fix(firmware): download and verify before rebooting, and prove the reboot (v0.23.31-beta)

Reported as rollouts failing with "Device rebooted but still reports 7.24.1 —
the update did not apply", intermittently and on ad-hoc updates too. Three
separate faults, found by watching one rollout that succeeded on an access point
and failed on a switch in the same run.

**The device had never rebooted.** Its uptime was one week. The old sequence
issued `/system/package/update/install`, slept a fixed 25 seconds, reconnected,
read the version and — finding it unchanged — declared the upgrade failed. It
could not tell a device that restarted unchanged from one that never restarted
at all, and 25 seconds is nowhere near long enough: the access point that
*worked* took 245 seconds. So a device still busy downloading was routinely
reported as broken, and with halt-on-failure that aborted the rest of the fleet.

**`install` was the wrong command.** It bundles download and reboot into one
fire-and-forget call whose progress cannot be observed. On the CCR2216 it
accepted the command and then neither downloaded nor rebooted, reporting
nothing; an explicit `download` on the same device worked immediately. The
sequence is now download → confirm the device reports the image as downloaded →
reboot, which makes success verifiable *before* anything restarts and means a
device is never rebooted for an image it does not have.

**A successful reboot was being read as a failure.** Rebooting necessarily kills
the session that asked for it, so the transport error that follows is the
expected outcome. Unhandled, it surfaced as "Update failed before reboot:
Connection closed" on a device that had in fact rebooted and upgraded correctly.

The reboot is now proven by comparing uptime across it rather than assumed from
elapsed time, so "never restarted" is reported as itself — with the device left
running and reachable — instead of being mistaken for a failed upgrade.

Download failures now carry the device's free space, because that is the usual
cause and "did not finish in ten minutes" otherwise sends someone hunting the
network. A CRS309 with 1.06 MB free now says so instead of timing out silently.

Verified on hardware: the CCR2216 that produced the original report upgraded
7.24.1 to 7.24.2 cleanly, and the CRS309 that genuinely cannot fit the image
failed with an accurate message and was left untouched — same version, uptime
unbroken, never rebooted.

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

backend/package.json

@@ -1,6 +1,6 @@
 {
   "name": "mikrotik-manager-backend",
-  "version": "0.23.30-beta",
+  "version": "0.23.31-beta",
   "description": "Mikrotik Network Management Platform - Backend API",
   "main": "dist/index.js",
   "scripts": {

backend/src/services/FirmwareOrchestrator.ts

@@ -13,6 +13,11 @@ import { DeviceCollector, DeviceRow } from './mikrotik/DeviceCollector';
 import { BackupService } from './BackupService';
 
 const REBOOT_GRACE_MS = 25_000;      // let the device actually go down
+const DOWNLOAD_TIMEOUT_MS = 10 * 60_000; // large images on slow links
+const DOWNLOAD_POLL_MS = 10_000;
+
+/** Reads better than a bare version in a sentence that may have none. */
+const latestLabel = (v: string) => (v ? `Version ${v}` : 'The update');
 const REBOOT_POLL_MS = 15_000;       // probe cadence while waiting
 const REBOOT_TIMEOUT_MS = 12 * 60_000; // give slow flash writes room
 
@@ -150,8 +155,17 @@ export class FirmwareOrchestrator {
       }
     }
 
-    // 2. Kick off the install (device downloads, installs, and reboots itself)
+    // 2. Download the image, and confirm it is actually on the device.
+    //
+    // Deliberately not `/system/package/update/install`, which bundles download
+    // and reboot into one call whose progress cannot be observed. A CCR2216 was
+    // seen to accept it, neither download nor reboot, and report nothing — which
+    // surfaced as "rebooted but still reports 7.24.1" because the check below
+    // could not tell a device that never restarted from one that restarted
+    // unchanged. Downloading first makes success verifiable *before* anything
+    // reboots, and means a device is never restarted for an image it lacks.
     await this.setItem(item.id, { status: 'upgrading' });
+    let uptimeBefore: number | null;
     const collector = new DeviceCollector(device);
     try {
       await collector.connect();
@@ -164,35 +178,85 @@ export class FirmwareOrchestrator {
         collector.disconnect();
         return true;
       }
-      await collector.installUpdate();
+
+      uptimeBefore = await collector.getUptimeSeconds();
+
+      // Free space is the usual reason a download never completes, and "did not
+      // finish in ten minutes" sends someone hunting the network when the device
+      // simply has nowhere to put the image.
+      const res = await collector.getSystemResource().catch(() => ({} as Record<string, string>));
+      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;
+      let downloaded = false;
+      while (Date.now() < dlDeadline) {
+        if (this.cancelRequested) break;
+        const s = await collector.getUpdateStatus();
+        if (/downloaded/i.test(s)) { downloaded = true; break; }
+        if (/error|fail/i.test(s)) {
+          collector.disconnect();
+          return fail(`Device could not download ${latest}: ${s}.${spaceNote}`);
+        }
+        await sleep(DOWNLOAD_POLL_MS);
+      }
+      if (!downloaded) {
+        collector.disconnect();
+        return fail(
+          `Device did not finish downloading ${latest} within ` +
+          `${Math.round(DOWNLOAD_TIMEOUT_MS / 60000)} minutes. Nothing was rebooted.${spaceNote}` +
+          (freeMb > 0 && freeMb < 20 ? ' That is unlikely to be enough for a RouterOS image.' : '')
+        );
+      }
+
+      await this.setItem(item.id, { to_version: latest });
+      await collector.reboot();
     } catch (e) {
       collector.disconnect();
-      return fail(`Update install failed: ${(e as Error).message}`);
+      return fail(`Update failed before reboot: ${(e as Error).message}`);
     }
     collector.disconnect();
 
-    // 3. Ride out the reboot
+    // 3. Ride out the reboot, and prove one happened.
     await this.setItem(item.id, { status: 'rebooting' });
     await sleep(REBOOT_GRACE_MS);
     const deadline = Date.now() + REBOOT_TIMEOUT_MS;
     let backOnline = false;
+    let rebootProven = false;
     let newVersion = '';
     while (Date.now() < deadline) {
       if (this.cancelRequested) break;
       const probe = new DeviceCollector(device);
       try {
         await probe.connect();
-        await this.setItem(item.id, { status: 'verifying' });
         const resource = await probe.getSystemResource();
         newVersion = (resource['version'] || '').split(' ')[0];
+        const uptimeNow = await probe.getUptimeSeconds();
         probe.disconnect();
-        backOnline = true;
-        break;
+
+        // An uptime that did not fall means we are talking to the same running
+        // system, not a restarted one — so keep waiting rather than reading the
+        // old version and calling it a failed upgrade.
+        rebootProven = uptimeBefore == null || uptimeNow == null || uptimeNow < uptimeBefore;
+        if (rebootProven) { await this.setItem(item.id, { status: 'verifying' }); backOnline = true; break; }
+        await sleep(REBOOT_POLL_MS);
       } catch {
         probe.disconnect();
         await sleep(REBOOT_POLL_MS);
       }
     }
+
+    if (backOnline && !rebootProven) {
+      return fail(
+        `${latestLabel(newVersion)} was downloaded, but the device never restarted. ` +
+        `It is still running and reachable — reboot it to complete the upgrade.`
+      );
+    }
     if (!backOnline) {
       return fail(`Device did not come back online within ${Math.round(REBOOT_TIMEOUT_MS / 60000)} minutes after the upgrade — check it manually (a pre-upgrade backup ${rollout.pre_backup ? 'exists' : 'was NOT taken'})`);
     }

backend/src/services/mikrotik/DeviceCollector.ts

@@ -3681,6 +3681,38 @@ export class DeviceCollector {
     return (result[0] as Record<string, string>) || {};
   }
 
+  /**
+   * Download the pending update without rebooting.
+   *
+   * Preferred over `install` because it is *verifiable*: afterwards the device
+   * reports "Downloaded, please reboot", so the caller can confirm the image is
+   * actually on the device before restarting anything. `install` bundles the
+   * download and the reboot into one fire-and-forget call whose success cannot
+   * 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');
+  }
+
+  /** The `status` line from /system/package/update, e.g. "Downloaded, please reboot…". */
+  async getUpdateStatus(): Promise<string> {
+    const rows = await this.client.execute('/system/package/update/print').catch(() => []);
+    return (rows[0]?.['status'] || '').trim();
+  }
+
+  /** Seconds since the device last booted, for proving a reboot happened. */
+  async getUptimeSeconds(): Promise<number | null> {
+    const res = await this.getSystemResource().catch(() => ({} as Record<string, string>));
+    const raw = res['uptime'];
+    if (!raw) return null;
+    const units: Record<string, number> = { w: 604800, d: 86400, h: 3600, m: 60, s: 1 };
+    let total = 0;
+    let matched = false;
+    for (const [, n, u] of raw.matchAll(/(\d+)([wdhms])/g)) { total += parseInt(n, 10) * units[u]; matched = true; }
+    return matched ? total : null;
+  }
+
   async installUpdate(): Promise<void> {
     await this.client.execute('/system/package/update/install');
   }
@@ -3699,8 +3731,22 @@ export class DeviceCollector {
     await this.client.execute('/system/reboot');
   }
 
+  /**
+   * Reboot the device.
+   *
+   * A reboot that works necessarily kills the session that asked for it, so the
+   * transport error that follows is the *expected* outcome, not a failure. Left
+   * unhandled it surfaced as "Update failed before reboot: Connection closed" on
+   * a device that had in fact rebooted and upgraded correctly.
+   */
   async reboot(): Promise<void> {
-    await this.client.execute('/system/reboot');
+    try {
+      await this.client.execute('/system/reboot');
+    } catch (e) {
+      const msg = (e as Error).message || '';
+      if (/closed|reset|EPIPE|not connected|timed? ?out|socket/i.test(msg)) return;
+      throw e;
+    }
   }
 
   // ─── Ethernet monitor / SFP DDM ───────────────────────────────────────────