fix(firmware): halt-on-failure stopped waiting for the wave to finish (v0.24.5-beta)

A regression I introduced in v0.24.3, and described inaccurately in its own commit message. Adding concurrency moved the halt check to the end of a wave. I wrote that "at the default of 1 those are the same moment". That is only true of a wave holding one device. With several devices in one wave and the default sequential setting, the old code stopped the moment one failed; mine went on to attempt every remaining device in that wave first. Anyone relying on halt-on-failure with a wide wave 1 got a wider blast radius than before, without changing a setting. Halting now stops further devices from *starting* as soon as a failure is known, whatever the concurrency. Devices already in flight are allowed to finish, because interrupting one mid-write is how you brick it. At concurrency 1 that is precisely the previous behaviour. Above 1, only the devices genuinely running side by side can carry a bad build -- which is the unavoidable part of the trade in #135, rather than a cost imposed on rollouts that never asked for concurrency. The firmware documentation said the same wrong thing and is corrected. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
fix(logs): a line with no RouterOS id had no protection against re-insertion (v0.24.4-beta)
2026-09-08 08:54:51 -07:00 · 2026-09-08 08:45:29 -07:00 · 2026-09-07 22:01:40 -07:00 · 2026-09-07 21:42:55 -07:00 · 2026-09-07 21:23:08 -07:00 · 2026-09-07 21:05:55 -07:00
32 changed files with 2054 additions and 161 deletions
@@ -9,8 +9,7 @@ name: Release
 on:
  push:
    branches: [main]
-    paths:
-      - 'frontend/package.json'
+  workflow_dispatch:

 permissions:
  contents: write
@@ -21,17 +20,26 @@ jobs:
    steps:
      - uses: actions/checkout@v5
        with:
-          fetch-depth: 2
+          # Full history and tags: the check below asks whether this version has
+          # already been released, which needs both.
+          fetch-depth: 0

-      - name: Read version, and stop if it did not change
+      - name: Read version, and stop if it is already released
        id: v
        run: |
          VERSION=$(node -p "require('./frontend/package.json').version")
-          PREV=$(git show HEAD~1:frontend/package.json 2>/dev/null | node -p "JSON.parse(require('fs').readFileSync(0,'utf8')).version" || echo "")
          echo "version=$VERSION" >> "$GITHUB_OUTPUT"
-          if [ "$VERSION" = "$PREV" ]; then
+          # Asking "does a tag for this version exist?" rather than "did the
+          # version change since the previous commit?".
+          #
+          # The old comparison was against HEAD~1, which silently skipped the
+          # release whenever a version bump was pushed together with any
+          # follow-up commit — the bump was no longer the tip, so nothing looked
+          # changed. v0.24.3 was lost exactly that way. This form does not care
+          # how commits are batched, and is safe to re-run.
+          if git rev-parse -q --verify "refs/tags/v$VERSION" >/dev/null; then
            echo "unchanged=true" >> "$GITHUB_OUTPUT"
-            echo "Version unchanged ($VERSION) — nothing to release."
+            echo "v$VERSION is already tagged — nothing to release."
          fi

      - name: Compose notes from the commit that shipped it
@@ -44,8 +52,11 @@ jobs:
          # a shell command lets the shell evaluate it — which is both why the
          # first attempt failed and a way to run arbitrary code in CI from a
          # commit message.
+          # The commit that last touched the version, not necessarily the tip:
+          # a version bump is often followed by docs or CI commits.
+          SHIP=$(git log -1 --format=%H -- frontend/package.json)
          {
-            git log -1 --pretty=%B
+            git log -1 --pretty=%B "$SHIP"
            printf '\n---\n'
            printf 'Container image: `ghcr.io/2gt-media-group-llc/mikrotik-manager:%s`\n\n' "$VERSION"
            printf 'Documentation: https://2gt-media-group-llc.github.io/mikrotik-manager/\n'
@@ -6,7 +6,7 @@
 </p>

 <p align="center">
-  <img src="https://img.shields.io/badge/version-0.24.0_Beta-blue" alt="Version" />
+  <img src="https://img.shields.io/badge/version-0.24.5_Beta-blue" alt="Version" />
  <img src="https://img.shields.io/badge/license-AGPLv3-blue" alt="License" />
  <img src="https://img.shields.io/badge/docker-compose-2496ED?logo=docker&logoColor=white" alt="Docker" />
  <img src="https://img.shields.io/badge/TypeScript-5.3-3178C6?logo=typescript&logoColor=white" alt="TypeScript" />
@@ -339,8 +339,8 @@ Each network service supports multi-device management with conflict detection:

 ### Fleet operations

- **Firmware orchestration** — fleet version overview, live update checks, and MikroTik's official release notes in-app. Staged rollouts run in **waves** (wave 1 = canary) through a verified pipeline per device: pre-upgrade backup → install → ride out the reboot → confirm it returned healthy on the new version → next. **Halt on failure** stops the rollout if any device fails, and a device that comes back on the *old* version counts as a failure. Rollouts can be scheduled, and cancelling never interrupts an in-flight flash
- **Backups** — on demand or on a daily/weekly/monthly schedule for all online devices, downloadable from the UI. Config snapshots and their restorable `.rsc` are one artifact: delete either and both go, so they never drift apart
+- **Firmware orchestration** — fleet version overview, live update checks, and MikroTik's official release notes in-app. Staged rollouts run in **waves** (wave 1 = canary) through a verified pipeline per device: pre-upgrade backup → download and confirm the image landed → reboot → prove it restarted and came back healthy on the new version → next. **Halt on failure** stops the rollout if any device fails, and a device that comes back on the *old* version counts as a failure. Devices in a wave upgrade one at a time by default; raise **at once** to run them concurrently when a fleet of five-minute reboots would otherwise take all evening — waves stay sequential either way, so the canary still gates the rest. Rollouts can be scheduled, and cancelling never interrupts an in-flight flash
+- **Backups** — on demand or on a daily/weekly/monthly schedule for all online devices. Filter by device, type and date range; read one in place instead of downloading it; and compare any two as a diff — they are `/export` text, not opaque blobs. Config snapshots and their restorable `.rsc` are one artifact: delete either and both go, so they never drift apart, and a bulk delete says how many snapshots it will take with it
 - **Configuration templates** — reusable sets (DNS, NTP, syslog) pushed to many devices with per-device result reporting
 - **Bulk device add** — "Try All" on discovered devices runs as a server-side job that survives a closed browser tab, with live progress and cancel
 - **Device organisation** — colour-coded tags, rack location, physical address with map support, and per-device notes
@@ -370,6 +370,8 @@ so it moves when behaviour does.
 | [Configuration](docs/configuration.md) | Environment variables, secret management, key rotation, TLS |
 | [Change Guard and Config Health](docs/change-guard.md) | How the safety system works, its settings, and what to expect when it fires |
 | [Sites](docs/sites.md) | Grouping devices into sites, switching between them, and what each site scopes |
+| [Firmware rollouts](docs/firmware.md) | Waves, the upgrade pipeline, and what upgrading several at once costs |
+| [Backups](docs/backups.md) | Finding, reading, comparing and deleting configuration backups |
 | [Alerting](docs/alerting.md) | Rules, channels, ntfy priority mapping, webhooks, maintenance windows |
 | [Single sign-on (OIDC)](docs/sso-oidc.md) | Identity provider setup, group-to-role mapping, break-glass behaviour |
 | [CAPsMAN](docs/capsman.md) | How centrally provisioned APs are modelled, and current limits |
@@ -1,6 +1,6 @@
 {
  "name": "mikrotik-manager-backend",
-  "version": "0.24.0-beta",
+  "version": "0.24.5-beta",
  "description": "Mikrotik Network Management Platform - Backend API",
  "main": "dist/index.js",
  "scripts": {
@@ -608,6 +608,10 @@ ALTER TABLE devices ADD COLUMN IF NOT EXISTS wifi_package VARCHAR(16);
 -- now meant visiting every device by hand — the exact task staged rollouts exist to
 -- remove.
 ALTER TABLE firmware_rollouts ADD COLUMN IF NOT EXISTS routerboot_after BOOLEAN NOT NULL DEFAULT FALSE;
+-- How many devices in a wave may upgrade at once (issue #135). Default 1 keeps
+-- the sequential behaviour every existing rollout was built around; raising it
+-- trades canary strictness for wall-clock time, which is the operator's call.
+ALTER TABLE firmware_rollouts ADD COLUMN IF NOT EXISTS wave_concurrency INTEGER NOT NULL DEFAULT 1;

 -- Temporarily dismissed "Things to handle" items (github issue #102).
 --
@@ -7,6 +7,7 @@ import morgan from 'morgan';
 import { createServer } from 'http';
 import { Server as SocketServer } from 'socket.io';
 import { Client as SshClient } from 'ssh2';
+import { resolveAuth, type SshExecDevice } from './services/sshExec';
 import type { ClientChannel } from 'ssh2';
 import dotenv from 'dotenv';

@@ -30,7 +31,6 @@ import { rateLimitRedis } from './middleware/rateLimitRedis';
 import { initSecrets } from './utils/secrets';
 import { reencryptStaleCredentials } from './db/reencryptCredentials';
 import { reconcileStaleGuards } from './services/changeGuard/ChangeGuard';
-import { decrypt } from './utils/crypto';
 import { corsMiddlewareOptions, socketIoCorsOptions } from './utils/corsOrigins';

 import authRoutes from './routes/auth';
@@ -189,23 +189,28 @@ terminalNs.on('connection', (socket) => {
        socket.emit('error', 'Too many terminal sessions started. Please wait a moment and try again.');
        return;
      }
-      const device = await queryOne<{
-        ip_address: string;
-        ssh_port: number | null;
-        ssh_username: string | null;
-        ssh_password_encrypted: string | null;
-      }>(
-        `SELECT ip_address, ssh_port, ssh_username, ssh_password_encrypted FROM devices WHERE id = $1`,
+      const device = await queryOne<SshExecDevice>(
+        `SELECT id, name, ip_address, ssh_port, ssh_username, ssh_password_encrypted,
+                api_username, api_password_encrypted
+           FROM devices WHERE id = $1`,
        [deviceId]
      );

      if (!device) { socket.emit('error', 'Device not found'); return; }
-      if (!device.ssh_username || !device.ssh_password_encrypted) {
-        socket.emit('error', 'No SSH credentials configured for this device. Add an SSH username and password in device settings.');
+
+      // Resolve credentials the same way every other SSH consumer does, so a
+      // deployed key works here too. Doing this locally, and looking only at
+      // the stored password, is what broke the console once a key replaced the
+      // password (#133).
+      let auth: Awaited<ReturnType<typeof resolveAuth>>;
+      try {
+        auth = await resolveAuth(device);
+      } catch (e) {
+        socket.emit('error',
+          `${(e as Error).message}. Add an SSH username and password in device settings, ` +
+          `or deploy an SSH key for this device.`);
        return;
      }
-
-      const password = decrypt(device.ssh_password_encrypted);
      sshClient = new SshClient();

      sshClient.on('ready', () => {
@@ -214,7 +219,7 @@ terminalNs.on('connection', (socket) => {
          (err, stream) => {
            if (err) { socket.emit('error', err.message); return; }
            shellStream = stream;
-            auditTerminal(user, clientIp, `opened SSH shell on device ${deviceId} (${device.ip_address})`, deviceId);
+            auditTerminal(user, clientIp, `opened SSH shell on device ${deviceId} (${device.ip_address}) using ${auth.kind} auth`, deviceId);
            socket.emit('ready');

            stream.on('data', (data: Buffer) => {
@@ -238,8 +243,8 @@ terminalNs.on('connection', (socket) => {
      sshClient.connect({
        host: device.ip_address,
        port: device.ssh_port ?? 22,
-        username: device.ssh_username,
-        password,
+        username: auth.username,
+        ...auth.auth,
        readyTimeout: 10_000,
        algorithms: {
          kex: [
@@ -12,27 +12,130 @@ router.use(requireAuth);

 const backupService = new BackupService();

-// GET /api/backups
-router.get('/', async (req: Request, res: Response) => {
-  const { deviceId } = req.query;
-  let sql = `
-    SELECT b.*, d.name as device_name
-    FROM backups b JOIN devices d ON d.id = b.device_id
-    WHERE 1=1
-  `;
-  const params: unknown[] = [];
-  if (deviceId) {
-    sql += ` AND b.device_id = $1`;
-    params.push(deviceId);
-  }
-  const siteFilter = siteScopeByDevice(activeSite(req), 'b.device_id');
-  if (siteFilter) sql += ` AND ${siteFilter}`;
-  sql += ' ORDER BY b.created_at DESC';
+/** Preview reads the file into memory, so it needs a ceiling. */
+const MAX_PREVIEW_BYTES = 2 * 1024 * 1024;
+/** A bulk delete is irreversible; keep one click from reaching an unbounded set. */
+const MAX_BULK_DELETE = 200;

-  const backups = await query(sql, params);
+// GET /api/backups — filter by device, type, date range and free text (#134)
+router.get('/', async (req: Request, res: Response) => {
+  const { deviceId, type, from, to, search } = req.query as Record<string, string | undefined>;
+
+  const filters: string[] = [];
+  const params: unknown[] = [];
+  /**
+   * Bind a value and return its placeholder.
+   *
+   * Derived from params.length rather than a hand-kept counter: with a counter,
+   * the last filter's increment is dead and whoever adds the next one inherits a
+   * stale index. This cannot get out of step.
+   */
+  const bind = (v: unknown): string => { params.push(v); return `$${params.length}`; };
+
+  if (deviceId) filters.push(`b.device_id = ${bind(deviceId)}`);
+  if (type)     filters.push(`b.backup_type = ${bind(type)}`);
+  // Dates are inclusive at both ends: a picker showing "to: 3 Sep" should include
+  // everything that happened on the 3rd, not stop at its first instant.
+  if (from)     filters.push(`b.created_at >= ${bind(from)}`);
+  if (to)       filters.push(`b.created_at < (${bind(to)}::date + INTERVAL '1 day')`);
+  if (search) {
+    const p = bind(`%${search}%`);
+    filters.push(`(b.filename ILIKE ${p} OR b.notes ILIKE ${p} OR d.name ILIKE ${p})`);
+  }
+
+  const siteFilter = siteScopeByDevice(activeSite(req), 'b.device_id');
+  if (siteFilter) filters.push(siteFilter);
+
+  const where = filters.length ? `WHERE ${filters.join(' AND ')}` : '';
+  const backups = await query(
+    `SELECT b.*, d.name as device_name
+       FROM backups b JOIN devices d ON d.id = b.device_id
+       ${where}
+      ORDER BY b.created_at DESC`,
+    params
+  );
  res.json(backups);
 });

+// GET /api/backups/types — the backup types actually present, for the filter
+router.get('/types', async (req: Request, res: Response) => {
+  const siteFilter = siteScopeByDevice(activeSite(req), 'b.device_id');
+  const rows = await query<{ backup_type: string; n: string }>(
+    `SELECT b.backup_type, COUNT(*)::text AS n
+       FROM backups b
+       ${siteFilter ? `WHERE ${siteFilter}` : ''}
+      GROUP BY b.backup_type ORDER BY b.backup_type`
+  );
+  res.json(rows.map((r) => ({ type: r.backup_type, count: parseInt(r.n, 10) })));
+});
+
+/**
+ * POST /api/backups/bulk-delete
+ *
+ * Deleting a backup cascades to the config snapshot that shares it — they are
+ * deliberately one artifact so they cannot drift apart. Doing that fifty times
+ * from one click deserves to say so, so the count of snapshots that will go with
+ * them is reported here and shown in the confirmation.
+ */
+router.post('/bulk-delete', requireWrite, async (req: Request, res: Response) => {
+  const { ids } = req.body as { ids?: unknown };
+  if (!Array.isArray(ids) || ids.length === 0) {
+    return res.status(400).json({ error: 'ids array is required' });
+  }
+  const clean = ids.map((v) => parseInt(String(v), 10)).filter(Number.isInteger);
+  if (clean.length === 0) return res.status(400).json({ error: 'ids array is required' });
+  if (clean.length > MAX_BULK_DELETE) {
+    return res.status(400).json({ error: `Cannot delete more than ${MAX_BULK_DELETE} backups at once` });
+  }
+
+  // Only ever touch backups in the active site, so a bulk action cannot reach
+  // across into another site's history.
+  const siteFilter = siteScopeByDevice(activeSite(req), 'device_id');
+  const allowed = await query<{ id: number }>(
+    `SELECT id FROM backups WHERE id = ANY($1::int[]) ${siteFilter ? `AND ${siteFilter}` : ''}`,
+    [clean]
+  );
+  const allowedIds = allowed.map((r) => r.id);
+  if (allowedIds.length === 0) return res.status(404).json({ error: 'No matching backups' });
+
+  let deleted = 0;
+  const failures: { id: number; error: string }[] = [];
+  for (const id of allowedIds) {
+    try { await backupService.deleteBackup(id); deleted++; }
+    catch (e) { failures.push({ id, error: (e as Error).message }); }
+  }
+  return res.json({ deleted, requested: clean.length, failures });
+});
+
+// GET /api/backups/bulk-delete/preview?ids=1,2,3 — what a bulk delete would remove
+router.get('/bulk-delete/preview', async (req: Request, res: Response) => {
+  const ids = String(req.query.ids || '')
+    .split(',').map((v) => parseInt(v, 10)).filter(Number.isInteger);
+  if (ids.length === 0) return res.json({ backups: 0, snapshots: 0, devices: [] });
+
+  const siteFilter = siteScopeByDevice(activeSite(req), 'b.device_id');
+  const rows = await query<{ n: string; snapshots: string }>(
+    `SELECT COUNT(*)::text AS n,
+            COUNT(dc.id)::text AS snapshots
+       FROM backups b
+       LEFT JOIN device_configs dc ON dc.backup_id = b.id
+      WHERE b.id = ANY($1::int[]) ${siteFilter ? `AND ${siteFilter}` : ''}`,
+    [ids]
+  );
+  const devices = await query<{ name: string; n: string }>(
+    `SELECT d.name, COUNT(*)::text AS n
+       FROM backups b JOIN devices d ON d.id = b.device_id
+      WHERE b.id = ANY($1::int[]) ${siteFilter ? `AND ${siteFilter}` : ''}
+      GROUP BY d.name ORDER BY d.name`,
+    [ids]
+  );
+  return res.json({
+    backups: parseInt(rows[0]?.n || '0', 10),
+    snapshots: parseInt(rows[0]?.snapshots || '0', 10),
+    devices: devices.map((d) => ({ name: d.name, count: parseInt(d.n, 10) })),
+  });
+});
+
 // POST /api/backups
 router.post('/', requireWrite, async (req: Request, res: Response) => {
  const { deviceId, notes } = req.body;
@@ -93,4 +196,76 @@ router.delete('/:id', requireWrite, async (req: Request, res: Response) => {
  return res.json({ message: 'Backup deleted' });
 });

+/**
+ * Read a backup's text.
+ *
+ * Backups are `/export compact` output, i.e. plain RouterOS script — not the
+ * binary `.backup` blob — so they can be read in place instead of downloaded
+ * and opened elsewhere (#134).
+ */
+async function readBackupText(id: string | number): Promise<
+  { meta: BackupMeta; text: string } | { error: string; status: number }
+> {
+  const meta = await queryOne<BackupMeta>(
+    `SELECT b.id, b.filename, b.size_bytes, b.backup_type, b.notes, b.created_at,
+            b.file_path, b.device_id, d.name AS device_name
+       FROM backups b JOIN devices d ON d.id = b.device_id
+      WHERE b.id = $1`,
+    [id]
+  );
+  if (!meta) return { error: 'Backup not found', status: 404 };
+  if (!fs.existsSync(meta.file_path)) {
+    return { error: 'Backup file not found on disk', status: 404 };
+  }
+  const stat = fs.statSync(meta.file_path);
+  if (stat.size > MAX_PREVIEW_BYTES) {
+    return {
+      error: `This backup is ${Math.round(stat.size / 1024)} KB, too large to preview. Download it instead.`,
+      status: 413,
+    };
+  }
+  return { meta, text: fs.readFileSync(meta.file_path, 'utf8') };
+}
+
+interface BackupMeta {
+  id: number;
+  filename: string;
+  size_bytes: number | null;
+  backup_type: string | null;
+  notes: string | null;
+  created_at: string;
+  file_path: string;
+  device_id: number;
+  device_name: string;
+}
+
+// GET /api/backups/:id/content — preview in place
+router.get('/:id/content', async (req: Request, res: Response) => {
+  const result = await readBackupText(req.params.id);
+  if ('error' in result) return res.status(result.status).json({ error: result.error });
+  const { file_path: _omit, ...meta } = result.meta;
+  return res.json({ ...meta, content: result.text });
+});
+
+// GET /api/backups/:fromId/diff/:toId — compare two backups
+// Mirrors the config-history diff contract: both texts are returned and the
+// browser renders them with the same lineDiff used for config snapshots.
+router.get('/:fromId/diff/:toId', async (req: Request, res: Response) => {
+  const [from, to] = await Promise.all([
+    readBackupText(req.params.fromId),
+    readBackupText(req.params.toId),
+  ]);
+  for (const r of [from, to]) {
+    if ('error' in r) return res.status(r.status).json({ error: r.error });
+  }
+  const f = from as { meta: BackupMeta; text: string };
+  const t = to as { meta: BackupMeta; text: string };
+  return res.json({
+    from: { id: f.meta.id, filename: f.meta.filename, device_name: f.meta.device_name,
+            created_at: f.meta.created_at, text: f.text },
+    to:   { id: t.meta.id, filename: t.meta.filename, device_name: t.meta.device_name,
+            created_at: t.meta.created_at, text: t.text },
+  });
+});
+
 export default router;
@@ -5,6 +5,8 @@ import { siteScopeDevices, siteScopeByDevice } from '../utils/siteScope';
 import { activeSite } from '../middleware/site';
 import { DeviceCollector, DeviceRow } from '../services/mikrotik/DeviceCollector';
 import { firmwareOrchestrator } from '../services/FirmwareOrchestrator';
+import { clampConcurrency, MAX_WAVE_CONCURRENCY } from '../utils/concurrency';
+import { findUpstreamWithinSelection } from '../utils/rolloutTopology';

 const router = Router();
 router.use(requireAuth);
@@ -64,9 +66,13 @@ router.post('/check-all', requireWrite, async (req: Request, res: Response) => {

 // POST /api/firmware/rollouts — create a rollout (optionally scheduled)
 router.post('/rollouts', requireWrite, async (req: Request, res: Response) => {
-  const { name, halt_on_failure, pre_backup, routerboot_after, scheduled_at, devices, start } = req.body as {
+  const {
+    name, halt_on_failure, pre_backup, routerboot_after, scheduled_at, devices, start,
+    wave_concurrency,
+  } = req.body as {
    name?: string; halt_on_failure?: boolean; pre_backup?: boolean; routerboot_after?: boolean; scheduled_at?: string | null;
    devices?: { device_id: number; wave: number }[]; start?: boolean;
+    wave_concurrency?: number;
  };
  if (!name || !name.trim()) return res.status(400).json({ error: 'name is required' });
  if (!Array.isArray(devices) || devices.length === 0) return res.status(400).json({ error: 'devices array is required' });
@@ -77,10 +83,46 @@ router.post('/rollouts', requireWrite, async (req: Request, res: Response) => {
  }
  if (scheduled_at && isNaN(Date.parse(scheduled_at))) return res.status(400).json({ error: 'scheduled_at must be a valid timestamp' });

+  // Refuse before creating anything (#139). The rollout used to be written
+  // first and started second, so a rejected start left the rows behind: the
+  // user saw "already running", pressed again, and each press quietly banked
+  // another rollout for the same devices. Nothing is persisted unless the
+  // request can actually be honoured.
+  const busy = await query<{ device_id: number; name: string; rollout_id: number }>(
+    `SELECT frd.device_id, d.name, frd.rollout_id
+       FROM firmware_rollout_devices frd
+       JOIN firmware_rollouts r ON r.id = frd.rollout_id
+       JOIN devices d ON d.id = frd.device_id
+      WHERE frd.device_id = ANY($1::int[])
+        AND r.status IN ('pending','running')
+        AND frd.status NOT IN ('success','failed','skipped')`,
+    [devices.map((d) => d.device_id)]
+  );
+  if (busy.length > 0) {
+    const names = [...new Set(busy.map((b) => b.name))];
+    return res.status(409).json({
+      error:
+        `${names.join(', ')} ${names.length === 1 ? 'is' : 'are'} already part of ` +
+        `rollout #${busy[0].rollout_id}, which has not finished. ` +
+        `Wait for it to complete or cancel it first.`,
+      conflicting_rollout_id: busy[0].rollout_id,
+      device_ids: [...new Set(busy.map((b) => b.device_id))],
+    });
+  }
+  // An immediate start needs the orchestrator free; a scheduled one does not.
+  if (start && !scheduled_at && firmwareOrchestrator.running) {
+    return res.status(409).json({
+      error: `Rollout #${firmwareOrchestrator.running} is still running. ` +
+             `Wait for it to finish, or schedule this one for later.`,
+      conflicting_rollout_id: firmwareOrchestrator.running,
+    });
+  }
+
  const rollout = await queryOne<{ id: number }>(
-    `INSERT INTO firmware_rollouts (name, halt_on_failure, pre_backup, routerboot_after, scheduled_at)
-     VALUES ($1,$2,$3,$4,$5) RETURNING id`,
-    [name.trim().slice(0, 100), halt_on_failure !== false, pre_backup !== false, routerboot_after === true, scheduled_at || null]);
+    `INSERT INTO firmware_rollouts (name, halt_on_failure, pre_backup, routerboot_after, scheduled_at, wave_concurrency)
+     VALUES ($1,$2,$3,$4,$5,$6) RETURNING id`,
+    [name.trim().slice(0, 100), halt_on_failure !== false, pre_backup !== false, routerboot_after === true,
+     scheduled_at || null, clampConcurrency(wave_concurrency ?? 1)]);
  for (const d of devices) {
    await query(
      `INSERT INTO firmware_rollout_devices (rollout_id, device_id, wave) VALUES ($1,$2,$3)`,
@@ -89,11 +131,52 @@ router.post('/rollouts', requireWrite, async (req: Request, res: Response) => {

  if (start && !scheduled_at) {
    try { await firmwareOrchestrator.start(rollout!.id); }
-    catch (e) { return res.status(409).json({ error: (e as Error).message, id: rollout!.id }); }
+    catch (e) {
+      // Lost a race for the orchestrator. Remove the rollout we just created
+      // rather than leaving an unstartable shell behind.
+      await query(`DELETE FROM firmware_rollouts WHERE id = $1`, [rollout!.id]).catch(() => {});
+      return res.status(409).json({ error: (e as Error).message });
+    }
  }
  res.status(201).json({ id: rollout!.id });
 });

+/**
+ * GET /api/firmware/rollouts/upstream-check?ids=1,2,3
+ *
+ * Which of the selected devices sit upstream of the others (#135).
+ *
+ * Running a wave concurrently means several devices reboot together, and a
+ * switch that carries the uplink to the rest takes them with it -- the manager
+ * then cannot reach the devices it is mid-upgrade on. Change Guard does not
+ * cover this: it protects configuration, not the path back to the device.
+ *
+ * Advisory only. It warns, it does not refuse: the operator may know the
+ * topology better than the discovered links do.
+ */
+router.get('/rollouts/upstream-check', async (req: Request, res: Response) => {
+  const ids = String(req.query.ids || '')
+    .split(',').map((v) => parseInt(v, 10)).filter(Number.isInteger);
+  if (ids.length < 2) return res.json({ upstream: [] });
+
+  const links = await query<{
+    from_device_id: number; to_device_id: number | null;
+    from_interface: string | null; stp_role: string | null;
+  }>(
+    `SELECT from_device_id, to_device_id, from_interface, stp_role
+       FROM topology_links WHERE to_device_id IS NOT NULL`
+  );
+  const names = await query<{ id: number; name: string }>(
+    `SELECT id, name FROM devices WHERE id = ANY($1::int[])`, [ids]
+  );
+  const nameById = new Map(names.map((n) => [n.id, n.name]));
+
+  const upstream = findUpstreamWithinSelection(ids, links).map((id) => ({
+    id, name: nameById.get(id) ?? `Device ${id}`,
+  }));
+  return res.json({ upstream, maxConcurrency: MAX_WAVE_CONCURRENCY });
+});
+
 // GET /api/firmware/rollouts — recent rollouts with progress counts
 router.get('/rollouts', async (req: Request, res: Response) => {
  // A rollout is shown when it touched at least one device in the active site.
@@ -1,6 +1,7 @@
 // Staged firmware rollout orchestrator.
 //
-// Executes a rollout wave-by-wave (wave 1 = canary), each device sequentially:
+// Executes a rollout wave-by-wave (wave 1 = canary). Devices within a wave run
+// sequentially by default, or up to wave_concurrency at once (#135):
 //   pre-upgrade backup → install RouterOS update → wait through the reboot →
 //   verify it came back healthy on the new version → next device.
 // A failure marks the device failed and (with halt_on_failure) stops the whole
@@ -11,6 +12,7 @@
 import { query, queryOne } from '../config/database';
 import { DeviceCollector, DeviceRow } from './mikrotik/DeviceCollector';
 import { BackupService } from './BackupService';
+import { mapWithConcurrency, clampConcurrency } from '../utils/concurrency';

 const REBOOT_GRACE_MS = 25_000;      // let the device actually go down
 const DOWNLOAD_TIMEOUT_MS = 10 * 60_000; // large images on slow links
@@ -28,6 +30,8 @@ interface RolloutRow {
  halt_on_failure: boolean; pre_backup: boolean;
  /** Follow the RouterOS upgrade with the pending RouterBOOT upgrade (issue #113). */
  routerboot_after: boolean;
+  /** How many devices in a wave may upgrade at once; 1 is sequential (#135). */
+  wave_concurrency: number;
 }
 interface RolloutDeviceRow {
  id: number; rollout_id: number; device_id: number; wave: number; status: string;
@@ -87,18 +91,54 @@ export class FirmwareOrchestrator {
      `SELECT * FROM firmware_rollout_devices WHERE rollout_id=$1 ORDER BY wave ASC, id ASC`,
      [rollout.id]);

+    const concurrency = clampConcurrency(rollout.wave_concurrency);
+    const waves = [...new Set(items.map((i) => i.wave))].sort((a, b) => a - b);
    let halted = false;
-    for (const item of items) {
-      if (this.cancelRequested || halted) {
-        await query(`UPDATE firmware_rollout_devices SET status='skipped',
-          error=$2 WHERE id=$1 AND status='pending'`,
-          [item.id, this.cancelRequested ? 'Rollout cancelled' : 'Halted: earlier device failed']);
-        continue;
+
+    // Waves stay strictly sequential -- that is the decision point, and it is
+    // what makes wave 1 a canary. Concurrency applies only *within* a wave.
+    //
+    // Halt-on-failure stops *starting* further devices the moment a failure is
+    // known, rather than at the end of the wave. Devices already in flight are
+    // allowed to finish, because interrupting a device mid-write is how you
+    // brick it.
+    //
+    // At the default concurrency of 1 that is exactly the previous behaviour:
+    // a failure stops the next device immediately. Only devices genuinely
+    // running side by side can carry a bad build, which is the unavoidable part
+    // of the trade in #135 -- not a wider blast radius imposed on rollouts that
+    // never asked for concurrency.
+    for (const wave of waves) {
+      if (this.cancelRequested || halted) break;
+      const inWave = items.filter((i) => i.wave === wave);
+
+      if (concurrency > 1 && inWave.length > 1) {
+        console.log(`[Firmware] rollout #${rollout.id}: wave ${wave} — ` +
+                    `${inWave.length} device(s), up to ${concurrency} at once`);
+      }
+
+      let failedInWave = false;
+      const results = await mapWithConcurrency(inWave, concurrency, async (item) => {
+        if (this.cancelRequested) return false;
+        // Do not begin another device once this wave has already failed.
+        if (rollout.halt_on_failure && failedInWave) return false;
+        const ok = await this.upgradeDevice(rollout, item);
+        if (!ok) failedInWave = true;
+        return ok;
+      });
+
+      if (rollout.halt_on_failure && results.some((ok) => !ok)) {
+        halted = true;
+        console.warn(`[Firmware] rollout #${rollout.id}: halting after failures in wave ${wave}`);
      }
-      const ok = await this.upgradeDevice(rollout, item);
-      if (!ok && rollout.halt_on_failure) halted = true;
    }

+    // Anything never reached is skipped rather than failed -- it did not run.
+    await query(
+      `UPDATE firmware_rollout_devices SET status='skipped', error=$2, finished_at=NOW()
+        WHERE rollout_id=$1 AND status='pending'`,
+      [rollout.id, this.cancelRequested ? 'Rollout cancelled' : 'Halted: earlier device failed']);
+
    const finalStatus = this.cancelRequested ? 'cancelled' : halted ? 'failed' : 'completed';
    await query(`UPDATE firmware_rollouts SET status=$2, finished_at=NOW() WHERE id=$1`, [rollout.id, finalStatus]);
    console.log(`[Firmware] rollout #${rollout.id} ${finalStatus}`);
@@ -188,16 +228,37 @@ export class FirmwareOrchestrator {
      const freeMb = Math.round((parseInt(res['free-hdd-space'] || '0', 10) / 1048576) * 10) / 10;
      const spaceNote = freeMb > 0 ? ` The device reports ${freeMb} MB free.` : '';

-      await collector.downloadUpdate();
-
-      // Wait for the device to say the image has landed. Large images on slow
-      // links take minutes, and the old fixed 25-second guess is what made this
-      // look like a failure while the device was still working.
-      const dlDeadline = Date.now() + DOWNLOAD_TIMEOUT_MS;
+      // The download command blocks until the image has landed, which on an
+      // ordinary link is a minute or more. If that outruns its budget the
+      // connection can no longer be trusted, so it is replaced rather than
+      // reused -- but a timed-out download is *not* evidence of failure. The
+      // device is asked on a fresh connection what actually happened, because
+      // declaring failure here is what made a working download look broken
+      // when several devices ran in one wave (#136).
      let downloaded = false;
+      try {
+        await collector.downloadUpdate(DOWNLOAD_TIMEOUT_MS);
+      } catch (e) {
+        console.warn(`[Firmware] ${device.name}: download command did not return cleanly ` +
+                     `(${(e as Error).message}); re-checking on a new connection`);
+        collector.disconnect();
+        try {
+          await collector.connect();
+        } catch (re) {
+          return fail(
+            `Lost contact with the device while downloading ${latest}: ${(re as Error).message}. ` +
+            `Nothing was rebooted.${spaceNote}`
+          );
+        }
+      }
+
+      // Confirm the image is on the device. Normally this is true immediately,
+      // because the download command only returns once it is; the wait covers
+      // RouterOS versions that return early and report progress instead.
+      const dlDeadline = Date.now() + DOWNLOAD_TIMEOUT_MS;
      while (Date.now() < dlDeadline) {
        if (this.cancelRequested) break;
-        const s = await collector.getUpdateStatus();
+        const s = await collector.getUpdateStatus().catch(() => '');
        if (/downloaded/i.test(s)) { downloaded = true; break; }
        if (/error|fail/i.test(s)) {
          collector.disconnect();
@@ -19,6 +19,18 @@ import {
 import { bytesSince, periodKey, shouldSend } from '../../utils/dataCap';
 import { parseDeviceLogTime } from '../../utils/deviceTime';
 import { BackupService } from '../BackupService';
+
+/** RouterOS update commands reach out to MikroTik's servers and are slow by nature. */
+const UPDATE_CHECK_TIMEOUT_MS = 90_000;
+const UPDATE_DOWNLOAD_TIMEOUT_MS = 10 * 60_000;
+/**
+ * `/log/print` returns the device's entire ring buffer -- a thousand sentences
+ * is normal -- and measured against real hardware it routinely outruns the 30s
+ * default. Timing it out cost the device's whole log *and*, before the client
+ * learned to abandon a timed-out connection, desynchronised the session (#137).
+ */
+const LOG_READ_TIMEOUT_MS = 120_000;
+import { selectNewLogLines, highestStoredId, surrogateLogId, type RawLogLine } from '../../utils/logDedup';
 import { alertService } from '../AlertService';

 /** DB column limits for topology_links (see migrate.ts); reject oversize rows instead of silent truncation. */
@@ -800,22 +812,21 @@ export class DeviceCollector {

  async collectEvents(): Promise<void> {
    try {
-      const logs = await this.client.execute('/log/print');
+      const logs = await this.client.execute(
+        '/log/print', {}, [], { timeoutMs: LOG_READ_TIMEOUT_MS }
+      );
      if (!logs.length) return;

-      // RouterOS log .id values are hex strings like "*1A2F". Parse to int for comparison.
-      const parseRosId = (id: string): number => {
-        const hex = (id || '').replace(/^\*/, '');
-        return hex ? parseInt(hex, 16) : 0;
-      };
-
-      // Find the highest RouterOS log ID we've already stored for this device.
-      const lastRow = await queryOne<{ log_id: string }>(
+      // The highest id we hold, taken across recent rows rather than from the
+      // single most recently inserted one. Those are usually the same, but when
+      // they differ the watermark slips backwards and the entire buffer looks
+      // new on every poll (#137).
+      const storedIds = await query<{ log_id: string }>(
        `SELECT log_id FROM events WHERE device_id = $1 AND log_id IS NOT NULL AND log_id != ''
-         ORDER BY id DESC LIMIT 1`,
+         ORDER BY id DESC LIMIT 2000`,
        [this.device.id]
      );
-      const lastIdNum = parseRosId(lastRow?.log_id || '');
+      const lastIdNum = highestStoredId(storedIds.map((r) => r.log_id));

      // Always get the latest stored event time — needed for timestamp fallback.
      const latestStored = await queryOne<{ event_time: Date }>(
@@ -824,45 +835,61 @@ export class DeviceCollector {
      );
      const latestTime = latestStored?.event_time ? new Date(latestStored.event_time) : new Date(0);

-      // Detect log buffer overflow or device reboot: if all current IDs are below
-      // our stored lastIdNum, RouterOS has cleared/reset its log buffer.
-      // In that case, fall back to timestamp-based deduplication to avoid missing events.
-      const currentIds = logs.map(l => parseRosId(l['.id'] || '')).filter(id => id > 0);
-      const maxCurrentId = currentIds.length > 0 ? Math.max(...currentIds) : 0;
-      const logReset = lastRow && maxCurrentId > 0 && maxCurrentId < lastIdNum;
+
+      // Collect the candidates first, then insert them in batches.
+      //
+      // This used to issue one INSERT per line and count every attempt as new,
+      // so a poll that re-read the whole ring buffer and inserted nothing still
+      // reported "Collected 1000 new log entries" — a thousand round trips a
+      // minute per device, described as work that had not happened (#137).
+      // Counting what the database actually accepted makes the log line true
+      // and the duplicate case cheap.
+      const fresh = selectNewLogLines(logs as RawLogLine[], {
+        lastStoredId: lastIdNum,
+        latestStoredTime: latestTime,
+        parseTime: (raw) => this.parseLogTime(raw),
+      });
+
+      const pending: unknown[][] = [];
+      for (const log of fresh) {
+        const rawId = (log['.id'] || '') as string;
+        const topics = (log['topics'] as string) || '';
+        const message = (log['message'] as string) || '';
+        const time = this.parseLogTime(log['time'] || '');
+        const severity = this.mapLogSeverity(topics);
+        // Never store a null log_id: the unique index that stops re-insertion
+        // does not constrain nulls, so a device whose lines carry no `.id`
+        // would accumulate the same entries for ever (#137).
+        const logId = rawId || surrogateLogId(log['time'] || '', topics, message);
+        pending.push([
+          this.device.id,
+          time.toISOString(),
+          severity,
+          topics || null,
+          message,
+          JSON.stringify(log),
+          logId,
+        ]);
+      }

      let newCount = 0;
-      for (const log of logs) {
-        const logId = (log['.id'] || '') as string;
-        const logIdNum = parseRosId(logId);
-
-        if (logId && !logReset) {
-          // Primary: skip entries already stored by RouterOS ID
-          if (logIdNum <= lastIdNum) continue;
-        } else {
-          // Fallback: no .id field, or log buffer has been reset — use timestamp deduplication
-          const time = this.parseLogTime(log['time'] || '');
-          if (time <= latestTime) continue;
-        }
-
-        const time = this.parseLogTime(log['time'] || '');
-        const severity = this.mapLogSeverity(log['topics'] || '');
-
-        await query(
+      const COLS = 7;
+      const BATCH = 200;
+      for (let i = 0; i < pending.length; i += BATCH) {
+        const chunk = pending.slice(i, i + BATCH);
+        const values = chunk
+          .map((_, r) => `(${Array.from({ length: COLS }, (_, c) => `$${r * COLS + c + 1}`).join(',')})`)
+          .join(',');
+        // RETURNING reports only the rows actually written; ON CONFLICT rows are
+        // silently dropped, which is exactly the count we want.
+        const inserted = await query<{ id: number }>(
          `INSERT INTO events (device_id, event_time, severity, topic, message, raw_json, log_id)
-           VALUES ($1,$2,$3,$4,$5,$6,$7)
-           ON CONFLICT (device_id, log_id) DO NOTHING`,
-          [
-            this.device.id,
-            time.toISOString(),
-            severity,
-            log['topics'] || null,
-            log['message'] || '',
-            JSON.stringify(log),
-            logId || null,
-          ]
+           VALUES ${values}
+           ON CONFLICT (device_id, log_id) DO NOTHING
+           RETURNING id`,
+          chunk.flat()
        );
-        newCount++;
+        newCount += inserted.length;
      }

      if (newCount > 0) {
@@ -3757,12 +3784,16 @@ export class DeviceCollector {
  // ─── Firmware updates ─────────────────────────────────────────────────────

  async checkForUpdates(): Promise<Record<string, string>> {
-    await this.client.execute('/system/package/update/check-for-updates').catch(() => {});
-    // Give the device a moment to complete the check
+    // Reaches out to MikroTik's servers, so it can outlast the ordinary read
+    // timeout on a slow or filtered link. The failure was previously swallowed
+    // (.catch(() => {})), which hid a timed-out check and left the caller
+    // reading whatever /print happened to return (#136).
+    await this.client.execute(
+      '/system/package/update/check-for-updates', {}, [], { timeoutMs: UPDATE_CHECK_TIMEOUT_MS }
+    );
+    // Give the device a moment to settle the result before reading it back.
    await new Promise<void>((resolve) => setTimeout(resolve, 3000));
-    const result = await this.client
-      .execute('/system/package/update/print')
-      .catch(() => [] as Record<string, string>[]);
+    const result = await this.client.execute('/system/package/update/print');
    return (result[0] as Record<string, string>) || {};
  }

@@ -3776,8 +3807,11 @@ export class DeviceCollector {
   * be observed — and on at least one CCR it neither downloaded nor rebooted
   * while reporting nothing at all (#firmware).
   */
-  async downloadUpdate(): Promise<void> {
-    await this.client.execute('/system/package/update/download');
+  async downloadUpdate(timeoutMs = UPDATE_DOWNLOAD_TIMEOUT_MS): Promise<void> {
+    // Blocks until the image has landed -- a minute or more is normal, and the
+    // 30-second default read timeout was cutting it off and corrupting the
+    // connection (#136).
+    await this.client.execute('/system/package/update/download', {}, [], { timeoutMs });
  }

  /** The `status` line from /system/package/update, e.g. "Downloaded, please reboot…". */
@@ -40,6 +40,8 @@ export class RouterOSClient extends EventEmitter {
    timeout: ReturnType<typeof setTimeout>;
  } | null = null;
  private sentenceQueue: RouterOSSentence[] = [];
+  /** Set when a reply stream can no longer be trusted; distinct from `connected`. */
+  private poisoned = false;

  private tagCounter = 0;

@@ -62,6 +64,10 @@ export class RouterOSClient extends EventEmitter {

  async connect(): Promise<void> {
    if (this.connected) return;
+    // A fresh connection starts trusted, so a client that was abandoned earlier
+    // can be reconnected and used again.
+    this.poisoned = false;
+    this.sentenceQueue = [];

    await new Promise<void>((resolve, reject) => {
      const timer = setTimeout(() => {
@@ -124,10 +130,18 @@ export class RouterOSClient extends EventEmitter {
  }

  // Enqueue a command so concurrent calls don't interleave
+  /**
+   * @param opts.timeoutMs  How long to wait for each reply sentence. The 30s
+   *   default suits ordinary reads, but some RouterOS commands legitimately run
+   *   far longer -- `/system/package/update/download` takes a minute or more on
+   *   an ordinary link -- and timing those out corrupted the connection (#136).
+   *   Callers issuing a long-running command must say so.
+   */
  async execute(
    command: string,
    params: Record<string, string> = {},
-    queries: string[] = []
+    queries: string[] = [],
+    opts: { timeoutMs?: number } = {}
  ): Promise<Record<string, string>[]> {
    let result!: Record<string, string>[];
    let error!: Error;
@@ -135,7 +149,7 @@ export class RouterOSClient extends EventEmitter {
    await new Promise<void>((resolve) => {
      this.operationChain = this.operationChain.then(async () => {
        try {
-          result = await this._executeRaw(command, params, queries);
+          result = await this._executeRaw(command, params, queries, opts.timeoutMs);
        } catch (err) {
          error = err as Error;
        }
@@ -150,7 +164,8 @@ export class RouterOSClient extends EventEmitter {
  private async _executeRaw(
    command: string,
    params: Record<string, string>,
-    queries: string[]
+    queries: string[],
+    timeoutMs?: number
  ): Promise<Record<string, string>[]> {
    if (!this.socket || !this.connected) {
      throw new RouterOSError('Not connected');
@@ -168,7 +183,7 @@ export class RouterOSClient extends EventEmitter {

    const results: Record<string, string>[] = [];
    while (true) {
-      const sentence = await this.readNextSentence();
+      const sentence = await this.readNextSentence(timeoutMs);
      if (sentence.type === '!done') break;
      if (sentence.type === '!re') {
        results.push(sentence.words);
@@ -341,6 +356,17 @@ export class RouterOSClient extends EventEmitter {
      const sentence = this.tryParseSentence();
      if (!sentence) break;

+      // Once a connection is abandoned, anything still arriving belongs to the
+      // command that gave up. Queueing it would hand it to the next caller as
+      // that caller's own reply, which is the desynchronisation poison() exists
+      // to prevent -- destroying the socket usually stops this, but data already
+      // buffered can still surface.
+      //
+      // This must test `poisoned`, not `connected`: `connected` is only set once
+      // login has *finished*, so testing it here discarded the login exchange
+      // itself and every connection timed out.
+      if (this.poisoned) continue;
+
      if (this.pendingRead) {
        clearTimeout(this.pendingRead.timeout);
        const { resolve } = this.pendingRead;
@@ -425,6 +451,19 @@ export class RouterOSClient extends EventEmitter {
    return new Promise((resolve, reject) => {
      const timer = setTimeout(() => {
        this.pendingRead = null;
+        // A timed-out read leaves a reply in flight. Previously the socket was
+        // left open, so when that reply finally arrived processBuffer() found no
+        // pending read and queued it -- and the *next* command took it as its
+        // own answer. Every later command on that connection then returned the
+        // previous one's data.
+        //
+        // That is how a slow `/system/package/update/download` turned into
+        // "did not finish downloading within 10 minutes" (#136): the status
+        // polls afterwards were reading shifted replies and could never see
+        // "Downloaded". A desynchronised connection can also make an unrelated
+        // command look like it succeeded, so the only safe response is to stop
+        // using it.
+        this.poison('Read timeout waiting for API response');
        reject(new RouterOSError('Read timeout waiting for API response'));
      }, timeoutMs);

@@ -436,6 +475,22 @@ export class RouterOSClient extends EventEmitter {
    });
  }

+  /**
+   * Abandon a connection whose reply stream can no longer be trusted.
+   *
+   * Anything already queued belongs to a command that gave up, so it is
+   * discarded rather than handed to the next caller.
+   */
+  private poison(reason: string): void {
+    this.poisoned = true;
+    this.connected = false;
+    this.sentenceQueue = [];
+    this.buffer = Buffer.alloc(0);
+    try { this.socket?.destroy(); } catch { /* already gone */ }
+    this.socket = null;
+    this.emit('poisoned', reason);
+  }
+
  // Execute a streaming/generator command (one that never sends !done on its own,
  // e.g. /tool/ip-scan). Collects !re rows until maxDurationMs elapses, then
  // sends /cancel to terminate the command and drains the cleanup response.
@@ -77,3 +77,91 @@ describe('sentence parsing', () => {
    expect(REPEATED_KEY).toBe(REPEATED_ATTRIBUTES_KEY);
  });
 });
+
+/**
+ * Read-timeout handling (#136).
+ *
+ * A timed-out read leaves a reply in flight. The connection used to stay open,
+ * so when that reply eventually arrived it was queued and handed to the *next*
+ * command as its own answer — every subsequent call on that connection returned
+ * the previous one's data.
+ *
+ * That is what turned a slow `/system/package/update/download` into "did not
+ * finish downloading within 10 minutes": the status polls afterwards were
+ * reading shifted replies and could never see "Downloaded". The same
+ * desynchronisation could make any unrelated command appear to succeed, so
+ * these tests matter well beyond firmware.
+ */
+describe('read timeout', () => {
+  type Internals = {
+    buffer: Buffer;
+    connected: boolean;
+    socket: unknown;
+    sentenceQueue: RouterOSSentence[];
+    pendingRead: unknown;
+    readNextSentence(timeoutMs?: number): Promise<RouterOSSentence>;
+    processBuffer(): void;
+  };
+
+  const makeClient = () => {
+    const client = new RouterOSClient('127.0.0.1', 8728, 'x', 'x');
+    const internals = client as unknown as Internals;
+    internals.connected = true;
+    internals.socket = { destroy() {} };
+    return { client, internals };
+  };
+
+  // The regression these were written without: the discard guard first tested
+  // `connected`, which is only set *after* login succeeds. Every login reply was
+  // therefore thrown away and every connection timed out — the whole fleet went
+  // offline. The tests above all pre-set connected = true, so none of them saw
+  // it. "Not yet connected" and "abandoned" are different states.
+  it('delivers replies that arrive before login has completed', () => {
+    const { internals } = makeClient();
+    internals.connected = false;          // mid-login, as connect() leaves it
+    internals.buffer = sentence('ret=abc123');
+    internals.processBuffer();
+    expect(internals.sentenceQueue).toHaveLength(1);
+  });
+
+  it('rejects when no reply arrives in time', async () => {
+    const { internals } = makeClient();
+    await expect(internals.readNextSentence(20)).rejects.toThrow(/Read timeout/);
+  });
+
+  it('does not hand a late reply to the next command', async () => {
+    const { internals } = makeClient();
+
+    await expect(internals.readNextSentence(20)).rejects.toThrow(/Read timeout/);
+
+    // The reply the timed-out command was waiting for now turns up.
+    internals.buffer = sentence('status=Downloaded, please reboot');
+    internals.processBuffer();
+
+    // It must not be sitting in the queue waiting to be mistaken for the next
+    // command's answer.
+    expect(internals.sentenceQueue).toHaveLength(0);
+  });
+
+  it('marks the connection unusable so it cannot be reused blind', async () => {
+    const { client, internals } = makeClient();
+    await expect(internals.readNextSentence(20)).rejects.toThrow(/Read timeout/);
+
+    expect(internals.connected).toBe(false);
+    // A caller that carries on regardless gets a clean error rather than
+    // another command's data.
+    await expect(client.execute('/system/resource/print')).rejects.toThrow(/Not connected/);
+  });
+
+  // The guard above must not break ordinary pipelining: a healthy connection
+  // legitimately queues replies that arrive faster than they are read.
+  it('still serves queued replies while the connection is healthy', async () => {
+    const { internals } = makeClient();
+    internals.buffer = sentence('status=ok');
+    internals.processBuffer();
+    expect(internals.sentenceQueue).toHaveLength(1);
+    await expect(internals.readNextSentence(20)).resolves.toMatchObject({
+      words: { status: 'ok' },
+    });
+  });
+});
@@ -13,6 +13,7 @@
 import { Client as SSHClient } from 'ssh2';
 import { queryOne } from '../config/database';
 import { decrypt } from '../utils/crypto';
+import { preferredAuth, type KeyStatus } from '../utils/sshKeys';

 export interface SshExecDevice {
  id: number;
@@ -39,19 +40,33 @@ const DEFAULT_TIMEOUT_MS = 30_000;
 *
 * A key is used only when it has been *proved* — a key that was generated or
 * pushed but never authenticated is not evidence the device will accept it.
+ *
+ * Exported because every SSH consumer must agree on this. The interactive
+ * terminal used to resolve credentials itself, looked only at the stored
+ * password, and so broke the moment a key was deployed and the password
+ * retired — the exact workflow the key feature exists to enable (#133).
 */
-async function resolveAuth(device: SshExecDevice): Promise<{
+export async function resolveAuth(device: SshExecDevice): Promise<{
  username: string;
  auth: { password: string } | { privateKey: string };
  kind: 'key' | 'password';
 }> {
-  const key = await queryOne<{ private_key_encrypted: string; ssh_username: string | null }>(
-    `SELECT private_key_encrypted, ssh_username FROM device_ssh_keys
-      WHERE device_id = $1 AND status = 'verified'`,
+  const key = await queryOne<{
+    private_key_encrypted: string; ssh_username: string | null; status: KeyStatus;
+  }>(
+    `SELECT private_key_encrypted, ssh_username, status FROM device_ssh_keys
+      WHERE device_id = $1`,
    [device.id]
  );

-  if (key) {
+  const encryptedPassword = device.ssh_password_encrypted || device.api_password_encrypted;
+  const choice = preferredAuth(
+    key?.status ?? null,
+    !!key?.private_key_encrypted,
+    !!encryptedPassword
+  );
+
+  if (choice === 'key' && key) {
    const username = key.ssh_username || device.ssh_username || device.api_username;
    if (!username) throw new Error('No SSH username available for key authentication');
    return { username, auth: { privateKey: decrypt(key.private_key_encrypted) }, kind: 'key' };
@@ -59,9 +74,8 @@ async function resolveAuth(device: SshExecDevice): Promise<{

  const username = device.ssh_username || device.api_username;
  if (!username) throw new Error('No SSH username configured');
-  const encrypted = device.ssh_password_encrypted || device.api_password_encrypted;
-  if (!encrypted) throw new Error('No SSH credential configured');
-  return { username, auth: { password: decrypt(encrypted) }, kind: 'password' };
+  if (choice === 'none' || !encryptedPassword) throw new Error('No SSH credential configured');
+  return { username, auth: { password: decrypt(encryptedPassword) }, kind: 'password' };
 }

 /**
@@ -0,0 +1,103 @@
+import { mapWithConcurrency, clampConcurrency, MAX_WAVE_CONCURRENCY } from '../concurrency';
+
+const tick = (ms = 5) => new Promise((r) => setTimeout(r, ms));
+
+describe('mapWithConcurrency', () => {
+  it('returns results in input order, not completion order', async () => {
+    // First item is slowest, so completion order is the reverse of input order.
+    const out = await mapWithConcurrency([30, 20, 1], 3, async (ms) => {
+      await tick(ms);
+      return ms;
+    });
+    expect(out).toEqual([30, 20, 1]);
+  });
+
+  it('never exceeds the limit', async () => {
+    let inFlight = 0;
+    let peak = 0;
+    await mapWithConcurrency(Array.from({ length: 12 }, (_, i) => i), 3, async () => {
+      inFlight++;
+      peak = Math.max(peak, inFlight);
+      await tick();
+      inFlight--;
+    });
+    expect(peak).toBe(3);
+  });
+
+  // The behaviour every existing rollout depends on: unchanged unless asked for.
+  it('is strictly sequential at a limit of 1', async () => {
+    const order: number[] = [];
+    let inFlight = 0;
+    let peak = 0;
+    await mapWithConcurrency([1, 2, 3, 4], 1, async (n) => {
+      inFlight++; peak = Math.max(peak, inFlight);
+      await tick();
+      order.push(n);
+      inFlight--;
+    });
+    expect(peak).toBe(1);
+    expect(order).toEqual([1, 2, 3, 4]);
+  });
+
+  it('runs every item exactly once', async () => {
+    const seen: number[] = [];
+    await mapWithConcurrency(Array.from({ length: 25 }, (_, i) => i), 4, async (n) => {
+      await tick(1);
+      seen.push(n);
+    });
+    expect(seen.sort((a, b) => a - b)).toEqual(Array.from({ length: 25 }, (_, i) => i));
+  });
+
+  it('handles an empty list without starting anything', async () => {
+    let started = 0;
+    const out = await mapWithConcurrency([], 5, async () => { started++; return 1; });
+    expect(out).toEqual([]);
+    expect(started).toBe(0);
+  });
+
+  it('does not start more runners than there is work', async () => {
+    let peak = 0;
+    let inFlight = 0;
+    await mapWithConcurrency([1, 2], 10, async () => {
+      inFlight++; peak = Math.max(peak, inFlight);
+      await tick();
+      inFlight--;
+    });
+    expect(peak).toBe(2);
+  });
+
+  // A rejecting worker would abandon the wave mid-flight; the orchestrator
+  // catches per device, but the helper must not be the thing that hides it.
+  it('propagates a worker rejection', async () => {
+    await expect(
+      mapWithConcurrency([1, 2, 3], 2, async (n) => {
+        if (n === 2) throw new Error('boom');
+        return n;
+      })
+    ).rejects.toThrow('boom');
+  });
+});
+
+describe('clampConcurrency', () => {
+  it('defaults to sequential for anything unusable', () => {
+    for (const bad of [undefined, null, '', 'abc', NaN, {}, []]) {
+      expect(clampConcurrency(bad)).toBe(1);
+    }
+  });
+
+  it('never returns less than 1', () => {
+    expect(clampConcurrency(0)).toBe(1);
+    expect(clampConcurrency(-8)).toBe(1);
+  });
+
+  // This number decides how many devices may be rebooting at once.
+  it('caps the upper end', () => {
+    expect(clampConcurrency(1000)).toBe(MAX_WAVE_CONCURRENCY);
+    expect(clampConcurrency(Infinity)).toBe(MAX_WAVE_CONCURRENCY);
+  });
+
+  it('floors fractional values', () => {
+    expect(clampConcurrency(3.9)).toBe(3);
+    expect(clampConcurrency('4')).toBe(4);
+  });
+});
@@ -0,0 +1,140 @@
+import {
+  parseRosId, highestStoredId, isLogReset, selectNewLogLines,
+  surrogateLogId, isSurrogateLogId, type RawLogLine,
+} from '../logDedup';
+
+// Fixed clock so timestamp comparisons are deterministic.
+const T = (iso: string) => new Date(iso);
+const parseTime = (raw: string) => (raw ? T(raw) : new Date(0));
+
+const line = (id: string, time = '', message = 'm'): RawLogLine =>
+  ({ '.id': id, time, message, topics: 'system,info' });
+
+describe('parseRosId', () => {
+  it('reads RouterOS hex ids', () => {
+    expect(parseRosId('*1A2F')).toBe(0x1a2f);
+    expect(parseRosId('1a2f')).toBe(0x1a2f);
+  });
+
+  it('returns 0 for anything unusable', () => {
+    for (const bad of ['', undefined, '*', '*zz', 'not-hex', '*-5']) {
+      expect(parseRosId(bad as string)).toBe(0);
+    }
+  });
+});
+
+describe('highestStoredId', () => {
+  // The bug this exists for: taking the last *inserted* row instead of the
+  // highest id lets the watermark move backwards, after which every line looks
+  // new and the whole buffer is reprocessed every poll.
+  it('takes the maximum, not the last element', () => {
+    expect(highestStoredId(['*10', '*FF', '*20'])).toBe(0xff);
+  });
+
+  it('ignores nulls and junk', () => {
+    expect(highestStoredId([null, undefined, '', '*zz', '*7'])).toBe(7);
+    expect(highestStoredId([])).toBe(0);
+  });
+});
+
+describe('isLogReset', () => {
+  it('detects ids going backwards', () => {
+    expect(isLogReset(5, 900)).toBe(true);
+  });
+
+  it('is not fooled by a first run or an empty buffer', () => {
+    expect(isLogReset(900, 0)).toBe(false);
+    expect(isLogReset(0, 900)).toBe(false);
+  });
+
+  it('treats normal forward movement as no reset', () => {
+    expect(isLogReset(950, 900)).toBe(false);
+  });
+});
+
+describe('selectNewLogLines', () => {
+  const latestStoredTime = T('2026-09-08T00:00:00Z');
+
+  it('returns nothing when the whole buffer is already stored', () => {
+    // The reported symptom: a thousand lines re-offered every minute.
+    const logs = Array.from({ length: 1000 }, (_, i) => line(`*${(i + 1).toString(16)}`));
+    const out = selectNewLogLines(logs, { lastStoredId: 1000, latestStoredTime, parseTime });
+    expect(out).toHaveLength(0);
+  });
+
+  it('returns only the lines above the watermark', () => {
+    const logs = [line('*8'), line('*9'), line('*A'), line('*B')];
+    const out = selectNewLogLines(logs, { lastStoredId: 9, latestStoredTime, parseTime });
+    expect(out.map((l) => l['.id'])).toEqual(['*A', '*B']);
+  });
+
+  it('falls back to timestamps when the buffer has reset', () => {
+    // Ids went backwards, so they cannot be compared against the watermark.
+    const logs = [
+      line('*1', '2026-09-07T23:00:00Z'), // older than what we hold
+      line('*2', '2026-09-08T00:30:00Z'), // newer
+    ];
+    const out = selectNewLogLines(logs, { lastStoredId: 900, latestStoredTime, parseTime });
+    expect(out.map((l) => l['.id'])).toEqual(['*2']);
+  });
+
+  it('uses timestamps for lines carrying no id', () => {
+    const logs = [
+      { time: '2026-09-07T23:00:00Z', message: 'old' },
+      { time: '2026-09-08T01:00:00Z', message: 'new' },
+    ];
+    const out = selectNewLogLines(logs, { lastStoredId: 5, latestStoredTime, parseTime });
+    expect(out.map((l) => l.message)).toEqual(['new']);
+  });
+
+  it('treats a line exactly on the boundary as already stored', () => {
+    const logs = [{ time: '2026-09-08T00:00:00Z', message: 'boundary' }];
+    expect(selectNewLogLines(logs, { lastStoredId: 0, latestStoredTime, parseTime })).toHaveLength(0);
+  });
+});
+
+describe('surrogateLogId', () => {
+  const line = ['sep/08 02:38:10', 'system,info', 'router rebooted'] as const;
+
+  it('is stable for the same line', () => {
+    expect(surrogateLogId(...line)).toBe(surrogateLogId(...line));
+  });
+
+  it('differs when any field differs', () => {
+    const base = surrogateLogId(...line);
+    expect(surrogateLogId('sep/08 02:38:11', line[1], line[2])).not.toBe(base);
+    expect(surrogateLogId(line[0], 'system,error', line[2])).not.toBe(base);
+    expect(surrogateLogId(line[0], line[1], 'something else')).not.toBe(base);
+  });
+
+  // Field boundaries are separated, so shifting text between fields cannot
+  // produce the same key.
+  it('does not confuse field boundaries', () => {
+    expect(surrogateLogId('a', 'b', 'c')).not.toBe(surrogateLogId('ab', '', 'c'));
+  });
+
+  // The column is VARCHAR(20); an over-long key would throw on insert.
+  it('fits the log_id column', () => {
+    const id = surrogateLogId('x'.repeat(500), 'y'.repeat(500), 'z'.repeat(5000));
+    expect(id.length).toBeLessThanOrEqual(20);
+  });
+
+  it('cannot be mistaken for a RouterOS id', () => {
+    const id = surrogateLogId(...line);
+    expect(id.startsWith('#')).toBe(true);
+    expect(isSurrogateLogId(id)).toBe(true);
+    expect(isSurrogateLogId('*CC75')).toBe(false);
+    expect(isSurrogateLogId(null)).toBe(false);
+  });
+
+  /**
+   * The watermark is a max over RouterOS's own hex ids. A synthesised id must
+   * not be parsed as one, or it would poison the high-water mark and make every
+   * real line look already-seen.
+   */
+  it('is ignored by the id watermark', () => {
+    const id = surrogateLogId(...line);
+    expect(parseRosId(id)).toBe(0);
+    expect(highestStoredId([id, '*10'])).toBe(0x10);
+  });
+});
@@ -0,0 +1,80 @@
+import { findUpstreamWithinSelection, type TopologyEdge } from '../rolloutTopology';
+
+const link = (
+  from: number, to: number | null, iface = 'ether1', role: string | null = 'root'
+): TopologyEdge => ({ from_device_id: from, to_device_id: to, from_interface: iface, stp_role: role });
+
+describe('findUpstreamWithinSelection', () => {
+  // The case the warning exists for: two devices reach the network through a
+  // switch, and all three are being upgraded together.
+  it('flags a device reached through, when the link is unambiguous', () => {
+    const links = [
+      link(2, 1, 'ether1', 'root'),
+      link(3, 1, 'ether1', 'root'),
+    ];
+    expect(findUpstreamWithinSelection([1, 2, 3], links)).toEqual([1]);
+  });
+
+  /**
+   * The failure that prompted the rewrite. Links are bidirectional and a trunk
+   * port often resolves to several neighbours, so counting adjacency flagged
+   * every device on a real four-device fleet. A warning that fires on
+   * everything trains people to ignore it.
+   */
+  it('says nothing when a port resolves to several neighbours', () => {
+    const links = [
+      link(7, 1, 'sfp28-2', 'root'),
+      link(7, 8, 'sfp28-2', 'root'),
+      link(7, 2, 'sfp28-2', 'root'),
+    ];
+    expect(findUpstreamWithinSelection([1, 2, 7, 8], links)).toEqual([]);
+  });
+
+  it('is not fooled by the reverse direction of the same link', () => {
+    // Both ends report each other; only the root-port end means "upstream".
+    const links = [
+      link(2, 1, 'ether1', 'root'),
+      link(1, 2, 'sfp1', 'designated'),
+    ];
+    expect(findUpstreamWithinSelection([1, 2], links)).toEqual([1]);
+  });
+
+  it('ignores designated ports, which face away from the root', () => {
+    const links = [link(1, 2, 'sfp1', 'designated')];
+    expect(findUpstreamWithinSelection([1, 2], links)).toEqual([]);
+  });
+
+  it('tolerates the -port suffix RouterOS sometimes reports', () => {
+    const links = [link(2, 1, 'ether1', 'root-port')];
+    expect(findUpstreamWithinSelection([1, 2], links)).toEqual([1]);
+  });
+
+  it('ignores links to devices outside the selection', () => {
+    // A switch upstream of something not in this rollout is not a risk to it.
+    const links = [link(50, 1, 'ether1', 'root')];
+    expect(findUpstreamWithinSelection([1, 2], links)).toEqual([]);
+  });
+
+  it('needs at least two selected devices', () => {
+    expect(findUpstreamWithinSelection([1], [link(2, 1)])).toEqual([]);
+    expect(findUpstreamWithinSelection([], [link(2, 1)])).toEqual([]);
+  });
+
+  it('tolerates unresolved neighbours, self links and missing roles', () => {
+    const links = [
+      link(2, null, 'ether1', 'root'),
+      link(1, 1, 'ether1', 'root'),
+      link(3, 1, 'ether5', null),
+      link(2, 1, 'ether9', 'root'),
+    ];
+    expect(findUpstreamWithinSelection([1, 2, 3], links)).toEqual([1]);
+  });
+
+  it('reports several upstream devices in a stable order', () => {
+    const links = [
+      link(3, 2, 'ether1', 'root'),
+      link(4, 1, 'ether1', 'root'),
+    ];
+    expect(findUpstreamWithinSelection([1, 2, 3, 4], links)).toEqual([1, 2]);
+  });
+});
@@ -67,7 +67,8 @@ describe('preferredAuth', () => {
    // bound to them, confirmed on hardware by removing the key and watching
    // password login start working again.
    expect(preferredAuth('verified', true, true)).toBe('key');
-    expect(preferredAuth('deployed', true, true)).toBe('key');
+    // Pushed but never proved: not yet evidence the device will accept it.
+    expect(preferredAuth('deployed', true, true)).toBe('password');
  });

  it('uses the password only when no key is installed', () => {
@@ -0,0 +1,54 @@
+/**
+ * Run tasks with a ceiling on how many are in flight (issue #135).
+ *
+ * Firmware upgrades are not like bulk commands. A command is a short round
+ * trip, so CommandRunner can fire a whole wave at once with Promise.all. An
+ * upgrade holds an API connection for minutes across a download, a reboot and a
+ * verification — and connection pressure is precisely where this system has
+ * been shown to break. So concurrency is bounded rather than unlimited.
+ *
+ * Results are returned in input order regardless of completion order, because
+ * callers pair them with the devices they came from.
+ */
+export async function mapWithConcurrency<T, R>(
+  items: T[],
+  limit: number,
+  worker: (item: T, index: number) => Promise<R>
+): Promise<R[]> {
+  const max = Math.max(1, Math.floor(limit) || 1);
+  const results = new Array<R>(items.length);
+  let next = 0;
+
+  async function runner(): Promise<void> {
+    while (true) {
+      const i = next++;
+      if (i >= items.length) return;
+      results[i] = await worker(items[i], i);
+    }
+  }
+
+  // Never start more runners than there is work for them to do.
+  await Promise.all(
+    Array.from({ length: Math.min(max, items.length) }, () => runner())
+  );
+  return results;
+}
+
+/**
+ * Clamp an operator-supplied concurrency to something sane.
+ *
+ * Rejecting silly values matters more here than elsewhere: this number decides
+ * how many devices can be rebooting simultaneously.
+ */
+export const MAX_WAVE_CONCURRENCY = 20;
+
+export function clampConcurrency(raw: unknown): number {
+  const n = Number(raw);
+  // NaN means "not a number at all" -> fall back to sequential. Infinity means
+  // "as many as possible", so it clamps to the cap: treating it as unusable
+  // would quietly do the opposite of what was asked.
+  if (Number.isNaN(n)) return 1;
+  if (n === Infinity) return MAX_WAVE_CONCURRENCY;
+  if (n === -Infinity) return 1;
+  return Math.min(MAX_WAVE_CONCURRENCY, Math.max(1, Math.floor(n)));
+}
@@ -0,0 +1,117 @@
+/**
+ * Deciding which RouterOS log lines are actually new (issue #137).
+ *
+ * `/log/print` returns the device's whole ring buffer every time — commonly a
+ * thousand lines — so almost all of it is already stored. Getting this decision
+ * right is the difference between inserting a handful of rows and re-offering
+ * the entire buffer once a minute.
+ */
+
+import { createHash } from 'crypto';
+
+export interface RawLogLine {
+  '.id'?: string;
+  time?: string;
+  topics?: string;
+  message?: string;
+  [k: string]: unknown;
+}
+
+/** RouterOS log ids are hex strings like "*1A2F"; 0 means "no usable id". */
+export function parseRosId(id: string | undefined): number {
+  const hex = (id || '').replace(/^\*/, '');
+  if (!hex || !/^[0-9a-fA-F]+$/.test(hex)) return 0;
+  const n = parseInt(hex, 16);
+  return Number.isSafeInteger(n) && n > 0 ? n : 0;
+}
+
+/**
+ * The highest log id we have already stored.
+ *
+ * Taken as the maximum over recent rows rather than "the last row inserted".
+ * Those are usually the same, but not always — and when they differ the
+ * watermark goes backwards, every later line looks new, and the whole buffer is
+ * reprocessed on every poll.
+ */
+export function highestStoredId(storedIds: (string | null | undefined)[]): number {
+  let max = 0;
+  for (const id of storedIds) {
+    const n = parseRosId(id ?? undefined);
+    if (n > max) max = n;
+  }
+  return max;
+}
+
+/**
+ * Has the device's log buffer been cleared or the device rebooted?
+ *
+ * Detected by ids going backwards. When it happens the id watermark is
+ * meaningless and timestamps are the only thing left to compare.
+ */
+export function isLogReset(currentMaxId: number, lastStoredId: number): boolean {
+  return lastStoredId > 0 && currentMaxId > 0 && currentMaxId < lastStoredId;
+}
+
+/**
+ * Which lines should be inserted.
+ *
+ * `parseTime` is injected so the caller's RouterOS timestamp handling is used
+ * without this module needing to know about it.
+ */
+export function selectNewLogLines(
+  logs: RawLogLine[],
+  opts: {
+    lastStoredId: number;
+    latestStoredTime: Date;
+    parseTime: (raw: string) => Date;
+  }
+): RawLogLine[] {
+  const ids = logs.map((l) => parseRosId(l['.id'])).filter((n) => n > 0);
+  const currentMax = ids.length ? Math.max(...ids) : 0;
+  const reset = isLogReset(currentMax, opts.lastStoredId);
+
+  const out: RawLogLine[] = [];
+  for (const log of logs) {
+    const id = parseRosId(log['.id']);
+    if (id > 0 && !reset) {
+      if (id <= opts.lastStoredId) continue;
+    } else {
+      // No usable id, or the buffer reset: fall back to timestamps.
+      if (opts.parseTime(log.time || '') <= opts.latestStoredTime) continue;
+    }
+    out.push(log);
+  }
+  return out;
+}
+
+/**
+ * A stable identifier for a log line that RouterOS gave no `.id`.
+ *
+ * The events table has a unique index on (device_id, log_id) and relies on it
+ * to reject lines already stored. PostgreSQL treats NULLs as *distinct* in a
+ * unique index, so a NULL log_id conflicts with nothing: a device whose log
+ * entries carry no `.id` had no protection at all and re-inserted the same
+ * lines on every poll, without bound.
+ *
+ * Deriving the key from the line's own content restores that protection. The
+ * `#` prefix keeps it out of the space of real RouterOS ids, which are `*`
+ * followed by hex, so the two can never collide or be mistaken for each other.
+ *
+ * The trade, stated plainly: two genuinely separate lines with the same
+ * timestamp, topic and text collapse into one. RouterOS timestamps are
+ * second-resolution, so that is possible. Losing a repeat of an identical
+ * message is a far smaller loss than growing the table by the whole buffer
+ * every minute for ever.
+ */
+export function surrogateLogId(time: string, topics: string, message: string): string {
+  const hash = createHash('sha256')
+    .update(`${time}\u0000${topics}\u0000${message}`)
+    .digest('hex')
+    .slice(0, 16);
+  return `#${hash}`;           // 17 chars, inside the column's VARCHAR(20)
+}
+
+/** True for the ids this module synthesised, as opposed to RouterOS's own. */
+export function isSurrogateLogId(id: string | null | undefined): boolean {
+  return typeof id === 'string' && id.startsWith('#');
+}
@@ -96,13 +96,19 @@ export type KeyStatus = 'pending' | 'deployed' | 'verified' | 'failed';
 *
 * The real recovery path is the binary API, which is unaffected by any of this
 * and can always remove the key.
+ *
+ * Only a *verified* key counts. One that was generated or pushed but never
+ * authenticated is not evidence the device will accept it, and 'deployed' is a
+ * transient state on the way to being proved. This function previously admitted
+ * it, which disagreed with the resolver that actually ran -- the disagreement
+ * went unnoticed because nothing outside these tests ever called this (#133).
 */
 export function preferredAuth(
  status: KeyStatus | null,
  hasPrivateKey: boolean,
  hasPassword: boolean,
 ): 'key' | 'password' | 'none' {
-  if (hasPrivateKey && (status === 'verified' || status === 'deployed')) return 'key';
+  if (hasPrivateKey && status === 'verified') return 'key';
  if (hasPassword) return 'password';
  return 'none';
 }
@@ -14,6 +14,8 @@ whether to try it, the [project README](https://github.com/2GT-Media-Group-LLC/m
 | [Cellular (LTE)](cellular.md) | Signal, carriers, tower movement, and the data-cap reset SMS |
 | [SSH keys](ssh-keys.md) | Per-device keypairs, rotation, and what installing one costs |
 | [Bulk commands](commands.md) | Running one command across a fleet, in waves |
+| [Firmware rollouts](firmware.md) | Waves, the upgrade pipeline, and what upgrading several at once costs |
+| [Backups](backups.md) | Finding, reading, comparing and deleting configuration backups |
 | [Polling and scaling](scaling.md) | Poller health, headroom, tuning, and clearing a backlog |
 | [API and automation](api.md) | Scoped tokens, authentication, and driving the platform from scripts |
 | [Architecture](architecture.md) | How the system is put together, for contributors |
@@ -0,0 +1,85 @@
+# Backups
+
+[← Documentation index](README.md)
+
+Configuration backups for every device, on demand or on a schedule, with the tools to
+find, read and compare them.
+
+## What a backup actually is
+
+`/export compact` run over SSH, stored as a `.rsc` file — **plain RouterOS script, not
+the binary `.backup` blob**.
+
+That distinction shapes everything else on this page. A `.rsc` can be read, diffed,
+edited and applied selectively; a `.backup` can only be restored wholesale onto the
+same device. It also means a backup is human-readable text, which is why previewing
+and comparing them is possible at all.
+
+It also means a backup does **not** capture things `/export` omits: user passwords,
+certificates and files on disk. A `.rsc` restores a configuration, not a device.
+
+## Finding one
+
+The table filters by **device**, **type**, a **date range** and free text across
+device name, filename and notes. Date ranges include both ends — "to: 3 September"
+covers everything that happened on the 3rd.
+
+Types:
+
+| Type | Created by |
+|---|---|
+| `manual` | The Create Backup button |
+| `scheduled` | The daily/weekly/monthly schedule |
+| `pre-upgrade` | A firmware rollout, before it touches the device |
+| `config-snapshot` | Config History, when it captures a change |
+
+## Reading and comparing
+
+Click any row to read the backup in place. Select exactly two and choose **Compare**
+for a line diff — the same diff Config History uses for snapshots.
+
+The comparison is always ordered oldest to newest regardless of the order you clicked,
+so additions read as additions.
+
+The **filename** is not a column in the table; it appears in the preview and diff
+headers instead. It is a device name and a timestamp — useful when you are looking at
+a specific file or quoting it to someone, less useful as a column competing for width
+with the device and date you actually filter on.
+
+## Deleting, and the one thing to know first
+
+A backup and its Config History snapshot are **one artifact**. They are joined in the
+database with `ON DELETE CASCADE` precisely so they cannot drift apart — a snapshot
+whose restorable `.rsc` had been deleted would offer a rollback it could not perform.
+
+So deleting a `config-snapshot` backup also removes that snapshot from the device's
+Config History, and the ability to roll back to it.
+
+Single deletes of a snapshot-linked backup ask for confirmation and say this plainly.
+**Bulk delete** does the same, but with the numbers: it reports how many backups per
+device will go, and how many Config History snapshots will go with them, before you
+confirm. There is no undo.
+
+Bulk delete is capped at 200 per action and is scoped to the [current site](sites.md),
+so it cannot reach into another site's history.
+
+## Restoring
+
+Restore uploads the `.rsc` and applies it. This is a real configuration change with
+all the risk that implies — read the backup first if you are not certain what is in
+it, which is what the preview is for.
+
+## API
+
+| Method | Path | Purpose |
+|---|---|---|
+| `GET` | `/api/backups` | List, with `deviceId`, `type`, `from`, `to`, `search` |
+| `GET` | `/api/backups/types` | Types present, with counts |
+| `POST` | `/api/backups` | Create one for a device |
+| `GET` | `/api/backups/:id/content` | Read the text (capped at 2 MB) |
+| `GET` | `/api/backups/:from/diff/:to` | Both texts, for a client-side diff |
+| `GET` | `/api/backups/:id/download` | Download the `.rsc` |
+| `POST` | `/api/backups/:id/restore` | Apply it to the device |
+| `DELETE` | `/api/backups/:id` | Delete one |
+| `GET` | `/api/backups/bulk-delete/preview` | What a bulk delete would remove |
+| `POST` | `/api/backups/bulk-delete` | Delete many |
@@ -0,0 +1,104 @@
+# Firmware rollouts
+
+[← Documentation index](README.md)
+
+Upgrade RouterOS across a fleet in waves, with a verified pipeline per device and
+a stopping point before a bad build reaches everything.
+
+## The pipeline
+
+Each device goes through the same sequence:
+
+1. **Pre-upgrade backup** (optional, on by default)
+2. **Check for updates** — a device already on the target version is *skipped*, not failed
+3. **Download the image, and confirm it landed**
+4. **Reboot**
+5. **Prove the reboot happened** — uptime must have gone *backwards*
+6. **Verify the version moved**
+7. **RouterBOOT** (optional) — a second flash and a second reboot
+
+Steps 3 and 5 exist because of a specific failure. `/system/package/update/install`
+bundles download and reboot into one call whose progress cannot be observed, and a
+CCR2216 was seen to accept it, do neither, and report nothing. That surfaced as
+"rebooted but still reports 7.24.1", because nothing could tell a device that never
+restarted from one that restarted unchanged.
+
+Downloading first makes success verifiable *before* anything reboots, so a device is
+never restarted for an image it does not have.
+
+!!! note "Downloads are slow, and that is normal"
+    `/system/package/update/download` blocks until the image has landed — a minute or
+    more on an ordinary link. It is given a ten-minute budget. If the command does not
+    return cleanly the device is asked again on a fresh connection what actually
+    happened, rather than being declared failed: a slow download is not a broken one.
+
+## Waves
+
+Waves are the point. Devices are assigned to wave 1, 2 or 3, and **waves run strictly
+in sequence**. Wave 1 is your canary: put one device in it, let it finish, and the
+rest of the fleet is still untouched if it goes wrong.
+
+**Halt on failure** stops the rollout rather than continuing into the next wave.
+Devices never reached are marked `skipped`, not `failed` — they did not run.
+
+A device that comes back on the *old* version counts as a failure.
+
+## Upgrading several devices at once
+
+By default the devices inside a wave upgrade **one at a time**. That is safe and slow:
+a CRS switch can take five minutes to reboot, so ten of them is most of an hour spent
+waiting.
+
+The **At once** control raises how many devices in the same wave may upgrade
+simultaneously. Waves remain sequential either way, so the canary still gates
+everything after it.
+
+There are two costs, and both are worth understanding before you raise it.
+
+**Halting becomes less precise.** A failure stops any further device from *starting*,
+immediately — but devices already running finish, because interrupting one mid-write is
+how you brick it. So with **At once** set to five, up to five devices can carry a bad
+build before anything stops. At the default of 1 nothing else has started, so a failure
+stops the rollout exactly as it always has.
+
+**Devices reboot together.** That is fine for independent routers. It is not fine for
+a switch that the others are reached *through*: rebooting it alongside them cuts the
+path to devices that are mid-upgrade. Change Guard does not cover this — it protects
+configuration, and a device that is unreachable because its uplink rebooted has no
+configuration change to revert.
+
+The rollout builder warns when it can see that one selected device carries another's
+traffic. That warning is deliberately conservative: it fires only when a link is the
+device's spanning-tree root port *and* that port resolves to exactly one neighbour.
+Discovered topology is often ambiguous — a trunk port can resolve to several
+neighbours at once — and naming the wrong switch would be worse than saying nothing.
+**Absence of a warning is not proof that it is safe to reboot everything together.**
+
+A reasonable pattern for a large fleet on a well-known patch release:
+
+| Wave | Devices | At once |
+|---|---|---|
+| 1 | one representative device | 1 |
+| 2 | the access layer | 5–10 |
+| 3 | anything upstream of wave 2 | 1 |
+
+## Scheduling and cancelling
+
+A rollout can be scheduled; pair it with a maintenance window by scheduling inside
+one. One rollout runs at a time.
+
+Devices already belonging to an unfinished rollout are refused by name when you try
+to create another for them, before anything is written.
+
+**Cancelling never interrupts an in-flight flash.** It stops before the *next* device,
+because interrupting a device mid-write is how you brick it. The UI says so when you
+press the button — the currently upgrading device will finish first.
+
+## When it fails
+
+Failure messages name the mechanism rather than the symptom. "Device did not finish
+downloading within ten minutes. Nothing was rebooted. The device reports 91.6 MB free"
+tells you where to look; "upgrade failed" does not.
+
+If a rollout reports a device stuck at the old version, check the device's own
+`/system/package/update` state before re-running — it may have the image already.
@@ -1,6 +1,6 @@
 {
  "name": "mikrotik-manager-frontend",
-  "version": "0.24.0-beta",
+  "version": "0.24.5-beta",
  "private": true,
  "type": "module",
  "scripts": {
@@ -0,0 +1,172 @@
+import { useQuery } from '@tanstack/react-query';
+import { X, FileText, GitCompare, Download, Loader2, AlertCircle } from 'lucide-react';
+import { format } from 'date-fns';
+import clsx from 'clsx';
+import { backupsApi } from '../../services/api';
+import { lineDiff } from '../../utils/lineDiff';
+
+/**
+ * Reads a backup in place, or compares two (#134).
+ *
+ * Backups are `/export compact` output — plain RouterOS script, not the binary
+ * `.backup` blob — so both are just text. The diff reuses the same lineDiff
+ * that Config History already uses, rather than introducing a second one.
+ */
+interface Props {
+  /** One id previews; two compare, oldest first. */
+  ids: number[];
+  onClose: () => void;
+  onDownload: (id: number, filename: string) => void;
+}
+
+function errorText(e: unknown): string {
+  const err = e as { response?: { data?: { error?: string } }; message?: string };
+  return err.response?.data?.error ?? err.message ?? 'Could not read this backup.';
+}
+
+export default function BackupViewerModal({ ids, onClose, onDownload }: Props) {
+  const comparing = ids.length === 2;
+
+  const single = useQuery({
+    queryKey: ['backup-content', ids[0]],
+    queryFn: () => backupsApi.content(ids[0]).then((r) => r.data),
+    enabled: !comparing && ids.length === 1,
+    retry: false,
+  });
+
+  const pair = useQuery({
+    queryKey: ['backup-diff', ids[0], ids[1]],
+    queryFn: () => backupsApi.diff(ids[0], ids[1]).then((r) => r.data),
+    enabled: comparing,
+    retry: false,
+  });
+
+  const loading = comparing ? pair.isLoading : single.isLoading;
+  const error = comparing ? pair.error : single.error;
+
+  const rows = pair.data ? lineDiff(pair.data.from.text, pair.data.to.text) : [];
+  const added = rows.filter((r) => r.type === 'add').length;
+  const removed = rows.filter((r) => r.type === 'del').length;
+
+  return (
+    <div
+      className="fixed inset-0 z-[60] flex items-center justify-center bg-black/50 backdrop-blur-sm p-4"
+      onClick={onClose}
+    >
+      <div
+        className="card w-full max-w-5xl max-h-[90vh] flex flex-col"
+        onClick={(e) => e.stopPropagation()}
+      >
+        <div className="flex items-start gap-3 p-4 border-b border-gray-200 dark:border-slate-700">
+          {comparing
+            ? <GitCompare className="w-5 h-5 text-blue-500 mt-0.5 flex-shrink-0" />
+            : <FileText className="w-5 h-5 text-blue-500 mt-0.5 flex-shrink-0" />}
+          <div className="min-w-0 flex-1">
+            {comparing ? (
+              <>
+                <h3 className="text-sm font-semibold text-gray-900 dark:text-white">
+                  Comparing backups
+                </h3>
+                {pair.data && (
+                  // The filename lives here rather than in the table: it is how you
+                  // would refer to this file to someone else, so it belongs where
+                  // you are actually looking at the file.
+                  <p className="text-xs font-mono text-gray-500 dark:text-slate-400 break-all mt-0.5">
+                    {pair.data.from.filename} → {pair.data.to.filename}
+                  </p>
+                )}
+              </>
+            ) : (
+              <>
+                <h3 className="text-sm font-semibold text-gray-900 dark:text-white">
+                  {single.data?.device_name ?? 'Backup'}
+                </h3>
+                {single.data && (
+                  <p className="text-xs font-mono text-gray-500 dark:text-slate-400 break-all mt-0.5">
+                    {single.data.filename}
+                  </p>
+                )}
+              </>
+            )}
+            {!comparing && single.data && (
+              <p className="text-xs text-gray-400 dark:text-slate-500 mt-0.5">
+                {format(new Date(single.data.created_at), 'MMM d, yyyy HH:mm')}
+                {single.data.notes ? ` · ${single.data.notes}` : ''}
+              </p>
+            )}
+            {comparing && pair.data && (
+              <p className="text-xs mt-0.5">
+                <span className="text-green-600 dark:text-green-400">+{added}</span>{' '}
+                <span className="text-red-600 dark:text-red-400">−{removed}</span>
+                <span className="text-gray-400 dark:text-slate-500">
+                  {' '}· {format(new Date(pair.data.from.created_at), 'MMM d HH:mm')}
+                  {' → '}{format(new Date(pair.data.to.created_at), 'MMM d HH:mm')}
+                </span>
+              </p>
+            )}
+          </div>
+          {!comparing && single.data && (
+            <button
+              onClick={() => onDownload(single.data!.id, single.data!.filename)}
+              className="p-1.5 rounded text-gray-400 hover:text-blue-500 hover:bg-blue-50 dark:hover:bg-blue-900/20"
+              title="Download"
+            >
+              <Download className="w-4 h-4" />
+            </button>
+          )}
+          <button
+            onClick={onClose}
+            className="p-1.5 rounded text-gray-400 hover:text-gray-600 dark:hover:text-slate-300"
+          >
+            <X className="w-5 h-5" />
+          </button>
+        </div>
+
+        <div className="flex-1 overflow-auto p-4">
+          {loading && (
+            <div className="flex items-center justify-center gap-2 py-16 text-gray-400">
+              <Loader2 className="w-4 h-4 animate-spin" /> Reading…
+            </div>
+          )}
+
+          {!loading && error && (
+            <div className="flex items-start gap-2 p-3 rounded-lg bg-red-50 dark:bg-red-900/20 border border-red-200 dark:border-red-800">
+              <AlertCircle className="w-4 h-4 text-red-500 mt-0.5 flex-shrink-0" />
+              <p className="text-sm text-red-600 dark:text-red-400">{errorText(error)}</p>
+            </div>
+          )}
+
+          {!loading && !error && comparing && (
+            added + removed === 0 ? (
+              <p className="text-sm text-gray-400 py-8 text-center">
+                These two backups are identical.
+              </p>
+            ) : (
+              <pre className="text-xs font-mono bg-gray-50 dark:bg-slate-900/60 border border-gray-200 dark:border-slate-700 rounded-lg p-3 leading-relaxed">
+                {rows.map((row, i) => (
+                  <div
+                    key={i}
+                    className={clsx(
+                      'whitespace-pre-wrap',
+                      row.type === 'add' && 'bg-green-50 dark:bg-green-900/20 text-green-700 dark:text-green-400',
+                      row.type === 'del' && 'bg-red-50 dark:bg-red-900/20 text-red-700 dark:text-red-400',
+                      row.type === 'ctx' && 'text-gray-500 dark:text-slate-400'
+                    )}
+                  >
+                    {row.type === 'add' ? '+' : row.type === 'del' ? '−' : ' '} {row.text}
+                  </div>
+                ))}
+              </pre>
+            )
+          )}
+
+          {!loading && !error && !comparing && single.data && (
+            <pre className="text-xs font-mono bg-gray-50 dark:bg-slate-900/60 border border-gray-200 dark:border-slate-700 rounded-lg p-3 leading-relaxed whitespace-pre-wrap">
+              {single.data.content}
+            </pre>
+          )}
+        </div>
+      </div>
+    </div>
+  );
+}
@@ -1,12 +1,25 @@
 import { useState } from 'react';
 import { useQuery, useMutation, useQueryClient } from '@tanstack/react-query';
-import { HardDrive, Plus, Download, RotateCcw, Trash2, AlertCircle, AlertTriangle, Loader2, X } from 'lucide-react';
+import {
+  HardDrive, Plus, Download, RotateCcw, Trash2, AlertCircle, AlertTriangle,
+  Loader2, X, Search, GitCompare, FileText, FilterX,
+} from 'lucide-react';
 import { backupsApi, devicesApi } from '../services/api';
 import { useCanWrite } from '../hooks/useCanWrite';
 import type { Backup } from '../types';
+import BackupViewerModal from '../components/backups/BackupViewerModal';
+import { orderForDiff } from '../utils/backupCompare';
 import { format } from 'date-fns';
 import clsx from 'clsx';

+/** One home for backup-type wording, used by the filter and the table badge. */
+const TYPE_LABEL: Record<string, string> = {
+  'config-snapshot': 'Config Snapshot',
+  scheduled: 'Scheduled',
+  manual: 'Manual',
+  'pre-upgrade': 'Pre-upgrade',
+};
+
 function formatBytes(bytes?: number): string {
  if (!bytes) return '—';
  if (bytes >= 1024) return `${(bytes / 1024).toFixed(1)} KB`;
@@ -25,11 +38,63 @@ export default function BackupsPage() {
  const [snapshotDeleteTarget, setSnapshotDeleteTarget] = useState<Backup | null>(null);
  const [error, setError] = useState('');

+  // Filters (#134). Held together so the query key is a single object.
+  const [filters, setFilters] = useState({ deviceId: '', type: '', from: '', to: '', search: '' });
+  const activeFilterCount = Object.values(filters).filter(Boolean).length;
+  const clearFilters = () => setFilters({ deviceId: '', type: '', from: '', to: '', search: '' });
+
+  const [selected, setSelected] = useState<number[]>([]);
+  const [viewing, setViewing] = useState<number[] | null>(null);
+  const [bulkConfirm, setBulkConfirm] = useState(false);
+
  const { data: backups = [], isLoading } = useQuery({
-    queryKey: ['backups'],
-    queryFn: () => backupsApi.list().then((r) => r.data),
+    queryKey: ['backups', filters],
+    queryFn: () => backupsApi.list(filters).then((r) => r.data),
  });

+  const { data: types = [] } = useQuery({
+    queryKey: ['backup-types'],
+    queryFn: () => backupsApi.types().then((r) => r.data),
+  });
+
+  // What a bulk delete would actually remove, including the config snapshots
+  // that go with it. Fetched before asking, so the confirmation can be specific
+  // rather than "are you sure?".
+  const { data: bulkPreview } = useQuery({
+    queryKey: ['backup-bulk-preview', selected],
+    queryFn: () => backupsApi.bulkDeletePreview(selected).then((r) => r.data),
+    enabled: bulkConfirm && selected.length > 0,
+  });
+
+  const bulkDeleteMutation = useMutation({
+    mutationFn: () => backupsApi.bulkDelete(selected).then((r) => r.data),
+    onSuccess: (res) => {
+      queryClient.invalidateQueries({ queryKey: ['backups'] });
+      queryClient.invalidateQueries({ queryKey: ['backup-types'] });
+      setBulkConfirm(false);
+      setSelected([]);
+      if (res.failures.length > 0) {
+        setError(`${res.deleted} deleted, ${res.failures.length} failed.`);
+      }
+    },
+    onError: (err: unknown) => {
+      const msg = (err as { response?: { data?: { error?: string } } })?.response?.data?.error;
+      setError(msg || 'Bulk delete failed');
+      setBulkConfirm(false);
+    },
+  });
+
+  const toggle = (id: number) =>
+    setSelected((cur) => (cur.includes(id) ? cur.filter((x) => x !== id) : [...cur, id]));
+  const allShownSelected = backups.length > 0 && backups.every((b) => selected.includes(b.id));
+  const toggleAll = () =>
+    setSelected(allShownSelected ? [] : backups.map((b) => b.id));
+
+  // Exactly two, oldest first, so the diff reads forwards in time (tested in
+  // utils/backupCompare — a reversed diff shows additions as deletions and
+  // looks perfectly normal while doing it).
+  const comparePair = orderForDiff(selected, backups);
+
  const { data: devices = [] } = useQuery({
    queryKey: ['devices'],
    queryFn: () => devicesApi.list().then((r) => r.data),
@@ -169,16 +234,110 @@ export default function BackupsPage() {
        </div>
      )}

+      {/* Filters (#134) */}
+      <div className="card p-3 flex flex-wrap items-end gap-2">
+        <div className="flex-1 min-w-[180px]">
+          <label className="label text-[11px]">Search</label>
+          <div className="relative">
+            <Search className="w-3.5 h-3.5 absolute left-2.5 top-1/2 -translate-y-1/2 text-gray-400" />
+            <input
+              className="input pl-8"
+              placeholder="Device, filename or note…"
+              value={filters.search}
+              onChange={(e) => setFilters((f) => ({ ...f, search: e.target.value }))}
+            />
+          </div>
+        </div>
+        <div className="min-w-[150px]">
+          <label className="label text-[11px]">Device</label>
+          <select
+            className="input"
+            value={filters.deviceId}
+            onChange={(e) => setFilters((f) => ({ ...f, deviceId: e.target.value }))}
+          >
+            <option value="">All devices</option>
+            {devices.map((d) => <option key={d.id} value={d.id}>{d.name}</option>)}
+          </select>
+        </div>
+        <div className="min-w-[140px]">
+          <label className="label text-[11px]">Type</label>
+          <select
+            className="input"
+            value={filters.type}
+            onChange={(e) => setFilters((f) => ({ ...f, type: e.target.value }))}
+          >
+            <option value="">All types</option>
+            {types.map((t) => (
+              <option key={t.type} value={t.type}>{TYPE_LABEL[t.type] ?? t.type} ({t.count})</option>
+            ))}
+          </select>
+        </div>
+        <div>
+          <label className="label text-[11px]">From</label>
+          <input type="date" className="input"
+            value={filters.from}
+            onChange={(e) => setFilters((f) => ({ ...f, from: e.target.value }))} />
+        </div>
+        <div>
+          <label className="label text-[11px]">To</label>
+          <input type="date" className="input"
+            value={filters.to}
+            onChange={(e) => setFilters((f) => ({ ...f, to: e.target.value }))} />
+        </div>
+        {activeFilterCount > 0 && (
+          <button onClick={clearFilters} className="btn-secondary flex items-center gap-1.5 text-xs">
+            <FilterX className="w-3.5 h-3.5" /> Clear
+          </button>
+        )}
+      </div>
+
+      {/* Selection actions */}
+      {selected.length > 0 && (
+        <div className="card px-4 py-2.5 flex flex-wrap items-center gap-3 border-blue-200 dark:border-blue-800">
+          <span className="text-sm font-medium text-gray-900 dark:text-white">
+            {selected.length} selected
+          </span>
+          <button onClick={() => setSelected([])} className="text-xs text-gray-400 hover:text-gray-600">
+            Clear
+          </button>
+          <div className="ml-auto flex items-center gap-2">
+            <button
+              onClick={() => comparePair && setViewing(comparePair)}
+              disabled={!comparePair}
+              title={comparePair ? 'Compare the two selected backups' : 'Select exactly two backups to compare'}
+              className="btn-secondary flex items-center gap-1.5 text-xs disabled:opacity-40"
+            >
+              <GitCompare className="w-3.5 h-3.5" /> Compare
+            </button>
+            {canWrite && (
+              <button
+                onClick={() => setBulkConfirm(true)}
+                className="btn-danger flex items-center gap-1.5 text-xs"
+              >
+                <Trash2 className="w-3.5 h-3.5" /> Delete {selected.length}
+              </button>
+            )}
+          </div>
+        </div>
+      )}
+
      {isLoading ? (
        <div className="flex items-center justify-center h-48 text-gray-400">Loading...</div>
      ) : backups.length === 0 ? (
        <div className="card p-12 flex flex-col items-center gap-4 text-center">
          <HardDrive className="w-16 h-16 text-gray-300 dark:text-slate-600" />
          <div>
-            <p className="font-medium text-gray-700 dark:text-slate-300">No backups yet</p>
-            <p className="text-sm text-gray-400 dark:text-slate-500 mt-1">
-              Create a backup to save your device configurations
+            <p className="font-medium text-gray-700 dark:text-slate-300">
+              {activeFilterCount > 0 ? 'No backups match these filters' : 'No backups yet'}
            </p>
+            <p className="text-sm text-gray-400 dark:text-slate-500 mt-1">
+              {activeFilterCount > 0
+                ? 'Try widening the date range or clearing a filter.'
+                : 'Create a backup to save your device configurations'}
+            </p>
+            {activeFilterCount > 0 && (
+              <button onClick={clearFilters} className="btn-secondary mt-3 text-xs">Clear filters</button>
+            )}
          </div>
        </div>
      ) : (
@@ -187,24 +346,46 @@ export default function BackupsPage() {
          <table className="w-full text-sm">
            <thead>
              <tr className="border-b border-gray-200 dark:border-slate-700">
+                <th className="table-header px-3 py-2.5 w-9">
+                  <input
+                    type="checkbox"
+                    aria-label="Select all shown"
+                    checked={allShownSelected}
+                    onChange={toggleAll}
+                    className="cursor-pointer"
+                  />
+                </th>
                <th className="table-header px-4 py-2.5 text-left">Device</th>
-                <th className="table-header px-4 py-2.5 text-left">Filename</th>
                <th className="table-header px-4 py-2.5 text-left">Type</th>
                <th className="table-header px-4 py-2.5 text-left">Size</th>
                <th className="table-header px-4 py-2.5 text-left">Created</th>
                <th className="table-header px-4 py-2.5 text-left">Notes</th>
-                <th className="table-header px-4 py-2.5 text-left w-32">Actions</th>
+                <th className="table-header px-4 py-2.5 text-left w-36">Actions</th>
              </tr>
            </thead>
            <tbody className="divide-y divide-gray-100 dark:divide-slate-700 table-zebra">
              {backups.map((backup) => (
-                <tr key={backup.id} className="hover:bg-gray-50 dark:hover:bg-slate-700/30">
+                <tr
+                  key={backup.id}
+                  onClick={() => setViewing([backup.id])}
+                  title="Click to preview"
+                  className={clsx(
+                    'cursor-pointer hover:bg-gray-50 dark:hover:bg-slate-700/30',
+                    selected.includes(backup.id) && 'bg-blue-50/60 dark:bg-blue-900/10'
+                  )}
+                >
+                  <td className="px-3 py-2.5" onClick={(e) => e.stopPropagation()}>
+                    <input
+                      type="checkbox"
+                      aria-label={`Select ${backup.filename}`}
+                      checked={selected.includes(backup.id)}
+                      onChange={() => toggle(backup.id)}
+                      className="cursor-pointer"
+                    />
+                  </td>
                  <td className="px-4 py-2.5 font-medium text-gray-900 dark:text-white">
                    {backup.device_name || '—'}
                  </td>
-                  <td className="px-4 py-2.5 font-mono text-xs text-gray-500 dark:text-slate-400">
-                    {backup.filename}
-                  </td>
                  <td className="px-4 py-2.5">
                    <span
                      className={clsx(
@@ -222,11 +403,7 @@ export default function BackupsPage() {
                          : undefined
                      }
                    >
-                      {backup.backup_type === 'config-snapshot'
-                        ? 'Config Snapshot'
-                        : backup.backup_type === 'scheduled'
-                          ? 'Scheduled'
-                          : 'Manual'}
+                      {TYPE_LABEL[backup.backup_type ?? ''] ?? backup.backup_type ?? 'Manual'}
                    </span>
                  </td>
                  <td className="px-4 py-2.5 text-gray-500 dark:text-slate-400">
@@ -238,8 +415,15 @@ export default function BackupsPage() {
                  <td className="px-4 py-2.5 text-xs text-gray-400 dark:text-slate-500 italic">
                    {backup.notes || ''}
                  </td>
-                  <td className="px-4 py-2.5">
+                  <td className="px-4 py-2.5" onClick={(e) => e.stopPropagation()}>
                    <div className="flex items-center gap-1">
+                      <button
+                        onClick={() => setViewing([backup.id])}
+                        className="p-1.5 rounded text-gray-400 hover:text-blue-500 hover:bg-blue-50 dark:hover:bg-blue-900/20 transition-colors"
+                        title="Preview"
+                      >
+                        <FileText className="w-3.5 h-3.5" />
+                      </button>
                      <button
                        onClick={() => downloadBackup(backup.id, backup.filename)}
                        className="p-1.5 rounded text-gray-400 hover:text-blue-500 hover:bg-blue-50 dark:hover:bg-blue-900/20 transition-colors"
@@ -312,6 +496,97 @@ export default function BackupsPage() {
        </div>
      )}

+      {viewing && (
+        <BackupViewerModal
+          ids={viewing}
+          onClose={() => setViewing(null)}
+          onDownload={downloadBackup}
+        />
+      )}
+
+      {/* Bulk delete confirmation.
+          Backups and their config snapshots are deliberately one artifact, so a
+          bulk delete can remove restore points the operator never looked at.
+          The confirmation therefore names exactly what goes, per device, and
+          says plainly that there is no undo. */}
+      {bulkConfirm && (
+        <div className="fixed inset-0 z-[60] flex items-center justify-center bg-black/50 backdrop-blur-sm p-4">
+          <div className="card w-full max-w-lg">
+            <div className="flex items-center justify-between p-5 border-b border-gray-200 dark:border-slate-700">
+              <h3 className="text-lg font-semibold text-gray-900 dark:text-white flex items-center gap-2">
+                <AlertTriangle className="w-5 h-5 text-amber-500" />
+                Delete {selected.length} backup{selected.length === 1 ? '' : 's'}?
+              </h3>
+              <button
+                onClick={() => setBulkConfirm(false)}
+                disabled={bulkDeleteMutation.isPending}
+                className="p-1 rounded text-gray-400 hover:text-gray-600 dark:hover:text-slate-300"
+              >
+                <X className="w-5 h-5" />
+              </button>
+            </div>
+            <div className="p-5 space-y-4">
+              {!bulkPreview ? (
+                <div className="flex items-center gap-2 text-sm text-gray-400 py-2">
+                  <Loader2 className="w-4 h-4 animate-spin" /> Checking what this removes…
+                </div>
+              ) : (
+                <>
+                  <div className="text-sm text-gray-600 dark:text-slate-300">
+                    <ul className="space-y-1">
+                      {bulkPreview.devices.map((d) => (
+                        <li key={d.name} className="flex justify-between gap-4">
+                          <span className="text-gray-900 dark:text-white">{d.name}</span>
+                          <span className="text-gray-400">{d.count} backup{d.count === 1 ? '' : 's'}</span>
+                        </li>
+                      ))}
+                    </ul>
+                  </div>
+
+                  {bulkPreview.snapshots > 0 && (
+                    <div className="rounded-lg border border-amber-300 bg-amber-50 dark:bg-amber-900/20 dark:border-amber-700 p-3">
+                      <p className="text-sm text-amber-800 dark:text-amber-300">
+                        <span className="font-semibold">{bulkPreview.snapshots}</span> of these
+                        {bulkPreview.snapshots === 1 ? ' is' : ' are'} tied to a{' '}
+                        <span className="font-medium">Config History snapshot</span>. Deleting
+                        {bulkPreview.snapshots === 1 ? ' it' : ' them'} also removes
+                        {bulkPreview.snapshots === 1 ? ' that snapshot' : ' those snapshots'} and the
+                        ability to roll back to {bulkPreview.snapshots === 1 ? 'it' : 'them'}.
+                      </p>
+                    </div>
+                  )}
+
+                  <p className="text-sm text-gray-500 dark:text-slate-400">
+                    This cannot be undone.
+                  </p>
+                </>
+              )}
+
+              <div className="flex items-center justify-end gap-2 pt-1">
+                <button
+                  onClick={() => setBulkConfirm(false)}
+                  disabled={bulkDeleteMutation.isPending}
+                  className="btn-secondary"
+                >
+                  Cancel
+                </button>
+                <button
+                  onClick={() => bulkDeleteMutation.mutate()}
+                  disabled={bulkDeleteMutation.isPending || !bulkPreview}
+                  className="btn-danger inline-flex items-center gap-2 disabled:opacity-50"
+                >
+                  {bulkDeleteMutation.isPending
+                    ? <Loader2 className="w-4 h-4 animate-spin" />
+                    : <Trash2 className="w-4 h-4" />}
+                  Delete {bulkPreview?.backups ?? selected.length}
+                  {bulkPreview && bulkPreview.snapshots > 0 ? ' + snapshots' : ''}
+                </button>
+              </div>
+            </div>
+          </div>
+        </div>
+      )}
+
      {/* Confirm deletion of a snapshot-linked backup (removes the config snapshot too) */}
      {snapshotDeleteTarget && (
        <div className="fixed inset-0 z-[60] flex items-center justify-center bg-black/50 backdrop-blur-sm">
@@ -2,7 +2,7 @@ import { useMemo, useState } from 'react';
 import { useQuery, useMutation, useQueryClient } from '@tanstack/react-query';
 import {
  ArrowUpCircle, RefreshCw, CheckCircle, XCircle, AlertTriangle, Clock,
-  HardDrive, ShieldAlert, Rocket, Ban, ChevronRight, Cpu,
+  HardDrive, ShieldAlert, Rocket, Ban, ChevronRight, Cpu, Zap,
 } from 'lucide-react';
 import clsx from 'clsx';
 import { firmwareApi } from '../services/api';
@@ -48,9 +48,22 @@ function RolloutPanel({ rolloutId, canWrite }: { rolloutId: number; canWrite: bo
    },
  });

+  // The API answers a cancel with a note explaining that the device currently
+  // being upgraded is never interrupted mid-write. That note was discarded, so
+  // pressing Cancel looked like it did nothing at all (#138).
+  const [cancelNote, setCancelNote] = useState<string | null>(null);
  const cancelMutation = useMutation({
-    mutationFn: () => firmwareApi.cancelRollout(rolloutId),
-    onSuccess: () => qc.invalidateQueries({ queryKey: ['fw-rollout', rolloutId] }),
+    mutationFn: () => firmwareApi.cancelRollout(rolloutId).then(r => r.data),
+    onSuccess: (data) => {
+      setCancelNote(
+        data?.note ?? 'Rollout cancelled. No further devices will be upgraded.'
+      );
+      qc.invalidateQueries({ queryKey: ['fw-rollout', rolloutId] });
+    },
+    onError: (e: unknown) => {
+      const err = e as { response?: { data?: { error?: string } } };
+      setCancelNote(err.response?.data?.error ?? 'Could not cancel the rollout.');
+    },
  });

  if (!rollout) return null;
@@ -74,15 +87,28 @@ function RolloutPanel({ rolloutId, canWrite }: { rolloutId: number; canWrite: bo
          {rollout.pre_backup && <span className="flex items-center gap-1"><HardDrive className="w-3 h-3" />pre-backup</span>}
          {rollout.halt_on_failure && <span className="flex items-center gap-1"><ShieldAlert className="w-3 h-3" />halt on failure</span>}
          {rollout.routerboot_after && <span className="flex items-center gap-1"><Cpu className="w-3 h-3" />+ RouterBOOT</span>}
+          {(rollout.wave_concurrency ?? 1) > 1 && (
+            <span className="flex items-center gap-1" title="Devices in a wave upgrading at once">
+              <Zap className="w-3 h-3" />{rollout.wave_concurrency} at once
+            </span>
+          )}
          {canWrite && active && (
-            <button onClick={() => cancelMutation.mutate()} disabled={cancelMutation.isPending}
-              className="flex items-center gap-1 px-2 py-1 rounded-lg text-red-600 dark:text-red-400 border border-red-200 dark:border-red-800 hover:bg-red-50 dark:hover:bg-red-900/20 transition-colors">
-              <Ban className="w-3 h-3" />Cancel
+            <button onClick={() => cancelMutation.mutate()} disabled={cancelMutation.isPending || !!cancelNote}
+              className="flex items-center gap-1 px-2 py-1 rounded-lg text-red-600 dark:text-red-400 border border-red-200 dark:border-red-800 hover:bg-red-50 dark:hover:bg-red-900/20 transition-colors disabled:opacity-50">
+              <Ban className="w-3 h-3" />
+              {cancelMutation.isPending ? 'Cancelling…' : cancelNote ? 'Cancelling' : 'Cancel'}
            </button>
          )}
        </span>
      </div>

+      {cancelNote && (
+        <div className="px-5 py-2 flex items-start gap-2 text-xs bg-amber-50 dark:bg-amber-950/30 text-amber-800 dark:text-amber-300 border-b border-amber-200 dark:border-amber-900">
+          <Ban className="w-3.5 h-3.5 mt-px flex-shrink-0" />
+          <span>{cancelNote}</span>
+        </div>
+      )}
+
      {waves.map(w => (
        <div key={w}>
          <div className="px-5 py-1.5 bg-gray-50 dark:bg-slate-800/50 text-[11px] font-semibold uppercase tracking-wide text-gray-400 dark:text-slate-500">
@@ -127,6 +153,9 @@ export default function FirmwarePage() {
  const [haltOnFailure, setHaltOnFailure] = useState(true);
  // Off by default: it is a second flash and a second reboot per device (#113).
  const [routerbootAfter, setRouterbootAfter] = useState(false);
+  // 1 = sequential, the long-standing behaviour. Raising it trades canary
+  // strictness for wall-clock time, which is the operator's call (#135).
+  const [waveConcurrency, setWaveConcurrency] = useState(1);
  const [scheduleAt, setScheduleAt] = useState('');
  const [viewRolloutId, setViewRolloutId] = useState<number | null>(null);
  const [checkResult, setCheckResult] = useState<string | null>(null);
@@ -174,6 +203,7 @@ export default function FirmwarePage() {
      halt_on_failure: haltOnFailure,
      pre_backup: preBackup,
      routerboot_after: routerbootAfter,
+      wave_concurrency: waveConcurrency,
      scheduled_at: scheduleAt ? new Date(scheduleAt).toISOString() : null,
      start: !scheduleAt,
      devices: [...selected.entries()].map(([device_id, wave]) => ({ device_id, wave })),
@@ -196,6 +226,17 @@ export default function FirmwarePage() {
  const setWave = (id: number, wave: number) => setSelected(prev => new Map(prev).set(id, wave));

  const selectedCount = selected.size;
+  // Advisory only: which selected devices other selected devices reach the
+  // network through. Discovered topology is incomplete, so this warns rather
+  // than blocks (#135).
+  const selectedIds = useMemo(() => [...selected.keys()].sort((a, b) => a - b), [selected]);
+  const { data: upstreamCheck } = useQuery({
+    queryKey: ['fw-upstream', selectedIds],
+    queryFn: () => firmwareApi.upstreamCheck(selectedIds).then(r => r.data),
+    enabled: waveConcurrency > 1 && selectedIds.length > 1,
+  });
+  const upstreamWarning = upstreamCheck?.upstream ?? [];
+
  const waveSummary = useMemo(() => {
    const byWave = new Map<number, number>();
    for (const w of selected.values()) byWave.set(w, (byWave.get(w) ?? 0) + 1);
@@ -356,11 +397,51 @@ export default function FirmwarePage() {
                <input type="checkbox" className="w-4 h-4 rounded" checked={routerbootAfter} onChange={e => setRouterbootAfter(e.target.checked)} />
                Then RouterBOOT
              </label>
+              <label
+                className="flex items-center gap-1.5 text-sm text-gray-700 dark:text-slate-300"
+                title="How many devices in the same wave may upgrade at once. 1 upgrades them one at a time."
+              >
+                <Zap className="w-4 h-4 text-gray-400" />
+                At once
+                <select
+                  className="input py-1 text-xs w-auto"
+                  value={waveConcurrency}
+                  onChange={e => setWaveConcurrency(parseInt(e.target.value, 10))}
+                >
+                  {[1, 2, 3, 5, 8, 10].map(n => (
+                    <option key={n} value={n}>{n === 1 ? '1 — one at a time' : n}</option>
+                  ))}
+                </select>
+              </label>
              <label className="flex items-center gap-1.5 text-sm text-gray-700 dark:text-slate-300">
                <Clock className="w-4 h-4 text-gray-400" />
                <input type="datetime-local" className="input py-1 text-xs w-auto" value={scheduleAt} onChange={e => setScheduleAt(e.target.value)} />
              </label>
            </div>
+
+            {waveConcurrency > 1 && (
+              <div className="flex items-start gap-2 text-xs rounded-lg border border-amber-300 bg-amber-50 dark:bg-amber-900/20 dark:border-amber-700 px-3 py-2 text-amber-800 dark:text-amber-300">
+                <ShieldAlert className="w-3.5 h-3.5 mt-px flex-shrink-0" />
+                <div className="space-y-1">
+                  <p>
+                    Up to {waveConcurrency} devices in a wave will reboot at the same time, and a
+                    failure stops the rollout at the <em>end</em> of the wave — so up to{' '}
+                    {waveConcurrency} devices can take a bad build before it halts.
+                  </p>
+                  {upstreamWarning.length > 0 && (
+                    <p>
+                      <span className="font-semibold">
+                        {upstreamWarning.map(u => u.name).join(', ')}
+                      </span>{' '}
+                      {upstreamWarning.length === 1 ? 'carries' : 'carry'} traffic for other selected
+                      devices. Rebooting {upstreamWarning.length === 1 ? 'it' : 'them'} alongside those
+                      devices can cut the path to them mid-upgrade. Consider putting{' '}
+                      {upstreamWarning.length === 1 ? 'it' : 'them'} in a later wave.
+                    </p>
+                  )}
+                </div>
+              </div>
+            )}
            <div className="flex items-center gap-3">
              <button className="btn-primary flex items-center gap-2" disabled={createRollout.isPending}
                onClick={() => createRollout.mutate()}>
@@ -892,6 +892,8 @@ export interface FirmwareDeviceRow {
 export interface FirmwareRollout {
  id: number; name: string; status: 'pending' | 'running' | 'completed' | 'failed' | 'cancelled';
  halt_on_failure: boolean; pre_backup: boolean; routerboot_after: boolean;
+  /** Devices per wave allowed to upgrade at once; 1 is sequential (#135). */
+  wave_concurrency?: number;
  scheduled_at: string | null; started_at: string | null; finished_at: string | null; created_at: string;
  device_count?: number; success_count?: number; failed_count?: number;
 }
@@ -912,13 +914,21 @@ export const firmwareApi = {
  createRollout: (data: {
    name: string; halt_on_failure: boolean; pre_backup: boolean; routerboot_after?: boolean;
    scheduled_at?: string | null; start?: boolean;
+    /** Devices per wave allowed to upgrade at once; 1 is sequential (#135). */
+    wave_concurrency?: number;
    devices: { device_id: number; wave: number }[];
  }) => api.post<{ id: number }>('/firmware/rollouts', data),
+  /** Advisory: selected devices that other selected devices depend on. */
+  upstreamCheck: (ids: number[]) =>
+    api.get<{ upstream: { id: number; name: string }[]; maxConcurrency: number }>(
+      '/firmware/rollouts/upstream-check', { params: { ids: ids.join(',') } }
+    ),
  listRollouts: () => api.get<FirmwareRollout[]>('/firmware/rollouts'),
  getRollout: (id: number) =>
    api.get<FirmwareRollout & { devices: FirmwareRolloutDevice[] }>(`/firmware/rollouts/${id}`),
  startRollout: (id: number) => api.post(`/firmware/rollouts/${id}/start`),
-  cancelRollout: (id: number) => api.post(`/firmware/rollouts/${id}/cancel`),
+  cancelRollout: (id: number) =>
+    api.post<{ ok: boolean; note?: string }>(`/firmware/rollouts/${id}/cancel`),
  changelog: (version: string) =>
    api.get<{ version: string; url: string; text: string }>(`/firmware/changelog/${encodeURIComponent(version)}`),
 };
@@ -964,15 +974,60 @@ export const operationsApi = {
  syncAll: () => api.post<OpsActionResult>('/operations/sync-all', undefined, { timeout: 180_000 }),
 };

+export interface BackupFilters {
+  deviceId?: number | string;
+  type?: string;
+  from?: string;
+  to?: string;
+  search?: string;
+}
+
+export interface BackupContent {
+  id: number;
+  filename: string;
+  device_name: string;
+  device_id: number;
+  backup_type: string | null;
+  notes: string | null;
+  size_bytes: number | null;
+  created_at: string;
+  content: string;
+}
+
+export interface BackupDiffSide {
+  id: number;
+  filename: string;
+  device_name: string;
+  created_at: string;
+  text: string;
+}
+
 export const backupsApi = {
-  list: (deviceId?: number) =>
-    api.get<Backup[]>('/backups', { params: deviceId ? { deviceId } : {} }),
+  list: (filters: BackupFilters = {}) => {
+    // Drop empty values so an unset filter doesn't become `?type=`.
+    const params = Object.fromEntries(
+      Object.entries(filters).filter(([, v]) => v !== undefined && v !== '')
+    );
+    return api.get<Backup[]>('/backups', { params });
+  },
+  types: () => api.get<{ type: string; count: number }[]>('/backups/types'),
  create: (deviceId: number, notes?: string) =>
    api.post<Backup>('/backups', { deviceId, notes }),
  download: (id: number) =>
    api.get(`/backups/${id}/download`, { responseType: 'blob' }),
+  content: (id: number) => api.get<BackupContent>(`/backups/${id}/content`),
+  diff: (fromId: number, toId: number) =>
+    api.get<{ from: BackupDiffSide; to: BackupDiffSide }>(`/backups/${fromId}/diff/${toId}`),
  restore: (id: number) => api.post(`/backups/${id}/restore`),
  delete: (id: number) => api.delete(`/backups/${id}`),
+  bulkDeletePreview: (ids: number[]) =>
+    api.get<{ backups: number; snapshots: number; devices: { name: string; count: number }[] }>(
+      '/backups/bulk-delete/preview', { params: { ids: ids.join(',') } }
+    ),
+  bulkDelete: (ids: number[]) =>
+    api.post<{ deleted: number; requested: number; failures: { id: number; error: string }[] }>(
+      '/backups/bulk-delete', { ids }
+    ),
 };

 // ─── Config History ────────────────────────────────────────────────────────────
@@ -0,0 +1,44 @@
+import { describe, it, expect } from 'vitest';
+import { orderForDiff, type ComparableBackup } from './backupCompare';
+
+const backups: ComparableBackup[] = [
+  { id: 10, created_at: '2026-09-01T10:00:00Z' },
+  { id: 20, created_at: '2026-09-05T10:00:00Z' },
+  { id: 30, created_at: '2026-09-08T10:00:00Z' },
+];
+
+describe('orderForDiff', () => {
+  // The property that matters: whichever order the user clicked in, the diff
+  // reads forwards in time. Selecting newest-then-oldest must not invert it.
+  it('always returns [older, newer]', () => {
+    expect(orderForDiff([10, 30], backups)).toEqual([10, 30]);
+    expect(orderForDiff([30, 10], backups)).toEqual([10, 30]);
+    expect(orderForDiff([30, 20], backups)).toEqual([20, 30]);
+  });
+
+  it('refuses anything other than a pair', () => {
+    expect(orderForDiff([], backups)).toBeNull();
+    expect(orderForDiff([10], backups)).toBeNull();
+    expect(orderForDiff([10, 20, 30], backups)).toBeNull();
+  });
+
+  // Two backups taken in the same second is unlikely but possible; the result
+  // must still be stable rather than depending on click order.
+  it('is stable when timestamps tie', () => {
+    const tied: ComparableBackup[] = [
+      { id: 7, created_at: '2026-09-01T10:00:00Z' },
+      { id: 9, created_at: '2026-09-01T10:00:00Z' },
+    ];
+    expect(orderForDiff([9, 7], tied)).toEqual([7, 9]);
+    expect(orderForDiff([7, 9], tied)).toEqual([7, 9]);
+  });
+
+  it('falls back to id when a backup is missing or undated', () => {
+    expect(orderForDiff([99, 98], [])).toEqual([98, 99]);
+    const bad: ComparableBackup[] = [
+      { id: 1, created_at: 'not-a-date' },
+      { id: 2, created_at: 'not-a-date' },
+    ];
+    expect(orderForDiff([2, 1], bad)).toEqual([1, 2]);
+  });
+});
@@ -0,0 +1,40 @@
+/**
+ * Ordering two selected backups for comparison (#134).
+ *
+ * A diff is directional: rendered the wrong way round, every addition reads as a
+ * deletion and vice versa. Nothing about the output looks broken when this is
+ * backwards, which is exactly why it is worth pinning down — selection order in
+ * the table is whatever the user happened to click, and carries no meaning.
+ */
+
+export interface ComparableBackup {
+  id: number;
+  created_at: string;
+}
+
+/**
+ * Returns [older, newer] so the diff reads forwards in time.
+ *
+ * Returns null unless exactly two are selected: comparing one thing, or three,
+ * is not a diff, and the caller should disable the action rather than guess.
+ */
+export function orderForDiff(
+  selectedIds: number[],
+  backups: ComparableBackup[]
+): [number, number] | null {
+  if (selectedIds.length !== 2) return null;
+
+  const at = (id: number): number => {
+    const b = backups.find((x) => x.id === id);
+    const t = b ? new Date(b.created_at).getTime() : NaN;
+    return Number.isNaN(t) ? 0 : t;
+  };
+
+  const [a, b] = selectedIds;
+  if (at(a) === at(b)) {
+    // Same timestamp, or both unknown: fall back to id, which still increases
+    // with time, so the direction stays meaningful rather than arbitrary.
+    return a <= b ? [a, b] : [b, a];
+  }
+  return at(a) < at(b) ? [a, b] : [b, a];
+}
@@ -1 +1 @@
-export const APP_VERSION = 'v0.24.0 Beta';
+export const APP_VERSION = 'v0.24.5 Beta';
@@ -65,6 +65,8 @@ nav:
      - Alerting: alerting.md
      - SSH keys: ssh-keys.md
      - Bulk commands: commands.md
+      - Firmware rollouts: firmware.md
+      - Backups: backups.md
  - Devices:
      - Cellular (LTE): cellular.md
      - CAPsMAN: capsman.md
Author	SHA1	Message	Date
2GT-RichandClaude Opus 5	1a4b635052	fix(firmware): halt-on-failure stopped waiting for the wave to finish (v0.24.5-beta) CI / Deployment files in sync (push) Canceled after 0s Details CI / Frontend — Lint, Type-check, Test & Build (push) Canceled after 0s Details CI / Backend — Lint, Type-check & Build (push) Canceled after 0s Details CI / Security Audit (push) Canceled after 0s Details CodeQL Security Analysis / Analyze (javascript-typescript) (push) Canceled after 0s Details Documentation / Build (check only) (push) Canceled after 0s Details Documentation / Publish versioned site (push) Canceled after 0s Details Publish to GHCR / Build & Push Backend (push) Canceled after 0s Details Publish to GHCR / Build & Push Nginx (Frontend) (push) Canceled after 0s Details Release / release (push) Canceled after 0s Details A regression I introduced in v0.24.3, and described inaccurately in its own commit message. Adding concurrency moved the halt check to the end of a wave. I wrote that "at the default of 1 those are the same moment". That is only true of a wave holding one device. With several devices in one wave and the default sequential setting, the old code stopped the moment one failed; mine went on to attempt every remaining device in that wave first. Anyone relying on halt-on-failure with a wide wave 1 got a wider blast radius than before, without changing a setting. Halting now stops further devices from starting as soon as a failure is known, whatever the concurrency. Devices already in flight are allowed to finish, because interrupting one mid-write is how you brick it. At concurrency 1 that is precisely the previous behaviour. Above 1, only the devices genuinely running side by side can carry a bad build -- which is the unavoidable part of the trade in #135, rather than a cost imposed on rollouts that never asked for concurrency. The firmware documentation said the same wrong thing and is corrected. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-09-08 08:54:51 -07:00
2GT-RichandClaude Opus 5	d9fa120abc	fix(logs): a line with no RouterOS id had no protection against re-insertion (v0.24.4-beta) Follows #137. The v0.24.1 fix made the count honest and stopped a thousand round trips per poll, but it assumed the unique index on (device_id, log_id) would reject lines already stored. For most devices it does. For a device whose log entries carry no `.id` it never could: the column was set to NULL, and PostgreSQL treats NULLs as distinct in a unique index, so every NULL conflicts with nothing. Those devices re-inserted their entire buffer on every poll, for ever, and the constraint that was supposed to stop it was structurally incapable of firing. A line with no id now gets a key derived from its own content -- timestamp, topics and message -- so the existing index does the work. The `#` prefix keeps it out of the space of real RouterOS ids (`*` plus hex), so the two cannot collide, and parseRosId already rejects it, which matters because the watermark is a max over real ids and must not be poisoned by a synthetic one. The trade, stated rather than buried: two genuinely separate lines sharing a timestamp, topic and message collapse into one. RouterOS timestamps are second-resolution, so that can happen. Losing a repeat of an identical message is a much smaller loss than growing the table by the whole buffer every minute without bound. This was found by reading a comment on #137 I had not read before closing it -- log output from a device with eleven lines total, reporting eleven new entries every minute for forty-five minutes. My own fleet cannot reproduce it: every event here carries a real `.id`, with no nulls and no duplicates, which is exactly why the hole was invisible from here. So this closes a defect demonstrable from the schema; whether it is the whole of that reporter's problem is still an open question and the issue is reopened to ask. Verified no regression where ids are healthy: no surrogate keys generated, no nulls, no duplicates, and counts that vary with real activity. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-09-08 08:45:29 -07:00
2GT-RichandClaude Opus 5	6bc7480b89	ci(release): tag by version, not by comparing with the previous commit v0.24.3-beta shipped without a release. The workflow compared the version in HEAD against HEAD~1, so a bump pushed together with any follow-up commit — here a docs commit — was no longer the tip and nothing looked changed. It now asks whether a tag for the current version already exists. That does not care how commits are batched, is safe to re-run, and makes the job idempotent rather than dependent on push shape. The paths filter is dropped for the same reason: the tag check is the real guard. Release notes are taken from the commit that last touched the version rather than the tip, since a bump is often followed by docs or CI commits. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-09-07 22:01:40 -07:00
2GT-RichandClaude Opus 5	6c3d2aaa35	docs: firmware rollouts and backups Neither area had a page, and both changed materially in v0.24.2/v0.24.3. The firmware page records why the pipeline downloads and verifies before rebooting, and what raising "at once" actually costs: halting becomes less precise, devices reboot together, and the upstream warning is conservative enough that its absence is not proof of safety. The backups page leads with the fact that a backup is /export text rather than a binary blob, since that is what makes preview and diff possible and what limits what a restore can do. It also states plainly that a backup and its Config History snapshot are one artifact, so deleting one removes the other's rollback. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-09-07 21:42:55 -07:00
2GT-RichandClaude Opus 5	df8f6dca44	feat(firmware): optional parallel upgrades within a wave (v0.24.3-beta) Closes #135. The reporter's numbers: CRS switches take ~5 minutes each to reboot, so ten devices is an hour of mostly waiting. Devices in a wave can now upgrade concurrently, up to a per-rollout limit. Waves themselves stay strictly sequential — that is the decision point, and what makes wave 1 a canary. The default is 1, which is the behaviour every existing rollout was built around, and existing rows backfill to it. CommandRunner already runs a wave with Promise.all, but an upgrade is not a bulk command: it holds an API connection for minutes across a download, a reboot and a verification, and connection pressure is exactly where this system has been shown to break. So concurrency is bounded rather than unlimited, via mapWithConcurrency, capped at 20 and clamped server-side. The trade is stated rather than hidden. Halting is now evaluated at the end of a wave instead of the instant a device fails, so a wave of five can put a bad build on five devices before stopping. At the default of 1 those are the same moment. The UI says so plainly whenever the setting is above 1. An upstream warning covers the risk Change Guard cannot: several devices rebooting together is fine for independent routers and dangerous for a switch the others are reached through, and a device unreachable because its uplink rebooted has no config change to revert. The first implementation of that warning was wrong in an instructive way. It flagged any device another selected device linked to — and on the real four-device fleet it flagged all four, because discovered links are bidirectional and a trunk port often resolves to several neighbours at once. A warning that fires on everything is worse than none: it teaches people to dismiss it. It now requires the link to be the reporting device's STP root port and that port to resolve to exactly one neighbour, so it stays quiet when it cannot tell — which, on this fleet's ambiguous topology, it correctly does. Verified on hardware with devices already up to date, so the wave machinery ran end to end without flashing anything: - sequential: 04:20:21→:26, then :26→:31. No overlap, 12s. - concurrency 2: both started within 10ms of each other, 7s. - concurrency 5 across two waves: wave 2 started 3ms after wave 1 finished, never during it. Clamping checked at the API: 999 stored as 20, 0 and "abc" as 1. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-09-07 21:23:08 -07:00
2GT-RichandClaude Opus 5	719563dd22	feat(backups): filter, preview, compare and bulk delete (v0.24.2-beta) Closes #134. Backups are `/export compact` output — plain RouterOS script, not the binary `.backup` blob — which is what makes the reporter's "it's just a text, after all" correct and most of this cheap. - Filters: device, type, date range and free text over device name, filename and notes. Dates are inclusive at both ends, because a picker showing "to: 3 Sep" that stops at that day's first instant is a trap. - Preview in place, on row click. No round trip through the downloads folder to see what a backup contains. - Compare any two, reusing the lineDiff that Config History already uses rather than adding a second diff implementation. - Bulk delete, with selection and a confirmation that names what goes. Two deliberate choices. Bulk delete had to respect an existing invariant: a backup and its config snapshot are one artifact, joined by ON DELETE CASCADE so they cannot drift apart. Removing fifty in one click can therefore destroy restore points nobody looked at. The confirmation is built from a server-side preview that reports the backups per device and how many config snapshots will go with them, and it is capped at 200 per call. The action is also site-scoped, so it cannot reach into another site's history. The Filename column is gone from the table but the filename is not gone: it is the natural handle for a file — device and timestamp, the thing you would quote to a colleague — so it moved to the preview and diff headers where you are actually looking at that file, rather than being dropped. Diff direction is extracted and tested. Selection order in a table is whatever the user happened to click and carries no meaning, so the pair is ordered oldest-first; rendered the other way round every addition reads as a deletion, and nothing about the output looks wrong while it does. Incidentally fixed: 'pre-upgrade' backups were labelled "Manual" in the table because the badge only knew three types. Type wording now has one home. The list query's placeholder counter is replaced by a bind() helper deriving from params.length. The counter's final increment was dead, and whoever added the next filter would have inherited a stale index. Verified against 110 real backups: every filter and combination returns the expected counts and partitions cleanly, preview and diff read real .rsc with file_path withheld, and bulk delete removed three throwaway backups plus their files and left the other 110 untouched. Guards checked: empty, missing, non-numeric and injected ids, unknown ids, and the 200 cap. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-09-07 21:05:55 -07:00
2GT-RichandClaude Opus 5	63b5677522	fix(firmware,ssh,logs): five reported bugs (v0.24.1-beta) Closes #136. Also fixes #133, #137, #138, #139 (GitHub closes only the first). None of these came from multi-site; two came from me. #136 — waves failed on the second device. A regression from v0.23.31, the firmware rewrite that replaced fire-and-forget `install` with `download` plus polling. `/system/package/update/download` blocks for a minute or more, and the client's read timeout is 30 seconds. The timeout path was the real damage: it nulled the pending read but left the socket open, so when the reply finally arrived processBuffer() queued it and the next command took it as its own answer. Every later call on that connection returned the previous one's data, which is why the status polls could never see "Downloaded". - a timed-out read now abandons the connection instead of silently desynchronising it, so the failure is loud rather than wrong - long-running commands can state their own budget; check-for-updates, download and /log/print now do - a download whose command times out is re-checked on a fresh connection rather than assumed failed, because the image may well have landed #133 — the SSH terminal could not use a deployed key. It read only ssh_password_encrypted and refused to connect without one, so retiring the password after deploying a key -- the workflow the feature exists for -- broke the console. It now uses the same resolver as every other SSH consumer. preferredAuth(), written to make exactly this decision, had never run outside its own tests, and disagreed with the resolver that did: it accepted a 'deployed' key where the live path required 'verified'. Unified on the stricter rule and wired in. #139 — the rollout was written before start() was attempted, so a rejected start left the rows behind: "already running", press again, another rollout banked each time. Devices already in an unfinished rollout are now refused by name, before anything is persisted. #138 — cancel worked but the API's explanation that the in-flight device is never interrupted was discarded, so the button looked dead. It is shown. #137 — log overfetching. /log/print returns the whole ring buffer, and the count was incremented per attempted insert rather than per accepted row, so a poll that stored nothing still reported "Collected 1000 new log entries" after a thousand round trips. Inserts are batched and counted by RETURNING, and the id watermark is the maximum over recent rows rather than the last one inserted, which could move backwards and make the whole buffer look new. One self-inflicted wound worth recording. The first cut of the desync guard discarded sentences whenever `connected` was false -- but `connected` is only set after login succeeds, so it threw away every login reply and took the whole fleet offline until it was caught. "Not yet connected" and "abandoned" are different states and now have different flags. Every existing test pre-set connected = true, so none of them could see it; the test added for it fails against the bad version. Verified on hardware: devices back online, check-update 5.2s (was a 30s failure), full sync completes, and log collection now reports 298 / 22 / 12 / 12 across successive polls -- real decreasing counts rather than a constant 1000 -- with no failures. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-09-07 19:17:52 -07:00