Notification settings moved to the Messages category, but the App Settings
tile subtitle still advertised them. Retitled across all 18 locales to match
what the pane actually renders (appearance, translation, battery, map, Cyr2Lat).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Adversarial review caught a data-loss defect in the reactor-name
fallback. For a reaction in a room from an author not in the local
contacts, _resolveContactSenderName returns null and the chain fell
through to reactionContact?.name. But reactionContact is the room server,
not the reactor, so every unknown-author reaction was attributed to the
room's display name. Two distinct unknown authors then both resolved to
that one name, so applyReaction's per-reactor dedup treated the second as
a duplicate and dropped it, count and all.
Fall back to the per-author hex (from the frame's own fourByteRoomContact
Key) instead, which is unique per reactor. In a true 1:1 that hex is
empty and the contact genuinely is the reactor, so its name stays the
correct fallback.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Reactions stored only a Map<String,int> of emoji to count, so "who
reacted" was unanswerable and MeshCore One's tap-to-see-who had nothing
to show against. This captures the reactor for every reaction, both our
own r: format and the PocketMesh / MeshCore One format.
Data layer only. The tap-a-badge-to-see-who UI is a separate follow-on;
this makes the data available and does not change any screen.
Additive by design. A new reactionSenders field (Map<String,List<String>>,
emoji -> reactor names) sits alongside the existing count map, serialised
under a new JSON key. Both maps are always written. An older build reading
a newer store ignores the unknown key and still gets correct counts from
reactions; it never hits the hard `value as int` cast that would fail the
whole message-list load (the #355 data-loss shape). Records written before
this field load with an empty sender map, so their counts survive with
names simply absent.
- ChannelMessage + Message: new reactionSenders field, constructor,
copyWith
- both stores: serialise the new key; deserialise via a shared
ReactionHelper.reactionSendersFromJson that returns empty for a missing
key and skips malformed entries rather than throwing
- applyReaction: records the reactor and dedups per reactor per emoji.
This is persistent dedup (survives restart), unlike the connector's
in-memory processed-set. A pre-existing count with no sender list is
incremented from its stored value, not recomputed from the partial
list, so old counts are preserved.
- connector: threads the reactor name into all reaction paths. Channel
uses the frame sender; a room resolves the author via
fourByteRoomContactKey; an outgoing reaction is attributed to self.
- pending queue: retry now hands the stored reactor to its callback
Tests: reactor capture, two-reactor count, same-reactor no-double-count,
pre-#383 count preservation, and JSON round-trip + backward-compat +
malformed-entry handling for the new field. Full suite 604 passing.
Epic #376.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The button used Icons.ios_share for all mobile, so Android showed the iOS
share glyph. Split by platform: Android -> Icons.share, iOS -> Icons.ios_share,
desktop -> Icons.folder_open (reveal the saved logs).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Two of three Gemini findings held up against the code; the third did not.
Accepted, message-swallow risk: the emoji check looked only at the first
rune against a range list that included arrows, so a multi-line message
beginning with an arrow and ending in eight Crockford characters would
have been consumed and displayed as a reaction whose "emoji" was the
whole sentence. Ben's own capture has U+2192 mid-sentence in ordinary
channel traffic, so this was reachable, not theoretical.
- drop U+2190-U+21FF and U+2934-U+2935; arrows carry the Unicode Emoji
property but read as punctuation in prose
- cap the emoji segment at 8 runes, which is clear of the longest ZWJ
sequence and nowhere near a sentence. This is the guard that holds
regardless of how the range list evolves.
Accepted, dedup: the contact path keyed on target hash plus emoji only.
A room server is many-party over the 1:1 transport, so two members
sending the same emoji collapsed into one and the count stuck at 1, the
same defect already fixed on the channel path. The reacting author
prefix is now part of the key; in a true 1:1 it is empty and the key is
unchanged.
Rejected, room-server sender matching: the review claimed getSenderName
resolves to the room itself and that room text carries a "Sender: "
prefix. Neither is true. _resolveContactSenderName resolves the author
via fourByteRoomContactKey to the real per-sender contact name, and room
message text is stored bare with the author in its own field. One real
sub-case survives: an author who is not in our contacts resolves to null
and will not match, which degrades to queue-then-expire with a warn log.
Also rejected: switching @[ lookup from first to last occurrence. The
reference implementation uses the first, and diverging risks mismatching
payloads it accepts.
Reactions sent from MeshCore One arrived as junk text: an emoji line
followed by an 8-character token such as "dyps6yf0". Those tokens are
Crockford Base32 target-message hashes, the second line of a two-line
reaction payload we did not recognise.
Receive-side only. Offband keeps sending its own r:hhhh:ii format; their
client already parses ours, so nothing about what we transmit changes.
Wire format (confirmed against a live capture, see #378):
channel: {emoji}@[{targetSender}]\n{hash}
direct: {emoji}\n{hash}
hash: sha256(body utf8 + timestamp uint32 LE seconds)[0:5],
Crockford Base32, 8 chars, lowercase
The body is hashed without the channel "SenderName: " prefix, which is
why the sender travels in @[...] instead.
- crockford_base32.dart: encode and normalise, 12-bit accumulator so the
web target's 32-bit bitwise ops cannot truncate a 40-bit value
- pocketmesh_reaction.dart: hash and a parser mirroring the reference
implementation, with a conservative leading-emoji check so a real
message is never swallowed
- reaction_helper.dart: ReactionInfo carries a dialect; applyReaction
picks the matching hash and, for the channel form, requires an exact
sender-name match (a node name can carry emoji and variation
selectors)
- pending_reactions.dart: a reaction arriving before its target is held
and retried rather than silently dropped, bounded at 50 entries with a
15 minute TTL and a warn-level log on expiry (closes the silent-drop
path in #382)
- the channel dedup key now includes the reacting sender, so two people
sending the same emoji no longer collapse into one
- notification tray summarises the foreign format as a reaction instead
of showing the raw token
Known limitation: on channels with Smaz or Cyr2Lat enabled the wire text
differs from the text we store, so a hash computed by another client
will not match. Flagged for a decision rather than worked around.
Epic #376. Fast-follow #383 adds reactor identity.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
AppShell decided top-level vs detail by `selectedIndex != null`, but pushed
detail screens (a channel chat, the LOS map) also set selectedIndex to keep the
bottom bar visible. So Back from inside a channel backgrounded/left the app
instead of popping to the channel list.
Decouple the two concerns: add `isTopLevel` (default true), separate from
`selectedIndex`. Detail screens pass `isTopLevel: false` (keep the bar, but Back
pops). The decision is extracted into a pure `AppShell.backAction`
(drawer -> close; detail with a route below -> pop; else -> background), with an
assert that a detail is actually poppable. Unit tests cover the matrix; widget
tests exercise the real system-Back -> PopScope -> pop wiring.
Follow-up #390 tracks the separate AppBar back-arrow path.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
When the database can't open (e.g. the native sqlite library fails to load),
every read/write silently failed and the app opened to an empty, normal-looking
screen — the user thinks their history was wiped (SAFELANE §6 violation).
Probe the storage layer in main() before any store reads
(BlobStore.verifyReadWrite). On failure, a StorageHealthService one-way latch
records it, and a persistent, non-dismissable banner is shown above the whole
app: history is not lost, storage is unavailable, messages are NOT being saved,
restart after fixing. Cross-platform (probe goes through drift, covers web too).
Tests: service state/latch + banner show/hide widget test. Gemini-reviewed.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
#363 pins the DB to one directory and migrates ONE prior store in, but a user
who ran differently-built copies can have data split across several stores.
On startup (native only, after the prefs->drift migration) this discovers
every store on the machine and unions its bulk blobs (messages by id, contacts
by public key) into the current one, so nothing shows as a gap. Sources are
read-only and never deleted.
Not a one-shot: instead of a permanent flag it tracks each store's signature
(mtime+size), so a store that a stray older build later creates or grows is
re-merged rather than stranded. A store over a 200 MB guard, or one that
cannot be read, is skipped and NOT recorded as done, so it retries later.
Identity dedup is key-order independent. sqlite3 (dart:ffi) stays out of the
web build via a conditional import.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The VACUUM INTO snapshot import pulled dart:ffi via package:sqlite3, which
does not compile on web. Isolate it behind a conditional import (native impl
+ web stub); the migration is native-only and never runs on web.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The DB directory came from getApplicationSupportDirectory(), which on Windows
is derived from the executable's Company/Product version metadata. Two builds
with different metadata (or a future rebrand) resolved DIFFERENT %APPDATA%
folders and read DIFFERENT databases, so a user's history appeared to vanish
when they ran a different build.
Pin the desktop DB to a constant path (Windows: %APPDATA%\Offband MeshCore;
Linux pinned too, its support dir derives from the exe name). On first run at
the pinned location, migrate an existing DB in via SQLite VACUUM INTO (a
consistent snapshot that is safe even under a concurrent writer), choosing the
DB from the known canonical locations first and falling back to a bounded
scan of the app-data roots (no name blocklist) so a DB under an unknown
folder is still found. On any snapshot failure it aborts cleanly, leaving the
source intact and opening a fresh DB - never a raw file copy of a live WAL DB.
Adds sqlite3 as a direct dependency (pinned to drift 2.34.2's resolved 3.5.0).
Tests cover source selection, the folder-agnostic scan, and the snapshot
happy + abort paths.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The per-contact settings dialog (compression + telemetry grants) was
reachable only from a contact's chat via ellipsis -> Contact settings.
Extract it to a shared showContactSettingsDialog() and add a "Contact
settings" entry as the first item of the contact long-press/right-click
menu, so it opens the same surface from both places.
chat_screen's _showContactSettings now delegates to the shared dialog;
its _buildInfoRow stays (still used by the contact-info dialog).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Three findings from the adversarial review (standards#145), all real, all
fixed:
- Move-then-close inside the 400ms debounce lost the final position. The
service now setPreventClose(true) and saves on onWindowClose before
destroying the window, so the closing frame is always persisted.
- _isReachable mixed a display's visible ORIGIN with its total SIZE when
visibleSize was null, producing a rect offset from the real area. It now
uses the visible pair when both are known, else the total pair. Negative
visiblePosition (a monitor left of/above primary) is handled by intersect().
- The window listener and debounce timer are now released in onWindowClose.
flutter analyze clean, dart format clean, 509 tests pass.
The stock Flutter desktop runner never saved the window frame, so the window
always reopened at a default position and size (#349). Adds
WindowGeometryService (window_manager + screen_retriever): it saves the frame
on move/resize, debounced, and restores it on launch.
Restore is clamped against the connected displays. A window last placed on a
monitor that is now unplugged must not reopen off-screen where it cannot be
grabbed; if the saved frame does not overlap any display by at least 80px on
both axes it is discarded and the window opens at the default. A too-small or
undecodable saved value also falls through to the default (logged, not
swallowed).
Gated on PlatformInfo.isDesktop, so it is a no-op on mobile and web. Scoped
past just Windows deliberately: window_manager is desktop-only and the same gap
exists on Linux/macOS, so all three get the fix rather than Windows needing an
extra guard.
Known limitation, not addressed here: without a native runner change to start
the window hidden, restore repositions AFTER the window is shown, so there can
be a brief flash at the default position before it jumps to the saved frame.
Eliminating that needs a windows/runner edit; flagged for the owner to judge
during hardware test.
flutter analyze clean, dart format clean, tests pass. Windows-run verification
is the owner's (geometry is a real-desktop behaviour).
The migration's already-present branch called prefs.remove(key) and threw
the prefs copy away when drift already held the key. A build that writes to
SharedPreferences (a non-drift build, or any in-between test build) collects
new messages there, so the next drift run silently gapped that history out.
It cost 574 real channel messages, recovered from backups.
Union the prefs copy into drift by element identity (messageId, else
publicKey, else canonical JSON), keeping drift's live copy on a collision and
appending prefs-only elements. Verify the merged write before removing the
source. New `merged` counter in the report.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Replaces the stopgap (53f87f5) that committed
lib/storage/drift/offband_database.g.dart to green CI. Restores the
repo's "generated files are not committed" convention:
- dropped the .gitignore negation; the .g.dart is gitignored + untracked
(git rm --cached, working copy kept)
- added `dart run build_runner build --delete-conflicting-outputs`
immediately after `flutter pub get` in every Dart job: flutter_dart
(analyze), build.yml (all 6), release-signed (android-signed),
deploy-web (before build_pipe)
- keeps the drift_dev 2.34.0 / drift 2.34.2 / build_runner ^2.15.1 pins
from the stopgap for deterministic generation
Verified locally on the bench toolchain (3.44.1): deleted the working
.g.dart, ran build_runner from scratch (regenerated cleanly), confirmed
the generated file passes `dart format --set-exit-if-changed` and full
`flutter analyze --fatal-infos --fatal-warnings` is clean with the file
generated (not tracked).
For #335 / PR #348. Agent: SapphireCompass (session 8d755b5e)
CI failed on every job: offband_database.g.dart (drift's build_runner output)
was gitignored and no workflow runs build_runner, so analyze and all six
platform builds hit "Target of URI hasn't been generated". drift is the first
code in this repo needing codegen, which is why dev's CI never had to.
The workflows are not matrixed - six separate build jobs plus analyze, web
deploy and release-signed, each with its own `flutter pub get`. Adding a
codegen step to all of them is wide and easy to miss one. Instead the generated
file is committed (drift supports this): one file, works across every job and
workflow with no CI edits, and reproducible.
drift_dev is pinned (2.34.0) alongside the already-pinned drift, so a future
regeneration produces the same file rather than drifting from the committed
copy. Regenerated against the pinned versions before committing.
Follow-up worth having: a CI check that regenerates and diffs, so a schema
change without regen fails loudly rather than shipping a stale .g.dart.
Adversarial review (standards#145) of the storage/migration surface found four
real defects, all data-integrity. All confirmed against the code and fixed; no
false positives.
BLOCKER - concurrent save race. saveChannelMessages / saveMessages are
read-modify-write with an await gap, so two saves to the same key raced and the
second clobbered the first, silently losing messages (e.g. a message and its
delivery ack arriving together). BlobStore now provides synchronized(key, ...),
a per-key operation chain; every RMW - merge-save, remove, and the load-path
legacy migration - runs through it. Different keys stay concurrent. New test
fires two concurrent saves and asserts the union survives.
BLOCKER - non-atomic DB relocation. The documents->support move copied and
deleted each file (.sqlite/-wal/-shm) in turn, so a failure after the main file
moved stranded the DB across two locations and corrupted it. Now copies all
files, verifies each by size, and only then deletes the sources; on any failure
it rolls back the destination and leaves the original intact.
MAJOR - load-path legacy migration raced save. The #194 index->PSK adoption did
a blind write to the PSK key that could clobber a save that landed first. It now
runs under the key lock and MERGES (union) instead of overwriting, so both the
adopted history and any fresh message survive.
MINOR - channel merge key lacked a sender. Two senders posting identical text at
the same timestamp without a messageId would collide and lose one. The key now
includes the sender, matching MessageStore.
flutter analyze clean, dart format clean, 509 tests pass. Full Gemini log in
docs/llm-consultations/.
The persisted store was being overwritten with the windowed in-memory list, so
any channel or DM with more than _messageWindowSize (200) messages lost
everything older than the newest 200 on the first save after load. Observed
live: Public went 232 -> 201 in one session on a build that already had the
#333 fix, so this was not the race - it was windowing truncating the store.
This is the slow-erosion cause behind the whole 566 -> 231 -> 206 -> 201
history.
saveChannelMessages / saveMessages now MERGE into the persisted set instead of
overwriting: upsert by message identity so older persisted messages are kept,
new ones added, and the in-memory copy wins for edits/reactions/status. If the
existing history fails to decode, the save aborts loudly rather than merging
into an empty base and truncating (SAFELANE 6).
Deletion is now an explicit path - removeChannelMessage / removeMessage - since
the app deletes individual messages. Routing delete through the merging save
would resurrect them; the connector's deleteChannelMessage / deleteMessage now
call the explicit remove.
Windowing stays for display and memory; it no longer dictates what is stored.
Tests: 250-message history survives a 200-window save; new message appends;
edit is captured not duplicated; delete does not resurrect. 508 pass, analyze
and format clean.
Cost: a save now re-reads and re-encodes the channel/contact history. Cheap
with drift's per-key writes (#335); a future append-only schema removes even
that.
drift_flutter's native default is getApplicationDocumentsDirectory(), which on
Windows resolves to the user's Documents folder - redirected into OneDrive on
most machines (verified: drift_flutter 0.3.1 connect.dart). A live SQLite file
syncing to OneDrive risks lock contention and corruption, and it is simply the
wrong place for app data.
The DB now opens in getApplicationSupportDirectory() (%APPDATA% on Windows),
the same place SharedPreferences already lives, so all app data sits together.
Existing installs already have a DB in the old location, so opening a fresh one
there would abandon their migrated history. _appSupportDatabaseDirectory()
relocates it once on first run: it moves offband_store.sqlite and its -wal/-shm
sidecars from the documents dir to the support dir before drift opens, and
never deletes a source without a successful copy. If relocation fails it is
logged loudly and a fresh DB is created, with the migration re-running from
SharedPreferences rather than silently losing anything.
Web is unaffected: path_provider has no web backend and drift ignores
databaseDirectory there, using OPFS/IndexedDB.
Adds `path` as a direct dependency (was transitive).
flutter analyze clean, tests pass.
Wires the migration into startup and points the four bulk stores at drift.
SharedPreferences keeps the settings, which is what it is for.
Startup runs the migration after prefs are up and BEFORE any store reads, and
awaits it. Letting stores race a half-finished migration is precisely the
shape of #333, where a storage path silently chose the wrong key and 566 real
messages read as empty.
Converted: message_store, channel_message_store, contact_store,
contact_discovery_store. All four now have ZERO prefs get/set for bulk data.
Three safety properties, deliberately built in:
1. readWithPrefsFallback - if a key is somehow not in drift, the prefs copy is
still served rather than reading as empty. It logs a WARNING when it fires,
because a fallback during normal operation means the migration is
incomplete and someone needs to know. Silence here is what made #333 look
like data loss.
2. deleteEverywhere / keysWithPrefix span BOTH backends. A clear that only
removed the drift row would leave a pre-migration prefs copy to reappear
through the fallback, resurrecting deleted history.
3. The legacy-key migrations inside the stores now check both backends and
only touch prefs when a legacy key actually exists. This also carries the
#306 fix into this branch: the unconditional prefs.remove was still present
here, since this branched from dev rather than from the #306 work.
Verified: analyze clean, format clean, 504 tests pass, Windows and web both
build. Migration rehearsed earlier against a copy of a real 7 MB store: 69
keys, 5.21 MB, zero failures.
NOT yet run against live data - that happens on first launch of this build,
and the owner should have a prefs backup before that.
Moves message history, contacts and discovered contacts out of the settings
store. Settings stay in SharedPreferences, which is what it is for.
Ordering is the whole safety argument: WRITE, VERIFY BY READING BACK, and only
then remove the source. #333 was a storage path that chose a key silently and
made 566 real messages read as empty; deleting before verifying would make
that class of mistake permanent instead of cosmetic. On any failure the source
is left intact and the error is logged - never a silent drop (SAFELANE 6).
Idempotent by construction: a key already present in drift is not overwritten,
so re-running is a no-op. If an older build re-writes a migrated key into
prefs, the migrated copy wins and the stale prefs copy is discarded rather
than promoted.
Rehearsed against a COPY of a real 7 MB store, as the plan required before
touching live data:
REHEARSAL: 69 migrated, 0 failed, 5.21 MB, 69 bulk keys expected
Every key checked for exact length, confirmed removed from prefs, and every
settings key confirmed untouched. The live store was never opened.
Two things the tests caught that review would not have:
1. getString THROWS on a non-string value rather than returning null, so a
non-string under a bulk prefix was counted as a migration FAILURE. It now
type-checks with prefs.get() and skips. Alarming falsely is its own bug.
2. The `contacts` prefix was checked against the real store rather than
assumed: it matches only the 6 bulk contact blobs, and correctly does NOT
match contact_unread_count*.
Not yet wired into app startup - that is the switchover, and it is deliberately
a separate commit so this can be reviewed on its own.
flutter analyze clean, dart format clean, 504 tests pass.
Step 1 gate, web half. Building is not evidence, so this was run in a browser.
Assets: sqlite3.wasm (748 KB) and drift_worker.js (355 KB), both taken from
the SAME drift-2.34.2 release so they are built against each other. Validated
before use - the wasm has correct WebAssembly magic (0061 736d) and carries
SQLite 3.53.3; the worker is genuine compiled Dart JS.
Note the filename: the release ships `drift_worker.js`, not the
`drift_worker.dart.js` the docs name. The database URI was corrected to match,
which would otherwise have been a silent 404 at runtime.
Browser result, served over HTTP with COOP/COEP and Content-Type:
application/wasm:
Using WasmStorageImplementation.opfsLocks
PROBE-OK open+write+read
PROBE-OK 6MiB round-trip (localStorage cap is 5MiB)
drift selected OPFS, the top storage tier, and round-tripped 6 MiB - data the
current localStorage-backed web build physically cannot hold. No exceptions.
Adds web/_headers for Cloudflare Pages: forces application/wasm on the wasm
asset and sets COOP/COEP for cross-origin isolation. Verified to ship into
build/web. Without COOP/COEP drift still works, falling back to IndexedDB,
which is slower but still unbounded next to localStorage - a performance tier,
not a correctness requirement.
web_probe/ is a standalone entrypoint that exercises ONLY the drift stack, so
a pass or fail is unambiguous without BLE and the rest of the app in the way.
It is not part of the app build.
Still no user data touched and no store switched over. The migration is the
next step.
flutter analyze clean, dart format clean, 497 tests pass.
Step 1 of #335, deliberately before any data migration: prove the store works
before trusting it with message history.
Adds drift + drift_flutter, a key/value StoredBlobs table mapping 1:1 onto the
existing SharedPreferences keys, and verifyReadWrite() as a runtime probe.
Key/value rather than relational tables on purpose: it maps directly onto the
current store interfaces, so callers do not change and the migration can be
verified row-for-row against the old keys. Relational schemas come later, once
this is proven and querying is actually wanted.
Verified here:
- 3 tests pass, including a 6 MiB round-trip - larger than the 5 MiB
localStorage cap that makes the web build impossible today
- flutter analyze clean
- Windows release build succeeds
- Web release build compiles
Two findings from the gate that a plan-on-paper would have missed:
1. sqlite3_flutter_libs 0.6.0+eol is a DEPRECATED no-op. From sqlite3 v3.x it
is unnecessary and drift_flutter already covers it, so it is not a direct
dependency here.
2. The web build COMPILES BUT WOULD FAIL AT RUNTIME: drift needs sqlite3.wasm
and drift_worker.dart.js shipped in web/, and neither is present. Per drift
docs they are downloaded from GitHub releases, version-matched to
pubspec.lock (drift 2.34.2, sqlite3 3.5.0), and the server must serve .wasm
as Content-Type: application/wasm. Not done here - downloading files needs
explicit human approval.
Also noted: the drift_dev CLI does not compile at these versions
(allSchemaEntities missing from the drift3_preview GeneratedDatabase), so its
asset tooling is unavailable. build_runner code generation is unaffected.
No user data is touched. No store is switched over. The migration is the next
step and remains gated on the web assets question.
Measured, not guessed. Cumulative per-code frame timing named the owner:
frame handler cumulative (2002ms total sync): code=3 2002ms/105calls
code=3 is RESP_CODE_CONTACT, and it accounted for ALL of the synchronous
frame time at ~19ms per contact. That is why a per-call 100ms threshold never
fired: no single call is slow, the volume is.
The 19ms is _handleContact calling notifyListeners() for every contact. Each
notification is a synchronous full-tree rebuild, and each rebuild walks the
contact list, so the cost grows with the address book. The channel handler
directly alongside already guards its notify with _isLoadingChannels; the
contact handler had no equivalent.
Notifications are now throttled to one per 250ms while _isLoadingContacts is
set, and remain immediate outside a pull so single adverts still update live.
Throttled rather than suppressed so the sync progress bar keeps advancing. The
existing notifyListeners() when the pull completes still fires, so the final
state cannot be stale.
Scope of what this accounts for, stated honestly: measured synchronous frame
work was ~2s per report window and the pull showed ~4s total, against a ~30s
observed freeze. This removes the largest measured contributor. If the stall
persists, the remainder is outside the frame handlers and the cumulative
counters will show frame time has dropped, which narrows it further.
flutter analyze clean, dart format clean, 494 tests pass.
The conversation-load metric was misread, and nearly caused a fix to the wrong
thing. It reported store=30138ms across 300 contacts, which looked like
loadMessages costing ~100ms each. It is not:
- The device-wide fallback key `messages_<dev>` does not exist on the
reporting machine, and only 11 per-contact message keys exist in total, so
loadMessages does three in-memory map lookups and returns empty.
- storeWatch spans an `await`, so it measures WALL time including event-loop
queueing, not work. merge, which is synchronous and has no await, measures
0ms - consistent with the isolate being busy elsewhere while those awaits
wait their turn.
So the 30s is ~300 awaits each waiting for a busy isolate, not 30s of storage
work. The metric label now says so, rather than inviting the same misreading.
The per-call 100ms threshold cannot see the actual shape here: a handler that
takes 40ms and runs 234 times owns ~9s of isolate time and never trips it.
Frame handlers now accumulate synchronous time per frame code and report the
top codes by total, which names the owner regardless of per-call cost.
Diagnostics only, no behaviour change.
flutter analyze clean, dart format clean, 494 tests pass.
The stores violated the no-silent-failures rule, and that is why tonight's
diagnosis took as long as it did. Two classes:
1. Ten catch blocks swallowed decode errors and returned an empty collection.
`catch (_) { return []; }` means a corrupt or unreadable store presents to
the caller as "no data", so the user sees no contacts, no channels or no
history with nothing anywhere explaining why. Indistinguishable from data
loss. Every one now logs what failed, with the store named, before
returning.
2. ChannelMessageStore._storageKey fell back from the PSK-identity key to the
legacy slot-index key without a word. That fallback is correct before a
channel list exists, but once history has been migrated to the PSK key it
reads a DIFFERENT key and returns empty. That is exactly #333: a user's 566
Public messages were intact on disk while the app showed nothing, with no
error, no parse failure and no log line to follow.
It now warns when it falls back while a resolver is installed, since that
combination specifically means the channel list was not loaded first.
Had either of these been loud, #333 would have been a one-line log read
instead of a multi-hour investigation that looked like data loss.
flutter analyze clean, dart format clean, 494 tests pass.
The startup loads are now clean (all steps ~0ms, down from 37792ms), but the
contact pull still stalls in multi-second bursts: 8.0s and 7.7s at +25s..+52s
on the last run, matching the reported freeze from ~15s to ~49s.
No frame handler exceeds 100ms, because the work is not in one.
_loadMessagesForContact is fired unawaited from the contact handler, so it runs
after the handler returns and escapes that timing entirely.
This times it directly and reports cumulative store time versus merge+notify
time every 25 contacts, so the next run says which half is responsible rather
than requiring another guess. Two candidates it will discriminate:
- store: prefs read + jsonDecode per contact
- merge+notify: the per-contact notifyListeners, i.e. one full widget tree
rebuild per contact, each of which walks the contact list
Diagnostics only, no behaviour change.
flutter analyze clean, dart format clean.
The previous commit fixed loadSmazEnabled and killed the post-connect stall
(startup-load channelSettings: 37792ms -> 0ms, measured twice). The contact
pull still choked, in multi-second bursts, each landing immediately after an
"Added new contact" line.
No frame handler exceeded the 100ms threshold, because the work is not in a
handler: the contact handler fires _loadMessagesForContact unawaited, so it
runs after the handler returns and escapes that timing.
message_store.loadMessages had the same unconditional prefs.remove(oldKey),
and it runs ONCE PER CONTACT during a pull. On Windows every prefs mutation
rewrites the entire file, so a 234-contact pull meant 234 full multi-MB writes
for legacy keys that do not exist.
A sweep found the identical pattern in six more stores: contact_store,
unread_store, channel_store, community_store, contact_group_store and
channel_order_store. Those run once per load rather than per item, so they are
far cheaper, but it is the same bug and they are fixed the same way.
In every case the removal now happens only when a legacy key actually exists,
paired with the write that migrates it, so the cleanup still occurs on a real
migration.
flutter analyze clean, dart format clean, 494 tests pass.
Back at the root was calling SystemNavigator.pop(). That is not "background
the app": on Android it calls finish() on the activity, which tears down the
Flutter engine and drops the radio connection. Backing out and reopening
therefore landed on a disconnected radio needing a fresh connect. Wrong call
for the job, and worse than the behaviour it replaced.
Backgrounding without finishing needs moveTaskToBack, which has no Flutter
equivalent, so it goes over a method channel:
- MainActivity exposes meshcore_open/app_lifecycle with a moveTaskToBack
method, alongside the existing USB channel.
- AppBackgrounder wraps it and is a no-op off Android, where programmatic
backgrounding either does not apply (desktop windows close by their own
chrome) or is forbidden (iOS). On those platforms back at the root stays
unhandled rather than doing something destructive.
- AppShell awaits it instead of calling SystemNavigator.pop().
The activity stays alive, so the connection survives and reopening returns to
where the user was.
flutter analyze clean, dart format clean, 469 tests pass.
The scanner's auto-entry was gated on activeTransport == bluetooth, so the
dead end fixed in the previous commit still applied to USB and TCP: a user
landing on the scanner with a live serial or network connection stayed stuck,
and pressing Connect did nothing, since there was nothing left to connect.
Both are real deployment paths, so the gate is removed and the check is now
transport-agnostic.
The first-connect path for USB and TCP is unaffected. Those screens are pushed
on top of the scanner and navigate themselves with pushReplacement; while they
are connecting they are the current route, so the scanner's isCurrentRoute
guard is false and it does not also push. That guard, not the transport check,
is what prevents a double navigation.
flutter analyze clean, dart format clean, 469 tests pass.
Two bugs, reported together from an S25 FE test.
**Back popped to the scanner.** The previous commit made back pop whenever the
route could be popped. The primary views sit on top of the scanner, so from
the channel list back landed on the radio-connect screen. That is a regression
I introduced: the `PopScope(canPop: !isConnected)` I removed existed to
prevent exactly this, and I removed it without establishing why it was there.
Back now distinguishes the two cases. A primary view (one carrying the bottom
bar) hands back to the OS and backgrounds the app. A detail screen (a channel
chat, which has no selectedIndex and is genuinely pushed) pops to its list.
Reaching the scanner is what Disconnect is for, not what Back is for.
**The scanner was a dead end while connected.** It only routed into the app on
a connection transition, behind a one-shot flag that never reset while
connected. So once showing with a live connection it stayed there, and
pressing Connect did nothing, since there was nothing left to connect. It now
routes in whenever it is the visible route and a connection is live, guarded
against double-push by a dedicated flag rather than reusing
_changedNavigation, which separately gates the disconnect-on-dispose cleanup.
Known limitation, pre-existing and unchanged: this auto-entry is bluetooth
only. USB and TCP connect from their own screens, which navigate themselves.
flutter analyze clean, dart format clean, 469 tests pass.
Owner hardware test on an S25 FE and Windows turned up five issues. Three of
them shared one root cause.
**Map: always show names.** New mapAlwaysShowNames setting, toggled from the
map panel. Names were shown only past zoom 14; the override wins at any zoom.
Evaluated at build rather than only on camera movement, so flipping the switch
repaints immediately instead of waiting for the next pan.
**Panel footer on every screen.** Disconnect and Settings now also appear
inside a channel. The footer is app-level, so it belongs anywhere the panel
is, not just the primary views.
**Back button (three reported symptoms, one cause).** Channels, Contacts and
Map each wrapped themselves in `PopScope(canPop: !isConnected)`, so while
connected the system back press was swallowed whole. That explains all three:
an open drawer would not close, Android would not background the app, and on
Windows the pinned layout has no Scaffold drawer, so the app bar auto-implied
a back arrow whose press PopScope then discarded - a button that visibly did
nothing.
AppShell now owns back handling for every screen that uses it, in order:
close an open drawer, else pop the route (a pushed chat returns to its list),
else hand back to the OS via SystemNavigator.pop() so Android returns to the
home screen or the previous app. The three per-screen PopScope wrappers are
removed. AppShell becomes stateful to hold the scaffold key this needs.
flutter analyze clean, dart format clean, 469 tests pass. Not yet re-tested on
hardware.
Epic D. The Map view has no list, so its nav panel was empty. It now carries
the map's own layer controls.
Those six toggles (chat nodes, repeaters, other nodes, discovery contacts,
guessed locations, overlaps) previously existed only in a modal behind the
floating button, so filtering the map meant covering the map with a dialog to
do it. In the panel, and especially pinned on a wide screen, they stay visible
and the map updates underneath as they are flipped.
Both surfaces read and write the same AppSettings, so a change in one is
reflected in the other. The floating-button dialog is deliberately left in
place: on a phone it is fewer taps than opening the drawer, and the panel is
the pinned-desktop path. If that duplication is unwanted, removing the dialog
is a one-line follow-up.
No new settings or storage; the existing setters were reused unchanged.
flutter analyze clean, dart format clean, 469 tests pass. Not yet run on
hardware.
Epic C, rescoped. The original plan was to eliminate the overflow menu and
move everything into the drawer. An inventory showed that premise was wrong:
the ellipsis is six different menus sharing an icon, and most of what they
hold is screen-level (force flood mode, path management, delete all
discovered). Those cannot live in a global nav panel, which has no notion of
which contact or channel is meant.
Only Disconnect and Settings are app-level, which is exactly why they were
duplicated across three screens. Those move; everything else stays put.
- AppShell gains optional onDisconnect / onSettings, rendered as a pinned
footer in the nav panel. Disconnect keeps the error colour it had as a red
menu entry, since it drops the radio connection.
- Channels, Contacts and Map pass both and drop those two menu entries.
- Map's overflow menu is removed entirely, having nothing left.
- Channels keeps its menu only for Manage Communities, which was already
conditional on having joined a community.
- Contacts keeps its menu for Discovered contacts.
- No detail screen menu is touched.
Pinned on a wide screen, Disconnect and Settings are now visible without
opening anything.
flutter analyze clean, dart format clean, 469 tests pass. Not yet run on
hardware.
The connect freeze is a legacy-key migration that writes even when there is
nothing to migrate.
loadSmazEnabled called prefs.remove(oldKey) unconditionally whenever the
scoped key was absent, which is the normal case for a channel that has never
had the setting changed. On Windows shared_preferences re-serialises and
rewrites the ENTIRE prefs file on any mutation, so each of those no-op removes
cost a full multi-MB write. Repeated across every channel slot on every
connect, that is tens of full-file rewrites back to back.
Measured with the instrumentation from the previous commit, on the reporting
device (6.9 MB prefs, ~42 channel slots):
startup-load channelOrder 0ms
startup-load contactCache 12ms
startup-load channelSettings 37792ms <-- here
startup-load cachedChannels 0ms
startup-load channelMessages 21ms
startup-load unreadState 0ms
startup-load discoveredContacts 11ms
No frame handler exceeded 100ms, confirming the stall was local storage work
and not radio traffic.
The remove now happens only when a legacy key actually exists, alongside the
write that migrates it. contact_settings_store had the identical pattern per
contact and is fixed the same way.
This also explains the platform gap in the original report: Android's backend
batches into native storage, so the same code costs almost nothing there,
while every no-op remove on Windows is a full file rewrite.
flutter analyze clean, dart format clean, 469 tests pass.
Two reported freezes: one shortly after connect, one during channel sync,
with the sync progress bar never repainting and then completing all at once.
That is the UI isolate being blocked, and the log already shows the
signature - a long silence followed by many frames sharing one timestamp,
which is queued input flushing after the loop frees up.
The previous fix (batching 600 per-contact notifications) moved the stall from
43.6s to 40.1s, which is noise. It was the wrong culprit, so this replaces
guessing with measurement:
- The post-SELF_INFO cache load is split into named steps, each timed and
logged as `startup-load <step> took Nms`. The step order is unchanged, and
cachedChannels still runs before channelMessages so PSK-keyed history
resolves.
- Every RX frame handler is timed, and any that occupies the isolate for more
than 100ms logs `slow frame handler: code=N blocked UI for Nms`. This names
the handler wherever it is, rather than requiring a guess about which one.
No behaviour change. Diagnostics only, kept as Perf-tagged logging because
this class of stall is worth catching again.
flutter analyze clean, dart format clean, 469 tests pass.
Diagnosed from the app's own log. After SELF_INFO the client went completely
silent for 43.7s, then flushed a burst of radio frames all sharing the
identical timestamp - queued while the event loop was blocked, not a slow
radio.
Cause: loadContactCache() looped over every cached contact calling
_ensureContactSmazSettingLoaded and _ensureContactCyr2LatSettingLoaded, and
each of those notified on completion. With 300 contacts that is 600
notifyListeners() calls in a burst, each rebuilding the whole widget tree, and
each rebuild itself walks the contact list. O(contacts^2) on the UI isolate.
Both loaders are now awaitable and take notify: false for bulk use.
loadContactCache awaits them all and notifies once.
Measured on the reporting device (radio 4f6585264e, TCP): 300 contacts, 1090
discovered contacts, 12 channels holding 1841 messages, 6.9 MB prefs file.
Log evidence: 43.7s stall between "Pulled battery" and the first channel
response, followed by same-millisecond frame delivery.
This is the Windows-visible freeze in #306. The platform asymmetry is
consistent with contact-list size rather than anything desktop-specific, so
whether Android is genuinely faster or simply has a smaller address book on
its paired radio is still open - #306 stays open pending a like-for-like
re-measure.
flutter analyze clean, dart format clean, 469 tests pass.
Channel history is keyed by the channel's PSK (#194), and the resolver set on
the store reads that PSK out of _channels. After SELF_INFO, loadCachedChannels()
and loadAllChannelMessages() were both fired without awaiting, so the messages
could load while _channels was still empty. The resolver then returned null,
_storageKey() silently fell back to the legacy slot-index key, and channels
read as empty even though their history was intact under the PSK key.
Silent because the fallback is a legitimate code path: there is no error, no
parse failure and no log line, just an empty list. It is indistinguishable
from data loss to the user.
Observed on a live radio: Public held 566 messages under
channel_messages_<dev>psk_8b3387e9..., and the app displayed nothing.
loadAllChannelMessages() now runs only after loadCachedChannels() completes.
The startup call in main.dart already awaits in the right order; it runs before
any radio is connected, which is the source of the "Public key hex is not set"
warnings at launch, and is harmless once this post-connect load is correct.
Was previously committed against #291 on the nav-redesign branch; split out
here under its own issue.
formatNotificationText used GifHelper.parseGif (Giphy only) while the chat
renders via resolveGifUrl (Giphy + allowlisted Tenor CDN). An allowlisted
Tenor GIF therefore rendered inline but still dumped a raw URL into the
notification tray.
Point both at resolveGifUrl so the tray summarises exactly the set the chat
displays. Off-allowlist URLs still pass through as plain text, pinned by test.
This is the same duplicate-detection drift that caused #284; closing it at the
source rather than shipping a known inconsistency.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Pasted GIF links rendered as plain text because parseGif only matched the
compact forms. Adds GifHelper.resolveGifUrl, which maps any supported payload
to the URL that renders it, and widens coverage to the links people actually
paste.
Allowlist is deliberately narrow, Giphy and Tenor only. Off-list URLs return
null and stay plain tap-to-open links, never auto-fetched: auto-loading a
stranger's URL leaks the viewer's IP and enables tracking-pixel abuse, and
inline-rendering unmoderated hosts is a malware and inappropriate-content
vector. Tests pin the rejections, including a lookalike host
(giphy.com.evil.example) and a general image host (imgur).
Newly rendered:
- i.giphy.com/<id>.gif and .webp, the form a browser copy produces
- media.tenor.com and c.tenor.com direct assets
A tenor.com/view/<slug>-<id> PAGE url is deliberately NOT matched. The modern
Tenor CDN path uses an opaque hash that cannot be derived from the page id, so
resolving one requires the Tenor API and a key. Such a link stays plain rather
than silently rendering the wrong image. Supporting it is a separate decision.
Also removes the hardcoded 'https://media.giphy.com/media/$gifId/giphy.gif'
literal duplicated across all four render call sites. That duplication is the
same drift that caused the #284 notification regression, where a second
hand-rolled copy of GIF detection went stale.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Notifications summarised a GIF as "Sent a GIF" using their own hand-rolled
`^g:[A-Za-z0-9_-]+$` regex rather than GifHelper. #282 changed the payload to
`https://giphy.com/gifs/<id>`, which that regex does not match, so GIF
notifications regressed to dumping the raw URL into the tray.
Detection now goes through `GifHelper.parseGif`, matching how the adjacent
reaction case already delegates to ReactionHelper. That restores the summary
and covers every form the app can send, including replies (`@[Name] ` +
payload), which the old regex never matched even for legacy `g:<id>`.
Verified no other hand-rolled `^g:`/`^m:`/`^s:`/`^r:` payload regexes remain
outside their owning helpers, so this class of drift is closed.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A capability-gated control that doesn't appear is indistinguishable from
a broken one. Diagnosing "the FEM LNA toggle is missing" on real hardware
required a rebuild, because the caps byte was never surfaced and the app
debug log records nothing in a release build unless logging is enabled.
Adds an "Offband capabilities" row to Device Info showing the raw caps
byte, firmware version code, and which gated features it grants, e.g.
"0x02 (v16) - block". That turned an unexplained missing toggle into a
one-look answer: firmware v16 running, FEM LNA bit clear, client correct.
Also logs the same values at device-info parse for anyone who does have
logging on.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Completes the client half against the firmware as-built (#298):
- Device-info offset 83 carries the FEM LNA state on v16+, appended
unconditionally (reads 0 on non-capable boards). parseFemLnaState is
length-guarded and returns null pre-v16. Byte presence signals firmware
version, not capability — the cap BIT gates the UI.
- 0xC3 reply handler adopts the reported value verbatim: firmware returns
post-apply hardware state, not an echo, so a refused write surfaces as
truth rather than a lie.
- setFemLna / requestFemLnaState no-op unless the capability bit is set,
so a non-capable radio never sees 0xC3 traffic.
- Radio Settings toggle rendered only when the bit is set, driven by
connector state via ListenableBuilder rather than local optimistic
state, so it always shows what the radio reports.
- errCodeUnsupportedCmd documented: a mis-gated request draws [0x01][0x01],
not [0x01][0x06]. Neither is 0xC3-prefixed, so no new error path.
Gating is on the bit alone, never model or version: firmware derives it
from a runtime FEM probe, so two Heltec V4s can legitimately disagree and
rak3401 reports false by design (its SKY66122 gates LNA and PA together).
13 tests. Not hardware-validated — firmware 0xC3 is still on a branch.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Adds the client half of the Heltec V4 FEM LNA control surface at the
protocol level: 0xC3 CMD_OFFBAND_FEM_LNA (0x01 SET [value], 0x02 GET),
a 3-byte [0xC3][sub][value] reply parser, and the
OFFBAND_CAP_FEM_LNA = 0x04 capability gate.
Deliberately a fork-private command rather than a sixth byte on the
stock CMD_SET_OTHER_PARAMS (38): that frame is shared with upstream
MeshCore and is sent to every radio regardless of fork, so widening it
would perturb stock firmware. Nothing is emitted unless the capability
bit is set.
The gate checks the bit only, never model or version — firmware derives
it at runtime from the auto-detected FEM chip, so it is a per-unit
answer and two Heltec V4s can legitimately disagree.
PROVISIONAL: firmware owns the caps byte and has not yet confirmed 0x04
is free, and the 0xC3 setter is not built yet. Spec sent to
OffbandMesh/meshcore-firmware#298. Do not merge before firmware
confirms the shape as built.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Gemini adversarial review finding (minor, accepted). _pathDiag derived the
expected byte count from the connector's global _pathHashByteWidth, which is
the CONNECTED device's width, while each Contact now carries the width its
path was actually captured at.
Failure case: a contact discovered on a 1-byte net (3 hops, 3 bytes) viewed
while connected to a 2-byte device computes expected = 3 * 2 = 6 and logs
"bytes=3/6 TRUNCATED" for a complete, correct path. A false TRUNCATED in the
very diagnostic added to investigate truncation would actively mislead.
_pathDiag now takes the per-path width; all seven call sites pass
contact.pathHashWidth.
Refs #298, #309
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The firmware path-len byte is packed and self-describing (src/Packet.h:79-84):
high 2 bits = hash size - 1, low 6 bits = hash COUNT (hops), byte length =
count * size. The app treated the low 6 bits as a byte count and ignored the
width, on the strength of a comment asserting the high bits were "not reliably
populated". Both halves of that comment were wrong.
Receive: Contact.fromFrame read `count` bytes where firmware meant `count`
hops, so at 2-byte width it kept HALF of every path and discarded the rest.
DAYTON VA's 2-hop route became a 2-byte fragment, which is why every repeater
login in Bandit's capture timed out.
Send: buildUpdateContactPathFrame wrote the count raw, leaving mode bits 00.
That tells the radio "1-byte hashes" while handing it 2-byte hash data, so it
routed to nodes never on the route. This is the likelier cause of the observed
"routes via c6 and 5c separately" behaviour.
Also fixes a third unit inconsistency: setContactPath wrote customPath.length
(bytes) into pathLength (hops) after every path set.
Contact now carries pathHashWidth per path, so a stored path can never be
re-sliced at a width it was not captured at. realHopCount() is removed rather
than adjusted: it divided an already-correct hop count, halving it at 2-byte
width, and every call site was compensating for a bug that no longer exists.
Migration (Ben's call, option A): records written before this change have no
pathHashWidth and hold truncated bytes that cannot be recovered by
reinterpretation, so their path is dropped and the contact reverts to flood
until the radio re-supplies it. Logged, not silent.
Tests asserted the old semantics (expect(realHopCount(2, 2), 1)) and were
rewritten against firmware truth rather than adjusted to keep passing. Added
send-side encoding coverage, which did not exist. 456 tests pass.
Refs #240, #279, #299
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Strycher
