Keep Discover rows stable while query and publication status changes

docs/geocaching-client-runtime.md

@@ -179,6 +179,13 @@ a catalogue gives it a new lifetime token, so stale selections from a closed
 session cannot become valid when a new catalogue starts at generation one.
 Status messages and action buttons update independently of row rendering.
 
+Discover rows use the committed query page's content generation. Query phase,
+failure text, detail responses and unrelated publication progress do not change
+that version. A separate source-lifetime token and saved-membership generation
+invalidate selections when the browser session is replaced or Downloaded
+membership changes. Pagination retains the existing rows while requesting the
+next page; committing the replacement page invalidates the previous selection.
+
 Background publication recovery filters completed, cancelled and no-intent
 requests before opening their parent tasks. Exact cache lookups still read
 confirmed receipts, and eligible unfinished requests retain task/reference

modules/core_geocaching/tests/run_browse_runtime_asan.sh

@@ -35,3 +35,4 @@ g++ -std=c++17 -g -O1 -fsanitize=address -fno-omit-frame-pointer -DTRAIL_MATE_RE
     -o "$build_dir/browse_runtime"
 ASAN_OPTIONS=detect_leaks=1:halt_on_error=1 "$build_dir/browse_runtime" modules/core_geocaching/tests/fixtures
 ASAN_OPTIONS=detect_leaks=1:halt_on_error=1 "$build_dir/browse_runtime" modules/core_geocaching/tests/fixtures save-close-detail
+ASAN_OPTIONS=detect_leaks=1:halt_on_error=1 "$build_dir/browse_runtime" modules/core_geocaching/tests/fixtures legacy-index-upgrade

modules/core_geocaching/tests/test_browse_runtime.cpp

@@ -537,6 +537,9 @@ int main(int argc, char** argv)
           "online detail not displayed");
     require(detailText(item).description == expected_record.description && detailText(item).hint == expected_record.hint,
             "online detail omitted full description or hint");
+    ui::geocaching::Item discover_probe;
+    require(snapshot(Section::Discover).generation == generation && test::source->item(Section::Discover, 0, generation, discover_probe),
+            "detail response invalidated unchanged discovery rows");
     require(test::files.empty() && !test::blocked_io, "online detail required SD");
     require(router.delivery({{reply_remote.data(), 16}, {router.local.data(), 16}, {}, {last_reply.data(), last_reply.size()}}, router.context),
             "verified detail duplicate was not acknowledged");
@@ -601,6 +604,8 @@ int main(int argc, char** argv)
         tick();
     }
     require(router.sends == 4, "saving verified detail sent a redundant network request");
+    require(snapshot(Section::Discover).generation != generation && !test::source->item(Section::Discover, 0, generation, discover_probe),
+            "downloaded membership retained stale discovery rows");
     const auto released_response = test::allocations.find(open_response);
     require(released_response == test::allocations.end() || released_response->second.owner != "geocaching.rx",
             "saved detail retained its response buffer");
@@ -698,6 +703,7 @@ int main(int argc, char** argv)
               return view.count == 1 && test::source->item(Section::Published, 0, view.generation, item) && item.publication_confirmed && item.publication_revision == 1; },
           "publication receipt not reflected in draft list");
     require(router.identity.signs == 1, "publication signed more than once");
+    const auto discovery_before_edit = snapshot(Section::Discover).generation;
     // Exercise the UI field limits and the confirmed-version edit path through
     // the same device owner, not a direct store call.
     const std::string long_name(96, 'N'), long_description(2048, 'D'), long_hint(512, 'H');
@@ -727,6 +733,9 @@ int main(int argc, char** argv)
               return test::source->item(Section::Published, 0, view.generation, item) && item.publication_confirmed && item.publication_revision == 2; },
           "second publication receipt not reflected in list");
     require(router.identity.signs == 2, "version update signed more than once");
+    require(snapshot(Section::Discover).generation == discovery_before_edit &&
+                test::source->item(Section::Discover, 0, discovery_before_edit, discover_probe),
+            "unrelated publication invalidated unchanged discovery rows");
     // Isolate archive conformance from the following restart fixture, which
     // intentionally retains an active revision-2 draft for its map assertions.
     const auto active_fixture = test::files;
@@ -1419,8 +1428,11 @@ int main(int argc, char** argv)
             if (found == test::files.end()) removed_staging = true;
             return removed_staging && snapshot(Section::Discover).has_more && found != test::files.end() && found->second.empty(); },
               "pagination test did not enter maintenance");
+        const auto rows_before_pagination = snapshot(Section::Discover).generation;
         require(test::source->loadMore(), "maintenance rejected pagination intent");
         require(!test::source->loadMore() && !snapshot(Section::Discover).has_more, "duplicate pagination was accepted");
+        require(snapshot(Section::Discover).generation == rows_before_pagination,
+                "pagination status invalidated the retained query page");
         require(!test::ui_io, "pagination callback touched storage");
         until([&]
               { return router.sends == paging_sends + 3; },
@@ -1431,6 +1443,9 @@ int main(int argc, char** argv)
               { const auto view = snapshot(Section::Discover); return view.can_refresh && !view.has_more && view.count == 0; },
               "final pagination response was not displayed");
         require(router.sends == paging_sends + 3, "pagination request sent twice");
+        require(snapshot(Section::Discover).generation != rows_before_pagination &&
+                    !test::source->item(Section::Discover, 0, rows_before_pagination, discover_probe),
+                "replacement query page retained a stale selection");
         closeRuntime();
         const auto journalCount = []
         {

modules/core_geocaching/tests/test_sd_journal.cpp

@@ -456,8 +456,11 @@ int checkBrowseFlow(const char* capabilities_path, const char* query_path)
     Destination wrong;
     wrong.bytes[0] = 1;
     if (port.accepted(wrong, id, {original.data(), original.size()})) return 127;
+    const auto retained_rows = snapshot.generation;
     source.refresh(::ui::geocaching::Section::Discover);
-    if (source.item(::ui::geocaching::Section::Discover, 0, snapshot.generation, item)) return 128;
+    source.snapshot(::ui::geocaching::Section::Discover, snapshot);
+    if (snapshot.generation != retained_rows || snapshot.can_refresh ||
+        !source.item(::ui::geocaching::Section::Discover, 0, retained_rows, item) || std::strcmp(item.name.data(), "Test")) return 128;
     if (!client->tick(10) || !advance() || client->phase() != QueryClientPhase::Querying) return 129;
     client->tick(10);
     if (!client->tick(10 + QueryClient::kReplyTimeoutMs) || client->phase() != QueryClientPhase::Cancelling) return 130;
@@ -480,7 +483,8 @@ int checkBrowseFlow(const char* capabilities_path, const char* query_path)
     storage::PendingRequestView pending;
     if (storage::nextPendingRequest(state.view(), {}, {}, pending) != storage::PendingRequestResult::None) return 133;
     source.snapshot(::ui::geocaching::Section::Discover, snapshot);
-    if (!snapshot.can_refresh || !std::strstr(snapshot.status.data(), "did not reply")) return 134;
+    if (!snapshot.can_refresh || !std::strstr(snapshot.status.data(), "did not reply") ||
+        !source.item(::ui::geocaching::Section::Discover, 0, snapshot.generation, item) || std::strcmp(item.name.data(), "Test")) return 134;
     fixture::files.clear();
     return 0;
 }

platform/esp/arduino_common/include/platform/esp/arduino_common/geocaching/query_browse_source.h

@@ -84,7 +84,7 @@ class QueryBrowseSource final : public ::ui::geocaching::Source
         QueryBrowseSource& source;
         bool locked;
     };
-    uint64_t generation() const { return (store_.generation() << 8) | static_cast<uint8_t>(client_.phase()) | (static_cast<uint8_t>(client_.failure()) << 4); }
+    uint64_t generation() const { return store_.generation(); }
     const char* status(::geocaching::QueryClientPhase phase) const
     {
         using Phase = ::geocaching::QueryClientPhase;

platform/esp/arduino_common/src/geocaching/browse_runtime.cpp

@@ -86,6 +86,7 @@ struct Session
 {
     uint64_t local_map_revision = 0;
     uint64_t saved_catalog_epoch = 0, draft_catalog_epoch = 0;
+    uint64_t discover_catalog_epoch = 0, discover_saved_epoch = 0, discover_saved_generation = 0;
     struct Publication
     {
         enum class DraftStage : uint8_t
@@ -326,6 +327,17 @@ std::atomic<uint32_t> next_step{0};
 std::atomic<uint32_t> replies_seen{0}, replies_busy{0}, replies_queued{0};
 uint32_t replies_processed = 0, replies_accepted = 0;
 uint64_t epoch = 0;
+uint64_t discoverScope(Session& s)
+{
+    const auto saved_generation = s.saved ? s.saved->generation() : 0;
+    if (s.discover_saved_epoch != s.saved_catalog_epoch || s.discover_saved_generation != saved_generation)
+    {
+        s.discover_saved_epoch = s.saved_catalog_epoch;
+        s.discover_saved_generation = saved_generation;
+        s.discover_catalog_epoch = ++epoch;
+    }
+    return s.discover_catalog_epoch;
+}
 void reportPublication(const char* event, const gc::PublishAttempt& attempt)
 {
     const auto& id = attempt.cacheId().bytes;
@@ -888,7 +900,7 @@ void advanceBrowse(Session& s)
             return;
         }
         s.client->query(kWorld);
-        ++epoch;
+        s.discover_catalog_epoch = ++epoch;
     }
     drainReply(s);
     if (auto* pending = pending_announcement.exchange(nullptr, std::memory_order_acq_rel))
@@ -1661,9 +1673,10 @@ class Facade final : public ::ui::geocaching::Source
             out.can_create = out.has_more = false;
         }
         // List ownership and its own content version determine row validity.
-        // Transport progress and other sections must not invalidate local rows.
+        // Transport and operation progress must not invalidate unchanged rows.
         const auto scope = section == ::ui::geocaching::Section::Published && session    ? session->draft_catalog_epoch
                            : section == ::ui::geocaching::Section::Downloaded && session ? session->saved_catalog_epoch
+                           : section == ::ui::geocaching::Section::Discover && session   ? discoverScope(*session)
                                                                                          : epoch;
         out.generation ^= scope << 32;
     }
@@ -1768,7 +1781,7 @@ class Facade final : public ::ui::geocaching::Source
         }
         if (guard.locked && section == ::ui::geocaching::Section::Downloaded && session && session->saved && !session->needsRecovery())
             return session->saved->item(index, generation ^ (session->saved_catalog_epoch << 32), out);
-        if (!guard.locked || !session || !session->source || !session->source->item(section, index, generation ^ (epoch << 32), out)) return false;
+        if (!guard.locked || !session || !session->source || !session->source->item(section, index, generation ^ (discoverScope(*session) << 32), out)) return false;
         out.downloaded = session->saved && session->saved->contains(out.id, out.revision_hash);
         out.can_download = !out.downloaded && !downloadActive() && !publicationActive() && !draftSaveActive() && (!session->store || !session->store->commitPending()) &&
                            !session->needsRecovery() && session->phase != Phase::Failed && storage::sd_card_ready() && !storage::sd_external_block_owner_active() &&
@@ -2091,7 +2104,7 @@ class Facade final : public ::ui::geocaching::Source
         if (session->saved && session->saved->contains(item.id, item.revision_hash)) return false;
         ::ui::geocaching::Snapshot snapshot;
         session->source->snapshot(::ui::geocaching::Section::Discover, snapshot);
-        if ((generation ^ (epoch << 32)) != snapshot.generation) return false;
+        if ((generation ^ (discoverScope(*session) << 32)) != snapshot.generation) return false;
         gc::protocol::SummaryView summary;
         bool found = false;
         for (size_t i = 0; i < snapshot.count; ++i)