Avoid Companion request tag collisions after login

docs/releases/1.17.1.9.md

@@ -22,6 +22,10 @@ layout before installing an image.
 - Access Control replies fit the encrypted LoRa route budget and the
   Companion frame envelope. Large ACL listings use bounded pages. ESP32 ACL
   persistence avoids repeated metadata opens while retaining recovery checks.
+- Companion requests skip recently used reply tags. A login reply whose
+  server timestamp matches the next client timestamp no longer causes an
+  immediate status or ACL request to fail locally. Stale replies remain
+  blocked, and tag selection does not change the radio's wall clock.
 - nRF52 keeps its 8 KiB loop stack. Crypto, contact CRC and background storage
   work reduce peak stack use and long pauses. OTA metadata is cached after
   verification instead of repeatedly reopening storage for status displays.

examples/companion_radio/MyMesh.cpp

@@ -5130,6 +5130,20 @@ bool MyMesh::allowRequestTag(uint32_t tag) {
   return !_request_tag_rejected;
 }
 
+bool MyMesh::allocateRequestTag(uint32_t& tag) {
+  // Login replies use the server's clock, so their retired tag can equal
+  // the next locally generated request tag even with synchronized clocks.
+  // Skip held tags instead of reporting BAD_STATE to the app. One history
+  // slot belongs to this reservation, leaving at most HISTORY_SIZE - 1
+  // collisions. Keep the wall clock and all stale-response guards intact.
+  for (unsigned attempt = 0;
+       attempt < mesh::CompanionDelayedReplies::HISTORY_SIZE; ++attempt) {
+    tag = getRTCClock()->getCurrentTimeUnique();
+    if (allowRequestTag(tag)) return true;
+  }
+  return false;
+}
+
 void MyMesh::armPendingRequest(uint32_t tag, uint32_t timeout, bool flood) {
   const uint32_t now = _ms->getMillis();
   _delayed_replies.armRequest(tag, timeout, flood, now);

examples/companion_radio/MyMesh.h

@@ -416,6 +416,7 @@ protected:
   uint32_t calcDirectTimeoutMillisFor(uint32_t pkt_airtime_millis, uint8_t path_len) const override;
   void onSendTimeout() override;
   bool allowRequestTag(uint32_t tag) override;
+  bool allocateRequestTag(uint32_t& tag) override;
 
   // DataStoreHost methods
   bool onContactLoaded(const ContactInfo& contact) override { return addContact(contact); }

src/helpers/BaseChatMesh.cpp

@@ -814,12 +814,16 @@ int BaseChatMesh::sendLogin(const ContactInfo& recipient, const char* password,
   return MSG_SEND_FAILED;
 }
 
+bool BaseChatMesh::allocateRequestTag(uint32_t& tag) {
+  tag = getRTCClock()->getCurrentTimeUnique();
+  return allowRequestTag(tag);
+}
+
 int BaseChatMesh::sendAnonReq(const ContactInfo& recipient, const uint8_t* data, uint8_t len, uint32_t& tag, uint32_t& est_timeout) {
   mesh::Packet* pkt;
   {
     uint8_t temp[MAX_PACKET_PAYLOAD];
-    tag = getRTCClock()->getCurrentTimeUnique();
-    if (!allowRequestTag(tag)) return MSG_SEND_FAILED;
+    if (!allocateRequestTag(tag)) return MSG_SEND_FAILED;
     memcpy(temp, &tag, 4);   // tag to match later (also extra blob to help make packet_hash unique)
     memcpy(&temp[4], data, len);
 
@@ -846,8 +850,7 @@ int  BaseChatMesh::sendRequest(const ContactInfo& recipient, const uint8_t* req_
   mesh::Packet* pkt;
   {
     uint8_t temp[MAX_PACKET_PAYLOAD];
-    tag = getRTCClock()->getCurrentTimeUnique();
-    if (!allowRequestTag(tag)) return MSG_SEND_FAILED;
+    if (!allocateRequestTag(tag)) return MSG_SEND_FAILED;
     memcpy(temp, &tag, 4);   // mostly an extra blob to help make packet_hash unique
     memcpy(&temp[4], req_data, data_len);
 
@@ -872,8 +875,7 @@ int  BaseChatMesh::sendRequest(const ContactInfo& recipient, uint8_t req_type, u
   mesh::Packet* pkt;
   {
     uint8_t temp[13];
-    tag = getRTCClock()->getCurrentTimeUnique();
-    if (!allowRequestTag(tag)) return MSG_SEND_FAILED;
+    if (!allocateRequestTag(tag)) return MSG_SEND_FAILED;
     memcpy(temp, &tag, 4);   // mostly an extra blob to help make packet_hash unique
     temp[4] = req_type;
     memset(&temp[5], 0, 4);  // reserved (possibly for 'since' param)

src/helpers/BaseChatMesh.h

@@ -182,6 +182,7 @@ protected:
   virtual void onContactResponse(const ContactInfo& contact, const uint8_t* data, uint8_t len) = 0;
   // Reject a recently retired reflected tag before allocating/transmitting.
   virtual bool allowRequestTag(uint32_t tag) { return true; }
+  virtual bool allocateRequestTag(uint32_t& tag);
   virtual void handleReturnPathRetry(const ContactInfo& contact, const uint8_t* path, uint8_t path_len);
 
   virtual bool sendFloodScoped(const ContactInfo& recipient, mesh::Packet* pkt, uint32_t delay_millis=0);

test/fixtures/companion_delayed_reply_delivery/test.cpp

@@ -87,6 +87,7 @@ struct BaseChatMesh {
   RtcBoundary* getRTCClock() { return &rtc; }
   RngBoundary* getRNG() { return &rng; }
   virtual bool allowRequestTag(uint32_t) { return true; }
+  virtual bool allocateRequestTag(uint32_t&);
   virtual void onContactResponse(const ContactInfo&, const uint8_t*, uint8_t) = 0;
   bool canMutateContacts() const { return false; }
   void onContactPathUpdated(ContactInfo&) {}
@@ -159,6 +160,7 @@ struct MyMesh : BaseChatMesh {
   size_t writePendingSerialFrame(const uint8_t*, size_t, uint32_t);
   bool beginPendingRequest(Kind, const ContactInfo&, bool = false);
   bool allowRequestTag(uint32_t) override;
+  bool allocateRequestTag(uint32_t&) override;
   void armPendingRequest(uint32_t, uint32_t, bool);
   void finishPendingRequest(int, uint32_t, uint32_t);
   void abandonPendingRequest();
@@ -633,8 +635,29 @@ int main() {
     f.arm(Tracker::Status, 18); reply(f, Tracker::Status, nullptr, 18); f.drain();
     assert(count(f.wire(), PUSH_CODE_STATUS_RESPONSE) == 1); ++checks;
   }
-  // Real Base send methods and real command branches prove rejection happens
-  // before packet allocation and transmission, including RTC tag re-use.
+  // A real login response carries the server's clock, not the client's
+  // request timestamp. It can equal the client's next timestamp. The app's
+  // immediate status/ACL request must select another tag without accepting
+  // the old login response or changing the wall clock.
+  for (bool tcp : {false, true}) for (Kind kind : {
+      Tracker::Status, Tracker::Binary, Tracker::Telemetry, Tracker::Discovery}) {
+    Fixture f(tcp); f.command(Tracker::Login);
+    const uint32_t server_tag = f.mesh.rtc.wall + 1;
+    reply(f, Tracker::Login, nullptr, server_tag); f.drain();
+    const size_t request_start = f.wire().size();
+    if (kind == Tracker::Binary) f.aclCommand(); else f.command(kind);
+    assert(f.mesh.transmissions == 2 && f.mesh.allocations == 2);
+    const uint32_t tag = f.mesh._delayed_replies.request.tag;
+    assert(tag > server_tag && f.mesh.rtc.wall == 1000);
+    reply(f, Tracker::Login, nullptr, server_tag);
+    assert(f.mesh._delayed_replies.request.phase == Tracker::AwaitRadio);
+    reply(f, kind, nullptr, tag); f.drain();
+    assert(count(f.wire(), RESP_CODE_ERR) == 0);
+    assert(count(f.wire(), PUSH_CODE_LOGIN_SUCCESS) == 1);
+    assertSentBeforeFinal(Bytes(f.wire().begin() + request_start, f.wire().end()),
+                          kind, tag); ++checks;
+  }
+  // An active request still rejects overlap before packet allocation/TX.
   for (Kind kind : {Tracker::Login, Tracker::Status, Tracker::Telemetry, Tracker::Binary, Tracker::Discovery, Tracker::Trace}) {
     Fixture f; f.command(kind); assert(f.mesh.transmissions == 1 && f.mesh.allocations == 1);
     f.command(kind); assert(f.mesh.transmissions == 1 && f.mesh.allocations == 1); ++checks;
@@ -715,17 +738,45 @@ int main() {
       testReservedHistorySurvivesIdle(request_service, start, kind); ++checks;
     }
   }
-  {
-    Fixture f; f.command(Tracker::Binary, true); assert(f.mesh.transmissions == 1);
+  // RTC correction also skips recent tags. Old replies remain quarantined;
+  // the newly transmitted tag is the one reported to the host and matched.
+  for (bool anonymous : {false, true}) for (Kind kind : {
+      Tracker::Status, Tracker::Binary, Tracker::Telemetry, Tracker::Discovery}) {
+    Fixture f; f.command(kind, anonymous);
+    const uint32_t old_tag = f.mesh._delayed_replies.request.tag;
+    reply(f, kind, nullptr, old_tag); f.drain();
+    const size_t request_start = f.wire().size();
+    f.mesh.rtc.resetUniqueTime(old_tag); f.command(kind, anonymous);
+    assert(f.mesh.transmissions == 2 && f.mesh.allocations == 2);
     const uint32_t tag = f.mesh._delayed_replies.request.tag;
-    reply(f, Tracker::Binary, nullptr, tag); f.drain();
-    f.mesh.rtc.resetUniqueTime(tag); f.command(Tracker::Binary, true);
-    assert(f.mesh.transmissions == 1 && f.mesh.allocations == 1); ++checks;
+    assert(tag == old_tag + 1);
+    reply(f, kind, nullptr, old_tag);
+    assert(f.mesh._delayed_replies.request.phase == Tracker::AwaitRadio);
+    reply(f, kind, nullptr, tag); f.drain();
+    assertSentBeforeFinal(Bytes(f.wire().begin() + request_start, f.wire().end()),
+                          kind, tag); ++checks;
   }
-  for (Kind kind : {Tracker::Status, Tracker::Binary, Tracker::Telemetry}) {
-    Fixture f; f.command(kind); const uint32_t tag = f.mesh._delayed_replies.request.tag;
-    reply(f, kind, nullptr, tag); f.drain(); f.mesh.rtc.resetUniqueTime(tag); f.command(kind);
-    assert(f.mesh.transmissions == 1 && f.mesh.allocations == 1); ++checks;
+  {
+    Fixture f;
+    for (unsigned i = 0; i < Tracker::HISTORY_SIZE - 1; ++i) {
+      f.command(Tracker::Binary);
+      reply(f, Tracker::Binary, nullptr, f.mesh._delayed_replies.request.tag);
+      f.drain();
+    }
+    f.mesh.rtc.resetUniqueTime(1000); f.command(Tracker::Binary);
+    assert(f.mesh.transmissions == Tracker::HISTORY_SIZE);
+    assert(f.mesh._delayed_replies.request.tag == 1000 + Tracker::HISTORY_SIZE - 1);
+    assert(f.mesh.rtc.wall == 1000); ++checks;
+  }
+  {
+    struct RejectAllTags : MyMesh {
+      unsigned attempts = 0;
+      bool allowRequestTag(uint32_t) override { ++attempts; return false; }
+    } mesh;
+    uint32_t tag = 0;
+    assert(!mesh.allocateRequestTag(tag));
+    assert(mesh.attempts == Tracker::HISTORY_SIZE);
+    assert(mesh.allocations == 0 && mesh.transmissions == 0); ++checks;
   }
   {
     Fixture f;

test/fixtures/contact_cache/test_contact_cache.cpp

@@ -193,6 +193,7 @@ public:
   int sendLogin(const ContactInfo&, const char*, uint32_t&);
   int sendAnonReq(const ContactInfo&, const uint8_t*, uint8_t, uint32_t&, uint32_t&);
   virtual bool allowRequestTag(uint32_t) { return true; }
+  virtual bool allocateRequestTag(uint32_t&);
   int sendRequest(const ContactInfo&, const uint8_t*, uint8_t, uint32_t&, uint32_t&);
   int sendRequest(const ContactInfo&, uint8_t, uint32_t&, uint32_t&);
 };

test/test_client_acl_response.py

@@ -101,7 +101,7 @@ class ClientAclResponseTest(unittest.TestCase):
     def test_same_clock_generates_login_and_binary_request_tags(self):
         source = (ROOT / "src/helpers/BaseChatMesh.cpp").read_text()
         for signature in ("int BaseChatMesh::sendLogin(",
-                          "int  BaseChatMesh::sendRequest(const ContactInfo& recipient, const uint8_t*"):
+                          "bool BaseChatMesh::allocateRequestTag("):
             body = extract_braced(source, signature)
             self.assertEqual(body.count("getRTCClock()->getCurrentTimeUnique()"), 1)
 

test/test_companion_delayed_reply_delivery.py

@@ -25,7 +25,8 @@ def production_methods(source, base, packet, dispatcher):
         "void MyMesh::onContactResponse(", "bool MyMesh::onContactPathRecv(",
         "void MyMesh::onTraceRecv(", "void MyMesh::clearPendingReqs(",
         "bool MyMesh::hasPendingReqs(", "bool MyMesh::beginPendingRequest(",
-        "bool MyMesh::allowRequestTag(", "void MyMesh::armPendingRequest(",
+        "bool MyMesh::allowRequestTag(", "bool MyMesh::allocateRequestTag(",
+        "void MyMesh::armPendingRequest(",
         "void MyMesh::finishPendingRequest(",
         "void MyMesh::abandonPendingRequest(", "void MyMesh::servicePendingSerialReply(",
         "void MyMesh::servicePendingSerialReply(uint32_t now)",
@@ -47,6 +48,7 @@ def production_methods(source, base, packet, dispatcher):
     methods += "\n#if COMPANION_FEATURE_TEXT_TERMINAL\n" + terminal + "\n#endif\n"
     methods += "\n" + "\n".join(extract_braced(base, signature) for signature in (
         "bool BaseChatMesh::onContactPathRecv(", "int BaseChatMesh::sendLogin(",
+        "bool BaseChatMesh::allocateRequestTag(",
         "int BaseChatMesh::sendAnonReq(",
         "int  BaseChatMesh::sendRequest(const ContactInfo& recipient, const uint8_t*",
         "int  BaseChatMesh::sendRequest(const ContactInfo& recipient, uint8_t",

test/test_contact_cache.py

@@ -108,6 +108,7 @@ class ContactCacheTest(unittest.TestCase):
                 "namespace mesh {\n" + method(mesh, "bool Mesh::sendDirect(") + "\n}\n"
                 + "\n".join(method(chat, signature) for signature in (
                     "int BaseChatMesh::sendLogin(", "int BaseChatMesh::sendAnonReq(",
+                    "bool BaseChatMesh::allocateRequestTag(",
                     "int  BaseChatMesh::sendRequest(const ContactInfo& recipient, const uint8_t*",
                     "int  BaseChatMesh::sendRequest(const ContactInfo& recipient, uint8_t"))
             )

test/test_official_app_compatibility.py

@@ -140,12 +140,22 @@ int main(int argc, char** argv) {
   const Bytes command = fromHex(argv[1]); assert(command.size() == 40);
   Fixture f; f.mesh.direct_timeout = 2000;
   memcpy(f.mesh.recipient.id.pub_key, command.data() + 1, 32);
+  // Access Control is opened after login. A synchronized repeater can return
+  // the timestamp the Companion would otherwise choose for this app query.
+  f.command(Tracker::Login);
+  const uint32_t server_tag = f.mesh.rtc.wall + 1;
+  reply(f, Tracker::Login, nullptr, server_tag); f.drain();
+  assert(count(f.wire(), PUSH_CODE_LOGIN_SUCCESS) == 1);
+  f.usb_stream.output.clear();
   memcpy(f.mesh.cmd_frame, command.data(), command.size());
   f.mesh.handleRequestFrame(command.size());
   const auto& payload = f.mesh.transmitted_payload;
   assert(payload.size() == 11 && !memcmp(payload.data() + 4, command.data() + 33, 7));
   uint32_t tag; memcpy(&tag, payload.data(), 4);
   assert(tag == slot(f, Tracker::Binary).tag && !slot(f, Tracker::Binary).sent_pending);
+  assert(tag > server_tag && f.mesh.rtc.wall == 1000);
+  reply(f, Tracker::Login, nullptr, server_tag);
+  assert(slot(f, Tracker::Binary).phase == Tracker::AwaitRadio);
   if (argc == 2) { printHex("REQUEST", payload); return 0; }
   Bytes radio = fromHex(argv[2]);
   if (atoi(argv[3]) == 0) {