Harden firmware recovery and retries

Improve companion BLE delivery and idempotent remote CLI retries, make FPF7 persistence atomic with FPF6 migration, verify staged OTA data on resume, and clarify filter policy wildcard and BlackHole semantics.

test/README.md

@@ -41,8 +41,9 @@ does not reflect the GoogleTest count -- run the built binary directly
 | `test_telemetry_history` | `src/helpers/TelemetryHistory.h` | 30-minute rings; seven-day temperature/voltage and dynamically sized GPS retention; exact 1 C temperature/status encoding; separate Base64 series payloads; 14-bit GPS differentials; resize preservation, heap budgets, and 1-based paging bounds |
 | `test_flood_filter_policy` | `src/helpers/FloodFilterPolicy.h` | unordered blacklist matching; ordered 1/2/3-byte pbyte rule prefixes; original incoming scope classes and canonical region-name identity; channel-authentication cache key comparison; priority ordering and terminal stop masks; bridge-bucket and regionless channel-target selector encoding; `require=region` and per-channel scope-gate truth tables; fast/slow timing; adding, replacing, and preserving packet scope |
 | `test_logical_message_cache` | `src/helpers/LogicalMessageCache.h` | bounded logical-message mapping; stable retry timestamps; exact older retries after newer messages; stale and same-timestamp mismatch rejection |
-| `test_remote_cli_reply_cache` | `src/helpers/RemoteCliReplyCache.h` | authenticated sender/timestamp/command matching; owned response storage; empty-response completion; on-air truncation; replacement and clearing |
-| `test_companion_frame_queue` | `src/helpers/CompanionFrameQueue.h` | reserved reply capacity; response priority; push eviction under saturation; response preservation |
+| `test_remote_cli_reply_cache` | `src/helpers/RemoteCliReplyCache.h`, `src/helpers/RemoteCliRequest.h` | authenticated logical-request matching; bounded recent-reply history; backward-compatible retry identity; empty-response completion; on-air truncation and clearing |
+| `test_companion_frame_queue` | `src/helpers/CompanionFrameQueue.h` | response/required/best-effort classification; reserved capacity; stable priority; safe eviction; message-waiting coalescing |
+| `test_ble_tx_stall_watchdog` | `src/helpers/BleTxStallWatchdog.h` | exact BLE fragment progress; blocked-reply timeout; rollover-safe elapsed time; disconnect recovery retry and completion |
 | `test_utils` | `src/Utils.cpp` | `Utils::toHex` (upstream) |
 
 ## Conventions (and how to add a suite)

test/test_ble_tx_stall_watchdog/test_ble_tx_stall_watchdog.cpp

@@ -0,0 +1,155 @@
+#include <gtest/gtest.h>
+
+#include <helpers/BleTxStallWatchdog.h>
+
+#include <stdint.h>
+#include <vector>
+
+TEST(BleFrameChunks, WritesOneAttPayloadAtATime) {
+  const uint8_t frame[50] = {};
+  std::vector<size_t> chunk_lengths;
+
+  const size_t written = mesh::writeBleFrameInChunks(
+      frame, sizeof(frame), 20,
+      [&chunk_lengths](const uint8_t*, size_t len) {
+        chunk_lengths.push_back(len);
+        return len;
+      });
+
+  EXPECT_EQ(written, sizeof(frame));
+  ASSERT_EQ(chunk_lengths.size(), 3U);
+  EXPECT_EQ(chunk_lengths[0], 20U);
+  EXPECT_EQ(chunk_lengths[1], 20U);
+  EXPECT_EQ(chunk_lengths[2], 10U);
+}
+
+TEST(BleFrameChunks, StopsAfterAnAmbiguousLaterFragmentFailure) {
+  const uint8_t frame[50] = {};
+  size_t calls = 0;
+
+  const size_t written = mesh::writeBleFrameInChunks(
+      frame, sizeof(frame), 20,
+      [&calls](const uint8_t*, size_t len) {
+        calls++;
+        return calls == 1 ? len : 0;
+      });
+
+  EXPECT_EQ(written, 20U);
+  EXPECT_EQ(calls, 2U);
+}
+
+TEST(BleFrameChunks, PreservesAndStopsAtAShortChunkWrite) {
+  const uint8_t frame[50] = {};
+  size_t calls = 0;
+
+  const size_t written = mesh::writeBleFrameInChunks(
+      frame, sizeof(frame), 20,
+      [&calls](const uint8_t*, size_t len) {
+        calls++;
+        return calls == 1 ? len : 5U;
+      });
+
+  EXPECT_EQ(written, 25U);
+  EXPECT_EQ(calls, 2U);
+}
+
+TEST(BleFrameChunks, RejectsInvalidInputsWithoutCallingWriter) {
+  const uint8_t frame[1] = {};
+  size_t calls = 0;
+  auto writer = [&calls](const uint8_t*, size_t len) {
+    calls++;
+    return len;
+  };
+
+  EXPECT_EQ(mesh::writeBleFrameInChunks(nullptr, 1, 20, writer), 0U);
+  EXPECT_EQ(mesh::writeBleFrameInChunks(frame, 0, 20, writer), 0U);
+  EXPECT_EQ(mesh::writeBleFrameInChunks(frame, 1, 0, writer), 0U);
+  EXPECT_EQ(calls, 0U);
+}
+
+TEST(BleTxStallWatchdog, ExpiresAtTheExactBlockedBoundary) {
+  mesh::BleTxStallWatchdog watchdog;
+
+  EXPECT_FALSE(watchdog.noteBlocked(1000, 10000));
+  EXPECT_TRUE(watchdog.active());
+  EXPECT_FALSE(watchdog.noteBlocked(10999, 10000));
+  EXPECT_TRUE(watchdog.noteBlocked(11000, 10000));
+}
+
+TEST(BleTxStallWatchdog, ProgressResetStartsAFreshWindow) {
+  mesh::BleTxStallWatchdog watchdog;
+
+  EXPECT_FALSE(watchdog.noteBlocked(100, 1000));
+  EXPECT_FALSE(watchdog.noteBlocked(1099, 1000));
+
+  watchdog.reset();
+  EXPECT_FALSE(watchdog.active());
+  EXPECT_FALSE(watchdog.noteBlocked(1100, 1000));
+  EXPECT_FALSE(watchdog.noteBlocked(2099, 1000));
+  EXPECT_TRUE(watchdog.noteBlocked(2100, 1000));
+}
+
+TEST(BleTxStallWatchdog, ZeroIsAValidStartTime) {
+  mesh::BleTxStallWatchdog watchdog;
+
+  EXPECT_FALSE(watchdog.noteBlocked(0, 25));
+  EXPECT_FALSE(watchdog.noteBlocked(24, 25));
+  EXPECT_TRUE(watchdog.noteBlocked(25, 25));
+}
+
+TEST(BleTxStallWatchdog, ElapsedTimeSurvivesMillisRollover) {
+  mesh::BleTxStallWatchdog watchdog;
+  const uint32_t start = UINT32_MAX - 50U;
+
+  EXPECT_FALSE(watchdog.noteBlocked(start, 100));
+  EXPECT_FALSE(watchdog.noteBlocked(48, 100));
+  EXPECT_TRUE(watchdog.noteBlocked(49, 100));
+}
+
+TEST(BleElapsedAtLeast, HandlesExactBoundaryAndMillisRollover) {
+  EXPECT_FALSE(mesh::bleElapsedAtLeast(1099, 100, 1000));
+  EXPECT_TRUE(mesh::bleElapsedAtLeast(1100, 100, 1000));
+
+  const uint32_t start = UINT32_MAX - 50U;
+  EXPECT_FALSE(mesh::bleElapsedAtLeast(48, start, 100));
+  EXPECT_TRUE(mesh::bleElapsedAtLeast(49, start, 100));
+}
+
+TEST(BleDisconnectRecovery, AttemptsImmediatelyThenThrottlesRetries) {
+  mesh::BleDisconnectRecovery recovery;
+
+  EXPECT_FALSE(recovery.pending());
+  EXPECT_FALSE(recovery.shouldAttempt(100, 1000));
+
+  recovery.begin();
+  EXPECT_TRUE(recovery.pending());
+  EXPECT_TRUE(recovery.shouldAttempt(100, 1000));
+  EXPECT_FALSE(recovery.shouldAttempt(1099, 1000));
+  EXPECT_TRUE(recovery.shouldAttempt(1100, 1000));
+}
+
+TEST(BleDisconnectRecovery, RetryTimingSurvivesMillisRollover) {
+  mesh::BleDisconnectRecovery recovery;
+  const uint32_t start = UINT32_MAX - 50U;
+
+  recovery.begin();
+  EXPECT_TRUE(recovery.shouldAttempt(start, 100));
+  EXPECT_FALSE(recovery.shouldAttempt(48, 100));
+  EXPECT_TRUE(recovery.shouldAttempt(49, 100));
+}
+
+TEST(BleDisconnectRecovery, CompletionStopsRetries) {
+  mesh::BleDisconnectRecovery recovery;
+
+  recovery.begin();
+  EXPECT_TRUE(recovery.shouldAttempt(0, 100));
+  recovery.complete();
+
+  EXPECT_FALSE(recovery.pending());
+  EXPECT_FALSE(recovery.shouldAttempt(1000, 100));
+}
+
+int main(int argc, char** argv) {
+  ::testing::InitGoogleTest(&argc, argv);
+  return RUN_ALL_TESTS();
+}

test/test_companion_frame_queue/test_companion_frame_queue.cpp

@@ -29,6 +29,51 @@ TEST(CompanionFrameQueue, PushTrafficLeavesOneResponseSlot) {
   EXPECT_EQ(0x00, queue[0].buf[0]);
 }
 
+TEST(CompanionFrameQueue, RequiredPushEvictsBestEffortTraffic) {
+  TestFrame queue[4] = {
+      {1, {0x06}}, {1, {0x82}}, {1, {0x88}}, {1, {0x84}}};
+  size_t count = 4;
+
+  ASSERT_TRUE(enqueue(queue, count, 4, 0x8B));
+  ASSERT_EQ(4U, count);
+  EXPECT_EQ(0x06, queue[0].buf[0]);
+  EXPECT_EQ(0x82, queue[1].buf[0]);
+  EXPECT_EQ(0x8B, queue[2].buf[0]);
+  EXPECT_EQ(0x88, queue[3].buf[0]);
+}
+
+TEST(CompanionFrameQueue, RequiredPushesRemainFifoAheadOfLogs) {
+  TestFrame queue[6] = {};
+  size_t count = 0;
+
+  ASSERT_TRUE(enqueue(queue, count, 6, 0x88));
+  ASSERT_TRUE(enqueue(queue, count, 6, 0x82));
+  ASSERT_TRUE(enqueue(queue, count, 6, 0x84));
+  ASSERT_TRUE(enqueue(queue, count, 6, 0x85));
+
+  ASSERT_EQ(4U, count);
+  EXPECT_EQ(0x82, queue[0].buf[0]);
+  EXPECT_EQ(0x85, queue[1].buf[0]);
+  EXPECT_EQ(0x88, queue[2].buf[0]);
+  EXPECT_EQ(0x84, queue[3].buf[0]);
+}
+
+TEST(CompanionFrameQueue, MessageWaitingCoalesces) {
+  TestFrame queue[4] = {};
+  size_t count = 0;
+
+  ASSERT_TRUE(enqueue(queue, count, 4, 0x83));
+  ASSERT_TRUE(enqueue(queue, count, 4, 0x83));
+  EXPECT_EQ(1U, count);
+}
+
+TEST(CompanionFrameQueue, UnknownPushDefaultsToRequired) {
+  uint8_t unknown = 0xF1;
+  EXPECT_TRUE(mesh::companionFrameRequiresDelivery(&unknown, 1));
+  uint8_t packet_log = 0x88;
+  EXPECT_FALSE(mesh::companionFrameRequiresDelivery(&packet_log, 1));
+}
+
 TEST(CompanionFrameQueue, ResponsesRunBeforeQueuedPushesAndRemainFifo) {
   TestFrame queue[5] = {};
   size_t count = 0;

test/test_deferred_cli_command/test_deferred_cli_command.cpp

@@ -8,13 +8,19 @@ TEST(DeferredCliCommand, CopiesAuthenticatedCommandContext) {
   memset(secret, 0x5A, sizeof(secret));
   const char command[] = "del flood.moderation.all";
 
-  ASSERT_TRUE(deferred.enqueue(7, 123456U, 2, secret, command, strlen(command)));
+  ASSERT_TRUE(deferred.enqueue(7, 123456U, 2, secret, command,
+                               strlen(command), 654321U));
   EXPECT_TRUE(deferred.pending);
   EXPECT_EQ(7, deferred.client_index);
   EXPECT_EQ(123456U, deferred.sender_timestamp);
+  EXPECT_EQ(654321U, deferred.request_id);
   EXPECT_EQ(2, deferred.path_hash_size);
   EXPECT_EQ(0, memcmp(secret, deferred.secret, sizeof(secret)));
   EXPECT_STREQ(command, deferred.command);
+  EXPECT_TRUE(deferred.matches(7, 654321U, command, strlen(command)));
+  EXPECT_FALSE(deferred.matches(6, 654321U, command, strlen(command)));
+  EXPECT_FALSE(deferred.matches(7, 654322U, command, strlen(command)));
+  EXPECT_FALSE(deferred.matches(7, 654321U, "region save", 11));
 
   secret[0] = 0;
   EXPECT_EQ(0x5A, deferred.secret[0]);
@@ -33,7 +39,9 @@ TEST(DeferredCliCommand, RejectsSecondCommandUntilCleared) {
 
   deferred.clear();
   EXPECT_FALSE(deferred.pending);
+  EXPECT_FALSE(deferred.matches(1, 10U, first, strlen(first)));
   EXPECT_EQ(0, deferred.secret[0]);
+  EXPECT_EQ(0U, deferred.request_id);
   EXPECT_EQ(0, deferred.command[0]);
   ASSERT_TRUE(deferred.enqueue(2, 11U, 3, secret, second, strlen(second)));
   EXPECT_STREQ(second, deferred.command);

test/test_flood_filter_policy/test_flood_filter_policy.cpp

@@ -269,6 +269,22 @@ TEST(FloodRuleOrder, NoStopPreservesEveryMatch) {
                        0x0B, priorities, stop_flags, 4));
 }
 
+TEST(FloodRuleOrder, MissingRegionTargetDoesNotStopLowerSafetyRules) {
+  EXPECT_TRUE(FloodFilterPolicy::stopActionApplies(true, false, false));
+  EXPECT_TRUE(FloodFilterPolicy::stopActionApplies(true, true, true));
+  EXPECT_FALSE(FloodFilterPolicy::stopActionApplies(true, true, false));
+  EXPECT_FALSE(FloodFilterPolicy::stopActionApplies(false, true, true));
+
+  const uint8_t priorities[] = {200, 100};
+  const uint8_t stop_flags[] = {
+      (uint8_t)(FloodFilterPolicy::stopActionApplies(true, true, false)
+                    ? 1 : 0),
+      0,
+  };
+  EXPECT_EQ(0x03U, FloodFilterPolicy::truncateRulesAtStop(
+                       0x03, priorities, stop_flags, 2));
+}
+
 TEST(FloodRuleOrder, ThirtyOneSlotTableIncludesTheLastSlot) {
   uint8_t priorities[31] = {0};
   uint8_t stop_flags[31] = {0};

test/test_ota/test_ota_core.cpp

@@ -194,6 +194,26 @@ TEST(OtaMerkle, BinaryCounterMatchesLevelByLevel) {
   }
 }
 
+TEST(OtaMerkle, StreamingAccumulatorMatchesContiguousRoot) {
+  uint8_t leaves[257 * 4];
+  for (size_t i = 0; i < sizeof(leaves); ++i) {
+    leaves[i] = (uint8_t)(i * 29U + 7U);
+  }
+  MerkleAccumulator accumulator;
+  for (uint32_t i = 0; i < 257; ++i) {
+    ASSERT_TRUE(accumulator.add(leaves + i * 4));
+  }
+  uint8_t streamed[4], contiguous[4];
+  ASSERT_TRUE(accumulator.finish(streamed));
+  merkle_root(contiguous, leaves, 257);
+  EXPECT_EQ(0, std::memcmp(streamed, contiguous, 4));
+  EXPECT_EQ(257U, accumulator.count());
+
+  accumulator.reset();
+  EXPECT_FALSE(accumulator.finish(streamed));
+  EXPECT_EQ(0U, accumulator.count());
+}
+
 // Verify every block's proof for several tricky counts, using proofs generated by the Python
 // reference (the oracle) - covers deep promotion chains (100, 255, 256, ...).
 TEST(OtaMerkle, ReferenceProofsAllIndices) {
@@ -508,6 +528,29 @@ static void pump(OtaManager& client, int guard_max = 200000) {
   }
 }
 
+static void finish_staged_verification(OtaManager& manager) {
+  int guard = 10000;
+  while (manager.fetchState() == OtaManager::VERIFYING_STAGED
+         && guard-- > 0) {
+    manager.loop();
+  }
+  ASSERT_GT(guard, 0);
+}
+
+class FaultingResumeStore : public OtaStoreRam<4096> {
+public:
+  void failReadAt(uint32_t offset) { _fail_offset = offset; }
+
+  bool read(uint32_t offset, uint8_t* buffer,
+            uint32_t length) const override {
+    if (offset == _fail_offset) return false;
+    return OtaStoreRam<4096>::read(offset, buffer, length);
+  }
+
+private:
+  uint32_t _fail_offset = UINT32_MAX;
+};
+
 // A test MotaSource backing an external "folder" with one or more complete `.mota` images held in RAM -
 // the simplest concrete transport (a real device uses serial/BLE/WiFi/FS, same interface). describe()
 // parses each container for the catalog + region offsets; read() is a bounds-checked memcpy.
@@ -937,6 +980,8 @@ TEST(OtaTransfer, ResumeAfterReboot) {
   client2.begin(SIM_TARGET_ID, sim_send, &to_server2);
   client2.set_fetch_store(&store);
   ASSERT_TRUE(client2.resumeStaged(nullptr));               // adopt whatever is staged
+  EXPECT_EQ(client2.fetchState(), OtaManager::VERIFYING_STAGED);
+  finish_staged_verification(client2);
   EXPECT_EQ(client2.blocksHave(), had);                     // resumed exactly where we left off
   EXPECT_EQ(client2.fetchState(), OtaManager::FETCHING);
   EXPECT_EQ(client2.blocksTotal(), SIM_MOTA_1K_BLOCKS);
@@ -947,6 +992,68 @@ TEST(OtaTransfer, ResumeAfterReboot) {
   EXPECT_EQ(0, std::memcmp(store.data(), SIM_MOTA_1K, SIM_MOTA_1K_LEN));   // byte-identical to the original
 }
 
+TEST(OtaTransfer, ResumeRehashesPayloadBeforeTrustingPresentLeaf) {
+  g_q.clear();
+  OtaManager server, client;
+  OtaStoreRam<4096> store;
+  SendTo to_client{&client}, to_server{&server};
+  server.begin(0, sim_send, &to_client);
+  client.begin(SIM_TARGET_ID, sim_send, &to_server);
+  client.set_fetch_store(&store);
+  client.set_autofetch(OtaManager::AUTOFETCH_ANY);
+  ASSERT_TRUE(server.serve(SIM_MOTA, SIM_MOTA_LEN));
+  server.announce();
+  pump(client);
+  ASSERT_EQ(client.fetchState(), OtaManager::COMPLETE);
+
+  MotaManifest staged;
+  ASSERT_TRUE(mota_parse(store.data(), store.staged_size(), staged));
+  const uint32_t payload_offset =
+      (uint32_t)(staged.payload - store.data());
+  const uint32_t leaves_offset =
+      (uint32_t)(staged.leaves - store.data());
+  uint8_t damaged = (uint8_t)(staged.payload[0] ^ 0x5A);
+  ASSERT_TRUE(store.write(payload_offset, &damaged, 1));
+
+  OtaManager resumed;
+  resumed.begin(SIM_TARGET_ID, nullptr, nullptr);
+  resumed.set_fetch_store(&store);
+  ASSERT_TRUE(resumed.resumeStaged(nullptr));
+  finish_staged_verification(resumed);
+  EXPECT_EQ(resumed.fetchState(), OtaManager::FETCHING);
+  EXPECT_EQ(resumed.blocksHave() + 1, resumed.blocksTotal());
+  uint8_t marker[4] = {};
+  ASSERT_TRUE(store.read(leaves_offset, marker, sizeof(marker)));
+  const uint8_t missing[4] = {0xFF, 0xFF, 0xFF, 0xFF};
+  EXPECT_EQ(0, std::memcmp(marker, missing, sizeof(marker)));
+}
+
+TEST(OtaTransfer, ResumeReadFailureCanNeverBecomeComplete) {
+  g_q.clear();
+  OtaManager server, client;
+  FaultingResumeStore store;
+  SendTo to_client{&client}, to_server{&server};
+  server.begin(0, sim_send, &to_client);
+  client.begin(SIM_TARGET_ID, sim_send, &to_server);
+  client.set_fetch_store(&store);
+  client.set_autofetch(OtaManager::AUTOFETCH_ANY);
+  ASSERT_TRUE(server.serve(SIM_MOTA, SIM_MOTA_LEN));
+  server.announce();
+  pump(client);
+  ASSERT_EQ(client.fetchState(), OtaManager::COMPLETE);
+
+  MotaManifest staged;
+  ASSERT_TRUE(mota_parse(store.data(), store.staged_size(), staged));
+  store.failReadAt((uint32_t)(staged.leaves - store.data()));
+
+  OtaManager resumed;
+  resumed.begin(SIM_TARGET_ID, nullptr, nullptr);
+  resumed.set_fetch_store(&store);
+  ASSERT_TRUE(resumed.resumeStaged(nullptr));
+  finish_staged_verification(resumed);
+  EXPECT_EQ(resumed.fetchState(), OtaManager::FAILED);
+}
+
 TEST(OtaTransfer, ClientRejectsWrongTarget) {
   g_q.clear();
   OtaManager server, client;

test/test_remote_cli_reply_cache/test_remote_cli_reply_cache.cpp

@@ -1,6 +1,7 @@
 #include <gtest/gtest.h>
 
 #include <helpers/RemoteCliReplyCache.h>
+#include <helpers/RemoteCliRequest.h>
 
 TEST(RemoteCliReplyCache, ReplaysOnlyTheSameAuthenticatedRequest) {
   mesh::RemoteCliReplyCache cache;
@@ -25,7 +26,7 @@ TEST(RemoteCliReplyCache, ReplaysOnlyTheSameAuthenticatedRequest) {
       mesh::RemoteCliReplyCache::fingerprint("set repeat on", 13)));
 }
 
-TEST(RemoteCliReplyCache, OwnsAndReplacesTheRememberedResponse) {
+TEST(RemoteCliReplyCache, OwnsAndRetainsRecentResponses) {
   mesh::RemoteCliReplyCache cache;
   uint8_t first_sender[PUB_KEY_SIZE] = {};
   uint8_t sender[PUB_KEY_SIZE] = {};
@@ -39,11 +40,27 @@ TEST(RemoteCliReplyCache, OwnsAndReplacesTheRememberedResponse) {
   EXPECT_STREQ("first", cache.response());
 
   ASSERT_TRUE(cache.remember(sender, 2U, 20U, "second"));
-  EXPECT_FALSE(cache.matches(first_sender, 1U, 10U));
+  const char* first_response = nullptr;
+  EXPECT_TRUE(cache.lookup(first_sender, 1U, 10U, &first_response));
+  EXPECT_STREQ("first", first_response);
   EXPECT_TRUE(cache.matches(sender, 2U, 20U));
   EXPECT_STREQ("second", cache.response());
 }
 
+TEST(RemoteCliReplyCache, RoundRobinEvictionIsBounded) {
+  mesh::RemoteCliReplyCache cache;
+  uint8_t sender[PUB_KEY_SIZE] = {};
+  for (size_t i = 0; i < mesh::RemoteCliReplyCache::ENTRY_COUNT; ++i) {
+    ASSERT_TRUE(cache.remember(sender, (uint32_t)i + 1,
+                               (uint32_t)i + 100, "OK"));
+  }
+  EXPECT_TRUE(cache.matches(sender, 1U, 100U));
+  ASSERT_TRUE(cache.remember(sender, 999U, 999U, "new"));
+  EXPECT_FALSE(cache.matches(sender, 1U, 100U));
+  EXPECT_TRUE(cache.matches(sender, 2U, 101U));
+  EXPECT_TRUE(cache.matches(sender, 999U, 999U));
+}
+
 TEST(RemoteCliReplyCache, EmptyResponseStillMarksRequestComplete) {
   mesh::RemoteCliReplyCache cache;
   uint8_t sender[PUB_KEY_SIZE] = {};
@@ -79,6 +96,26 @@ TEST(RemoteCliReplyCache, ClearForgetsTheRequestAndResponse) {
   EXPECT_STREQ("", cache.response());
 }
 
+TEST(RemoteCliRequest, LogicalIdExtensionIsBackwardCompatible) {
+  uint8_t payload[64] = {};
+  const char command[] = "get stats";
+  memcpy(payload + 5, command, strlen(command));
+  const size_t length = mesh::RemoteCliRequest::append(
+      payload, sizeof(payload), 5, strlen(command), 0x12345678U);
+  ASSERT_GT(length, 0U);
+  EXPECT_STREQ(command, (const char*)payload + 5);
+
+  uint32_t logical_id = 0;
+  EXPECT_TRUE(mesh::RemoteCliRequest::parse(
+      payload, length, 5, logical_id));
+  EXPECT_EQ(0x12345678U, logical_id);
+
+  uint8_t legacy[32] = {};
+  memcpy(legacy + 5, command, strlen(command));
+  EXPECT_FALSE(mesh::RemoteCliRequest::parse(
+      legacy, sizeof(legacy), 5, logical_id));
+}
+
 int main(int argc, char** argv) {
   ::testing::InitGoogleTest(&argc, argv);
   return RUN_ALL_TESTS();