From 1a4b6350528b8c656ab7a84ee4806a8a86cf9932 Mon Sep 17 00:00:00 2001 From: 2GT-Rich <107219997+2GT-Rich@users.noreply.github.com> Date: Tue, 8 Sep 2026 08:54:51 -0700 Subject: [PATCH] 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 --- README.md | 2 +- backend/package.json | 2 +- backend/src/services/FirmwareOrchestrator.ts | 30 +++++++++++++------- docs/firmware.md | 9 +++--- frontend/package.json | 2 +- frontend/src/version.ts | 2 +- 6 files changed, 28 insertions(+), 19 deletions(-) diff --git a/README.md b/README.md index fa27a2f..305cb7d 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,7 @@

- Version + Version License Docker TypeScript diff --git a/backend/package.json b/backend/package.json index d0ee2a3..8da6905 100644 --- a/backend/package.json +++ b/backend/package.json @@ -1,6 +1,6 @@ { "name": "mikrotik-manager-backend", - "version": "0.24.4-beta", + "version": "0.24.5-beta", "description": "Mikrotik Network Management Platform - Backend API", "main": "dist/index.js", "scripts": { diff --git a/backend/src/services/FirmwareOrchestrator.ts b/backend/src/services/FirmwareOrchestrator.ts index 9050948..5fedd23 100644 --- a/backend/src/services/FirmwareOrchestrator.ts +++ b/backend/src/services/FirmwareOrchestrator.ts @@ -98,12 +98,16 @@ export class FirmwareOrchestrator { // Waves stay strictly sequential -- that is the decision point, and it is // what makes wave 1 a canary. Concurrency applies only *within* a wave. // - // At the default of 1 this is the previous behaviour with one difference - // worth stating: halting is evaluated at the end of a wave rather than the - // instant a device fails. With a wave of one those are the same moment. With - // a larger wave it is the trade the operator asked for (#135) -- more devices - // carry a bad build before anything stops, in exchange for not waiting out - // ten five-minute reboots in series. + // 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); @@ -113,11 +117,15 @@ export class FirmwareOrchestrator { `${inWave.length} device(s), up to ${concurrency} at once`); } - const results = await mapWithConcurrency(inWave, concurrency, (item) => - this.cancelRequested - ? Promise.resolve(false) - : this.upgradeDevice(rollout, item) - ); + 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; diff --git a/docs/firmware.md b/docs/firmware.md index 0f54fd8..24e53f2 100644 --- a/docs/firmware.md +++ b/docs/firmware.md @@ -55,10 +55,11 @@ everything after it. There are two costs, and both are worth understanding before you raise it. -**Halting becomes less precise.** With one device at a time, a failure stops the -rollout immediately. Running several at once, the wave finishes first — so a wave of -five can put a bad build on five devices before anything stops. At the default of 1 -these are the same moment. +**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 diff --git a/frontend/package.json b/frontend/package.json index 9edff39..6022fce 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -1,6 +1,6 @@ { "name": "mikrotik-manager-frontend", - "version": "0.24.4-beta", + "version": "0.24.5-beta", "private": true, "type": "module", "scripts": { diff --git a/frontend/src/version.ts b/frontend/src/version.ts index 7b1d20b..d471792 100644 --- a/frontend/src/version.ts +++ b/frontend/src/version.ts @@ -1 +1 @@ -export const APP_VERSION = 'v0.24.4 Beta'; +export const APP_VERSION = 'v0.24.5 Beta';