fix(messaging): time out orphaned "Sending..." messages into a retryable failure

[jamesarich]

fix(messaging): time out orphaned "Sending..." messages into a retryable failure

Why

An outbound message goes ENROUTE when handed to the radio and only a routing ACK/NAK resolves it. If that response never reaches the app — typically because the app was disconnected when it arrived — the message showed "Sending..." forever, with no retry affordance (ENROUTE is not retryable). iOS fixed the same defect in meshtastic/Meshtastic-Apple#2224 (v2.7.18) with a 5-minute send timeout; this brings Android to parity so the shared status vocabulary on meshtastic/design#43 can be finalized.

🐛 Bug Fixes

• A sent message still ENROUTE five minutes after handoff to the radio (matching iOS's sendAckTimeout) is now stamped ERROR + Routing.Error.TIMEOUT. It renders through the existing vocabulary — "Failed to deliver to mesh" with the detail "No acknowledgment was received in time…" — and gets the standard Resend affordance. No new strings.
• Timeouts are re-armed on every connection (PacketHandler.rearmSendAckTimeouts, called from onRadioConfigLoaded) so messages orphaned by a disconnect or app restart resolve too. Re-arm waits at least 30 s so an ACK/NAK still sitting in the radio's phone-queue backlog can resolve the message first; a late ACK always upgrades a timed-out message via the normal handleAckNak path.
• QUEUED messages are deliberately untouched: they auto-send on the next connection and time out only if that send's ACK never arrives.

Known limitation

iOS derives its timed-out state at render time, so it needs no timer. Android's status is persisted, so this uses a live timer plus the reconnect re-arm. If the app process is killed and the user never connects a radio again, an orphaned message keeps showing "Sending..." until the next connection. Closing that would mean deriving status at render time from the send timestamp, which ripples through the Message status APIs — left out to keep this change small.

Testing Performed

• New unit tests in PacketHandlerImplTest: unacked ENROUTE send times out to a retryable ERROR/TIMEOUT; a send resolved before the deadline is left untouched; re-arm times out a stale persisted packet after the reconnect grace; re-arm gives a fresh packet its full ack window.
• Full baseline: spotlessApply spotlessCheck detekt assembleDebug test allTests kmpSmokeCompile.

Refs meshtastic/design#43.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes

  • Improved handling of packet acknowledgment timeouts after reconnects or app restarts.
  • Packets that remain unacknowledged now transition to a retryable timeout error.
  • Persisted packets awaiting acknowledgment receive an appropriate timeout window when transmission resumes.

• Tests

  • Added coverage for acknowledged packets, timed-out packets, and restored packets with fresh or expired timeout windows.

[coderabbitai]

Important

Review skipped

Auto incremental reviews are disabled on this repository.

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: 7a2c1f9a-476d-4e3a-a710-d3e93c789131

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

The change adds routing-ack timeout handling for ENROUTE packets. It persists unresolved packets as ERROR with Routing.Error.TIMEOUT and rearms persisted timeouts after reconnect.

Changes

Packet acknowledgment timeout handling

Layer / File(s)	Summary
Timeout and persistence contracts core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/PacketHandler.kt, core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/PacketRepository.kt, core/data/src/commonMain/kotlin/org/meshtastic/core/data/repository/PacketRepositoryImpl.kt	The repository exposes persisted ENROUTE packets. The packet handler exposes rearmSendAckTimeouts().
En-route timeout processing core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/PacketHandlerImpl.kt	ENROUTE packets receive delayed acknowledgment timeout processing. Packets that remain unresolved become ERROR with Routing.Error.TIMEOUT.
Reconnect timeout rearming and validation core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/MeshConnectionManagerImpl.kt, core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/PacketHandlerImpl.kt, core/data/src/commonTest/kotlin/org/meshtastic/core/data/manager/PacketHandlerImplTest.kt	Radio configuration loading rearms persisted packet timeouts. Tests cover stale, fresh, resolved, and unresolved packets.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

• meshtastic/Meshtastic-Android#6133: Handles user-facing wording for Routing.Error.TIMEOUT.
• meshtastic/Meshtastic-Android#6598: Modifies PacketHandlerImpl timeout and dispatch behavior.
• meshtastic/Meshtastic-Android#6603: Modifies routing acknowledgment handling in PacketHandlerImpl.

Suggested labels: bugfix

🚥 Pre-merge checks | ✅ 5 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Tests Prove The Path, Not The End State	⚠️ Warning	The added send resolved before the timeout is left untouched test only verifies zero update calls; removing the timeout scheduler leaves that assertion passing.	Make the test exercise the resolution path, such as invoking handleAckNak, and verify the expected status update before advancing the timeout.

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: timing out orphaned sending messages and marking them as retryable failures.
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.
Sibling Call Sites And Presence Semantics	✅ Passed	The diff only adds packet timeout and re-arm logic. It changes no nullable field, zero-guard, presence check, or physical-metric default, so no sibling call site is affected.

---

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

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/PacketHandlerImpl.kt`:
- Around line 301-308: Make scheduleSendAckTimeout use an atomic repository/DAO
conditional update that sets ERROR with Routing.Error.TIMEOUT only when the
stored packet status remains ENROUTE, rather than writing the previously read
packet copy. Add the corresponding repository and DAO operation, then test that
an ACK or NAK resolving the packet between the read and persistence prevents the
timeout update from overwriting it.
- Around line 280-292: Track each send-ack timeout job by packet ID, including
jobs created by scheduleSendAckTimeout; cancel and remove the corresponding jobs
when disconnecting, and cancel any existing job before rearming in
rearmSendAckTimeouts. Ensure reconnect scheduling uses REARM_GRACE without
allowing the original timeout to mark the packet ERROR, and add a regression
test covering send, disconnect, reconnect before the original deadline, and the
grace window.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 2046cee7-747f-4125-991b-2c62aadc6bab

📥 Commits

Reviewing files that changed from the base of the PR and between 6c06601 and 8231191.

📒 Files selected for processing (6)

• core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/MeshConnectionManagerImpl.kt
• core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/PacketHandlerImpl.kt
• core/data/src/commonMain/kotlin/org/meshtastic/core/data/repository/PacketRepositoryImpl.kt
• core/data/src/commonTest/kotlin/org/meshtastic/core/data/manager/PacketHandlerImplTest.kt
• core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/PacketHandler.kt
• core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/PacketRepository.kt