fix(database): make DB updates atomic across device switches

[jeremiah-k]

Overview

This hardens database updates and cross-transport device association during connection changes.

Several packet, reaction, mesh-log, and traceroute operations previously performed related reads and writes through separate DAO calls. Failure, cancellation, or concurrent updates between those calls could leave partial or lost state.

Database association also needed stronger coordination between active operations, transport-session authority, merge publication, routing recovery, database retirement, and shutdown. Delayed callbacks or queued frames from an older BLE, TCP, or USB session could otherwise continue after a replacement connection became active and affect the wrong device database.

This builds on #6096’s cross-transport database unification, #6228’s trusted device-identity migration, #6231’s retry-idempotent merge marker, #6233’s write-through-withDb merge barrier, and #6236’s switching Discovery DAO.

Foundation for: #6255, #6259

Review guide

The three commits are intended to be reviewed in order:

1. Packet-state transactions — moves packet, reaction, last-read, deletion, and SFPP read-modify-write operations into transactional DAO methods.
2. Mesh-log and traceroute transactions — makes paged log deletion and traceroute snapshot replacement atomic.
3. Association and session lifecycle — carries transport-session authority through handshake, packet processing, persistence, database association, rollback, retirement, and shutdown.

The first two commits establish the transactional primitives. The third commit applies them to transport changes and contains most of the lifecycle and concurrency tests.

Scope boundary

This PR removes raw transport identifiers and payloads from diagnostic string output, but it does not redesign the persisted MeshLog payload or retention contract. That storage is consumed by telemetry, position, traceroute, and diagnostic-history features and requires a separate compatibility and privacy audit before its schema or retained payloads can safely change.

Changes

• Moved packet, reaction, last-read, SFPP, mesh-log deletion, and traceroute replacement operations into transactional DAO methods.
• Kept SQLite bind-limit chunking inside the owning transactions.
• Prevented confirmed SFPP statuses from being downgraded and preserved complete stored hashes during prefix-based status updates.
• Added writer admission and bounded draining around database association, switching, recovery, retirement, cleanup, and shutdown.
• Prevented arbitrary database callbacks from being transparently replayed after execution may have begun.
• Added recovery for merges committed before routing metadata was finalized.
• Kept published, retired, or still-owned Room instances open while process consumers may still reference them.
• Made cache cleanup, retirement, and shutdown bounded, idempotent, cancellation-safe, and failure-safe.
• Moved telemetry parsing outside the serialized database dispatcher while retaining bounded reads and atomic deletion.
• Carried immutable transport address and session generation with received radio frames.
• Revoked stale transport callbacks before teardown and rejected queued frames from sessions that are no longer active.
• Preserved session authority through handshake, configuration, packet processing, persistence, and notification side effects.
• Required matching address and generation before database association.
• Cancelled or rejected in-flight association work when session authority changed.
• Added awaited and per-node-serialized persistence where session authority must remain held until the write completes.
• Kept raw transport addresses, device IDs, and radio payloads out of diagnostic string output.

Validation

Added deterministic coverage for:

• transactional rollback and concurrent read-modify-write updates;
• chunked packet and mesh-log deletion;
• traceroute replacement rollback;
• writer admission, cancellation, merge failure, routing recovery, retirement, eviction, and shutdown;
• callback provenance, transport creation failure, teardown ordering, and same-address reconnect generations;
• queued old-session frames and early-packet replay;
• complete session-bound handshake and configuration processing;
• cancellation of stale in-flight association;
• packet, node, configuration, and notification persistence remaining inside session authority;
• fake-service behavior matching production transport admission and reconnect rules.

The corresponding source head passed the Android build, lint and KMP smoke compile, all applicable test shards, screenshot validation, and the aggregate workflow check.

Extended debug testing exercised telemetry, positions, messages, configuration, database cleanup, Activity restarts, weak-signal BLE disconnects, reconnection, and device switching without a closed Room pool, SQLite lock, database-acquisition timeout, stale database association, or process crash.

Summary by CodeRabbit

• New Features
  • Added transport-session awareness across radio reception, configuration handshake, messaging, telemetry, and association.
  • Introduced session-bound connection identity tracking for the current device session.
  • Improved atomic database operations for updates, deletes, and snapshot persistence.
• Bug Fixes
  • Discards stale/revoked session events so they can’t overwrite newer connection state, buffered packets, or persistence.
  • More reliable device switching and shutdown, with safer rollback and recovery around database merges and pending routes.
• Tests
  • Expanded coverage for session lifecycle, concurrency, transactional behavior, recovery, shutdown, and stale-data handling.

[coderabbitai]

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 3ea338cc-98ba-4ce7-8d51-141596ff58b7

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

📝 Walkthrough

Walkthrough

This change makes radio processing, handshake state, device association, database writes, and shutdown session-aware. It adds immutable received frames, stale-session rejection, atomic DAO operations, writer admission gates, pending-route recovery, logical retirement, and suspendable database shutdown.

Changes

Session-bound radio and handshake flow

Layer / File(s)	Summary
Transport session contracts and frame propagation core/repository/*, core/service/*, core/testing/*, desktopApp/.../NoopStubs.kt	Radio data now travels as ReceivedRadioFrame values carrying immutable RadioSessionContext; stale callbacks and frames are rejected.
Session-aware handshake and association core/data/.../manager/*, core/service/.../RadioControllerImpl.kt	Handshake callbacks, configuration persistence, node processing, connection identity, and device association now validate the active transport session.
Session and concurrency validation core/data/.../*Test.kt, core/service/.../*Test.kt, core/testing/.../*Test.kt	Tests cover stale frames, session revocation, identity publication, association cancellation, buffering, and persistence ordering.

Atomic database operations

Layer / File(s)	Summary
Transactional DAO and repository operations core/database/.../dao/*, core/data/.../repository/*	DAO-level transactions add key-based packet updates, SFPP status application, atomic message/log deletion, paginated log snapshots, and traceroute snapshot replacement.
DAO transaction coverage core/database/src/commonTest/.../dao/*Test.kt	In-memory database tests cover rollback, chunk boundaries, pagination, identity matching, no-downgrade status updates, and empty replacements.

Database lifecycle

Layer / File(s)	Summary
Database association and lifecycle state core/database/src/commonMain/.../DatabaseManager.kt, DatabaseMerger.kt, DatabaseConstants.kt	Database association now uses writer gates, active-session checks, pending-route metadata, merge rollback, logical retirement, cache coordination, and lifecycle-controlled shutdown.
Recovery, admission, retirement, and shutdown tests core/database/src/commonTest/.../DatabaseManager*Test.kt, DatabaseManagerTestFixture.kt, core/database/src/jvmTest/.../DatabaseManagerPendingRouteRecoveryJvmTest.kt	Tests exercise writer draining, cancellation, pending-route repair, restart recovery, retirement cleanup, close failures, shutdown races, and bounded admission.
Application shutdown integration androidApp/src/main/kotlin/org/meshtastic/app/MeshUtilApplication.kt	Application termination cancels the application scope before awaiting suspendable database closure with runBlocking.

Estimated code review effort: 5 (Critical) | ~120 minutes񎟙

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly reflects the main focus on atomic database updates during device switches.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[jamesarich]

This is strong work — I went through it in detail. The transactional consolidation checks out: the new @Transaction DAO methods (updateLastReadMessage, updatePacketByKey, updateReactionByKey, applySFPPStatus, deleteMessagesAtomic) genuinely replace the multi-call repo logic, chunking now lives inside the transactions with a cross-chunk test, and the writer-release paths (abort→source, commit→dest) are covered including cancellation. The traceroute rollback-injection test is a nice touch. No regression of the all-writes-through-withDb policy from #6233 that I can find.

Four issues to resolve before this leaves draft, roughly in severity order:

1. The withDb retry path bypasses the writer gate. beginWrite() blocks admission on writerGate while an association is in flight, but retryRegisteredDbBlock registers via registerWriter(db), which takes writerTrackerMutex and bumps activeWriters without checking the gate. Scenario: a writer registers on a pre-switch instance X; an association arms the gate for source S and drainWriters(S) sees zero (the writer is counted against X, not S); the merge starts; the writer's op on X throws a closed-pool exception; retryDb = _currentDb.value = S; the retry writes into S mid-copy and is silently lost at retirement. Routing the retry admission through beginWrite (or gate-checking inside registerWriter) closes it.

2. The routing-metadata failure path doesn't self-repair the way its comment says. If the datastore alias write fails after a committed merge, the comment claims the next connect repairs it via the already-unified branch — but a real reconnect resolves the address through the buildDbName fallback, which republishes the merged-away source as active. Writes land there until associateDevice runs, and then the merge-marker skip discards them. The existing test only exercises the no-switch in-process retry, which is why it passes. Suggest retrying the alias edit, or repairing the alias in switchActiveDatabase when the resolved name carries a merge marker.

3. The FTS backfill writes outside the barrier. backfillSearchIndexIfNeeded writes through a captured db under NonCancellable with no registerWriter, so a drain can't see it and backfill writes can land on a source mid-merge and be lost. Pre-existing, but this PR establishes the "no writer slips past the barrier" invariant for everything else — worth registering the backfill too while you're here.

4. gate.await() is unbounded, and a non-Exception Error between arming and release leaks the gate — after which every write in the app suspends forever. A finally-hardened release (and possibly a generous await timeout) would make this failure mode survivable. Related, smaller: close() cancels managerScope before joining, which can strand an in-flight retireDatabase at its mutex suspension so shutdown skips that file's deletion.

Two nits: the PR body says mesh-log cleanup is atomic, but the protobuf-parsing selection runs outside the transaction (the code comment acknowledges this — the body overstates it); and the closeCachedDatabase KDoc still says "5 connections per WAL-mode DB," which contradicts the deliberate single-connection-pool config.

On the retirement lifecycle: accumulation is bounded (one retired instance per merge event, realistically 1–2 per process) and process death is safe since SQLite recovers the WAL, so deferring physical close to shutdown is the right call. The one cost is retired DB files persisting on disk across process death until a later process's eviction — fine, but worth a line in the KDoc.

[jeremiah-k]

@jamesarich Thanks for the detailed review. I agree with all four findings.

I’m going to make the retry path reacquire admission through beginWrite() rather than registering a previously selected database directly, so a retry cannot bypass an active association gate.

For routing recovery, I’ll make the merge final an active association gate.

For routing recovery, I’ll make the merge finalization two-phase and crash-recoverable: persist a pending source/destination route before committing the merge, verify the destination’s merge marker during a later address switch when the final alias is missing, and repair the alias before publishing either database. A pre-commit failure will leave no valid marker and safely fall back to the source.

The FTS backfill will enter the same writer-admission accounting as other writes and skip its captured database if the admitted active database has changed.

I’ll also make gate release idempotent and finally-hardened for any Throwable, add a bounded gate wait, and record logical retirement synchronously inside the association critical section so shutdown cannot cancel it before registration.

I’ll add deterministic coverage for each path and correct the mesh-log wording, connection-pool KDoc, and process-death cleanup note before moving the PR out of draft.

[jeremiah-k]

I reworked this and updated it against current main.

The issues you called out are addressed:

• closed-pool retries and FTS backfill now go through normal writer admission;
• committed merges retain recoverable routing intent;
• writer-gate, drain, manager-job, and shutdown waits are bounded, with cleanup hardened around failure and cancellation;
• published Room instances remain open while consumers may still reference them;
• telemetry parsing no longer occupies the serialized database dispatcher;
• received frames retain their original transport address and generation, and stale-session association work is rejected or cancelled.

I also tightened the rollback, recovery, shutdown, reconnect, and stale-frame coverage. Exact-head CI is green.

[coderabbitai]

Actionable comments posted: 5

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

core/database/src/commonMain/kotlin/org/meshtastic/core/database/DatabaseManager.kt (1)

172-178: 🗄️ Data Integrity & Integration | 🔴 Critical | ⚡ Quick win

Resolve the protected database after acquiring the manager mutex.

Deferred enforcement can wait behind a later switch, leaving activeDbName stale. The new active database may then be selected, closed, and deleted while _currentDb still references it.

Proposed fix

-            enforceCacheLimit(activeDbName = currentDbName)
+            enforceCacheLimit()
...
-        launchManagerWork(dispatchers.io) { enforceCacheLimit(activeDbName = dbName) }
+        launchManagerWork(dispatchers.io) { enforceCacheLimit() }
...
-            launchManagerWork(dispatchers.io) { enforceCacheLimit(activeDbName = currentDbName) }
+            launchManagerWork(dispatchers.io) { enforceCacheLimit() }
...
-    private suspend fun enforceCacheLimit(activeDbName: String) = withManagerOperation {
+    private suspend fun enforceCacheLimit() = withManagerOperation {
         mutex.withLock {
+            val activeDbName = currentDbName
             val limit = getCurrentCacheLimit()

Also applies to: 437-449, 885-887, 1121-1135

🤖 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
`@core/database/src/commonMain/kotlin/org/meshtastic/core/database/DatabaseManager.kt`
around lines 172 - 178, Update setCacheLimit and the other affected enforcement
paths to resolve the protected database only after entering the manager mutex
via launchManagerWork. Read the current active database name and acquire/protect
that database inside the serialized block immediately before enforceCacheLimit,
rather than capturing currentDbName when scheduling deferred work, so
enforcement applies to the database active at execution time.

🤖 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
`@core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/MeshConfigFlowManagerImpl.kt`:
- Around line 255-264: The session re-check in handleMyInfo must occur before
handshakeState, setMyDeviceId, and setMyNodeNum are updated. Move the existing
isActiveSession(session) guard ahead of these identity writes so a session
change prevents all stale mutations, and add a regression test that flips
activeSession between admission checks and verifies no state is written.

In
`@core/data/src/commonTest/kotlin/org/meshtastic/core/data/manager/NodeManagerConnectionIdentityTest.kt`:
- Line 58: Replace testScope.runTest with top-level
runTest(testScope.coroutineContext) in all affected tests:
NodeManagerConnectionIdentityTest.kt lines 58, 77, 92, 109, 170, and 186.
Preserve each test body and existing testScope context.

In
`@core/service/src/commonMain/kotlin/org/meshtastic/core/service/RadioControllerImpl.kt`:
- Around line 112-119: Update the session-generation collector in
RadioControllerImpl so clearing connection identity is generation-aware and
cannot remove an identity already published for the new generation; only clear
when the stored identity does not match generation. Add a deterministic
regression test that publishes the new identity before the boundary collector
executes and verifies it is preserved.

In
`@core/service/src/commonMain/kotlin/org/meshtastic/core/service/SharedRadioInterfaceService.kt`:
- Around line 578-591: Add failure-safe rollback around transport creation in
the session admission flow using activeTransportSession, _activeSession,
_sessionGeneration, isStarted, runningTransportId, and radioTransport. If
transportFactory.createTransport throws, clear the admitted session and
transport state, reset the lifecycle fields to their inactive values, then
rethrow the original exception.

In
`@core/testing/src/commonMain/kotlin/org/meshtastic/core/testing/FakeRadioInterfaceService.kt`:
- Around line 59-68: The fake service currently creates sessions from
setDeviceAddress and never advances _sessionGeneration. Update setDeviceAddress
to only update the selected device, and move session creation plus generation
increments into connect() and restartTransport(), ensuring every
connect/restart—including same-address reconnects—gets a fresh generation and no
session exists before connect.

---

Outside diff comments:
In
`@core/database/src/commonMain/kotlin/org/meshtastic/core/database/DatabaseManager.kt`:
- Around line 172-178: Update setCacheLimit and the other affected enforcement
paths to resolve the protected database only after entering the manager mutex
via launchManagerWork. Read the current active database name and acquire/protect
that database inside the serialized block immediately before enforceCacheLimit,
rather than capturing currentDbName when scheduling deferred work, so
enforcement applies to the database active at execution time.

🪄 Autofix (Beta)

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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 28cdbc3b-674e-4a23-99df-ff8b2049af33

📥 Commits

Reviewing files that changed from the base of the PR and between 8403fc5 and 1eda08e.

📒 Files selected for processing (39)

• androidApp/src/main/kotlin/org/meshtastic/app/MeshUtilApplication.kt
• core/common/src/commonMain/kotlin/org/meshtastic/core/common/database/DatabaseManager.kt
• core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/FromRadioPacketHandlerImpl.kt
• core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/MeshConfigFlowManagerImpl.kt
• core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/MeshMessageProcessorImpl.kt
• core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/NodeManagerImpl.kt
• core/data/src/commonMain/kotlin/org/meshtastic/core/data/repository/MeshLogRepositoryImpl.kt
• core/data/src/commonMain/kotlin/org/meshtastic/core/data/repository/PacketRepositoryImpl.kt
• core/data/src/commonMain/kotlin/org/meshtastic/core/data/repository/TracerouteSnapshotRepositoryImpl.kt
• core/data/src/commonTest/kotlin/org/meshtastic/core/data/manager/FromRadioPacketHandlerImplTest.kt
• core/data/src/commonTest/kotlin/org/meshtastic/core/data/manager/MeshConfigFlowManagerImplTest.kt
• core/data/src/commonTest/kotlin/org/meshtastic/core/data/manager/MeshMessageProcessorImplTest.kt
• core/data/src/commonTest/kotlin/org/meshtastic/core/data/manager/NodeManagerConnectionIdentityTest.kt
• core/database/build.gradle.kts
• core/database/src/commonMain/kotlin/org/meshtastic/core/database/DatabaseConstants.kt
• core/database/src/commonMain/kotlin/org/meshtastic/core/database/DatabaseManager.kt
• core/database/src/commonMain/kotlin/org/meshtastic/core/database/dao/MeshLogDao.kt
• core/database/src/commonMain/kotlin/org/meshtastic/core/database/dao/PacketDao.kt
• core/database/src/commonMain/kotlin/org/meshtastic/core/database/dao/TracerouteNodePositionDao.kt
• core/database/src/commonTest/kotlin/org/meshtastic/core/database/DatabaseManagerAssociationTest.kt
• core/database/src/commonTest/kotlin/org/meshtastic/core/database/dao/MeshLogDaoTest.kt
• core/database/src/commonTest/kotlin/org/meshtastic/core/database/dao/PacketDaoAtomicTransactionTest.kt
• core/database/src/commonTest/kotlin/org/meshtastic/core/database/dao/TracerouteNodePositionDaoTest.kt
• core/database/src/jvmTest/kotlin/org/meshtastic/core/database/DatabaseManagerPendingRouteRecoveryJvmTest.kt
• core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/FromRadioPacketHandler.kt
• core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/MeshConfigFlowManager.kt
• core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/MeshMessageProcessor.kt
• core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/NodeManager.kt
• core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/RadioInterfaceService.kt
• core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/ReceivedRadioFrame.kt
• core/service/src/commonMain/kotlin/org/meshtastic/core/service/MeshServiceOrchestrator.kt
• core/service/src/commonMain/kotlin/org/meshtastic/core/service/RadioControllerImpl.kt
• core/service/src/commonMain/kotlin/org/meshtastic/core/service/SharedRadioInterfaceService.kt
• core/service/src/commonTest/kotlin/org/meshtastic/core/service/MeshServiceOrchestratorTest.kt
• core/service/src/commonTest/kotlin/org/meshtastic/core/service/RadioControllerImplTest.kt
• core/service/src/commonTest/kotlin/org/meshtastic/core/service/SharedRadioInterfaceServiceLivenessTest.kt
• core/testing/src/commonMain/kotlin/org/meshtastic/core/testing/FakeDatabaseManager.kt
• core/testing/src/commonMain/kotlin/org/meshtastic/core/testing/FakeRadioInterfaceService.kt
• desktopApp/src/main/kotlin/org/meshtastic/desktop/stub/NoopStubs.kt

[coderabbitai]

Actionable comments posted: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)

core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/MeshMessageProcessorImpl.kt (1)

153-179: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift

Sanitize audit records before persisting them.

logVariant stores NodeInfo.toPIIString() and complete configuration/channel representations in MeshLog, potentially retaining identity, location, or key material. Persist redacted summaries and sanitize or omit the stored fromRadio payload for sensitive variants.

As per coding guidelines, Kotlin code must never log or expose PII, location data, or cryptographic keys. As per path instructions, flag any such logging in **/*.kt.

🤖 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
`@core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/MeshMessageProcessorImpl.kt`
around lines 153 - 179, The logVariant method persists sensitive NodeInfo,
configuration, channel, and fromRadio data in MeshLog. Replace PII-bearing and
complete representations with redacted summaries, and sanitize or omit the
fromRadio payload for those sensitive variants; ensure no identity, location, or
cryptographic key material is stored while preserving safe logging for
non-sensitive variants.

Sources: Coding guidelines, Path instructions

core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/MeshDataHandlerImpl.kt (1)

124-173: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Retain session authority for deferred packet work.

session is only carried to the ADMIN branch. Text/reaction persistence, ACK updates, telemetry, and other detached scope jobs can begin after the originating lease ends and write against a newly associated device database. Thread the session into those asynchronous paths and enter runWhileSessionActive(session) before side effects; add a reconnect-generation regression test.

🤖 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
`@core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/MeshDataHandlerImpl.kt`
around lines 124 - 173, Preserve the originating session for all deferred packet
work, not only the ADMIN branch. Thread session through handleTextMessage,
reaction/ACK persistence, telemetry, specialized handlers, and detached scope
jobs, invoking runWhileSessionActive(session) before database or other side
effects. Add a regression test covering reconnect generation to verify stale
deferred work cannot write through a newly associated device database.

🧹 Nitpick comments (2)

core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/NodeManager.kt (1)

152-162: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Declare transformation callbacks side-effect-free.

NodeManagerImpl Lines 242–249 can evaluate transform repeatedly after CAS contention, but this public contract does not warn callers. Side-effecting callbacks could therefore execute more than once. Document and audit the purity requirement, or serialize callback evaluation.

🤖 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
`@core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/NodeManager.kt`
around lines 152 - 162, Update the public contracts for updateNode and
updateNodeAndPersist to explicitly require side-effect-free transformation
callbacks because NodeManagerImpl may reevaluate them after CAS contention.
Audit both implementations and call sites for callback side effects, preserving
repeated evaluation behavior rather than serializing callbacks unless required
by existing semantics.

core/database/src/commonTest/kotlin/org/meshtastic/core/database/DatabaseManagerWriterAdmissionTest.kt (1)

79-79: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Use await() here instead of getCompleted(). await() is the stable equivalent and matches the rest of these tests.

🤖 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
`@core/database/src/commonTest/kotlin/org/meshtastic/core/database/DatabaseManagerWriterAdmissionTest.kt`
at line 79, Update the assertion in DatabaseManagerWriterAdmissionTest to call
await() on newWriterDb instead of getCompleted(), preserving the existing
expected dbA value and assertion message.

🤖 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
`@core/data/src/commonTest/kotlin/org/meshtastic/core/data/manager/MeshConfigFlowManagerImplTest.kt`:
- Around line 303-319: The test around laterPersistence and releaseFirstClear
only verifies that the first clear completed before later persistence began;
strengthen it to assert that all clearLocalConfig, clearLocalModuleConfig,
clearDeviceUIConfig, clearFileManifest, and clearLoraRegionPresetMap operations
finish before laterPersistence starts. Use completion signals or equivalent
ordering verification for each clear, while preserving the existing
blocked-first-clear and later-persistence flow.

In
`@core/data/src/commonTest/kotlin/org/meshtastic/core/data/manager/NodeManagerImplTest.kt`:
- Around line 103-133: The test same-node persistence cannot finish with an
older snapshot currently cannot detect concurrent entry into the first upsert.
Track each nodeRepository.upsert invocation separately, assert the second
invocation has not started while the first is blocked, then release the first
and verify persisted order is [1, 2] along with the final in-memory node state.

In
`@core/database/src/commonMain/kotlin/org/meshtastic/core/database/DatabaseMerger.kt`:
- Around line 81-83: Update the transaction flow around DatabaseMerger’s
ensureAssociationActive and immediateTransaction so the association-generation
lease remains held through transaction commit, preventing rollover from
occurring between the authority check and commit. Coordinate session rollover
using the same synchronization primitive, and release the lease only after the
transaction completes or rolls back.

In
`@core/service/src/commonMain/kotlin/org/meshtastic/core/service/RadioControllerImpl.kt`:
- Around line 274-297: Update rollbackDeviceSwitch so transport and related
device-state restoration only proceed after
switchActiveDatabase(previousAddress) succeeds; if database rollback fails,
clear the transport selection, keep writes disabled, and propagate or surface
the rollback failure instead of merely suppressing it. Preserve suppression for
independent rollback failures where safe, and add a test covering database
rollback failure and the resulting fail-closed transport behavior.

---

Outside diff comments:
In
`@core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/MeshDataHandlerImpl.kt`:
- Around line 124-173: Preserve the originating session for all deferred packet
work, not only the ADMIN branch. Thread session through handleTextMessage,
reaction/ACK persistence, telemetry, specialized handlers, and detached scope
jobs, invoking runWhileSessionActive(session) before database or other side
effects. Add a regression test covering reconnect generation to verify stale
deferred work cannot write through a newly associated device database.

In
`@core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/MeshMessageProcessorImpl.kt`:
- Around line 153-179: The logVariant method persists sensitive NodeInfo,
configuration, channel, and fromRadio data in MeshLog. Replace PII-bearing and
complete representations with redacted summaries, and sanitize or omit the
fromRadio payload for those sensitive variants; ensure no identity, location, or
cryptographic key material is stored while preserving safe logging for
non-sensitive variants.

---

Nitpick comments:
In
`@core/database/src/commonTest/kotlin/org/meshtastic/core/database/DatabaseManagerWriterAdmissionTest.kt`:
- Line 79: Update the assertion in DatabaseManagerWriterAdmissionTest to call
await() on newWriterDb instead of getCompleted(), preserving the existing
expected dbA value and assertion message.

In
`@core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/NodeManager.kt`:
- Around line 152-162: Update the public contracts for updateNode and
updateNodeAndPersist to explicitly require side-effect-free transformation
callbacks because NodeManagerImpl may reevaluate them after CAS contention.
Audit both implementations and call sites for callback side effects, preserving
repeated evaluation behavior rather than serializing callbacks unless required
by existing semantics.

🪄 Autofix (Beta)

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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: b552890d-7f48-41ce-bcca-ce51a063cd54

📥 Commits

Reviewing files that changed from the base of the PR and between 1eda08e and 2350f49.

📒 Files selected for processing (62)

• androidApp/src/main/kotlin/org/meshtastic/app/MeshUtilApplication.kt
• core/common/src/commonMain/kotlin/org/meshtastic/core/common/database/DatabaseManager.kt
• core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/AdminPacketHandlerImpl.kt
• core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/FromRadioPacketHandlerImpl.kt
• core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/MeshConfigFlowManagerImpl.kt
• core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/MeshConfigHandlerImpl.kt
• core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/MeshDataHandlerImpl.kt
• core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/MeshMessageProcessorImpl.kt
• core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/NodeManagerImpl.kt
• core/data/src/commonMain/kotlin/org/meshtastic/core/data/repository/MeshLogRepositoryImpl.kt
• core/data/src/commonMain/kotlin/org/meshtastic/core/data/repository/PacketRepositoryImpl.kt
• core/data/src/commonMain/kotlin/org/meshtastic/core/data/repository/TracerouteSnapshotRepositoryImpl.kt
• core/data/src/commonTest/kotlin/org/meshtastic/core/data/manager/AdminPacketHandlerImplTest.kt
• core/data/src/commonTest/kotlin/org/meshtastic/core/data/manager/FromRadioPacketHandlerImplTest.kt
• core/data/src/commonTest/kotlin/org/meshtastic/core/data/manager/MeshConfigFlowManagerImplTest.kt
• core/data/src/commonTest/kotlin/org/meshtastic/core/data/manager/MeshConfigHandlerImplTest.kt
• core/data/src/commonTest/kotlin/org/meshtastic/core/data/manager/MeshDataHandlerTest.kt
• core/data/src/commonTest/kotlin/org/meshtastic/core/data/manager/MeshMessageProcessorImplTest.kt
• core/data/src/commonTest/kotlin/org/meshtastic/core/data/manager/NodeManagerConnectionIdentityTest.kt
• core/data/src/commonTest/kotlin/org/meshtastic/core/data/manager/NodeManagerImplTest.kt
• core/data/src/jvmTest/kotlin/org/meshtastic/core/data/repository/AirQualityChartReproTest.kt
• core/database/build.gradle.kts
• core/database/src/commonMain/kotlin/org/meshtastic/core/database/DatabaseConstants.kt
• core/database/src/commonMain/kotlin/org/meshtastic/core/database/DatabaseManager.kt
• core/database/src/commonMain/kotlin/org/meshtastic/core/database/DatabaseMerger.kt
• core/database/src/commonMain/kotlin/org/meshtastic/core/database/DatabaseProvider.kt
• core/database/src/commonMain/kotlin/org/meshtastic/core/database/dao/MeshLogDao.kt
• core/database/src/commonMain/kotlin/org/meshtastic/core/database/dao/PacketDao.kt
• core/database/src/commonMain/kotlin/org/meshtastic/core/database/dao/SwitchingDiscoveryDao.kt
• core/database/src/commonMain/kotlin/org/meshtastic/core/database/dao/TracerouteNodePositionDao.kt
• core/database/src/commonTest/kotlin/org/meshtastic/core/database/DatabaseManagerAssociationRecoveryTest.kt
• core/database/src/commonTest/kotlin/org/meshtastic/core/database/DatabaseManagerBackfillTest.kt
• core/database/src/commonTest/kotlin/org/meshtastic/core/database/DatabaseManagerRetirementTest.kt
• core/database/src/commonTest/kotlin/org/meshtastic/core/database/DatabaseManagerShutdownTest.kt
• core/database/src/commonTest/kotlin/org/meshtastic/core/database/DatabaseManagerTestFixture.kt
• core/database/src/commonTest/kotlin/org/meshtastic/core/database/DatabaseManagerWriterAdmissionTest.kt
• core/database/src/commonTest/kotlin/org/meshtastic/core/database/DatabaseMergerTest.kt
• core/database/src/commonTest/kotlin/org/meshtastic/core/database/dao/MeshLogDaoTest.kt
• core/database/src/commonTest/kotlin/org/meshtastic/core/database/dao/PacketDaoAtomicTransactionTest.kt
• core/database/src/commonTest/kotlin/org/meshtastic/core/database/dao/TracerouteNodePositionDaoTest.kt
• core/database/src/jvmTest/kotlin/org/meshtastic/core/database/DatabaseManagerPendingRouteRecoveryJvmTest.kt
• core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/AdminPacketHandler.kt
• core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/FromRadioPacketHandler.kt
• core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/MeshConfigFlowManager.kt
• core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/MeshConfigHandler.kt
• core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/MeshDataHandler.kt
• core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/MeshMessageProcessor.kt
• core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/NodeManager.kt
• core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/RadioInterfaceService.kt
• core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/ReceivedRadioFrame.kt
• core/service/src/commonMain/kotlin/org/meshtastic/core/service/MeshServiceOrchestrator.kt
• core/service/src/commonMain/kotlin/org/meshtastic/core/service/RadioControllerImpl.kt
• core/service/src/commonMain/kotlin/org/meshtastic/core/service/SharedRadioInterfaceService.kt
• core/service/src/commonTest/kotlin/org/meshtastic/core/service/MeshServiceOrchestratorTest.kt
• core/service/src/commonTest/kotlin/org/meshtastic/core/service/RadioControllerImplTest.kt
• core/service/src/commonTest/kotlin/org/meshtastic/core/service/SharedRadioInterfaceServiceLivenessTest.kt
• core/takserver/src/commonTest/kotlin/org/meshtastic/core/takserver/TAKMeshIntegrationTest.kt
• core/testing/src/commonMain/kotlin/org/meshtastic/core/testing/FakeDatabaseManager.kt
• core/testing/src/commonMain/kotlin/org/meshtastic/core/testing/FakeRadioInterfaceService.kt
• core/testing/src/commonTest/kotlin/org/meshtastic/core/testing/FakeRadioInterfaceServiceSessionTest.kt
• core/testing/src/commonTest/kotlin/org/meshtastic/core/testing/RepositoryFakesTest.kt
• desktopApp/src/main/kotlin/org/meshtastic/desktop/stub/NoopStubs.kt

🚧 Files skipped from review as they are similar to previous changes (20)

• core/database/build.gradle.kts
• core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/ReceivedRadioFrame.kt
• core/database/src/commonMain/kotlin/org/meshtastic/core/database/dao/TracerouteNodePositionDao.kt
• core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/FromRadioPacketHandler.kt
• core/common/src/commonMain/kotlin/org/meshtastic/core/common/database/DatabaseManager.kt
• core/data/src/commonMain/kotlin/org/meshtastic/core/data/repository/MeshLogRepositoryImpl.kt
• core/database/src/commonMain/kotlin/org/meshtastic/core/database/DatabaseConstants.kt
• core/service/src/commonMain/kotlin/org/meshtastic/core/service/MeshServiceOrchestrator.kt
• core/database/src/jvmTest/kotlin/org/meshtastic/core/database/DatabaseManagerPendingRouteRecoveryJvmTest.kt
• desktopApp/src/main/kotlin/org/meshtastic/desktop/stub/NoopStubs.kt
• androidApp/src/main/kotlin/org/meshtastic/app/MeshUtilApplication.kt
• core/database/src/commonTest/kotlin/org/meshtastic/core/database/dao/TracerouteNodePositionDaoTest.kt
• core/database/src/commonTest/kotlin/org/meshtastic/core/database/dao/MeshLogDaoTest.kt
• core/testing/src/commonMain/kotlin/org/meshtastic/core/testing/FakeRadioInterfaceService.kt
• core/service/src/commonTest/kotlin/org/meshtastic/core/service/MeshServiceOrchestratorTest.kt
• core/data/src/commonMain/kotlin/org/meshtastic/core/data/repository/PacketRepositoryImpl.kt
• core/database/src/commonTest/kotlin/org/meshtastic/core/database/dao/PacketDaoAtomicTransactionTest.kt
• core/service/src/commonMain/kotlin/org/meshtastic/core/service/SharedRadioInterfaceService.kt
• core/database/src/commonMain/kotlin/org/meshtastic/core/database/dao/PacketDao.kt
• core/database/src/commonMain/kotlin/org/meshtastic/core/database/DatabaseManager.kt

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/TracerouteHandlerImpl.kt (1)

64-101: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Unbounded logInsertJob?.join() inside a session lease.

RadioSessionAuthority.runWithSessionLease documents that callers "must keep the block bounded," since teardown waits for this block to finish before completing session teardown / starting a replacement transport. logInsertJob?.join() here has no timeout — if the mesh-log insert stalls (e.g. under DB writer-admission pressure), this session-lease block never returns, and session teardown/reconnect blocks indefinitely on it.

🐛 Proposed fix: bound the join with a timeout

-            if (logUuid != null) {
-                logInsertJob?.join()
+            if (logUuid != null) {
+                withTimeoutOrNull(LOG_INSERT_JOIN_TIMEOUT) { logInsertJob?.join() }
                 val routeNodeNums = (forwardRoute + returnRoute).distinct()

🤖 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
`@core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/TracerouteHandlerImpl.kt`
around lines 64 - 101, Bound the logInsertJob wait inside the
radioInterfaceService.launchSessionWork session-lease block: replace the
unbounded join in TracerouteHandlerImpl’s traceroute handling with a finite
timeout, while preserving the existing snapshot update flow after the wait.
Ensure a stalled mesh-log insert cannot prevent the lease block from returning
and session teardown or transport replacement from proceeding.

🧹 Nitpick comments (2)

core/data/src/commonTest/kotlin/org/meshtastic/core/data/manager/TelemetryPacketHandlerImplTest.kt (1)

58-230: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Missing coverage for the new session-rejection guard in two handlers.

This PR adds session-admission gating so stale-session work is skipped (runWithSessionLease/runWhileSessionActive returning false). StoreForwardPacketHandlerImplTest.kt, NodeManagerImplTest.kt, and MeshDataHandlerTest.kt all added a dedicated "retired/rejected session" test for this guard, but two sibling suites in this same cohort don't:

• core/data/src/commonTest/kotlin/org/meshtastic/core/data/manager/TelemetryPacketHandlerImplTest.kt#L58-L230: add a test that stubs radioInterfaceService.runWithSessionLease(radioSession, any()) to return false and asserts nodeManager.updateNodeForSession(...) is never reached, mirroring the pattern already used for StoreForwardPacketHandlerImplTest.kt's SFPP rejection test.
• core/data/src/commonTest/kotlin/org/meshtastic/core/data/manager/GeofenceMonitorTest.kt#L107-L133: add a test that calls onPositionReceived(...) with a non-null RadioSessionContext while the mocked RadioInterfaceService reports that session as inactive, and assert evaluate(...)/notifications never fire, to exercise the new session-gated branch added in GeofenceMonitor.kt.

🤖 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
`@core/data/src/commonTest/kotlin/org/meshtastic/core/data/manager/TelemetryPacketHandlerImplTest.kt`
around lines 58 - 230, Add rejected-session coverage in
TelemetryPacketHandlerImplTest.kt (lines 58-230) by stubbing
radioInterfaceService.runWithSessionLease for radioSession to return false,
invoking handleTelemetry, and asserting nodeManager.updateNodeForSession is
never called. Also update GeofenceMonitorTest.kt (lines 107-133) with a non-null
RadioSessionContext whose session is reported inactive, then call
onPositionReceived and verify evaluate and notifications do not run.

core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/TelemetryPacketHandlerImpl.kt (1)

61-88: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Extract the shared telemetry update helper. TelemetryPacketHandlerImpl and NodeManagerImpl.handleReceivedTelemetry both apply the same metrics copies and lastHeard clamping; move that logic into a shared private helper and call it from both paths so the behavior stays aligned.

🤖 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
`@core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/TelemetryPacketHandlerImpl.kt`
around lines 61 - 88, Extract the duplicated telemetry-to-Node transformation
from handleTelemetry and NodeManagerImpl.handleReceivedTelemetry into one shared
private helper. The helper must copy all telemetry metric fields and update
lastHeard using the existing telemetry-time fallback and
clampTimestampToNow/maxOf logic, then replace both inline implementations with
calls to it.

🤖 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
`@core/database/src/jvmTest/kotlin/org/meshtastic/core/database/DatabaseManagerPendingRouteRecoveryJvmTest.kt`:
- Line 23: Update the Room import in DatabaseManagerPendingRouteRecoveryJvmTest
to use androidx.room.Room instead of the incorrect androidx.room3.Room
namespace.

---

Outside diff comments:
In
`@core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/TracerouteHandlerImpl.kt`:
- Around line 64-101: Bound the logInsertJob wait inside the
radioInterfaceService.launchSessionWork session-lease block: replace the
unbounded join in TracerouteHandlerImpl’s traceroute handling with a finite
timeout, while preserving the existing snapshot update flow after the wait.
Ensure a stalled mesh-log insert cannot prevent the lease block from returning
and session teardown or transport replacement from proceeding.

---

Nitpick comments:
In
`@core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/TelemetryPacketHandlerImpl.kt`:
- Around line 61-88: Extract the duplicated telemetry-to-Node transformation
from handleTelemetry and NodeManagerImpl.handleReceivedTelemetry into one shared
private helper. The helper must copy all telemetry metric fields and update
lastHeard using the existing telemetry-time fallback and
clampTimestampToNow/maxOf logic, then replace both inline implementations with
calls to it.

In
`@core/data/src/commonTest/kotlin/org/meshtastic/core/data/manager/TelemetryPacketHandlerImplTest.kt`:
- Around line 58-230: Add rejected-session coverage in
TelemetryPacketHandlerImplTest.kt (lines 58-230) by stubbing
radioInterfaceService.runWithSessionLease for radioSession to return false,
invoking handleTelemetry, and asserting nodeManager.updateNodeForSession is
never called. Also update GeofenceMonitorTest.kt (lines 107-133) with a non-null
RadioSessionContext whose session is reported inactive, then call
onPositionReceived and verify evaluate and notifications do not run.

🪄 Autofix (Beta)

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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 2c27cc90-8fc1-4c80-b353-e9265da8dff2

📥 Commits

Reviewing files that changed from the base of the PR and between 2350f49 and 8567039.

📒 Files selected for processing (76)

• androidApp/src/main/kotlin/org/meshtastic/app/MeshUtilApplication.kt
• core/common/src/commonMain/kotlin/org/meshtastic/core/common/database/DatabaseManager.kt
• core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/AdminPacketHandlerImpl.kt
• core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/FromRadioPacketHandlerImpl.kt
• core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/GeofenceMonitor.kt
• core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/MeshConfigFlowManagerImpl.kt
• core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/MeshConfigHandlerImpl.kt
• core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/MeshDataHandlerImpl.kt
• core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/MeshMessageProcessorImpl.kt
• core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/NodeManagerImpl.kt
• core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/PacketHandlerImpl.kt
• core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/SessionWork.kt
• core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/StoreForwardPacketHandlerImpl.kt
• core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/TelemetryPacketHandlerImpl.kt
• core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/TracerouteHandlerImpl.kt
• core/data/src/commonMain/kotlin/org/meshtastic/core/data/repository/MeshLogRepositoryImpl.kt
• core/data/src/commonMain/kotlin/org/meshtastic/core/data/repository/PacketRepositoryImpl.kt
• core/data/src/commonMain/kotlin/org/meshtastic/core/data/repository/TracerouteSnapshotRepositoryImpl.kt
• core/data/src/commonTest/kotlin/org/meshtastic/core/data/manager/AdminPacketHandlerImplTest.kt
• core/data/src/commonTest/kotlin/org/meshtastic/core/data/manager/FromRadioPacketHandlerImplTest.kt
• core/data/src/commonTest/kotlin/org/meshtastic/core/data/manager/GeofenceMonitorTest.kt
• core/data/src/commonTest/kotlin/org/meshtastic/core/data/manager/MeshConfigFlowManagerImplTest.kt
• core/data/src/commonTest/kotlin/org/meshtastic/core/data/manager/MeshConfigHandlerImplTest.kt
• core/data/src/commonTest/kotlin/org/meshtastic/core/data/manager/MeshDataHandlerTest.kt
• core/data/src/commonTest/kotlin/org/meshtastic/core/data/manager/MeshMessageProcessorImplTest.kt
• core/data/src/commonTest/kotlin/org/meshtastic/core/data/manager/NodeManagerConnectionIdentityTest.kt
• core/data/src/commonTest/kotlin/org/meshtastic/core/data/manager/NodeManagerImplTest.kt
• core/data/src/commonTest/kotlin/org/meshtastic/core/data/manager/SessionWorkTest.kt
• core/data/src/commonTest/kotlin/org/meshtastic/core/data/manager/TelemetryPacketHandlerImplTest.kt
• core/data/src/jvmTest/kotlin/org/meshtastic/core/data/manager/StoreForwardPacketHandlerImplTest.kt
• core/data/src/jvmTest/kotlin/org/meshtastic/core/data/repository/AirQualityChartReproTest.kt
• core/database/build.gradle.kts
• core/database/src/commonMain/kotlin/org/meshtastic/core/database/DatabaseConstants.kt
• core/database/src/commonMain/kotlin/org/meshtastic/core/database/DatabaseManager.kt
• core/database/src/commonMain/kotlin/org/meshtastic/core/database/DatabaseMerger.kt
• core/database/src/commonMain/kotlin/org/meshtastic/core/database/DatabaseProvider.kt
• core/database/src/commonMain/kotlin/org/meshtastic/core/database/dao/MeshLogDao.kt
• core/database/src/commonMain/kotlin/org/meshtastic/core/database/dao/PacketDao.kt
• core/database/src/commonMain/kotlin/org/meshtastic/core/database/dao/SwitchingDiscoveryDao.kt
• core/database/src/commonMain/kotlin/org/meshtastic/core/database/dao/TracerouteNodePositionDao.kt
• core/database/src/commonTest/kotlin/org/meshtastic/core/database/DatabaseManagerAssociationRecoveryTest.kt
• core/database/src/commonTest/kotlin/org/meshtastic/core/database/DatabaseManagerBackfillTest.kt
• core/database/src/commonTest/kotlin/org/meshtastic/core/database/DatabaseManagerRetirementTest.kt
• core/database/src/commonTest/kotlin/org/meshtastic/core/database/DatabaseManagerShutdownTest.kt
• core/database/src/commonTest/kotlin/org/meshtastic/core/database/DatabaseManagerTestFixture.kt
• core/database/src/commonTest/kotlin/org/meshtastic/core/database/DatabaseManagerWriterAdmissionTest.kt
• core/database/src/commonTest/kotlin/org/meshtastic/core/database/DatabaseMergerTest.kt
• core/database/src/commonTest/kotlin/org/meshtastic/core/database/dao/MeshLogDaoTest.kt
• core/database/src/commonTest/kotlin/org/meshtastic/core/database/dao/PacketDaoAtomicTransactionTest.kt
• core/database/src/commonTest/kotlin/org/meshtastic/core/database/dao/TracerouteNodePositionDaoTest.kt
• core/database/src/jvmTest/kotlin/org/meshtastic/core/database/DatabaseManagerPendingRouteRecoveryJvmTest.kt
• core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/AdminPacketHandler.kt
• core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/FromRadioPacketHandler.kt
• core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/MeshConfigFlowManager.kt
• core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/MeshConfigHandler.kt
• core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/MeshDataHandler.kt
• core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/MeshMessageProcessor.kt
• core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/NodeManager.kt
• core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/PacketHandler.kt
• core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/RadioInterfaceService.kt
• core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/ReceivedRadioFrame.kt
• core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/StoreForwardPacketHandler.kt
• core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/TelemetryPacketHandler.kt
• core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/TracerouteHandler.kt
• core/service/src/commonMain/kotlin/org/meshtastic/core/service/MeshServiceOrchestrator.kt
• core/service/src/commonMain/kotlin/org/meshtastic/core/service/RadioControllerImpl.kt
• core/service/src/commonMain/kotlin/org/meshtastic/core/service/SharedRadioInterfaceService.kt
• core/service/src/commonTest/kotlin/org/meshtastic/core/service/MeshServiceOrchestratorTest.kt
• core/service/src/commonTest/kotlin/org/meshtastic/core/service/RadioControllerImplTest.kt
• core/service/src/commonTest/kotlin/org/meshtastic/core/service/SharedRadioInterfaceServiceLivenessTest.kt
• core/takserver/src/commonTest/kotlin/org/meshtastic/core/takserver/TAKMeshIntegrationTest.kt
• core/testing/src/commonMain/kotlin/org/meshtastic/core/testing/FakeDatabaseManager.kt
• core/testing/src/commonMain/kotlin/org/meshtastic/core/testing/FakeRadioInterfaceService.kt
• core/testing/src/commonTest/kotlin/org/meshtastic/core/testing/FakeRadioInterfaceServiceSessionTest.kt
• core/testing/src/commonTest/kotlin/org/meshtastic/core/testing/RepositoryFakesTest.kt
• desktopApp/src/main/kotlin/org/meshtastic/desktop/stub/NoopStubs.kt

🚧 Files skipped from review as they are similar to previous changes (49)

• core/database/src/commonMain/kotlin/org/meshtastic/core/database/dao/SwitchingDiscoveryDao.kt
• core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/AdminPacketHandler.kt
• core/database/src/commonMain/kotlin/org/meshtastic/core/database/DatabaseProvider.kt
• core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/AdminPacketHandlerImpl.kt
• androidApp/src/main/kotlin/org/meshtastic/app/MeshUtilApplication.kt
• core/database/src/commonMain/kotlin/org/meshtastic/core/database/dao/TracerouteNodePositionDao.kt
• core/data/src/commonMain/kotlin/org/meshtastic/core/data/repository/TracerouteSnapshotRepositoryImpl.kt
• core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/ReceivedRadioFrame.kt
• core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/MeshConfigHandler.kt
• core/common/src/commonMain/kotlin/org/meshtastic/core/common/database/DatabaseManager.kt
• core/database/src/commonMain/kotlin/org/meshtastic/core/database/DatabaseMerger.kt
• core/database/src/commonTest/kotlin/org/meshtastic/core/database/dao/TracerouteNodePositionDaoTest.kt
• core/database/src/commonTest/kotlin/org/meshtastic/core/database/dao/MeshLogDaoTest.kt
• core/takserver/src/commonTest/kotlin/org/meshtastic/core/takserver/TAKMeshIntegrationTest.kt
• core/database/src/commonMain/kotlin/org/meshtastic/core/database/DatabaseConstants.kt
• core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/MeshMessageProcessor.kt
• core/service/src/commonMain/kotlin/org/meshtastic/core/service/MeshServiceOrchestrator.kt
• core/testing/src/commonMain/kotlin/org/meshtastic/core/testing/FakeDatabaseManager.kt
• core/database/src/commonTest/kotlin/org/meshtastic/core/database/dao/PacketDaoAtomicTransactionTest.kt
• core/database/src/commonTest/kotlin/org/meshtastic/core/database/DatabaseMergerTest.kt
• core/database/src/commonMain/kotlin/org/meshtastic/core/database/dao/MeshLogDao.kt
• desktopApp/src/main/kotlin/org/meshtastic/desktop/stub/NoopStubs.kt
• core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/NodeManager.kt
• core/database/src/commonTest/kotlin/org/meshtastic/core/database/DatabaseManagerTestFixture.kt
• core/database/src/commonTest/kotlin/org/meshtastic/core/database/DatabaseManagerBackfillTest.kt
• core/data/src/jvmTest/kotlin/org/meshtastic/core/data/repository/AirQualityChartReproTest.kt
• core/data/src/commonTest/kotlin/org/meshtastic/core/data/manager/FromRadioPacketHandlerImplTest.kt
• core/service/src/commonTest/kotlin/org/meshtastic/core/service/MeshServiceOrchestratorTest.kt
• core/service/src/commonTest/kotlin/org/meshtastic/core/service/SharedRadioInterfaceServiceLivenessTest.kt
• core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/FromRadioPacketHandlerImpl.kt
• core/database/src/commonTest/kotlin/org/meshtastic/core/database/DatabaseManagerAssociationRecoveryTest.kt
• core/data/src/commonMain/kotlin/org/meshtastic/core/data/repository/MeshLogRepositoryImpl.kt
• core/data/src/commonTest/kotlin/org/meshtastic/core/data/manager/NodeManagerConnectionIdentityTest.kt
• core/data/src/commonMain/kotlin/org/meshtastic/core/data/repository/PacketRepositoryImpl.kt
• core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/MeshConfigHandlerImpl.kt
• core/database/src/commonTest/kotlin/org/meshtastic/core/database/DatabaseManagerRetirementTest.kt
• core/data/src/commonTest/kotlin/org/meshtastic/core/data/manager/MeshConfigHandlerImplTest.kt
• core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/MeshMessageProcessorImpl.kt
• core/database/src/commonTest/kotlin/org/meshtastic/core/database/DatabaseManagerShutdownTest.kt
• core/data/src/commonTest/kotlin/org/meshtastic/core/data/manager/AdminPacketHandlerImplTest.kt
• core/database/src/commonMain/kotlin/org/meshtastic/core/database/dao/PacketDao.kt
• core/testing/src/commonTest/kotlin/org/meshtastic/core/testing/RepositoryFakesTest.kt
• core/service/src/commonMain/kotlin/org/meshtastic/core/service/RadioControllerImpl.kt
• core/database/src/commonTest/kotlin/org/meshtastic/core/database/DatabaseManagerWriterAdmissionTest.kt
• core/service/src/commonMain/kotlin/org/meshtastic/core/service/SharedRadioInterfaceService.kt
• core/service/src/commonTest/kotlin/org/meshtastic/core/service/RadioControllerImplTest.kt
• core/data/src/commonTest/kotlin/org/meshtastic/core/data/manager/MeshConfigFlowManagerImplTest.kt
• core/data/src/commonTest/kotlin/org/meshtastic/core/data/manager/MeshMessageProcessorImplTest.kt
• core/database/src/commonMain/kotlin/org/meshtastic/core/database/DatabaseManager.kt

[jeremiah-k]

@coderabbitai I verified the three outside-diff/nit findings against the current session architecture.

Traceroute log-insert join

I’m keeping the current ordering rather than adding a local timeout.

logInsertJob is not an arbitrary external job: the mesh-log insert is itself launched through the session-bound work path and acquires an independent lifecycle lease before the packet handler returns. Teardown therefore already waits for that insert lease even if traceroute stops joining it.

The join exists to preserve parent-before-child persistence ordering. traceroute_node_position.log_uuid is a foreign key to MeshLog.uuid; timing out the join could race the snapshot insert ahead of its required parent row and produce a foreign-key failure, while still not releasing the insert job’s own session lease.

If database persistence itself needs a stronger global time bound, that should be enforced at the database operation/admission layer rather than by weakening this specific dependency.

Rejected-session test coverage

The proposed telemetry assertion would test the wrong boundary. TelemetryPacketHandlerImpl is called from an already-admitted parent packet operation. updateNodeForSession intentionally applies the in-memory transformation and delegates only the deferred repository persistence to nested session admission.

That rejection behavior is already covered by:

• SessionWorkTest, which proves retired-session work does not execute; and
• NodeManagerImplTest, which proves a session-bound node update may update memory but does not call nodeRepository.upsert when the nested lease is rejected.

A direct Geofence rejection test would be additional branch-level coverage, but the implementation already places non-null-session evaluation inside runWhileSessionActive. I’m not expanding this already-large lifecycle PR for duplicate coverage of the shared authority mechanism.

Telemetry transform deduplication

I’m leaving the two small transforms local in this PR. The active radio path is TelemetryPacketHandlerImpl; NodeManager.handleReceivedTelemetry currently has no production caller beyond its interface/implementation test seam.

Extracting a shared cross-class helper for that legacy path would add coupling and review churn without changing current behavior. This can be revisited separately if that API is retained or gains a production caller.

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Locate the files named in the follow-up, then map their declarations before
# reading only the relevant implementation/test regions.
fd -t f '^(TracerouteHandlerImpl|MeshMessageProcessorImpl|SessionWork|NodeManagerImpl|GeofenceMonitor|TelemetryPacketHandlerImpl|SessionWorkTest|NodeManagerImplTest)\.kt$' .

ast-grep outline core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/TracerouteHandlerImpl.kt --items all
ast-grep outline core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/MeshMessageProcessorImpl.kt --items all
ast-grep outline core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/SessionWork.kt --items all
ast-grep outline core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/NodeManagerImpl.kt --items all
ast-grep outline core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/GeofenceMonitor.kt --items all
ast-grep outline core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/TelemetryPacketHandlerImpl.kt --items all

# Verify the stated call/lease relationships and find production uses of the
# legacy telemetry API, excluding tests.
rg -n -C 5 -e '\blogInsertJob\b' \
  -e '\blaunchSessionWork\b' \
  -e '\brunWhileSessionActive\b' \
  -e '\bupdateNodeForSession\b' \
  -e '\bhandleReceivedTelemetry\s*\(' \
  -e 'traceroute_node_position' \
  core/data/src/commonMain core/repository/src/commonMain core/database/src/commonMain \
  -g '*.kt'

rg -nP --glob '*.kt' --glob '!**/test/**' --glob '!**/commonTest/**' --glob '!**/jvmTest/**' \
  '\bhandleReceivedTelemetry\s*\(' .

Length of output: 75122

---

@jeremiah-k Thanks for the detailed verification. I checked the current implementation and agree with all three decisions.

• logInsertJob.join() preserves the required parent (MeshLog) before child (traceroute_node_position) foreign-key ordering. Since the log insert owns its own nested session lease, a local timeout would not improve teardown liveness and could introduce an FK race.
• updateNodeForSession updates in-memory state before nested, lease-gated persistence; SessionWorkTest and NodeManagerImplTest cover that rejection boundary. GeofenceMonitor also correctly wraps session-bound evaluation in runWhileSessionActive, so extra direct coverage would be redundant here.
• Production telemetry dispatch goes through TelemetryPacketHandlerImpl; NodeManager.handleReceivedTelemetry has no production caller. Keeping the transforms local avoids unnecessary coupling in this PR.

I’m withdrawing these outside-diff/nit findings.

🐇💭

---

✏️ Learnings added

Learnt from: jeremiah-k
URL: https://github.com/meshtastic/Meshtastic-Android/pull/6256

Timestamp: 2026-07-19T21:31:05.342Z
Learning: In the Meshtastic Android session architecture, `TracerouteHandlerImpl` must join the session-bound mesh-log insertion job before persisting traceroute node positions because `traceroute_node_position.log_uuid` has a foreign key to `MeshLog`. The insert job owns an independent lifecycle lease, so this join is an intentional parent-before-child ordering dependency rather than an unbounded teardown hazard.

---

Learnt from: jeremiah-k
URL: https://github.com/meshtastic/Meshtastic-Android/pull/6256

Timestamp: 2026-07-19T21:31:05.343Z
Learning: In `NodeManagerImpl`, `updateNodeForSession` intentionally applies the node-state mutation in memory before scheduling repository persistence through nested `RadioInterfaceService.launchSessionWork`; a rejected retired-session lease prevents persistence but does not roll back the in-memory update. `SessionWorkTest` and `NodeManagerImplTest` cover this shared authority boundary.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

[jamesarich]

I've now been through this PR three times as it evolved — a full pass on the first rework, an adversarial pass over the new recovery machinery, and a delta pass on the current head — so this consolidates everything against 8567039e.

What checks out. The original four findings from my first review are genuinely resolved, and most of what my later passes flagged has been fixed along the way: the withManagerOperation token release is now NonCancellable-wrapped, createTransport failure rolls back the admitted session (with the revoked-callback test to prove it), the traceroute empty-positions semantics went back to an atomic transactional replace, and the handleMyInfo session re-check ordering CodeRabbit asked about was done correctly (identity writes and publish inside runIfSessionActive, stale discard before any write). The two-phase pending-route recovery held up under an adversarial pass — crash between route-write and merge falls back cleanly, marker-before-alias repairs before publishing, and I found no path for one device's MyNodeInfo to associate another device's DB. The writer-admission, flap, and session-gating test coverage is genuinely strong. For anyone triaging the CodeRabbit thread: the testScope.runTest "Critical" was a false positive (public kotlinx-coroutines-test API, used throughout this repo's tests, green in CI).

Two small items I'd like fixed before merge:

1. A throwing transport factory still crashes the process from two call sites. The rollback inside startTransportLocked is right, and the rethrow is contained by ignoreExceptionSuspend in connect() — but the same rethrow escapes uncaught into processLifecycle.coroutineScope from setDeviceAddress and from the liveness-restart launch. A BLE stack throwing in createTransport during a liveness restart takes the app down instead of logging a failed reconnect. Same wrap as connect() at both sites.

2. LRU eviction can delete a pending route's destination — the recovery's commit proof. The eviction exclusions cover logically-retired/detached/legacy/default, and selectEvictionVictims protects the active name, but nothing protects DBs named in a persisted pending route. Crash after the merge commits but before the finalize edit, then connect to a different device first: the destination — holding the only merged copy plus the marker — is evictable before the address-scoped recovery ever runs. (File-mtime recency softens the window but doesn't close it.) Small fix: exclude pending-route source/destination names from eviction candidates.

Follow-ups, fine as separate issues:

3. Nothing bounds the merge, and the new lease chaining widens the blast radius. DatabaseMerger.merge runs NonCancellable and unbounded while beginWrite waiters throw at 30s — so a >30s merge already turns every withDb into an exception. On this head the association also runs under a session lease and revokeTransportSession awaits lease drain unbounded, so the same long merge now additionally wedges disconnect()/setDeviceAddress() behind transportMutex/deviceSwitchMutex. A merge timeout with the same abort→release-to-source contract as the drain timeout would bound the whole chain.

4. The merge marker is still name-presence-only. isMerged(sourceName) gates the copy-skip and serves as commit proof; mergedAt exists but is never consulted, and markers are never deleted on physical retirement. With deterministic buildDbName hashing, a recreated source name after wiped claims silently no-op-merges. Consulting mergedAt (or a route id) would future-proof marker↔route matching.

5. Minor: the same-address branch in RadioControllerImpl now fires onDeviceAddressChanged and cycles the transport on a pure no-op reselect when not connected — looks like intentional "repair," worth a confirming comment. And there's an "UNDISTPATCHED" typo in the handleMyInfo clear-lane comment.

With 1 and 2 in, this has my approval — the iteration quality on this PR has been excellent.

[jeremiah-k]

@jamesarich Thanks again for the focused third pass. Both approval blockers are addressed in the latest head.

Transport-factory failure containment

The setDeviceAddress() and BLE liveness-restart fire-and-forget launches now contain recoverable startTransportLocked() failures through the same ignoreExceptionSuspend boundary used by connect().

startTransportLocked() still performs the authoritative rollback first: it revokes the partially admitted session, clears transport state, and leaves the failed generation consumed. The containing launch logs the recoverable failure instead of allowing it to escape the process-lifecycle coroutine. Cancellation and fatal errors still propagate, and the explicit suspend restart path remains observable to its caller.

The added tests verify that a throwing factory publishes no replacement transport, leaves no active failed session, and allows a later retry with a strictly newer generation.

Pending-route eviction protection

Cache enforcement now resolves all persisted pending source and destination database names while holding the manager mutex and excludes both endpoints from LRU candidates.

Protected databases remain counted toward the cache limit, so cleanup can intentionally remain over limit while recovery evidence exists. This preserves both the source required for pre-commit fallback and the destination that may contain the committed merged copy and its marker.

The tests cover multiple pending routes and verify that even the oldest route endpoints cannot be selected for eviction while an unrelated safe candidate can.

I also clarified the disconnected same-address repair behavior and corrected the UNDISTPATCHED typo.

I agree that bounding the complete merge/writer-gate/session-lease chain and adding route-bound or versioned markers should be focused follow-ups rather than local fixes inside this final amendment.

[coderabbitai]

🧹 Nitpick comments (1)

core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/MeshConfigHandlerImpl.kt (1)

66-71: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Duplicated runForSession/runWhileForSession wrappers across handler classes.

These two-line delegating wrappers to radioInterfaceService.runIfSessionActive / runWhileSessionActive are duplicated verbatim in MeshConfigFlowManagerImpl (see runForSession(session) at line 148 and runWhileForSession(session) at line 224 in that file). Consider hoisting these as extension functions directly on RadioInterfaceService (e.g. RadioInterfaceService.runForSession(...)) so every handler (AdminPacketHandlerImpl, MeshDataHandlerImpl, TelemetryPacketHandlerImpl, etc.) shares one implementation instead of re-declaring it per class.

♻️ Suggested extraction

// e.g. in core/repository RadioInterfaceService.kt
fun RadioInterfaceService.runForSession(session: RadioSessionContext, block: () -> Unit): Boolean =
    runIfSessionActive(session, block)

suspend fun RadioInterfaceService.runWhileForSession(session: RadioSessionContext, block: suspend () -> Unit): Boolean =
    runWhileSessionActive(session, block)

Also applies to: 166-171

🤖 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
`@core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/MeshConfigHandlerImpl.kt`
around lines 66 - 71, Extract the duplicated runForSession and
runWhileForSession delegation into shared extension functions on
RadioInterfaceService, reusing runIfSessionActive and runWhileSessionActive.
Remove the corresponding private wrappers from MeshConfigHandlerImpl and
MeshConfigFlowManagerImpl, and update affected handlers to use the shared
extensions.

🤖 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.

Nitpick comments:
In
`@core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/MeshConfigHandlerImpl.kt`:
- Around line 66-71: Extract the duplicated runForSession and runWhileForSession
delegation into shared extension functions on RadioInterfaceService, reusing
runIfSessionActive and runWhileSessionActive. Remove the corresponding private
wrappers from MeshConfigHandlerImpl and MeshConfigFlowManagerImpl, and update
affected handlers to use the shared extensions.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: b0c1c6cf-ea5a-46a7-9f57-2a7de22bda0b

📥 Commits

Reviewing files that changed from the base of the PR and between 8567039 and 4d9807a.

📒 Files selected for processing (77)

• androidApp/src/main/kotlin/org/meshtastic/app/MeshUtilApplication.kt
• core/common/src/commonMain/kotlin/org/meshtastic/core/common/database/DatabaseManager.kt
• core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/AdminPacketHandlerImpl.kt
• core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/FromRadioPacketHandlerImpl.kt
• core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/GeofenceMonitor.kt
• core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/MeshConfigFlowManagerImpl.kt
• core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/MeshConfigHandlerImpl.kt
• core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/MeshDataHandlerImpl.kt
• core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/MeshMessageProcessorImpl.kt
• core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/NodeManagerImpl.kt
• core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/PacketHandlerImpl.kt
• core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/SessionWork.kt
• core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/StoreForwardPacketHandlerImpl.kt
• core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/TelemetryPacketHandlerImpl.kt
• core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/TracerouteHandlerImpl.kt
• core/data/src/commonMain/kotlin/org/meshtastic/core/data/repository/MeshLogRepositoryImpl.kt
• core/data/src/commonMain/kotlin/org/meshtastic/core/data/repository/PacketRepositoryImpl.kt
• core/data/src/commonMain/kotlin/org/meshtastic/core/data/repository/TracerouteSnapshotRepositoryImpl.kt
• core/data/src/commonTest/kotlin/org/meshtastic/core/data/manager/AdminPacketHandlerImplTest.kt
• core/data/src/commonTest/kotlin/org/meshtastic/core/data/manager/FromRadioPacketHandlerImplTest.kt
• core/data/src/commonTest/kotlin/org/meshtastic/core/data/manager/GeofenceMonitorTest.kt
• core/data/src/commonTest/kotlin/org/meshtastic/core/data/manager/MeshConfigFlowManagerImplTest.kt
• core/data/src/commonTest/kotlin/org/meshtastic/core/data/manager/MeshConfigHandlerImplTest.kt
• core/data/src/commonTest/kotlin/org/meshtastic/core/data/manager/MeshDataHandlerTest.kt
• core/data/src/commonTest/kotlin/org/meshtastic/core/data/manager/MeshMessageProcessorImplTest.kt
• core/data/src/commonTest/kotlin/org/meshtastic/core/data/manager/NodeManagerConnectionIdentityTest.kt
• core/data/src/commonTest/kotlin/org/meshtastic/core/data/manager/NodeManagerImplTest.kt
• core/data/src/commonTest/kotlin/org/meshtastic/core/data/manager/SessionWorkTest.kt
• core/data/src/commonTest/kotlin/org/meshtastic/core/data/manager/TelemetryPacketHandlerImplTest.kt
• core/data/src/jvmTest/kotlin/org/meshtastic/core/data/manager/StoreForwardPacketHandlerImplTest.kt
• core/data/src/jvmTest/kotlin/org/meshtastic/core/data/repository/AirQualityChartReproTest.kt
• core/database/build.gradle.kts
• core/database/src/commonMain/kotlin/org/meshtastic/core/database/DatabaseConstants.kt
• core/database/src/commonMain/kotlin/org/meshtastic/core/database/DatabaseManager.kt
• core/database/src/commonMain/kotlin/org/meshtastic/core/database/DatabaseMerger.kt
• core/database/src/commonMain/kotlin/org/meshtastic/core/database/DatabaseProvider.kt
• core/database/src/commonMain/kotlin/org/meshtastic/core/database/dao/MeshLogDao.kt
• core/database/src/commonMain/kotlin/org/meshtastic/core/database/dao/PacketDao.kt
• core/database/src/commonMain/kotlin/org/meshtastic/core/database/dao/SwitchingDiscoveryDao.kt
• core/database/src/commonMain/kotlin/org/meshtastic/core/database/dao/TracerouteNodePositionDao.kt
• core/database/src/commonTest/kotlin/org/meshtastic/core/database/DatabaseManagerAssociationRecoveryTest.kt
• core/database/src/commonTest/kotlin/org/meshtastic/core/database/DatabaseManagerBackfillTest.kt
• core/database/src/commonTest/kotlin/org/meshtastic/core/database/DatabaseManagerEvictionTest.kt
• core/database/src/commonTest/kotlin/org/meshtastic/core/database/DatabaseManagerRetirementTest.kt
• core/database/src/commonTest/kotlin/org/meshtastic/core/database/DatabaseManagerShutdownTest.kt
• core/database/src/commonTest/kotlin/org/meshtastic/core/database/DatabaseManagerTestFixture.kt
• core/database/src/commonTest/kotlin/org/meshtastic/core/database/DatabaseManagerWriterAdmissionTest.kt
• core/database/src/commonTest/kotlin/org/meshtastic/core/database/DatabaseMergerTest.kt
• core/database/src/commonTest/kotlin/org/meshtastic/core/database/dao/MeshLogDaoTest.kt
• core/database/src/commonTest/kotlin/org/meshtastic/core/database/dao/PacketDaoAtomicTransactionTest.kt
• core/database/src/commonTest/kotlin/org/meshtastic/core/database/dao/TracerouteNodePositionDaoTest.kt
• core/database/src/jvmTest/kotlin/org/meshtastic/core/database/DatabaseManagerPendingRouteRecoveryJvmTest.kt
• core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/AdminPacketHandler.kt
• core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/FromRadioPacketHandler.kt
• core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/MeshConfigFlowManager.kt
• core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/MeshConfigHandler.kt
• core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/MeshDataHandler.kt
• core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/MeshMessageProcessor.kt
• core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/NodeManager.kt
• core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/PacketHandler.kt
• core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/RadioInterfaceService.kt
• core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/ReceivedRadioFrame.kt
• core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/StoreForwardPacketHandler.kt
• core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/TelemetryPacketHandler.kt
• core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/TracerouteHandler.kt
• core/service/src/commonMain/kotlin/org/meshtastic/core/service/MeshServiceOrchestrator.kt
• core/service/src/commonMain/kotlin/org/meshtastic/core/service/RadioControllerImpl.kt
• core/service/src/commonMain/kotlin/org/meshtastic/core/service/SharedRadioInterfaceService.kt
• core/service/src/commonTest/kotlin/org/meshtastic/core/service/MeshServiceOrchestratorTest.kt
• core/service/src/commonTest/kotlin/org/meshtastic/core/service/RadioControllerImplTest.kt
• core/service/src/commonTest/kotlin/org/meshtastic/core/service/SharedRadioInterfaceServiceLivenessTest.kt
• core/takserver/src/commonTest/kotlin/org/meshtastic/core/takserver/TAKMeshIntegrationTest.kt
• core/testing/src/commonMain/kotlin/org/meshtastic/core/testing/FakeDatabaseManager.kt
• core/testing/src/commonMain/kotlin/org/meshtastic/core/testing/FakeRadioInterfaceService.kt
• core/testing/src/commonTest/kotlin/org/meshtastic/core/testing/FakeRadioInterfaceServiceSessionTest.kt
• core/testing/src/commonTest/kotlin/org/meshtastic/core/testing/RepositoryFakesTest.kt
• desktopApp/src/main/kotlin/org/meshtastic/desktop/stub/NoopStubs.kt

🚧 Files skipped from review as they are similar to previous changes (72)

• core/database/build.gradle.kts
• core/service/src/commonMain/kotlin/org/meshtastic/core/service/MeshServiceOrchestrator.kt
• core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/FromRadioPacketHandler.kt
• core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/ReceivedRadioFrame.kt
• core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/SessionWork.kt
• core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/StoreForwardPacketHandler.kt
• core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/PacketHandler.kt
• core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/AdminPacketHandler.kt
• core/takserver/src/commonTest/kotlin/org/meshtastic/core/takserver/TAKMeshIntegrationTest.kt
• core/testing/src/commonMain/kotlin/org/meshtastic/core/testing/FakeDatabaseManager.kt
• core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/TelemetryPacketHandler.kt
• core/database/src/commonTest/kotlin/org/meshtastic/core/database/dao/TracerouteNodePositionDaoTest.kt
• core/data/src/commonMain/kotlin/org/meshtastic/core/data/repository/TracerouteSnapshotRepositoryImpl.kt
• core/database/src/commonMain/kotlin/org/meshtastic/core/database/DatabaseProvider.kt
• core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/MeshDataHandler.kt
• core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/TracerouteHandler.kt
• core/common/src/commonMain/kotlin/org/meshtastic/core/common/database/DatabaseManager.kt
• desktopApp/src/main/kotlin/org/meshtastic/desktop/stub/NoopStubs.kt
• core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/MeshConfigHandler.kt
• core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/PacketHandlerImpl.kt
• core/database/src/commonTest/kotlin/org/meshtastic/core/database/dao/MeshLogDaoTest.kt
• core/data/src/commonTest/kotlin/org/meshtastic/core/data/manager/SessionWorkTest.kt
• core/data/src/jvmTest/kotlin/org/meshtastic/core/data/repository/AirQualityChartReproTest.kt
• core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/MeshConfigFlowManager.kt
• core/database/src/commonMain/kotlin/org/meshtastic/core/database/dao/TracerouteNodePositionDao.kt
• androidApp/src/main/kotlin/org/meshtastic/app/MeshUtilApplication.kt
• core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/MeshMessageProcessor.kt
• core/database/src/commonMain/kotlin/org/meshtastic/core/database/DatabaseMerger.kt
• core/data/src/commonTest/kotlin/org/meshtastic/core/data/manager/GeofenceMonitorTest.kt
• core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/AdminPacketHandlerImpl.kt
• core/database/src/commonTest/kotlin/org/meshtastic/core/database/DatabaseManagerWriterAdmissionTest.kt
• core/testing/src/commonTest/kotlin/org/meshtastic/core/testing/RepositoryFakesTest.kt
• core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/GeofenceMonitor.kt
• core/testing/src/commonTest/kotlin/org/meshtastic/core/testing/FakeRadioInterfaceServiceSessionTest.kt
• core/data/src/jvmTest/kotlin/org/meshtastic/core/data/manager/StoreForwardPacketHandlerImplTest.kt
• core/data/src/commonMain/kotlin/org/meshtastic/core/data/repository/MeshLogRepositoryImpl.kt
• core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/RadioInterfaceService.kt
• core/database/src/commonTest/kotlin/org/meshtastic/core/database/dao/PacketDaoAtomicTransactionTest.kt
• core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/NodeManagerImpl.kt
• core/data/src/commonTest/kotlin/org/meshtastic/core/data/manager/NodeManagerConnectionIdentityTest.kt
• core/data/src/commonMain/kotlin/org/meshtastic/core/data/repository/PacketRepositoryImpl.kt
• core/database/src/commonMain/kotlin/org/meshtastic/core/database/dao/PacketDao.kt
• core/database/src/commonTest/kotlin/org/meshtastic/core/database/DatabaseMergerTest.kt
• core/service/src/commonMain/kotlin/org/meshtastic/core/service/RadioControllerImpl.kt
• core/database/src/jvmTest/kotlin/org/meshtastic/core/database/DatabaseManagerPendingRouteRecoveryJvmTest.kt
• core/database/src/commonTest/kotlin/org/meshtastic/core/database/DatabaseManagerBackfillTest.kt
• core/testing/src/commonMain/kotlin/org/meshtastic/core/testing/FakeRadioInterfaceService.kt
• core/database/src/commonMain/kotlin/org/meshtastic/core/database/dao/SwitchingDiscoveryDao.kt
• core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/TracerouteHandlerImpl.kt
• core/data/src/commonTest/kotlin/org/meshtastic/core/data/manager/MeshConfigHandlerImplTest.kt
• core/data/src/commonTest/kotlin/org/meshtastic/core/data/manager/TelemetryPacketHandlerImplTest.kt
• core/database/src/commonTest/kotlin/org/meshtastic/core/database/DatabaseManagerRetirementTest.kt
• core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/NodeManager.kt
• core/service/src/commonTest/kotlin/org/meshtastic/core/service/SharedRadioInterfaceServiceLivenessTest.kt
• core/data/src/commonTest/kotlin/org/meshtastic/core/data/manager/FromRadioPacketHandlerImplTest.kt
• core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/MeshConfigFlowManagerImpl.kt
• core/database/src/commonTest/kotlin/org/meshtastic/core/database/DatabaseManagerTestFixture.kt
• core/data/src/commonTest/kotlin/org/meshtastic/core/data/manager/MeshMessageProcessorImplTest.kt
• core/service/src/commonTest/kotlin/org/meshtastic/core/service/MeshServiceOrchestratorTest.kt
• core/database/src/commonTest/kotlin/org/meshtastic/core/database/DatabaseManagerShutdownTest.kt
• core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/FromRadioPacketHandlerImpl.kt
• core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/TelemetryPacketHandlerImpl.kt
• core/data/src/commonTest/kotlin/org/meshtastic/core/data/manager/MeshDataHandlerTest.kt
• core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/StoreForwardPacketHandlerImpl.kt
• core/database/src/commonTest/kotlin/org/meshtastic/core/database/DatabaseManagerAssociationRecoveryTest.kt
• core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/MeshMessageProcessorImpl.kt
• core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/MeshDataHandlerImpl.kt
• core/data/src/commonTest/kotlin/org/meshtastic/core/data/manager/MeshConfigFlowManagerImplTest.kt
• core/data/src/commonTest/kotlin/org/meshtastic/core/data/manager/NodeManagerImplTest.kt
• core/service/src/commonTest/kotlin/org/meshtastic/core/service/RadioControllerImplTest.kt
• core/service/src/commonMain/kotlin/org/meshtastic/core/service/SharedRadioInterfaceService.kt
• core/database/src/commonMain/kotlin/org/meshtastic/core/database/DatabaseManager.kt

[codecov]

⚠️ JUnit XML file not found

The CLI was unable to find any JUnit XML files to upload.
For more help, visit our troubleshooting guide.

[jamesarich]

🎉