Preserve verified cache details while saving and installing

modules/core_geocaching/tests/CMakeLists.txt

@@ -435,6 +435,7 @@ target_compile_features(geocaching_browse_runtime_test PRIVATE cxx_std_17)
 target_compile_definitions(geocaching_browse_runtime_test PRIVATE ESP_PLATFORM=1)
 target_link_libraries(geocaching_browse_runtime_test PRIVATE geocaching_native_hash)
 add_test(NAME geocaching_browse_runtime COMMAND geocaching_browse_runtime_test ${CMAKE_CURRENT_SOURCE_DIR}/fixtures)
+add_test(NAME geocaching_browse_runtime_close_detail COMMAND geocaching_browse_runtime_test ${CMAKE_CURRENT_SOURCE_DIR}/fixtures save-close-detail)
 add_test(NAME geocaching_draft_only_map COMMAND geocaching_browse_runtime_test ${CMAKE_CURRENT_SOURCE_DIR}/fixtures local-draft-only)
 
 add_executable(geocaching_local_map_test test_local_map_overlay.cpp)

modules/core_geocaching/tests/run_browse_runtime_asan.sh

@@ -34,3 +34,4 @@ g++ -std=c++17 -g -O1 -fsanitize=address -fno-omit-frame-pointer -DTRAIL_MATE_RE
     "$build_dir/ge.o" "$build_dir/sc.o" "$build_dir/sha512.o" \
     -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

modules/core_geocaching/tests/test_browse_runtime.cpp

@@ -251,7 +251,8 @@ DetailText detailText(const ui::geocaching::Item& item)
 }
 int main(int argc, char** argv)
 {
-    require(argc == 2 || argc == 3, "expected fixture directory and optional local-draft-only scenario");
+    require(argc == 2 || argc == 3, "expected fixture directory and optional runtime scenario");
+    const bool close_on_save = argc == 3 && std::strcmp(argv[2], "save-close-detail") == 0;
     const std::string folder(argv[1]);
     const auto encoded_record = fixture(folder, "record-v1.bin");
     geocaching::RecordView expected_record;
@@ -260,7 +261,7 @@ int main(int argc, char** argv)
     chat::MeshAdapterRouter router;
     LoraBoard board;
     rt::configure(router, board);
-    if (argc == 3)
+    if (argc == 3 && !close_on_save)
     {
         require(std::strcmp(argv[2], "local-draft-only") == 0, "unknown runtime scenario");
         router.ready = false;
@@ -468,13 +469,48 @@ int main(int argc, char** argv)
           "initial publication projection did not load");
     test::source->requestWindow(Section::Discover, 0, 1);
     generation = snapshot(Section::Discover).generation;
+    void* open_response = nullptr;
+    for (const auto& allocation : test::allocations)
+        if (allocation.second.owner == "geocaching.rx") open_response = allocation.first;
+    require(open_response != nullptr, "open detail response missing before save");
+    test::memory_available = false;
+    require(!test::source->download(item, generation), "save ignored unavailable PSRAM");
+    test::memory_available = true;
+    require(detailText(item).status == ui::geocaching::DetailStatus::Ready,
+            "failed save invalidated the open verified detail");
     require(test::source->download(item, generation), "download not queued");
-    test::source->closeDetail();
+    require(test::allocated("geocaching.download.detail"), "verified download did not own a response copy");
+    if (close_on_save)
+    {
+        // Release the original response before the installation's first tick.
+        // Every verified view must belong to the queued download's copy.
+        test::source->closeDetail();
+        until([&]
+              {
+                  const auto allocation = test::allocations.find(open_response);
+                  return allocation == test::allocations.end() || allocation->second.owner != "geocaching.rx"; },
+              "closing pending download retained the detail response");
+    }
     until([&]
-          { return std::strstr(snapshot(Section::Discover).status.data(), "Shared caches"); },
+          {
+              if (!close_on_save)
+                  require(detailText(item).status == ui::geocaching::DetailStatus::Ready,
+                          "saving invalidated the open verified detail");
+              return std::strstr(snapshot(Section::Discover).status.data(), "Shared caches"); },
           "download did not finish");
+    if (!close_on_save)
+    {
+        require(test::allocations.count(open_response), "saving released the open detail response");
+        require(detailText(item).description == expected_record.description && detailText(item).hint == expected_record.hint,
+                "saved open detail lost verified text");
+        require(!test::source->archiveCache(item.id, item.revision_hash), "saved foreign detail became archivable");
+        test::source->closeDetail();
+        tick();
+    }
     require(router.sends == 4, "saving verified detail sent a redundant network request");
-    require(!test::allocated("geocaching.detail.rx"), "saved detail retained its response buffer");
+    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");
     // A complete download writes requests, attempts, objects and installed
     // heads. None changes the publication projection we already loaded.
     require(std::strstr(snapshot(Section::Published).status.data(), "No local drafts"),
@@ -486,6 +522,22 @@ int main(int argc, char** argv)
               return view.count == 1 && test::source->item(Section::Downloaded, 0, view.generation, item) && item.downloaded; },
           "installed GPX missing from directory");
     const auto saved_id = item.id;
+    if (close_on_save)
+    {
+        test::source->open(item, snapshot(Section::Downloaded).generation);
+        until([&]
+              { return detailText(item).status == ui::geocaching::DetailStatus::Ready; },
+              "closed detail download lost installed details");
+        require(detailText(item).description == expected_record.description && detailText(item).hint == expected_record.hint,
+                "closed detail download corrupted installed text");
+        require(router.sends == 4, "installed detail required another network request");
+        test::source->closeDetail();
+        test::source->activate(false);
+        until([&]
+              { return test::allocations.empty(); },
+              "closed detail installation leaked PSRAM");
+        return 0;
+    }
     ui::geocaching::DraftInput draft;
     draft.name = "Published through device runtime";
     draft.description = "A local draft with a durable signed revision";

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

@@ -2085,13 +2085,26 @@ class Facade final : public ::ui::geocaching::Source
         if (!session->response && detail && detail->state == CacheDetail::State::Ready && detail->response &&
             detail->matches(item.id, item.revision_hash) && detail->remote.bytes == remote.bytes)
         {
-            job->verified.reset(::platform::memory::createPsram<gc::protocol::VerifiedRecordView>(detail->verified_record));
+            job->verified.reset(::platform::memory::createPsram<gc::protocol::VerifiedRecordView>());
             if (!job->verified) return false;
             job->request = detail->request;
-            job->response = detail->response;
             job->response_size = detail->response_size;
-            detail->response = nullptr;
-            detail->response_size = 0;
+            job->response = static_cast<uint8_t*>(mem::allocatePreferred("geocaching.download.detail", job->response_size, false));
+            if (!job->response) return false;
+            std::memcpy(job->response, detail->response, job->response_size);
+            // Both owners may outlive the other. Reparse the immutable copy so
+            // every borrowed field belongs to the download's response, while
+            // retaining the already verified identity/hash without more crypto.
+            gc::protocol::GetResponseView parsed;
+            if (!gc::protocol::decodeGetResponse({job->response, job->response_size}, job->request, 8192, parsed) || parsed.has_conflict) return false;
+            gc::protocol::CmpReader reader(parsed.signed_cache);
+            size_t count = 0;
+            gc::ByteView encoded;
+            if (!reader.array(count, 2) || count != 2 || !reader.binary(encoded, gc::kMaxRecordBytes) ||
+                !reader.binary(job->verified->signature, 64) || job->verified->signature.size != 64 || !reader.finished() ||
+                !gc::protocol::decodeGeocacheRecord(encoded, job->verified->record)) return false;
+            job->verified->id = detail->verified_record.id;
+            job->verified->hash = detail->verified_record.hash;
         }
         session->storage_requested = true;
         session->download_start = std::move(job);
@@ -2353,10 +2366,11 @@ void step()
         if (s.local_read_only && (draftSaveActive() || downloadActive() || publicationActive() ||
                                   s.needsRecovery() || (s.saved && s.saved->error()) || (s.draft_catalog && s.draft_catalog->failed)))
         {
-            // The audit and local readers share a frame lease. Release every
-            // reader first; queued edits own their bytes independently.
+            // The audit and local readers share a frame lease. Ready details
+            // own their response independently; release only their reader.
             s.workspace_owner.release(s.detail.get());
-            s.detail.reset();
+            if (s.detail && s.detail->state == CacheDetail::State::Ready) s.detail->saved_read.reset();
+            else s.detail.reset();
             s.pending_detail.reset();
             if (s.store) s.store->releaseDraftRead();
             s.saved.reset();