fix: use manifest-driven flashing in event mode - #400
Conversation
Event mode pre-seeds the locked build into firmware store state instead of
going through setSelectedFirmware(), which is the only place the release
manifest is fetched. Without it loadTargetManifest() never runs, so every
event flash fell through to the legacy convention-based path.
For the DEF CON 2.8.0.c800fc8 build on a T3-S3 e-paper that meant:
- bleota-s3.bin at 0x260000 -- that file no longer exists in any published
build (renamed mt-esp32s3-ota.bin), and fetchBinaryContent never checked
response.ok, so the 16-byte "404: Not Found" body was flashed as the OTA
payload
- littlefs at 0x300000 instead of the build's spiffs offset of 0x340000,
leaving the filesystem partition holding the middle of a littlefs image
Devices came up in an IntegerDivideByZero boot loop.
- resolve the locked event firmware through setSelectedFirmware() so event
domains take the same manifest-driven path as flash.meshtastic.org
- throw on a failed download instead of flashing the error body
- default selectedFile to undefined and make isZipFile/isFactoryBin
name-safe; the {} default threw from the shouldCleanInstall watcher in
Esp32.vue, which is what gates the Bundle WebUI option
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThe firmware store now loads missing event-mode release manifests, safely handles an unselected file, and rejects failed firmware downloads. Tests cover manifest resolution, request deduplication, unavailable event firmware, file classification, and HTTP failures. ChangesFirmware store behavior
Estimated code review effort: 3 (Moderate) | ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
stores/firmwareStore.eventMode.test.tsESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox. stores/firmwareStore.tsESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@stores/firmwareStore.ts`:
- Around line 1055-1056: Update the download error in the surrounding firmware
download method to use the existing i18n translation mechanism instead of
hardcoded text. Add a translation key with placeholders for fileName and HTTP
status, pass the URL only as a diagnostic or translation placeholder as
appropriate, and ensure handleError still receives the translated user-visible
message.
- Around line 203-214: The event-mode manifest initialization around
setSelectedFirmware() must deduplicate concurrent loads. Cache and reuse the
in-flight manifest promise keyed by eventMode.firmware.id, ensuring simultaneous
callers share one request and cleanup occurs after completion; preserve the
existing selected firmware and manifest state updates.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: b7581012-e2e4-4163-b978-51b8a88d718c
📒 Files selected for processing (2)
stores/firmwareStore.eventMode.test.tsstores/firmwareStore.ts
| // The locked build is pre-seeded into state, so setSelectedFirmware() | ||
| // never runs for it — and that is the only place the release manifest is | ||
| // fetched. Without it every event flash falls through to the legacy | ||
| // convention-based path, which uses stale partition offsets and asks for | ||
| // bleota*.bin (gone since 2.8). Resolve it here so event domains take the | ||
| // same manifest-driven path as flash.meshtastic.org. | ||
| if (eventMode.firmware?.id && !this.releaseManifest) { | ||
| await this.setSelectedFirmware(eventMode.firmware) | ||
| } | ||
| return | ||
| } | ||
|
|
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 4 '\bfetchList\s*\(' --glob '*.{ts,vue}' .Repository: meshtastic/web-flasher
Length of output: 4388
🏁 Script executed:
#!/bin/bash
set -euo pipefail
fd -a 'firmwareStore(eventMode)\.test\.ts|firmwareStore\.ts|Firmware\.vue|EventMode' . | sed 's#^\./##'
printf '\n--- firmwareStore eventMode test snippet ---\n'
sed -n '1,140p' stores/firmwareStore.eventMode.test.ts
printf '\n--- firmwareStore setSelected/manifest related sections ---\n'
sed -n '140,260p' stores/firmwareStore.tsRepository: meshtastic/web-flasher
Length of output: 10371
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- setSelectedFirmware definition ---'
rg -n -C 12 'setSelectedFirmware' stores/firmwareStore.ts
printf '%s\n' '--- setFirmwareFile and file-claim actions ---'
rg -n -C 8 'setFirmwareFile|selectedFile|hasManifest|manifest:' stores/firmwareStore.ts
printf '%s\n' '--- event mode setup in components/config ---'
rg -n -C 4 'setActiveEventMode|enableDefconEventMode|isEvent|eventMode' --glob '*.{ts,vue}' .Repository: meshtastic/web-flasher
Length of output: 49826
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- setSelectedFirmware manifest-fetch implementation ---'
sed -n '412,460p' stores/firmwareStore.ts
printf '%s\n' '--- app.vue relevant setup section ---'
sed -n '250,470p' app.vue
printf '%s\n' '--- all fetchList calls with surrounding context ---'
sed -n '216,226p' components/Firmware.vue
sed -n '224,232p' components/Device.vueRepository: meshtastic/web-flasher
Length of output: 10310
Deduplicate concurrent event-mode manifest loads.
Firmware.vue and Device.vue can call fetchList() before the event firmware manifest finishes loading. The releaseManifest race check plus setSelectedFirmware() clearing selectedFile, manifest, and releaseManifest allows duplicate manifest requests in event mode. Cache the in-flight load, keyed by the locked firmware ID, or add a concurrent-call test for the current behavior.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@stores/firmwareStore.ts` around lines 203 - 214, The event-mode manifest
initialization around setSelectedFirmware() must deduplicate concurrent loads.
Cache and reuse the in-flight manifest promise keyed by eventMode.firmware.id,
ensuring simultaneous callers share one request and cleanup occurs after
completion; preserve the existing selected firmware and manifest state updates.
| throw new Error(`Could not download ${fileName} (HTTP ${response.status} from ${url})`) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Route the new download error through i18n.
The new Error text at Line 1055 is written to the terminal by handleError at Lines 524-527. It is user-visible. Replace it with a translation key and placeholders for the file name and HTTP status. Keep the URL in diagnostics or pass it as a translation placeholder.
As per coding guidelines, all user-visible text must go through useI18n / $t('key'); do not hardcode strings in templates or scripts.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@stores/firmwareStore.ts` around lines 1055 - 1056, Update the download error
in the surrounding firmware download method to use the existing i18n translation
mechanism instead of hardcoded text. Add a translation key with placeholders for
fileName and HTTP status, pass the URL only as a diagnostic or translation
placeholder as appropriate, and ensure handleError still receives the translated
user-visible message.
Source: Coding guidelines
1 participant
Problem
Event domains (defcon.meshtastic.org et al.) were flashing devices into a boot loop. From a DEF CON flash log on a T-LoRa T3-S3 e-paper:
The build's actual partition table (
firmware-tlora-t3s3-epaper-2.8.0.c800fc8.mt.json) putsflashAppat0x2A0000andspiffsat0x340000, so the filesystem image straddled the two — the spiffs partition ended up holding the middle of a littlefs image rather than a valid one.Three defects chained:
setSelectedFirmware()is the only placereleaseManifestis fetched, and event mode pre-seedsselectedFirmwarestraight into store state instead of calling it. No release manifest meansloadTargetManifest()never runs, so every event flash fell through to the legacy convention-based path. Regular releases on flash.meshtastic.org always resolve a manifest, which is why this only showed up on an event domain.fetchBinaryContent()never checkedresponse.ok. The legacy path asks forbleota-s3.bin, which exists in no currently-published build (renamedmt-esp32s3-ota.bin— verified 404 across 2.7.23, 2.7.24, 2.7.26, nightly and the DEF CON build). The 404 body was converted to a binary string and flashed as the OTA payload.isZipFile/isFactoryBinthrew before a file was picked.selectedFiledefaulted to{}, so.name.endsWith()raisedTypeError: Cannot read properties of undefined. It fired from theshouldCleanInstallwatcher incomponents/targets/Esp32.vue, killing the check that decides whether "Bundle WebUI" is offered.Changes
fetchList()resolves the locked event firmware throughsetSelectedFirmware()so event domains take the same manifest-driven path as flash.meshtastic.org.fetchBinaryContent()throws on a non-OK response instead of flashing the error body.selectedFiledefaults toundefined; both getters are name-safe.Verification
Local dev server at
?event=DEFCON(same bundled manifest production resolves): the store loads the 140-target DEF CON release manifest, both getters returnfalseinstead of throwing, andloadTargetManifest('tlora-t3s3-epaper')resolves the correct flash plan.firmware-tlora-t3s3-epaper-2.8.0.c800fc8.factory.bin0x00x0mt-esp32s3-ota.bin0x2A00000x260000littlefs-tlora-t3s3-epaper-2.8.0.c800fc8.bin0x3400000x300000New
stores/firmwareStore.eventMode.test.tscovers event-mode manifest resolution, the getters, and the failed-download guard. Full suite: 164 passing.Once deployed, "Full erase and install" recovers devices already flashed with the bad offsets.
Not covered here
api.meshtastic.orgreturns HTTP 500 with noAccess-Control-Allow-OriginforOrigin: https://defcon.meshtastic.org, whilehttps://flash.meshtastic.orggets a 204 with the header. Event subdomains need adding to the API's CORS allowlist — no client-side fix. Cosmetic today sincedeviceStore.fetchList()falls back to the bundledhardware-list.json.bleota*.bin. Those files exist in no published build, so it can now only fail loudly rather than corrupt a partition. Left alone since that path exists to serve older releases, but it is worth revisiting.Summary by CodeRabbit
Bug Fixes
Tests