From 87e072ade8a94bff1286dbbe9cf17fb216936dca Mon Sep 17 00:00:00 2001 From: rcmerci Date: Thu, 15 Oct 2026 15:20:08 +0800 Subject: [PATCH 1/8] Checkpoint native graph picker and approved deletion contract --- app/application.ml | 149 ++++++++++------- app/journal_view.ml | 6 +- app/journal_view.mli | 1 + .../2026-10-15-switch-graph-native-picker.md | 35 ++++ logseq_sync/spec/pure_reducer/core.mli | 6 +- test/application_view_test.ml | 153 +++++++++++++++++- 6 files changed, 285 insertions(+), 65 deletions(-) create mode 100644 docs/agent-guide/proposed/feature/2026-10-15-switch-graph-native-picker.md diff --git a/app/application.ml b/app/application.ml index 4956f1c..b8934d2 100644 --- a/app/application.ml +++ b/app/application.ml @@ -327,6 +327,7 @@ type state = ; graph_error : graph_error option ; sync_error : sync_error_notice option ; manager : Graph_service.snapshot option + ; graph_picker_open : bool ; graph_state : Logseq_db_worker.graph_state ; diagnostics : Graph_service.diagnostics option ; admission_refresh : Admission_refresh.t @@ -420,6 +421,7 @@ let initial_state = ; graph_error = None ; sync_error = None ; manager = None + ; graph_picker_open = false ; graph_state = { generation = -1; graph_id = None; phase = Graph_closed; error = None } ; diagnostics = None ; admission_refresh = Admission_refresh.closed @@ -755,8 +757,7 @@ let apply_manager_state state (manager_state : Graph_service.state) = match state.modal, snapshot.selected_graph with | (Status_sheet _ | Capture_sheet | Append_sheet), _ when graph_context_changed -> No_modal - | Cache_reset_confirmation confirmation, Some selected - when Logseq_db_types.Graph_types.Uuid.equal confirmation selected -> state.modal + | Cache_reset_confirmation _, _ when not owner_changed -> state.modal | ( ( No_modal | Capture_sheet | Append_sheet @@ -769,6 +770,7 @@ let apply_manager_state state (manager_state : Graph_service.state) = let state = { state with manager = Some snapshot + ; graph_picker_open = state.graph_picker_open && not owner_changed ; diagnostics = Some manager_state.diagnostics ; e2ee_password ; e2ee_submission_pending = @@ -1693,6 +1695,8 @@ type command = | Retry_capture_imports | Close_error_info | Switch_graph + | Close_graph_picker + | Request_graph_local_cache_reset of string | Sign_out | Submit_e2ee_password | Request_local_cache_reset @@ -1751,28 +1755,12 @@ module Presentation = struct |> fun content -> V.Body.Vertical.create [ content ] ;; - let list children = - V.Body.Vertical.create - [ V.Body.Vertical.fixed - (V.Native_list.vertical - ~key:(Ui.Key.string "graph-list") - ~style:Inset - [ V.Native_list.section - ~key:(Ui.Key.string "graphs") - ~separator:Hidden - (List.mapi - (fun index child -> - let key = - match V.For_testing.test_id child with - | Some id -> Ui.Key.string id - | None -> Ui.Key.int index - in - V.Native_list.row ~key ~separator:Hidden child) - children) - ]) - ] - |> V.Body.Private.to_widget - |> V.frame ~max_height:Fill + let list rows = + V.Native_list.vertical ~key:(Ui.Key.string "graph-list") ~style:Inset + [ V.Native_list.section ~key:(Ui.Key.string "graphs") ~separator:Hidden rows ] + |> V.with_test_id (Ui.Test_id.string "graph-picker-list") + |> fun list -> V.column ~spacing:0. [list] + |> V.frame ~max_width:Fill ~max_height:Fill |> V.Body.static ;; @@ -2850,21 +2838,23 @@ let diagnostics_page ~snapshot ~graph ~admission diagnostics dispatch = ;; module Cache_confirmation = struct - let local_cache ~token dispatch body = + let local_cache ~token ~graph_name dispatch body = let request = Option.map (fun token -> V.Confirmation.request ~token - ~title:"Delete local graph copy?" + ~title:"Delete local copy?" ~message: - "This deletes the local copy, including unsaved drafts and pending local \ - changes, then returns to graph selection. The remote graph and cached \ - encryption key are retained. Select a graph to open or download it." + (Printf.sprintf + "Remove “%s” from this device only.\n\nUnsynced changes, drafts, and \ + attachments on this device will be lost.\n\nYour cloud copy will be \ + kept. You can download it again." + graph_name) [ V.Confirmation.action ~key:"cancel" ~title:"Cancel" ~role:Cancel () ; V.Confirmation.action ~key:"delete" - ~title:"Delete local graph copy" + ~title:"Delete local copy" ~role:Destructive () ]) @@ -3289,28 +3279,54 @@ let manager_page state dispatch = ~message:"Refresh to check for graphs available to your account." ~actions:(V.empty ()) | graphs -> - Presentation.list - (List.map - (fun (graph : Graph_service.graph) -> - let graph_id = - Logseq_db_types.Graph_types.Uuid.to_string graph.graph_id - in - button - ~id:("graph-picker:" ^ graph_id) - ~command:(Select_graph graph_id) - graph.name - (if graph.encrypted then "lock.doc" else "folder")) - graphs) + let rows = List.map (fun (graph : Graph_service.graph) -> + let id = Logseq_db_types.Graph_types.Uuid.to_string graph.graph_id in + let current = snapshot.selected_graph = Some graph.graph_id in + let select = + V.Navigation_link.create ~key:(Ui.Key.string ("graph-picker:" ^ id)) + ~activation_id:id ~disclosure:false + ~on_activate:(bind_action dispatch (Select_graph id)) + ~label:(V.row ~spacing:12. + [ V.symbol ~name:(if graph.encrypted then "lock.doc" else "folder") () + ; V.column ~spacing:3. ~alignment:Leading + (V.text graph.name :: if current then [V.text "Current graph" |> V.opacity 0.65] else []) + |> V.frame ~max_width:Fill + ; (if current then V.symbol ~name:"checkmark" () else V.empty ()) ] + |> V.frame ~max_width:Fill ~min_height:44.) + () |> V.with_test_id (Ui.Test_id.string ("graph-picker:" ^ id)) + |> V.frame ~max_width:Fill in + let options = + V.buttons ~key:(Ui.Key.string ("graph-picker-options:" ^ id)) + ~actions:[V.buttons_menu_action ~label:("Options for " ^ graph.name) + ~icon:"ellipsis" + ~on_select:(Ui.Event.Handler.create (function + | Ui.Event.Payload.Int64 1L when not current -> + dispatch.send (Request_graph_local_cache_reset id) + | _ -> ())) + ([V.Menu.action ~id:1L ~title:"Delete local copy" ~icon:"trash" + ~role:Destructive ~enabled:(not current) + ~test_id:(Ui.Test_id.string ("graph-picker-delete:" ^ id)) ()] + @ if current then [V.Menu.action ~id:2L + ~title:"Switch to another graph before deleting." ~enabled:false ()] else [])] + () |> V.with_test_id (Ui.Test_id.string ("graph-picker-options:" ^ id)) in + V.Native_list.row ~key:(Ui.Key.string id) ~separator:Hidden + (V.row ~spacing:8. [select; options] |> V.frame ~max_width:Fill) + ) graphs in + Presentation.list rows |> V.Body.with_test_id (Ui.Test_id.string "graph-picker-list") in toolbar ~sign_out:snapshot.startup.authenticated - "Choose a graph" - [ V.Toolbar.item + "Switch graph" + ((if Option.is_some snapshot.selected_graph then + [V.Toolbar.item ~key:(Ui.Key.string "graph-picker-close") ~placement:Cancellation_action + (button ~id:"graph-picker-close" ~command:Close_graph_picker "Close" "xmark")] + else []) + @ [ V.Toolbar.item ~key:(Ui.Key.string "graph-picker-refresh") ~placement:Primary_action refresh - ] + ]) content in let unlock error = @@ -3422,6 +3438,7 @@ let manager_page state dispatch = | Interaction startup -> startup | _ -> Journal_startup.derive ~snapshot ~graph:state.graph_state in (match startup.phase with + | _ when state.graph_picker_open -> graph_picker snapshot | _ when state.e2ee_submission_pending -> unlock None | Awaiting_selection -> graph_picker snapshot | Awaiting_e2ee_password -> unlock None @@ -5775,7 +5792,12 @@ let start let graph_id = value in (match Logseq_db_types.Graph_types.Uuid.of_string graph_id with | Error _ -> Effect.ignore - | Ok graph_id -> send_manager (Graph_service.Select_graph graph_id)) + | Ok graph_id -> + Effect.bind (update (fun state -> { state with graph_picker_open = false })) + ~f:(fun () -> + if Option.bind snapshot.manager (fun value -> value.selected_graph) = Some graph_id + then Effect.ignore + else send_manager (Graph_service.Select_graph graph_id))) | Refresh_catalog -> send_manager Graph_service.Refresh_catalog | Begin_online_recovery -> send_manager Graph_service.Begin_online_recovery | Open_capture @@ -5911,19 +5933,19 @@ let start | Close_error_info -> update (fun state -> { state with modal = No_modal }) | Switch_graph -> cancel_copy (); - Effect.many - [ update (fun state -> - { state with - modal = No_modal - ; e2ee_submission_pending = false - ; e2ee_password = - Journal_capture.create - ~session_number:state.next_local_sequence - ~source:"" - ; next_local_sequence = Int64.succ state.next_local_sequence - }) - ; send_manager Graph_service.Return_to_graph_picker - ] + update (fun state -> { state with graph_picker_open = true; modal = No_modal }) + | Close_graph_picker -> + update (fun state -> { state with graph_picker_open = false; modal = No_modal }) + | Request_graph_local_cache_reset value -> + update (fun state -> + match state.manager, Logseq_db_types.Graph_types.Uuid.of_string value with + | Some manager, Ok graph_id + when manager.startup.authenticated + && manager.selected_graph <> Some graph_id + && List.exists (fun (graph : Graph_service.graph) -> graph.graph_id = graph_id) manager.catalog -> + { state with modal = Cache_reset_confirmation graph_id + ; confirmation_sequence = Int64.succ state.confirmation_sequence } + | _ -> state) | Sign_out -> cancel_copy (); sign_out_in_flight := true; @@ -6448,7 +6470,8 @@ let start || Option.is_some (operation_failure state.timeline_notice) in let equal_root_presentation left right = - if (not left.graph_ready) || not right.graph_ready + if left.graph_picker_open <> right.graph_picker_open then false + else if (not left.graph_ready) || not right.graph_ready then left == right else Journal_startup.Presentation.equal_presentation left.startup_presentation right.startup_presentation @@ -6686,6 +6709,7 @@ let start let root = region "root" ~equal:equal_root_presentation (fun state -> match Journal_startup.Presentation.screen state.startup_presentation with + | _ when state.graph_picker_open -> manager_page state dispatch | Waiting | Interaction _ | Account_failure -> manager_page state dispatch | Timeline -> timeline_page @@ -7019,6 +7043,11 @@ let start && left.confirmation_sequence = right.confirmation_sequence) (fun current -> Cache_confirmation.local_cache + ~graph_name:(match current.modal, current.manager with + | Cache_reset_confirmation id, Some manager -> + (match List.find_opt (fun (graph : Graph_service.graph) -> graph.graph_id = id) manager.catalog with + | Some graph -> graph.name | None -> "this graph") + | _ -> "this graph") ~token: (match current.modal with | Cache_reset_confirmation _ -> Some current.confirmation_sequence diff --git a/app/journal_view.ml b/app/journal_view.ml index a5e15ed..292e91e 100644 --- a/app/journal_view.ml +++ b/app/journal_view.ml @@ -1966,10 +1966,11 @@ module View = struct ; icon : string option ; enabled : bool ; role : Button_role.t + ; test_id : Test_id.t option } - let action ~id ~title ?icon ?(enabled = true) ?(role = Button_role.Normal) () = - { id; title; icon; enabled; role } + let action ~id ~title ?icon ?(enabled = true) ?(role = Button_role.Normal) ?test_id () = + { id; title; icon; enabled; role; test_id } ;; let to_elements ~on_select entries = @@ -1977,6 +1978,7 @@ module View = struct (fun entry -> Lui_elements.menu_item ~key:(Int64.to_string entry.id) + ?accessibility_identifier:entry.test_id ~text:(if String.length entry.title = 0 then " " else entry.title) ?icon:(Option.map journal_icon entry.icon) ~disabled:(not entry.enabled) diff --git a/app/journal_view.mli b/app/journal_view.mli index cfd09f6..bbedc59 100644 --- a/app/journal_view.mli +++ b/app/journal_view.mli @@ -782,6 +782,7 @@ module View : sig -> ?icon:string -> ?enabled:bool -> ?role:Button_role.t + -> ?test_id:Test_id.t -> unit -> entry end diff --git a/docs/agent-guide/proposed/feature/2026-10-15-switch-graph-native-picker.md b/docs/agent-guide/proposed/feature/2026-10-15-switch-graph-native-picker.md new file mode 100644 index 0000000..320000b --- /dev/null +++ b/docs/agent-guide/proposed/feature/2026-10-15-switch-graph-native-picker.md @@ -0,0 +1,35 @@ +# Switch Graph Native Picker + +## Problem + +The graph picker uses uneven capsule buttons and loses the active graph when opened. It has no per-row local-copy management or durable presence observation. + +## Proposal + +Implement the user's selected concept A with native equal-width list rows, a soft header, close/refresh/existing account actions, an inline current marker and a separate row menu. Keep picker and request ownership in OCaml. Query one graph's mirror presence on demand with an account-generation fence; cache observations outside rendering. + +The user explicitly permits discarding unsynchronized local content after confirmation (Sentinel_3c2ff0ee96f88191ab349fca7f7befe9). The confirmation must explain this loss, device-only deletion and the retained cloud copy. The active graph requires switching first. Deletion-owner semantics remain gated on separate approval for the existing protected Core contract comment; this document does not grant that approval. + +## Alternatives considered + +### Reuse the existing graph-picker transition unchanged + +It detaches the current graph and clears its identity. Keep an application-owned picker presentation instead, and use the existing Select_graph lifecycle only for an actual switch. + +## Acceptance criteria + +- Uniform native rows preserve catalog order, expose the current marker and support long names and Dynamic Type. +- Close returns to the retained current graph; name activation switches through the worker. +- Row menus are independently reachable; current graph deletion is disabled with an explanation. +- Presence is bounded and requested only for the relevant graph, with stale and cancelled results ignored. +- Confirmation names the graph, warns that unsynchronized changes/drafts/attachments will be lost and says the cloud copy remains downloadable. +- Isolated synthetic Simulator acceptance covers cancellation, deletion, failure recovery and redownload before PR publication. + +## Risks + +- Presence can be unknown or fail; expose a clear retry/status rather than assuming a downloaded copy. +- The protected deletion contract approval is separate from this UI/presence implementation. + +## Questions + +- Design A, Simulator verification, discard semantics and PR publication are already answered by the user's recorded instructions. No unanswered UI/presence question blocks this scope. diff --git a/logseq_sync/spec/pure_reducer/core.mli b/logseq_sync/spec/pure_reducer/core.mli index 3295ef9..0a5d8ee 100644 --- a/logseq_sync/spec/pure_reducer/core.mli +++ b/logseq_sync/spec/pure_reducer/core.mli @@ -37,8 +37,10 @@ type startup_facts = ; presentation_generation : presentation_generation } -(** Non-resumable cleanup of the normally open selected graph. Success removes - this status only after Save_catalog acknowledges the cleared selection. *) +(** Cleanup of an authorized, non-current graph's local copy. The host must + obtain explicit confirmation that unsynchronized content will be discarded. + The selected graph and catalog entry are retained. Completion or failure + releases the deletion flight so selection and retry remain available. *) type local_deletion_stage = | Closing_graph | Deleting_mirror diff --git a/test/application_view_test.ml b/test/application_view_test.ml index fbe5630..5470cec 100644 --- a/test/application_view_test.ml +++ b/test/application_view_test.ml @@ -4889,6 +4889,151 @@ let test_application_token_control ~saturated ~failure () = !token_error) ;; +(* Picker presentation and native controls are owned by Application; Root_navigation + has no public picker event. Exercise its mounted public hooks and worker lane. *) +let test_graph_picker_controls scenario () = + let module W = Logseq_db_worker_lui.Journal_worker in + let module P = Logseq_db_worker.Protocol in + let uuid n = Logseq_db_types.Graph_types.Uuid.of_string + (Printf.sprintf "94000000-0000-4000-8000-%012d" n) |> Result.get_ok in + let first = uuid 1 and second = uuid 2 in + let long_name = "Synthetic very long graph name — 工作笔记 and attachments across multiple lines" in + let graph graph_id name : Service.graph = + { graph_id; name; schema = { major = 65; minor = 33; exact = true }; encrypted = false } in + let manager : Service.state = + { snapshot = + { sync_phase = Current; catalog = [graph first "Current synthetic graph"; graph second long_name] + ; selected_graph = Some first; applied_server_t = Some (server_cursor_of_int 0) + ; timeline_presentation_pending = false + ; startup = { authenticated = true; catalog_loading = false; awaiting_selection = false + ; restoring_local = false; bootstrapping = false; awaiting_e2ee_password = false + ; failure = None; account_generation = 1; graph_generation = 1; presentation_generation = 1 } + ; last_error = None; local_deletion = None } + ; diagnostics = { groups = [] } } in + let current = Atomic.make manager and commands = Atomic.make [] in + let client = ref None in + let service = W.Service.create ~push_topic_count:6 ~concurrency:Serial + ~init:(fun context _ -> + W.Session_context.emit context ~topic:Service.manager_topic (Service.Client_state_changed manager); + Ok ()) + ~handle:(fun context () request -> + let publish value = + Atomic.set current value; + W.Request_context.emit context ~topic:Service.manager_topic (Service.Client_state_changed value) in + match request with + | Service.Get_graph_state -> + let value = Atomic.get current in + Ok (Service.Graph_state { generation = value.snapshot.startup.graph_generation + ; graph_id = value.snapshot.selected_graph; phase = Graph_open; error = None }) + | Client_command command -> + Atomic.set commands (command :: Atomic.get commands); + let previous = Atomic.get current in + (match command with + | Return_to_graph_picker -> + publish { previous with snapshot = { previous.snapshot with selected_graph = None + ; startup = { previous.snapshot.startup with awaiting_selection = true } } } + | Select_graph id -> + let generation = previous.snapshot.startup.graph_generation + 1 in + publish { previous with snapshot = { previous.snapshot with selected_graph = Some id + ; startup = { previous.snapshot.startup with awaiting_selection = false; graph_generation = generation } } }; + W.Request_context.emit context ~topic:Service.graph_state_topic + (Service.Graph_state_changed { generation; graph_id = Some id; phase = Graph_open; error = None }) + | _ -> ()); + Ok Service.Client_command_completed + | Asset_command _ | Release_asset_file _ -> Ok Service.Client_command_completed + | Acquire_asset_file _ | Acquire_imported_file _ -> Ok (Service.Asset_file None) + | Graph_request request -> + let outcome = match request.P.command with + | V2_graph_info -> P.V2_graph_info_outcome + { graph_uuid = Option.get (Atomic.get current).snapshot.selected_graph + ; graph_name = "Synthetic"; schema = { major = 65; minor = 33 } + ; admission_facts = []; journal_title_format = None + ; limits = { response_budget_bytes = P.maximum_response_bytes + ; outbox_max_records = 4096; outbox_max_bytes = 8388608 + ; change_max_items = 4096; change_max_bytes = 1048576 + ; dispatcher_capacity = 256; wire_batch_max_bytes = 262144 } + ; generation = "g"; projection_revision = "p" } + | V2_list_journals _ -> P.V2_journals_outcome { items = []; next_cursor = None } + | V2_get_asset_descriptors _ | V2_list_assets _ -> + P.V2_assets_outcome { generation = "g"; projection_revision = "p"; items = []; next_cursor = None } + | _ -> P.V2_failed { code = "UnusedFixtureRead"; message = "Unused graph-picker fixture read" } in + Ok (Service.Graph_response (P.V2_response + { api_version = P.api_version; request_id = request.request_id; outcome })) + | Import_asset _ -> Error "Unused graph-picker fixture import") + ~shutdown:(fun () -> ()) () in + let hooks = Application.For_testing.app_with_service ~on_client:(fun value -> client := Some value) service in + let props = Hashtbl.create 128 in + let teardown = track_json_wire_teardown (Hashtbl.remove props) in + let consume source = + if source <> "" then + let open Yojson.Safe.Util in + Yojson.Safe.from_string source |> member "ops" |> to_list |> List.iter (fun op -> + teardown op; + match member "op" op |> to_string with + | "create-node" | "create-extension" -> Hashtbl.replace props (member "id" op |> to_int) [] + | "set-prop" | "set-extension-prop" -> + let id = member "id" op |> to_int in + let key = member "property" op |> to_string in + let old = Option.value (Hashtbl.find_opt props id) ~default:[] in + Hashtbl.replace props id ((key, member "value" op) :: List.remove_assoc key old) + | _ -> ()) in + let find key value = Hashtbl.fold (fun id values found -> + if List.assoc_opt key values = Some (`String value) then Some id else found) props None in + let node name = match find "accessibility-identifier" name with + | Some id -> id | None -> failwith ("Missing " ^ name) in + let press id = consume (hooks.dispatch (Lui_protocol.Press id)) in + let wait label predicate = + let deadline = Unix.gettimeofday () +. 3. in + while not (predicate ()) && Unix.gettimeofday () < deadline do + consume (hooks.pump ()); Unix.sleepf 0.001 + done; + Alcotest.(check bool) label true (predicate ()) in + let config = Logseq_db_worker.Config.create + ~application_support_directory:"/tmp/journal-graph-picker-synthetic" + ~target:(Managed_sync { base_url = "https://example.invalid" }) + ~compatibility_profile:Logseq_65_33_or_newer + ~response_budget_bytes:P.maximum_response_bytes ~default_page_size:P.default_page_size + |> Result.get_ok |> Journal_startup.encode |> Result.get_ok |> Bytes.to_string in + Fun.protect ~finally:(fun () -> + ignore (hooks.dispose ()); Option.iter Logseq_db_worker_lui.Journal_worker_runtime.stop !client) + (fun () -> + consume (hooks.init 2 2 config); + wait "Switch graph action mounts" (fun () -> Option.is_some (find "text" "Switch graph")); + press (Option.get (find "text" "Switch graph")); + wait "graph list mounts" (fun () -> Option.is_some (find "accessibility-identifier" "graph-picker-list")); + match scenario with + | `Current -> + Alcotest.(check bool) "current marker stays in catalog row" true + (Option.is_some (find "text" "Current graph")); + Alcotest.(check bool) "long name is preserved" true (Option.is_some (find "text" long_name)); + Alcotest.(check bool) "opening picker keeps selected graph" true + ((Atomic.get current).snapshot.selected_graph = Some first) + | `Close -> + press (node "graph-picker-close"); + wait "close returns to retained graph" (fun () -> + Option.is_none (find "accessibility-identifier" "graph-picker-list")); + Alcotest.(check bool) "picker never detaches current graph" false + (List.exists (function Service.Return_to_graph_picker -> true | _ -> false) (Atomic.get commands)) + | `Select -> + press (node ("graph-picker:" ^ Logseq_db_types.Graph_types.Uuid.to_string second)); + wait "name selection reaches worker" (fun () -> + List.exists (function Service.Select_graph id -> id = second | _ -> false) (Atomic.get commands)) + | `Delete_menu -> + ignore (node ("graph-picker-options:" ^ Logseq_db_types.Graph_types.Uuid.to_string second)); + wait "row action is exposed" (fun () -> Option.is_some (find "text" "Delete local copy")); + press (node ("graph-picker-delete:" ^ Logseq_db_types.Graph_types.Uuid.to_string second)); + wait "confirm names data loss" (fun () -> + Hashtbl.fold (fun _ values found -> found || List.exists (fun (_, value) -> + match value with `String text -> String.starts_with ~prefix:"Remove “" text + && String.contains text '\n' | _ -> false) values) props false); + Alcotest.(check bool) "requesting confirmation does not delete" false + (List.exists (function Service.Delete_local_cache _ -> true | _ -> false) (Atomic.get commands)) + | `Current_menu -> + ignore (node ("graph-picker-options:" ^ Logseq_db_types.Graph_types.Uuid.to_string first)); + Alcotest.(check bool) "current protection explains switching first" true + (Option.is_some (find "text" "Switch to another graph before deleting."))) +;; + let () = if Sys.getenv_opt "JOURNAL_UNLOCK_SYNTHETIC_HOST" = Some "1" then ( @@ -4896,7 +5041,13 @@ let () = Journal_bridge.register hooks) else Alcotest.run "application view" - [ ( "relative date chrome", [ + [ ( "graph picker", [ + Alcotest.test_case "current marker and long names" `Quick (test_graph_picker_controls `Current); + Alcotest.test_case "close retains current session" `Quick (test_graph_picker_controls `Close); + Alcotest.test_case "name switches graph" `Quick (test_graph_picker_controls `Select); + Alcotest.test_case "row deletion confirmation" `Quick (test_graph_picker_controls `Delete_menu); + Alcotest.test_case "current row deletion protection" `Quick (test_graph_picker_controls `Current_menu)]) + ; ( "relative date chrome", [ Alcotest.test_case "calendar boundaries and date updates" `Quick test_timeline_relative_date_headers; Alcotest.test_case "empty Detail navigation title" `Quick test_detail_empty_navigation_title; Alcotest.test_case "foreground day updates before delayed or failed feed" `Quick From 80df73fd1d6dca9d2926a57f3492f2c112504b6c Mon Sep 17 00:00:00 2001 From: rcmerci Date: Thu, 15 Oct 2026 17:14:02 +0800 Subject: [PATCH 2/8] Add scoped local-copy deletion and full-canvas graph list --- app/application.ml | 375 +++++++++++++----- app/journal_header.ml | 20 +- app/journal_header.mli | 7 + app/journal_startup.ml | 4 +- app/journal_view.ml | 29 +- app/journal_view.mli | 2 + .../2026-10-15-switch-graph-native-picker.md | 14 +- .../lib/effect_runner/effect_runner.ml | 162 ++++++-- logseq_db_worker/lib/logseq_db_worker.ml | 90 ++++- logseq_db_worker/lib/logseq_db_worker.mli | 10 + .../lui/logseq_db_worker_lui_service.ml | 49 +++ .../lui/logseq_db_worker_lui_service.mli | 11 + logseq_db_worker/test/test_lui_service.ml | 94 ++++- logseq_overlay_db/lib/database.ml | 63 +-- .../test/test_overlay_storage.ml | 17 +- .../lib/effect_runner/effect_runner.ml | 45 ++- logseq_sync/lib/pure_reducer/core.ml | 132 +++--- logseq_sync/test/core_contract.ml | 349 +++++----------- logseq_sync/test/runner_contract.ml | 19 + swift/JournalChrome.swift | 23 +- swift/JournalIcons.swift | 1 + test/application_view_test.ml | 100 ++++- test/journal_semantics_test.ml | 4 +- test/source_boundary_test.ml | 6 +- test/startup_test.ml | 26 +- 25 files changed, 1105 insertions(+), 547 deletions(-) diff --git a/app/application.ml b/app/application.ml index 9a2adf5..a5b1e00 100644 --- a/app/application.ml +++ b/app/application.ml @@ -258,6 +258,7 @@ type sync_error_notice = module Graph_drafts = Map.Make (String) module Reference_sources = Map.Make (String) +module Local_copies = Map.Make (String) type graph_drafts = { capture_draft : Journal_capture.t option @@ -328,6 +329,11 @@ type state = ; sync_error : sync_error_notice option ; manager : Graph_service.snapshot option ; graph_picker_open : bool + ; graph_catalog : Graph_service.graph Rrbvec.t + ; local_copies : (Graph_service.local_copy_presence, string) result Local_copies.t + ; local_copy_check : Graph_service.local_copy_request option + ; local_copy_deleting : Graph_service.local_copy_request option + ; local_copy_message : string option ; graph_state : Logseq_db_worker.graph_state ; diagnostics : Graph_service.diagnostics option ; admission_refresh : Admission_refresh.t @@ -422,6 +428,11 @@ let initial_state = ; sync_error = None ; manager = None ; graph_picker_open = false + ; graph_catalog = Rrbvec.empty + ; local_copies = Local_copies.empty + ; local_copy_check = None + ; local_copy_deleting = None + ; local_copy_message = None ; graph_state = { generation = -1; graph_id = None; phase = Graph_closed; error = None } ; diagnostics = None ; admission_refresh = Admission_refresh.closed @@ -704,6 +715,9 @@ let local_deletion_available state = let apply_manager_state state (manager_state : Graph_service.state) = let e2ee_submission_was_pending = state.e2ee_submission_pending in let snapshot = manager_state.snapshot in + let graph_catalog = match state.manager with + | Some previous when previous.catalog == snapshot.catalog -> state.graph_catalog + | _ -> Rrbvec.of_list snapshot.catalog in let owner_changed = match state.manager with | Some previous -> previous.startup.account_generation <> snapshot.startup.account_generation @@ -713,12 +727,17 @@ let apply_manager_state state (manager_state : Graph_service.state) = match state.manager with | Some previous when previous.startup.account_generation <> snapshot.startup.account_generation -> - clear_account_drafts state + { (clear_account_drafts state) with + local_copies = Local_copies.empty; local_copy_check = None + ; local_copy_deleting = None; local_copy_message = None } | None | Some _ -> state in let state = - if Option.is_some snapshot.local_deletion && not (local_deletion_active state) - then discard_local_graph_state state + if owner_changed then ( + let forget graph_id copies = Option.fold ~none:copies + ~some:(fun id -> Local_copies.remove (Logseq_db_types.Graph_types.Uuid.to_string id) copies) graph_id in + let copies = forget (Option.bind state.manager (fun manager -> manager.selected_graph)) state.local_copies in + { state with local_copies = forget snapshot.selected_graph copies }) else state in let previous_sync_error = @@ -770,6 +789,7 @@ let apply_manager_state state (manager_state : Graph_service.state) = let state = { state with manager = Some snapshot + ; graph_catalog ; graph_picker_open = state.graph_picker_open && not owner_changed ; diagnostics = Some manager_state.diagnostics ; e2ee_password @@ -1696,6 +1716,7 @@ type command = | Close_error_info | Switch_graph | Close_graph_picker + | Graph_picker_visible of int64 * int64 | Request_graph_local_cache_reset of string | Sign_out | Submit_e2ee_password @@ -1755,11 +1776,11 @@ module Presentation = struct |> fun content -> V.Body.Vertical.create [ content ] ;; - let list rows = - V.Native_list.vertical ~key:(Ui.Key.string "graph-list") ~style:Inset - [ V.Native_list.section ~key:(Ui.Key.string "graphs") ~separator:Hidden rows ] - |> V.with_test_id (Ui.Test_id.string "graph-picker-list") + let list ?header ?on_visible_range rows = + V.Native_list.vertical ~key:(Ui.Key.string "graph-list") ~style:Plain ?on_visible_range + [ V.Native_list.section ~key:(Ui.Key.string "graphs") ?header ~separator:Hidden rows ] |> fun list -> V.column ~spacing:0. [list] + |> V.with_test_id (Ui.Test_id.string "graph-picker-list") |> V.frame ~max_width:Fill ~max_height:Fill |> V.Body.static ;; @@ -3160,7 +3181,7 @@ let detail_page |> V.Body.with_test_id (Ui.Test_id.string "journal-detail-route") ;; -let manager_page state dispatch = +let manager_page ?picker_signal state dispatch = let button ?(role = V.Button_role.Normal) ?(enabled = true) ~id ~command title symbol = V.button ~key:(Ui.Key.string id) @@ -3191,8 +3212,7 @@ let manager_page state dispatch = "Diagnostics" "stethoscope" in - let toolbar ?(sign_out = false) title actions body = - let account_menu = + let account_menu ?(sign_out = false) () = V.buttons ~actions: [ V.buttons_menu_action @@ -3217,7 +3237,9 @@ let manager_page state dispatch = ] () |> V.with_test_id (Ui.Test_id.string "journal-startup-menu") - in + in + let toolbar ?(sign_out = false) title actions body = + let account_menu = account_menu ~sign_out () in body |> V.Body.toolbar ~items: @@ -3262,71 +3284,107 @@ let manager_page state dispatch = (if allow_graph_selection && can_choose_graph then choose_graph else V.empty ()) in let graph_picker snapshot = - let refresh = - button - ~id:"graph-picker-refresh" - ~command:Refresh_catalog - "Refresh graphs" - "arrow.clockwise" - |> V.help ~message:"Refresh the authorized graph catalog" + let reactive ?grow ~equal build = + match picker_signal with + | None -> build state + | Some signal -> V.of_lui + (Lui_elements.stack ?grow + [Lui_elements.dyn ~equal (fun current -> Ui.mount (build current)) signal]) + in + let icon_button ?(enabled = true) ?tooltip ~id ~command ~label icon = + V.of_lui (Lui_elements.button ~key:id ~accessibility_identifier:id + ~text:"" ~label ~icon ~size:`icon ~variant:`ghost ~disabled:(not enabled) ?tooltip + ~on_press:(fun _ -> dispatch.send command) []) + in + let refresh = reactive + ~equal:(fun left right -> left.local_copy_deleting = right.local_copy_deleting) + (fun owner -> icon_button ~enabled:(owner.local_copy_deleting = None) + ~id:"graph-picker-refresh" ~command:Refresh_catalog ~label:"Refresh graphs" + ~tooltip:"Refresh the authorized graph catalog" `refresh_cw) + in + let projected context project = Option.map (fun source -> + let signal = Signal.map project source in + Signal.on_dispose context.Lui_ui.ui_scope (fun () -> Signal.dispose_signal signal); + signal) picker_signal in + let status_text current = + match current.local_copy_deleting, current.local_copy_check, current.local_copy_message with + | Some _, _, _ -> "Deleting local copy…" + | _, Some _, _ -> "Checking local copy…" + | _, _, Some message -> message + | _ -> "" in + let status = V.of_lui (fun context parent -> + Lui_elements.text ~value:(status_text state) + ?value_signal:(projected context status_text) + ~foreground:"error-foreground" ~accessibility_identifier:"graph-picker-local-copy-status" [] context parent) in let content = match snapshot.Graph_service.catalog with - | [] -> - unavailable - ~title:"No graphs available" - ~symbol:"folder" - ~message:"Refresh to check for graphs available to your account." - ~actions:(V.empty ()) + | [] -> unavailable ~title:"No graphs available" ~symbol:"folder" + ~message:"Refresh to check for graphs available to your account." ~actions:(V.empty ()) | graphs -> let rows = List.map (fun (graph : Graph_service.graph) -> let id = Logseq_db_types.Graph_types.Uuid.to_string graph.graph_id in let current = snapshot.selected_graph = Some graph.graph_id in - let select = - V.Navigation_link.create ~key:(Ui.Key.string ("graph-picker:" ^ id)) - ~activation_id:id ~disclosure:false - ~on_activate:(bind_action dispatch (Select_graph id)) - ~label:(V.row ~spacing:12. - [ V.symbol ~name:(if graph.encrypted then "lock.doc" else "folder") () - ; V.column ~spacing:3. ~alignment:Leading - (V.text graph.name :: if current then [V.text "Current graph" |> V.opacity 0.65] else []) - |> V.frame ~max_width:Fill - ; (if current then V.symbol ~name:"checkmark" () else V.empty ()) ] - |> V.frame ~max_width:Fill ~min_height:44.) - () |> V.with_test_id (Ui.Test_id.string ("graph-picker:" ^ id)) - |> V.frame ~max_width:Fill in - let options = - V.buttons ~key:(Ui.Key.string ("graph-picker-options:" ^ id)) - ~actions:[V.buttons_menu_action ~label:("Options for " ^ graph.name) - ~icon:"ellipsis" - ~on_select:(Ui.Event.Handler.create (function - | Ui.Event.Payload.Int64 1L when not current -> - dispatch.send (Request_graph_local_cache_reset id) - | _ -> ())) - ([V.Menu.action ~id:1L ~title:"Delete local copy" ~icon:"trash" - ~role:Destructive ~enabled:(not current) - ~test_id:(Ui.Test_id.string ("graph-picker-delete:" ^ id)) ()] - @ if current then [V.Menu.action ~id:2L - ~title:"Switch to another graph before deleting." ~enabled:false ()] else [])] - () |> V.with_test_id (Ui.Test_id.string ("graph-picker-options:" ^ id)) in - V.Native_list.row ~key:(Ui.Key.string id) ~separator:Hidden - (V.row ~spacing:8. [select; options] |> V.frame ~max_width:Fill) + let presence owner = Local_copies.find_opt id owner.local_copies in + let subtitle owner = + if current then "Current graph" + else if Option.fold ~none:false ~some:(fun request -> request.Graph_service.graph_id = graph.graph_id) owner.local_copy_deleting + then "Deleting local copy…" + else match presence owner with + | Some (Ok Graph_service.Not_downloaded) -> "Not downloaded" + | Some (Error _) -> "Unable to check local copy" + | _ -> "" in + let selection_enabled owner = owner.local_copy_deleting = None in + let deletion_enabled owner = not current && owner.local_copy_check = None + && owner.local_copy_deleting = None && presence owner <> Some (Ok Graph_service.Not_downloaded) in + (* Native List cells retain their interactive children. Property signals + update status and enabled state without replacing callback owners. *) + let row = V.of_lui (fun context parent -> + let select = V.Navigation_link.create ~key:(Ui.Key.string ("graph-picker:" ^ id)) + ~activation_id:id ~disclosure:false ~enabled:(selection_enabled state) + ?enabled_signal:(projected context selection_enabled) + ~on_activate:(bind_action dispatch (Select_graph id)) + ~label:(V.row ~spacing:12. + [ V.symbol ~name:(if graph.encrypted then "lock.doc" else "folder") () + ; V.column ~spacing:3. ~alignment:Leading + [V.text graph.name; V.of_lui (fun context parent -> + Lui_elements.text ~value:(subtitle state) + ?value_signal:(projected context subtitle) + ~foreground:"secondary" [] context parent)] + |> V.frame ~max_width:Fill + ; (if current then V.symbol ~name:"checkmark.circle" () + else reactive ~equal:(fun left right -> presence left = presence right) + (fun owner -> if presence owner = Some (Ok Graph_service.Not_downloaded) + then V.symbol ~name:"icloud.and.arrow.down" () else V.column [])) ] + |> V.frame ~max_width:Fill ~min_height:44.) + () |> V.with_test_id (Ui.Test_id.string ("graph-picker:" ^ id)) + |> V.frame ~max_width:Fill in + let options = V.buttons ~key:(Ui.Key.string ("graph-picker-options:" ^ id)) + ~actions:[V.buttons_menu_action ~label:("Options for " ^ graph.name) ~icon:"ellipsis" + ~on_select:(Ui.Event.Handler.create (function + | Ui.Event.Payload.Int64 1L -> dispatch.send (Request_graph_local_cache_reset id) + | _ -> ())) + ([V.Menu.action ~id:1L ~title:"Delete local copy" ~icon:"trash" ~role:Destructive + ~enabled:(deletion_enabled state) ?enabled_signal:(projected context deletion_enabled) + ~test_id:(Ui.Test_id.string ("graph-picker-delete:" ^ id)) ()] + @ if current then [V.Menu.action ~id:2L + ~title:"Switch graphs first." ~enabled:false ()] else [])] + () |> V.with_test_id (Ui.Test_id.string ("graph-picker-options:" ^ id)) in + Ui.mount (V.row ~spacing:8. [select; options] |> V.frame ~max_width:Fill) context parent) + in + V.Native_list.row ~key:(Ui.Key.string id) ~separator:Hidden row ) graphs in - Presentation.list rows - |> V.Body.with_test_id (Ui.Test_id.string "graph-picker-list") + Presentation.list ~header:status + ~on_visible_range:(Ui.Event.Handler.create (function + | Ui.Event.Payload.Visible_range range -> + dispatch.send (Graph_picker_visible (range.first_index, range.last_exclusive)) + | _ -> ())) rows in - toolbar - ~sign_out:snapshot.startup.authenticated - "Switch graph" - ((if Option.is_some snapshot.selected_graph then - [V.Toolbar.item ~key:(Ui.Key.string "graph-picker-close") ~placement:Cancellation_action - (button ~id:"graph-picker-close" ~command:Close_graph_picker "Close" "xmark")] - else []) - @ [ V.Toolbar.item - ~key:(Ui.Key.string "graph-picker-refresh") - ~placement:Primary_action - refresh - ]) + Journal_header.graph_picker + ~leading:(V.column (if Option.is_some snapshot.selected_graph then + [icon_button ~id:"graph-picker-close" ~command:Close_graph_picker ~label:"Close" `x] + else [])) + ~controls:(V.row ~spacing:8. [refresh; account_menu ~sign_out:snapshot.startup.authenticated ()]) content in let unlock error = @@ -4041,6 +4099,57 @@ let start | Stopping -> unlock_log "phase=queue result=stopping"; reject_unlock owner ()) | _ -> ())) in + let local_copy_requests = Hashtbl.create 2 in + let visible_copy_queue = Queue.create () in + let drain_visible_copies = ref (fun () -> Effect.ignore) in + let cancel_local_copy_checks () = + Queue.clear visible_copy_queue; + let cancelled = Hashtbl.fold (fun id (_, deleting, _) ids -> if deleting then ids else id :: ids) + local_copy_requests [] in + List.iter (fun id -> Worker.cancel client ~request_id:id; Hashtbl.remove local_copy_requests id) cancelled + in + let local_copy_request ?(confirm = true) ~deleting (request : Graph_service.local_copy_request) = + Effect.of_thunk (fun () -> + let command = if deleting then Graph_service.Delete_local_copy request else Inspect_local_copy request in + match Worker.send client command with + | Worker.Accepted id -> Hashtbl.replace local_copy_requests id (request, deleting, confirm) + | Full | Not_ready | Stopping -> + set_state (fun state -> + if confirm then + { state with local_copy_check = None; local_copy_deleting = None + ; local_copy_message = Some "The local copy request could not start. Try again." } + else { state with local_copies = Local_copies.add + (Logseq_db_types.Graph_types.Uuid.to_string request.graph_id) + (Error "Unable to check the local copy. Try again.") state.local_copies }) ()) + in + drain_visible_copies := (fun () -> + Effect.of_thunk (fun () -> + let automatic_in_flight = Hashtbl.fold + (fun _ (_, deleting, confirm) active -> active || (not deleting && not confirm)) + local_copy_requests false in + if not automatic_in_flight && Hashtbl.length local_copy_requests < 2 then ( + let rec next () = match Queue.take_opt visible_copy_queue with + | None -> () + | Some (request : Graph_service.local_copy_request) -> + let state = !state_ref in + let key = Logseq_db_types.Graph_types.Uuid.to_string request.graph_id in + let valid = match state.manager with + | Some manager -> manager.startup.account_generation = request.account_generation + && (state.graph_picker_open || manager.startup.awaiting_selection) + && not (Local_copies.mem key state.local_copies) + | None -> false in + if valid then Effect.run (local_copy_request ~confirm:false ~deleting:false request) + else next () + in next ()))); + let discard_graph_copy_draft state graph_id = + let key = Logseq_db_types.Graph_types.Uuid.to_string graph_id in + Option.iter (fun draft -> + Option.iter (fun capture -> + List.iter Journal_asset_import.discard_staged_file (Journal_capture.pending_attachments capture)) draft.capture_draft; + List.iter Journal_asset_import.discard_staged_file (Journal_routes.retained_attachments draft.append_drafts)) + (Graph_drafts.find_opt key state.graph_drafts); + { state with graph_drafts = Graph_drafts.remove key state.graph_drafts } + in let submit request = Journal_graph_runtime.submit graph_runtime request in let fail_graph_transport state message = terminal_graph_state state (Transport_graph_error message) @@ -4292,15 +4401,11 @@ let start let termination_in_flight = ref false in let apply_manager_transition set_state set_state_and_effect manager_state = let manager = manager_state.Graph_service.snapshot in - if Option.is_some manager.local_deletion || not manager.startup.authenticated - then cancel_copy (); - if Option.is_some manager.local_deletion - then ( - Journal_graph_runtime.reset graph_runtime; - Hashtbl.clear graph_worker_requests; - Hashtbl.clear favorites_worker_requests; - Hashtbl.clear admission_worker_requests; - started_graph_generation := None); + let previous_owner = Option.map (fun manager -> manager.Graph_service.startup.account_generation, + manager.startup.graph_generation) (!state_ref).manager in + if previous_owner <> Some (manager.startup.account_generation, manager.startup.graph_generation) + then cancel_local_copy_checks (); + if not manager.startup.authenticated then cancel_copy (); let update = set_state (fun state -> apply_manager_state state manager_state) in let sign_out = if (not manager.Graph_service.startup.authenticated) && !sign_out_in_flight @@ -4345,6 +4450,42 @@ let start sync_media !state_ref; Journal_media_runtime.pump media_runtime; match event with + | Worker.Response { request_id; outcome; _ } + when Hashtbl.mem local_copy_requests request_id -> + let request, deleting, confirm = Hashtbl.find local_copy_requests request_id in + Hashtbl.remove local_copy_requests request_id; + Effect.bind (set_state (fun state -> + let owned = match state.manager with + | Some manager -> manager.startup.account_generation = request.Graph_service.account_generation + && (if deleting then state.local_copy_deleting = Some request else (not confirm || state.local_copy_check = Some request)) + | None -> false in + if not owned then state + else if deleting then ( + let result = match outcome with + | Worker.Completed (Graph_service.Local_copy_deleted (actual, result)) when actual = request -> result + | _ -> Error "Local copy deletion was interrupted. Check the copy and try again." in + let state = { state with local_copy_deleting = None; modal = No_modal } in + match result with + | Error message -> { state with local_copy_message = Some message } + | Ok () -> { (discard_graph_copy_draft state request.graph_id) with + local_copies = Local_copies.add (Logseq_db_types.Graph_types.Uuid.to_string request.graph_id) + (Ok Graph_service.Not_downloaded) state.local_copies + ; local_copy_message = None }) + else ( + let result = match outcome with + | Worker.Completed (Graph_service.Local_copy_inspected (actual, result)) when actual = request -> result + | _ -> Error "Unable to check the local copy. Try again." in + let manager = Option.get state.manager in + let visible = state.graph_picker_open || manager.startup.awaiting_selection in + let state = { state with local_copy_check = (if confirm then None else state.local_copy_check) + ; local_copies = Local_copies.add (Logseq_db_types.Graph_types.Uuid.to_string request.graph_id) + result state.local_copies } in + if not confirm || not visible || manager.selected_graph = Some request.graph_id then state + else match result with + | Ok Graph_service.Downloaded -> { state with modal = Cache_reset_confirmation request.graph_id; local_copy_message = None } + | Ok Not_downloaded -> { state with local_copy_message = Some "No local copy on this device." } + | Error message -> { state with local_copy_message = Some message }))) + ~f:(fun () -> !drain_visible_copies ()) | Worker.Response { request_id; outcome; _ } when Hashtbl.mem copy_worker_requests request_id -> let owner, ticket, protocol_id = Hashtbl.find copy_worker_requests request_id in @@ -4688,6 +4829,7 @@ let start then Effect.of_thunk (reject_unlock owner) else Effect.ignore | Worker.Response { outcome = Completed (Asset_file _); _ } -> Effect.ignore + | Worker.Response { outcome = Completed (Local_copy_inspected _ | Local_copy_deleted _); _ } -> Effect.ignore | Worker.Response { outcome = Completed Client_command_completed; _ } -> Effect.ignore | Worker.Response { outcome = Completed (Graph_state graph_state); _ } | Worker.Push { payload = Graph_state_changed graph_state; _ } -> @@ -5492,7 +5634,7 @@ let start when local_deletion_active snapshot && match payload with - | Open_diagnostics | Close_diagnostics + | Open_diagnostics | Close_diagnostics | Close_graph_picker | Sign_out | Native_payload (Ui.Event.Payload.Navigation_path_changed _) -> false | Asset_settings _ -> false | _ -> true -> Effect.ignore @@ -5538,9 +5680,17 @@ let start | Cache_reset_confirmation graph_id when response.token = state.confirmation_sequence -> (match response.result with - | Action "delete" when local_deletion_available state -> - ( Root_navigation.step state Local_copy_deleted - , send_manager (Graph_service.Delete_local_cache graph_id) ) + | Action "delete" -> + (match state.manager with + | Some manager when manager.startup.authenticated + && manager.selected_graph <> Some graph_id + && state.local_copy_deleting = None + && List.exists (fun (graph : Graph_service.graph) -> graph.graph_id = graph_id) manager.catalog -> + let request : Graph_service.local_copy_request = + { graph_id; account_generation = manager.startup.account_generation } in + { state with modal = No_modal; local_copy_deleting = Some request; local_copy_message = None }, + local_copy_request ~deleting:true request + | _ -> { state with modal = No_modal }, Effect.ignore) | Action "cancel" | Dismissed -> { state with modal = No_modal }, Effect.ignore | Action _ -> state, Effect.ignore) | _ -> state, Effect.ignore) @@ -5793,7 +5943,9 @@ let start (match Logseq_db_types.Graph_types.Uuid.of_string graph_id with | Error _ -> Effect.ignore | Ok graph_id -> - Effect.bind (update (fun state -> { state with graph_picker_open = false })) + cancel_local_copy_checks (); + Effect.bind (update (fun state -> { state with graph_picker_open = false + ; local_copy_check = None; local_copy_message = None; modal = No_modal })) ~f:(fun () -> if Option.bind snapshot.manager (fun value -> value.selected_graph) = Some graph_id then Effect.ignore @@ -5934,18 +6086,45 @@ let start | Switch_graph -> cancel_copy (); update (fun state -> { state with graph_picker_open = true; modal = No_modal }) + | Graph_picker_visible (first, last) -> + Effect.bind + (Effect.of_thunk (fun () -> + let state = !state_ref in + match state.manager with + | Some manager when state.graph_picker_open || manager.startup.awaiting_selection -> + let length = Rrbvec.length state.graph_catalog in + let first = Int64.max 0L (Int64.min first (Int64.of_int length)) |> Int64.to_int in + let last = Int64.max (Int64.of_int first) (Int64.min last (Int64.of_int length)) |> Int64.to_int in + (* Retain at most one viewport, and admit one background metadata read. *) + Queue.clear visible_copy_queue; + for index = first to min last (first + 20) - 1 do + let graph = Rrbvec.nth state.graph_catalog index in + let key = Logseq_db_types.Graph_types.Uuid.to_string graph.graph_id in + let in_flight = Hashtbl.fold (fun _ (request, _, _) found -> + found || request.Graph_service.graph_id = graph.graph_id) local_copy_requests false in + if not (Local_copies.mem key state.local_copies) && not in_flight then + Queue.add Graph_service.{ graph_id = graph.graph_id + ; account_generation = manager.startup.account_generation } visible_copy_queue + done + | _ -> ())) + ~f:(fun () -> !drain_visible_copies ()) | Close_graph_picker -> - update (fun state -> { state with graph_picker_open = false; modal = No_modal }) + cancel_local_copy_checks (); + update (fun state -> { state with graph_picker_open = false; modal = No_modal + ; local_copy_check = None; local_copy_message = None }) | Request_graph_local_cache_reset value -> - update (fun state -> + set_state_and_effect (fun state -> match state.manager, Logseq_db_types.Graph_types.Uuid.of_string value with | Some manager, Ok graph_id when manager.startup.authenticated && manager.selected_graph <> Some graph_id + && state.local_copy_check = None && state.local_copy_deleting = None && List.exists (fun (graph : Graph_service.graph) -> graph.graph_id = graph_id) manager.catalog -> - { state with modal = Cache_reset_confirmation graph_id - ; confirmation_sequence = Int64.succ state.confirmation_sequence } - | _ -> state) + let request : Graph_service.local_copy_request = + { graph_id; account_generation = manager.startup.account_generation } in + { state with local_copy_check = Some request; local_copy_message = None }, + local_copy_request ~deleting:false request + | _ -> state, Effect.ignore) | Sign_out -> cancel_copy (); sign_out_in_flight := true; @@ -5983,15 +6162,7 @@ let start })) ~f:(fun () -> send_manager (Graph_service.Submit_e2ee_password password))) | Request_local_cache_reset -> - update (fun state -> - match state.manager with - | Some { selected_graph = Some graph_id; _ } when local_deletion_available state - -> - { state with - modal = Cache_reset_confirmation graph_id - ; confirmation_sequence = Int64.succ state.confirmation_sequence - } - | None | Some _ -> state) + update (fun state -> { state with graph_picker_open = true; modal = No_modal }) | Delete_undo -> update (fun state -> match state.pending_delete with @@ -6470,7 +6641,15 @@ let start || Option.is_some (operation_failure state.timeline_notice) in let equal_root_presentation left right = - if left.graph_picker_open <> right.graph_picker_open then false + let picker state = state.graph_picker_open || Option.fold ~none:false + ~some:(fun manager -> manager.Graph_service.startup.awaiting_selection) state.manager in + if picker left || picker right then + picker left = picker right && left.graph_picker_open = right.graph_picker_open + && (match left.manager, right.manager with + | Some left, Some right -> left.catalog == right.catalog + && left.selected_graph = right.selected_graph + && left.startup.account_generation = right.startup.account_generation + | _ -> false) else if (not left.graph_ready) || not right.graph_ready then left == right else @@ -6709,8 +6888,8 @@ let start let root = region "root" ~equal:equal_root_presentation (fun state -> match Journal_startup.Presentation.screen state.startup_presentation with - | _ when state.graph_picker_open -> manager_page state dispatch - | Waiting | Interaction _ | Account_failure -> manager_page state dispatch + | _ when state.graph_picker_open -> manager_page ~picker_signal:model_signal state dispatch + | Waiting | Interaction _ | Account_failure -> manager_page ~picker_signal:model_signal state dispatch | Timeline -> timeline_page ~header_signal:(Some header_signal) diff --git a/app/journal_header.ml b/app/journal_header.ml index 04cfbbe..9c4eb18 100644 --- a/app/journal_header.ml +++ b/app/journal_header.ml @@ -59,6 +59,15 @@ let feedback ~key ~top ~visible ~compact ~expanded body = feedback_with_visibility ~key ~top ~visible ~compact ~expanded body ;; +let graph_picker ~leading ~controls body = + Ui.Native_widget.widget chrome + ~key:(Ui.Key.string "graph-picker-chrome") + ~props:(props ~mode:"graph-picker" ~connecting:false ~controls:true ()) + ~on_event:(fun _ -> ()) + ~children:[V.Body.Private.to_widget body; controls; V.column []; leading] () + |> V.Body.static +;; + let date_header ?relative_label ~title () = Ui.Native_widget.widget chrome @@ -241,7 +250,7 @@ let view_impl parent) in let cluster_for presentation = - let local_deletion_available = presentation.local_deletion_available in + let _ = presentation.local_deletion_available in let on_error_info = if presentation.error_available then on_error_info else None in let account_action = Option.map @@ -259,15 +268,6 @@ let view_impl , Switch_graph , V.Button_role.Normal ) ] - @ (if local_deletion_available - then - [ ( 3L - , "Delete local graph copy" - , "trash" - , Request_local_cache_reset - , V.Button_role.Destructive ) - ] - else []) @ [ ( 4L , "Sign out" , "rectangle.portrait.and.arrow.right" diff --git a/app/journal_header.mli b/app/journal_header.mli index 4eca353..4811230 100644 --- a/app/journal_header.mli +++ b/app/journal_header.mli @@ -83,3 +83,10 @@ val detail (** Stable native startup surface; action remains reachable during waiting. *) val startup : loading:bool -> initial:bool -> active:bool -> ?value:float -> message:string -> Journal_view.View.t -> Journal_view.View.Body.t + +(** Full-canvas native list with floating close and catalog controls. *) +val graph_picker + : leading:Journal_view.View.t + -> controls:Journal_view.View.t + -> Journal_view.View.Body.t + -> Journal_view.View.Body.t diff --git a/app/journal_startup.ml b/app/journal_startup.ml index 524d19b..af046b8 100644 --- a/app/journal_startup.ml +++ b/app/journal_startup.ml @@ -75,7 +75,7 @@ let derive ~snapshot ~(graph : Logseq_db_worker.graph_state) = && graph.phase = Logseq_db_worker.Graph_open && not snapshot.timeline_presentation_pending in - if Option.is_some snapshot.local_deletion + if Option.is_some snapshot.local_deletion && not ready then ( match snapshot.local_deletion with | Some (Graph_service.Deletion_in_progress _) -> @@ -236,7 +236,7 @@ module Presentation = struct | Some o -> (match o.manager with | Some manager when o.graph_ready && o.feed_loaded && not o.graph_error - && manager.local_deletion = None && manager.startup.authenticated + && manager.startup.authenticated && o.graph.phase = Logseq_db_worker.Graph_open && graph_matches manager o.graph -> Timeline | Some manager -> let startup = derive ~snapshot:manager ~graph:o.graph in diff --git a/app/journal_view.ml b/app/journal_view.ml index 292e91e..e248903 100644 --- a/app/journal_view.ml +++ b/app/journal_view.ml @@ -1758,6 +1758,7 @@ module View = struct ?key ~activation_id:_ ?(enabled = true) + ?enabled_signal ?(disclosure = true) ~on_activate ~label @@ -1765,10 +1766,15 @@ module View = struct = element ?key - (Lui_elements.list_item - ~disabled:(not enabled) + (fun context parent -> + let disabled_signal = Option.map (fun source -> + let mapped = Signal.map not source in + Signal.on_dispose context.Lui_ui.ui_scope (fun () -> Signal.dispose_signal mapped); + mapped) enabled_signal in + Lui_elements.list_item + ~disabled:(not enabled) ?disabled_signal ?on_press: - (if enabled + (if enabled || Option.is_some enabled_signal then Some (fun _ -> invoke on_activate Event.Payload.Unit) else None) (* Callers choose whether the native list item carries a disclosure icon. *) @@ -1776,7 +1782,7 @@ module View = struct ~icon_placement:`trailing (* A list-item must carry text or children; mount the label as the item content so composite labels render too. *) - [ label.mount ]) + [ label.mount ] context parent) ;; end @@ -1965,26 +1971,31 @@ module View = struct ; title : string ; icon : string option ; enabled : bool + ; enabled_signal : bool Signal.signal option ; role : Button_role.t ; test_id : Test_id.t option } - let action ~id ~title ?icon ?(enabled = true) ?(role = Button_role.Normal) ?test_id () = - { id; title; icon; enabled; role; test_id } + let action ~id ~title ?icon ?(enabled = true) ?enabled_signal ?(role = Button_role.Normal) ?test_id () = + { id; title; icon; enabled; enabled_signal; role; test_id } ;; let to_elements ~on_select entries = List.map - (fun entry -> + (fun entry context parent -> + let disabled_signal = Option.map (fun source -> + let mapped = Signal.map not source in + Signal.on_dispose context.Lui_ui.ui_scope (fun () -> Signal.dispose_signal mapped); + mapped) entry.enabled_signal in Lui_elements.menu_item ~key:(Int64.to_string entry.id) ?accessibility_identifier:entry.test_id ~text:(if String.length entry.title = 0 then " " else entry.title) ?icon:(Option.map journal_icon entry.icon) - ~disabled:(not entry.enabled) + ~disabled:(not entry.enabled) ?disabled_signal ?variant:(Button_role.lui_variant entry.role) ~on_press:(fun _ -> invoke on_select (Event.Payload.Int64 entry.id)) - []) + [] context parent) entries ;; end diff --git a/app/journal_view.mli b/app/journal_view.mli index bbedc59..0f37f9b 100644 --- a/app/journal_view.mli +++ b/app/journal_view.mli @@ -711,6 +711,7 @@ module View : sig : ?key:Key.t -> activation_id:string -> ?enabled:bool + -> ?enabled_signal:bool Signal.signal -> ?disclosure:bool -> on_activate:Event.handler -> label:t @@ -781,6 +782,7 @@ module View : sig -> title:string -> ?icon:string -> ?enabled:bool + -> ?enabled_signal:bool Signal.signal -> ?role:Button_role.t -> ?test_id:Test_id.t -> unit diff --git a/docs/agent-guide/proposed/feature/2026-10-15-switch-graph-native-picker.md b/docs/agent-guide/proposed/feature/2026-10-15-switch-graph-native-picker.md index 320000b..dd172de 100644 --- a/docs/agent-guide/proposed/feature/2026-10-15-switch-graph-native-picker.md +++ b/docs/agent-guide/proposed/feature/2026-10-15-switch-graph-native-picker.md @@ -6,9 +6,11 @@ The graph picker uses uneven capsule buttons and loses the active graph when ope ## Proposal -Implement the user's selected concept A with native equal-width list rows, a soft header, close/refresh/existing account actions, an inline current marker and a separate row menu. Keep picker and request ownership in OCaml. Query one graph's mirror presence on demand with an account-generation fence; cache observations outside rendering. +Implement the user's selected concept A with native equal-width list rows, a full-canvas List with Timeline-style floating close/refresh/existing account controls, an inline current marker and a separate row menu. Keep picker and request ownership in OCaml. Query one graph's mirror presence on demand with an account-generation fence; cache observations outside rendering. -The user explicitly permits discarding unsynchronized local content after confirmation (Sentinel_3c2ff0ee96f88191ab349fca7f7befe9). The confirmation must explain this loss, device-only deletion and the retained cloud copy. The active graph requires switching first. Deletion-owner semantics remain gated on separate approval for the existing protected Core contract comment; this document does not grant that approval. +The user explicitly permits discarding unsynchronized local content after confirmation (Sentinel_3c2ff0ee96f88191ab349fca7f7befe9). The confirmation must explain this loss, device-only deletion and the retained cloud copy. The active graph requires switching first. The user separately approved the minimal existing protected Core contract comment change (Sentinel_752ae13c79888191aeb7123ba1d9fd25). No other protected spec, Dune or lock edit is part of this change. + +The user subsequently requested removing the header entirely and making the whole UI a List like Timeline. Reuse the existing floating chrome; retain the native controls and per-row actions without a title bar. ## Alternatives considered @@ -28,7 +30,13 @@ It detaches the current graph and clears its identity. Keep an application-owned ## Risks - Presence can be unknown or fail; expose a clear retry/status rather than assuming a downloaded copy. -- The protected deletion contract approval is separate from this UI/presence implementation. +- Host ARM64 OCaml restamping is an experimental Simulator compatibility path; actual runtime acceptance does not establish full iOS cross-compilation or ABI validity. + +## Consequences + +Presence observations are cached by account and invalidated for a graph lifecycle change. Native row controls stay mounted while property signals update text and enabled state: real Simulator testing showed that replacing interactive subtrees could leave the retained List cell with an obsolete callback binding. Headless final-state assertions alone did not reproduce that native retention boundary, so the acceptance check is the real Simulator flow. + +Deletion uses a single account-scoped flight and exact terminal result. It preserves the selected graph and catalog, validates persisted local ownership, cancels target work, waits for native leases, and revalidates before mirror removal under the existing exclusive ownership primitive. Unsynchronized local content is intentionally discarded after the warning. Unbound legacy mirrors require opening once and switching away before removal. ## Questions diff --git a/logseq_db_worker/lib/effect_runner/effect_runner.ml b/logseq_db_worker/lib/effect_runner/effect_runner.ml index 26498b0..535a26b 100644 --- a/logseq_db_worker/lib/effect_runner/effect_runner.ml +++ b/logseq_db_worker/lib/effect_runner/effect_runner.ml @@ -296,6 +296,8 @@ let shutdown_id_token_cache cache = clear_id_token_cache cache "ID token cache is stopped.")) ;; +module Authorized_graphs = Set.Make (String) + type runtime = { fork : sw:Eio.Switch.t -> (unit -> unit) -> unit ; sleep : float -> unit @@ -374,6 +376,9 @@ type t = ; mutable attached : (string * Sync.graph_scope) option ; mutable next_database_id : int64 ; mutable stopped : bool + ; mutable latest_snapshot : Sync.snapshot option + ; mutable authorized_graphs : Authorized_graphs.t + ; mutable deletion_owner : Sync.mirror_deletion option } let runtime ~fork ~sleep = Ok { fork; sleep } @@ -405,6 +410,9 @@ let create ~sw dependencies ~post ~recovery_current ~upload_current ~asset_curre ; attached = None ; next_database_id = 0L ; stopped = false + ; latest_snapshot = None + ; authorized_graphs = Authorized_graphs.empty + ; deletion_owner = None } ;; @@ -1430,6 +1438,63 @@ let authoritative_transition_error = function "authoritative commit fatal state: " ^ message ;; +let mirror_directory t graph_id = + Filename.concat t.dependencies.config.application_support_directory + ("logseq-db-worker/synced-graphs/" ^ Graph.Uuid.to_string graph_id) +;; + +let local_owner (account : Sync.account_scope) graph_id = + Yojson.Safe.sort (`Assoc + ["user_id", `String account.user_id; "origin", `String (Uri.to_string account.managed_sync_origin) + ; "graph_id", `String (Graph.Uuid.to_string graph_id)]) +;; + +let check_local_owner t (account : Sync.account_scope) graph_id ~bind = + let directory = mirror_directory t graph_id in + let path = Filename.concat directory ".journal-local-owner.json" in + try + let expected = local_owner account graph_id in + if Sys.file_exists path then ( + let stat = Unix.lstat path in + if stat.st_kind <> Unix.S_REG || stat.st_nlink <> 1 || stat.st_size > 4096 + then Error "The local copy ownership could not be verified." + else if Yojson.Safe.sort (Yojson.Safe.from_file path) = expected then Ok () + else Error "This local copy belongs to another account.") + else if not bind then Error "Open this graph once, then switch to another graph before deleting." + else ( + let temporary, output = Filename.open_temp_file ~temp_dir:directory ".journal-owner-" ".tmp" in + Fun.protect ~finally:(fun () -> close_out_noerr output; + if Sys.file_exists temporary then Sys.remove temporary) + (fun () -> output_string output (Yojson.Safe.to_string expected); flush output; + Unix.fsync (Unix.descr_of_out_channel output); close_out output; Unix.rename temporary path); + Ok ()) + with _ -> Error "The local copy ownership could not be verified." +;; + +let open_owned_database t (request : Sync.mirror_request) inspection = + match Database.open_ ~sw:t.sw t.dependencies.overlay inspection ~graph_name:request.graph.name with + | Error _ as error -> error + | Ok database -> + (match check_local_owner t request.scope.account request.scope.graph_id ~bind:true with + | Ok () -> Ok database + | Error message -> ignore (Database.close database); Error (Overlay.Open_resource_failed message)) +;; + +let deletion_authorized t (request : Sync.mirror_deletion) = + match t.latest_snapshot with + | Some snapshot -> + snapshot.startup.authenticated + && snapshot.startup.account_generation = request.account.account_generation + && snapshot.selected_graph <> Some request.graph_id + && snapshot.local_deletion = Some (Sync.Deletion_in_progress Sync.Deleting_mirror) + && Authorized_graphs.mem (Graph.Uuid.to_string request.graph_id) t.authorized_graphs + | None -> false +;; + +let deletion_current t request = + not t.stopped && t.deletion_owner = Some request && deletion_authorized t request +;; + let handle_sync_worker_effect t = function | Sync.Inspect_mirror request -> (match @@ -1512,36 +1577,56 @@ let handle_sync_worker_effect t = function ~event:(Sync.Snapshot_activated { scope = request.scope }) ()))))) | Delete_mirror request -> - let key = Graph.Uuid.to_string request.graph_id in - (match Hashtbl.find_opt t.inspections key with - | None -> Error (effect_error "The mirror deletion inspection is unavailable.") - | Some inspection -> - (match - Result.bind - (Result.bind - (with_upload_store t (fun db -> - Logseq_db_storage.Asset_upload_store.delete_graph - db - ~origin:(Uri.to_string request.account.managed_sync_origin) - ~account:request.account.user_id - ~graph:request.graph_id)) - (fun () -> - asset_unit - t - ~scope: - { Sync.account = request.account - ; graph_id = request.graph_id - ; graph_generation = 0 - } - Sync.Delete_graph_assets) - |> Result.map_error (fun _ -> ())) - (fun () -> - Database.delete_mirror inspection |> Result.map_error (fun _ -> ())) - with - | Error _ -> Error (effect_error "The overlay mirror could not be deleted.") - | Ok _ -> - Hashtbl.remove t.inspections key; - Ok (sync_result ~event:(Sync.Mirror_deleted (request, Ok ())) ()))) + let ( let* ) = Result.bind in + let current () = deletion_current t request in + let result = + let* () = if current () then Ok () else Error "The graph account is no longer current." in + let* () = match t.attached with + | Some (_, scope) when scope.graph_id = request.graph_id -> + Error "Switch to another graph before deleting." + | _ -> Ok () in + let* inspection = Database.inspect_mirror + ~application_support_directory:t.dependencies.config.application_support_directory + ~graph_id:request.graph_id + |> Result.map_error (fun _ -> "The local copy could not be inspected.") in + let present = match Database.mirror_presence inspection with Overlay.Absent _ -> false | Available _ -> true in + let* () = if present then check_local_owner t request.account request.graph_id ~bind:false else Ok () in + let same_target (scope : Sync.graph_scope) = + scope.graph_id = request.graph_id + && scope.account.user_id = request.account.user_id + && Uri.equal scope.account.managed_sync_origin request.account.managed_sync_origin in + Hashtbl.iter (fun (ticket : Upload.ticket) resolve -> + if same_target ticket.scope then ignore (Eio.Promise.try_resolve resolve () : bool)) t.upload_operations; + let rec drain attempts = + if not (current ()) then Error "The graph account is no longer current." + else if not (Hashtbl.fold (fun (ticket : Upload.ticket) _ busy -> busy || same_target ticket.scope) + t.upload_operations false) then Ok () + else if attempts = 0 then Error "The local copy is still in use. Close attachment previews and try again." + else (t.dependencies.runtime.sleep 0.005; drain (attempts - 1)) in + let* () = drain 1000 in + with_staging_lock t (fun () -> + let* () = if current () then Ok () else Error "The graph account is no longer current." in + let* () = asset_unit t + ~scope:{ Sync.account = request.account; graph_id = request.graph_id; graph_generation = 0 } + Sync.Delete_graph_assets |> Result.map_error (fun _ -> + "The local copy is still in use. Close attachment previews and try again.") in + let* () = if current () then Ok () else Error "The graph account is no longer current." in + let* () = if present then check_local_owner t request.account request.graph_id ~bind:false else Ok () in + let* () = + if not present then Ok () + else Database.delete_mirror inspection |> Result.map (fun _ -> ()) + |> Result.map_error (function + | Overlay.Mirror_delete_busy -> "The local copy is still in use. Close attachment previews and try again." + | _ -> "Local graph deletion failed. Try again.") in + let* () = with_upload_store t (fun db -> + Logseq_db_storage.Asset_upload_store.delete_graph db + ~origin:(Uri.to_string request.account.managed_sync_origin) + ~account:request.account.user_id ~graph:request.graph_id) + |> Result.map_error (fun _ -> "Local graph deletion failed. Try again.") in + Hashtbl.remove t.inspections (Graph.Uuid.to_string request.graph_id); + Ok ()) + in + Ok (sync_result ~event:(Sync.Mirror_deleted (request, result)) ()) | Attach_graph request -> ignore (close_attached t); let key = Graph.Uuid.to_string request.graph.graph_id in @@ -1549,11 +1634,7 @@ let handle_sync_worker_effect t = function | None -> Error (effect_error "The graph attachment inspection is stale.") | Some inspection -> (match - Database.open_ - ~sw:t.sw - t.dependencies.overlay - inspection - ~graph_name:request.graph.name + open_owned_database t request inspection with | Error _ -> Error (effect_error "The overlay graph could not be opened.") | Ok database -> @@ -2016,6 +2097,9 @@ let submit t = function | Core.Run_asset (context, instruction) -> if not t.stopped then run_asset t context instruction | Close_asset_scope scope -> send_asset t ~scope Sync.Close_asset_scope + | Core.Run_worker (Core.Request (_, Core.Handle_sync_worker_effect (Sync.Delete_mirror deletion)) as request) -> + if deletion_authorized t deletion then t.deletion_owner <- Some deletion; + if not t.stopped then t.dependencies.runtime.fork ~sw:t.sw (fun () -> complete t request) | Core.Run_worker request -> if not t.stopped then t.dependencies.runtime.fork ~sw:t.sw (fun () -> complete t request) @@ -2040,6 +2124,14 @@ let submit t = function t.dependencies.sync_runner.submit sync_effect | Run_sync sync_effect -> if not t.stopped then t.dependencies.sync_runner.submit sync_effect + | Publish (Core.Sync_output (Sync.State_changed state) as output) -> + if not (Option.fold ~none:false ~some:(fun old -> old.Sync.catalog == state.snapshot.catalog) t.latest_snapshot) + then t.authorized_graphs <- List.fold_left (fun ids (graph : Sync.graph) -> + Authorized_graphs.add (Graph.Uuid.to_string graph.graph_id) ids) Authorized_graphs.empty state.snapshot.catalog; + t.latest_snapshot <- Some state.snapshot; + if not (Option.fold ~none:false ~some:(deletion_authorized t) t.deletion_owner) + then t.deletion_owner <- None; + if not t.stopped then t.dependencies.publish output | Publish output -> if not t.stopped then t.dependencies.publish output ;; diff --git a/logseq_db_worker/lib/logseq_db_worker.ml b/logseq_db_worker/lib/logseq_db_worker.ml index cbe26c6..3198c8d 100644 --- a/logseq_db_worker/lib/logseq_db_worker.ml +++ b/logseq_db_worker/lib/logseq_db_worker.ml @@ -67,12 +67,24 @@ type graph_state = Pure_reducer.graph_state = ; error : Error.t option } +type local_deletion_waiter = + { graph_id : Logseq_db_types.Graph_types.Uuid.t + ; account_generation : int + ; mutable request : Sync.mirror_deletion option + ; resolve : (unit, string) result Eio.Promise.u + } + +type queued_event = + | Event of Pure_reducer.event + | Delete_copy of local_deletion_waiter + type t = { state : Pure_reducer.state ref ; runner : Effect_runner.t - ; events : Pure_reducer.event Eio.Stream.t + ; events : queued_event Eio.Stream.t ; mutable next_request_id : int64 ; mutable stopped : bool + ; mutable local_deletion_waiter : local_deletion_waiter option } type create_error = Invalid_create of string @@ -82,6 +94,27 @@ let dispatch t event = let transition = Pure_reducer.step before event in observe_unlock before transition event; t.state := transition.next; + (match t.local_deletion_waiter with + | None -> () + | Some waiter -> + let finish result = + t.local_deletion_waiter <- None; + ignore (Eio.Promise.try_resolve waiter.resolve result : bool) in + let snapshot = (Pure_reducer.view transition.next).sync.snapshot in + if snapshot.startup.account_generation <> waiter.account_generation + then finish (Error "The graph account is no longer current.") + else ( + (match waiter.request, event with + | None, Pure_reducer.Sync_event (Sync.Local_cache_deletion_requested graph_id) + when graph_id = waiter.graph_id -> + waiter.request <- List.find_map (function + | Pure_reducer.Run_worker (Pure_reducer.Request (_, Pure_reducer.Handle_sync_worker_effect (Sync.Delete_mirror request))) -> + Some request + | _ -> None) transition.effects; + if waiter.request = None then finish (Error "This local copy cannot be deleted now.") + | _ -> ()); + if waiter.request <> None && snapshot.local_deletion = None + then finish (match snapshot.last_error with None -> Ok () | Some message -> Error message))); List.iter (Effect_runner.submit t.runner) transition.effects ;; @@ -96,7 +129,22 @@ let accepts_after_shutdown = function let route_event t event = if t.stopped then (if accepts_after_shutdown event then dispatch t event) - else Eio.Stream.add t.events event + else Eio.Stream.add t.events (Event event) +;; + +let dispatch_queued t = function + | Event event -> dispatch t event + | Delete_copy waiter -> + let snapshot = (Pure_reducer.view !(t.state)).sync.snapshot in + if not t.stopped && snapshot.startup.authenticated + && snapshot.startup.account_generation = waiter.account_generation + && snapshot.selected_graph <> Some waiter.graph_id + && Option.fold ~none:false ~some:(fun active -> active == waiter) t.local_deletion_waiter + then dispatch t (Pure_reducer.Sync_event (Sync.Local_cache_deletion_requested waiter.graph_id)) + else ( + if Option.fold ~none:false ~some:(fun active -> active == waiter) t.local_deletion_waiter + then t.local_deletion_waiter <- None; + ignore (Eio.Promise.try_resolve waiter.resolve (Error "The graph account is no longer current.") : bool)) ;; let create ~sw ~config ~runner_dependencies = @@ -105,7 +153,7 @@ let create ~sw ~config ~runner_dependencies = | Ok state -> let events = Eio.Stream.create 256 in let state_ref = ref state in - let event_sink = ref (fun event -> Eio.Stream.add events event) in + let event_sink = ref (fun event -> Eio.Stream.add events (Event event)) in (match Effect_runner.create ~sw @@ -121,13 +169,14 @@ let create ~sw ~config ~runner_dependencies = | Error (Effect_runner.Invalid_create message) -> Error (Invalid_create message) | Ok runner -> let t = - { state = state_ref; runner; events; next_request_id = 0L; stopped = false } + { state = state_ref; runner; events; next_request_id = 0L; stopped = false + ; local_deletion_waiter = None } in event_sink := route_event t; dispatch t Pure_reducer.Start; Eio.Fiber.fork ~sw (fun () -> while not t.stopped do - dispatch t (Eio.Stream.take events) + dispatch_queued t (Eio.Stream.take events) done); Ok t) ;; @@ -136,6 +185,26 @@ let post t event = route_event t event let view t = Pure_reducer.view !(t.state) let graph_state t = (view t).graph +let delete_local_copy t ~account_generation ~graph_id = + let snapshot = (view t).sync.snapshot in + if t.stopped || t.local_deletion_waiter <> None + || not snapshot.startup.authenticated + || snapshot.startup.account_generation <> account_generation + || snapshot.selected_graph = Some graph_id + || not (List.exists (fun (graph : Sync.graph) -> graph.graph_id = graph_id) snapshot.catalog) + then Error "This local copy cannot be deleted now." + else ( + let outcome, resolve = Eio.Promise.create () in + let waiter = { graph_id; account_generation; request = None; resolve } in + t.local_deletion_waiter <- Some waiter; + (try Eio.Stream.add t.events (Delete_copy waiter) + with error -> + if Option.fold ~none:false ~some:(fun active -> active == waiter) t.local_deletion_waiter + then t.local_deletion_waiter <- None; + raise error); + Eio.Promise.await outcome) +;; + let request t request = let id = Pure_reducer.request_id_of_int64 t.next_request_id in t.next_request_id <- Int64.succ t.next_request_id; @@ -147,16 +216,23 @@ let shutdown t = if not t.stopped then ( t.stopped <- true; + Option.iter (fun waiter -> + ignore (Eio.Promise.try_resolve waiter.resolve (Error "The Worker stopped.") : bool)) + t.local_deletion_waiter; + t.local_deletion_waiter <- None; dispatch t Pure_reducer.Shutdown; let rec drain () = match Eio.Stream.take_nonblocking t.events with | None -> () - | Some event -> + | Some (Event event) -> if accepts_after_shutdown event then dispatch t event; drain () + | Some (Delete_copy waiter) -> + ignore (Eio.Promise.try_resolve waiter.resolve (Error "The Worker stopped.") : bool); + drain () in drain (); - Eio.Stream.add t.events Pure_reducer.Shutdown; + Eio.Stream.add t.events (Event Pure_reducer.Shutdown); Effect_runner.shutdown t.runner) ;; diff --git a/logseq_db_worker/lib/logseq_db_worker.mli b/logseq_db_worker/lib/logseq_db_worker.mli index dc6817b..4c78ffe 100644 --- a/logseq_db_worker/lib/logseq_db_worker.mli +++ b/logseq_db_worker/lib/logseq_db_worker.mli @@ -30,6 +30,16 @@ val create val post : t -> Pure_reducer.event -> unit val view : t -> Pure_reducer.view val graph_state : t -> graph_state + +(** Waits for the exact local deletion flight's terminal result. The caller has + confirmed discard of unsynchronized local content; current and unauthorized + graphs, stale accounts and overlapping requests are rejected. *) +val delete_local_copy + : t + -> account_generation:int + -> graph_id:Logseq_db_types.Graph_types.Uuid.t + -> (unit, string) result + val request : t -> Protocol.request -> Protocol.response val shutdown : t -> unit diff --git a/logseq_db_worker/lui/logseq_db_worker_lui_service.ml b/logseq_db_worker/lui/logseq_db_worker_lui_service.ml index b922dc5..eac1ba9 100644 --- a/logseq_db_worker/lui/logseq_db_worker_lui_service.ml +++ b/logseq_db_worker/lui/logseq_db_worker_lui_service.ml @@ -6,6 +6,7 @@ module Sync_runner = Logseq_sync_effect_runner.Effect_runner module Protocol = Db.Protocol module Overlay = Logseq_overlay_db.Database module ID = Journal_worker_ids +module Authorized_graphs = Set.Make (String) let unlock_log_count = Atomic.make 0 @@ -237,7 +238,16 @@ type asset_command = | Retry_asset of Logseq_db_types.Graph_types.Uuid.t | Retry_upload of Logseq_db_types.Graph_types.Uuid.t +type local_copy_presence = Downloaded | Not_downloaded + +type local_copy_request = + { graph_id : graph_id + ; account_generation : int + } + type request = + | Inspect_local_copy of local_copy_request + | Delete_local_copy of local_copy_request | Import_asset of { graph_generation : int ; source : Logseq_db_types.Asset_import.t @@ -263,6 +273,8 @@ type request = } type response = + | Local_copy_inspected of local_copy_request * (local_copy_presence, string) result + | Local_copy_deleted of local_copy_request * (unit, string) result | Asset_imported of (Logseq_db_worker.import_receipt, string) result | Client_command_completed | Asset_file of (string * string) option @@ -509,6 +521,9 @@ let create ~(dependencies : dependencies) = let module Session = struct type t = { worker : Db.t + ; application_support_directory : string + ; mutable catalog_source : Sync.graph list + ; mutable authorized_graphs : Authorized_graphs.t ; token_cache : Worker_runner.id_token_cache ; authenticated_user : string option ref ; auth_seen : bool ref @@ -666,6 +681,9 @@ let create ~(dependencies : dependencies) = Ok Session. { worker + ; application_support_directory = config.Db.Config.application_support_directory + ; catalog_source = [] + ; authorized_graphs = Authorized_graphs.empty ; token_cache ; authenticated_user ; auth_seen @@ -676,6 +694,35 @@ let create ~(dependencies : dependencies) = })))) ~handle:(fun context session request -> match request with + | Delete_local_copy request -> + Eio.Condition.loop_no_mutex session.auth_wake (fun () -> + if !(session.auth_enqueued_sequence) >= !(session.auth_sequence) then Some () else None); + Ok (Local_copy_deleted (request, + Db.delete_local_copy session.worker ~account_generation:request.account_generation + ~graph_id:request.graph_id)) + | Inspect_local_copy request -> + let snapshot = (Db.view session.worker).sync.snapshot in + if session.catalog_source != snapshot.catalog then ( + session.catalog_source <- snapshot.catalog; + session.authorized_graphs <- List.fold_left (fun ids (graph : Sync.graph) -> + Authorized_graphs.add (Logseq_db_types.Graph_types.Uuid.to_string graph.graph_id) ids) + Authorized_graphs.empty snapshot.catalog); + let result = + if not snapshot.startup.authenticated + || request.account_generation <> snapshot.startup.account_generation + || not (Authorized_graphs.mem (Logseq_db_types.Graph_types.Uuid.to_string request.graph_id) session.authorized_graphs) + then Error "The graph account is no longer current." + else + match Overlay.inspect_mirror + ~application_support_directory:session.application_support_directory + ~graph_id:request.graph_id with + | Error _ -> Error "Unable to check the local copy. Try again." + | Ok inspection -> + Ok (match Overlay.mirror_presence inspection with + | Logseq_overlay_db.Types.Absent _ -> Not_downloaded + | Available _ -> Downloaded) + in + Ok (Local_copy_inspected (request, result)) | Import_asset { graph_generation; source } -> Ok (Asset_imported (Db.import_asset session.worker ~graph_generation source)) | Get_graph_state -> Ok (Graph_state (Db.graph_state session.worker)) @@ -734,6 +781,8 @@ let create ~(dependencies : dependencies) = Reassert the observed SDK state after restoration's ordered post. *) if !(session.auth_seen) then session.queue_auth !(session.authenticated_user); Ok Client_command_completed + | Client_command (Delete_local_cache _) -> + Error "Local deletion requires an account-scoped request." | Client_command command -> (match command with Submit_e2ee_password _ -> unlock_queue_log context | _ -> ()); Db.post session.worker (client_event command); diff --git a/logseq_db_worker/lui/logseq_db_worker_lui_service.mli b/logseq_db_worker/lui/logseq_db_worker_lui_service.mli index df3431e..57f2ee5 100644 --- a/logseq_db_worker/lui/logseq_db_worker_lui_service.mli +++ b/logseq_db_worker/lui/logseq_db_worker_lui_service.mli @@ -147,7 +147,16 @@ type asset_command = | Retry_asset of Logseq_db_types.Graph_types.Uuid.t | Retry_upload of Logseq_db_types.Graph_types.Uuid.t +type local_copy_presence = Downloaded | Not_downloaded + +type local_copy_request = + { graph_id : graph_id + ; account_generation : int + } + type request = + | Inspect_local_copy of local_copy_request + | Delete_local_copy of local_copy_request | Import_asset of { graph_generation : int ; source : Logseq_db_types.Asset_import.t @@ -173,6 +182,8 @@ type request = } type response = + | Local_copy_inspected of local_copy_request * (local_copy_presence, string) result + | Local_copy_deleted of local_copy_request * (unit, string) result | Asset_imported of (Logseq_db_worker.import_receipt, string) result | Client_command_completed | Asset_file of (string * string) option diff --git a/logseq_db_worker/test/test_lui_service.ml b/logseq_db_worker/test/test_lui_service.ml index 293053f..b7b4e93 100644 --- a/logseq_db_worker/test/test_lui_service.ml +++ b/logseq_db_worker/test/test_lui_service.ml @@ -1105,10 +1105,102 @@ let test_real_auth_control ~transition () = "revoked token was never applied")) ;; +let test_local_copy_presence mode () = + let module Core = Logseq_sync_pure_reducer.Core in + let module G = Logseq_db_types.Graph_types in + T.with_managed (fun fixture -> + let root = fixture.config.Logseq_db_worker.Config.application_support_directory in + let graph_id = G.Uuid.of_string "94000000-0000-4000-8000-000000000101" |> Result.get_ok in + let graph : Core.graph = { graph_id; name = "Synthetic presence"; encrypted = false + ; schema = { major = 65; minor = 33; exact = true } } in + let user = "synthetic-presence-user" in + let directory = Filename.concat root "logseq-db-worker" in + Unix.mkdir directory 0o700; + let catalogs = Filename.concat directory "sync-catalogs" in + Unix.mkdir catalogs 0o700; + let identity = user ^ "\000https://api.logseq.io" in + let name = Digestif.SHA256.digest_string identity |> Digestif.SHA256.to_hex in + let cache = Core.catalog_cache ~user_id:user ~graphs:[graph] ~selected_graph:None in + Out_channel.with_open_bin (Filename.concat catalogs (name ^ ".json")) + (fun output -> output_string output (Core.encode_catalog_cache cache)); + if List.mem mode ["downloaded"; "corrupt"; "delete"; "busy"; "owner"] then ( + let mirrors = Filename.concat directory "synced-graphs" in + Unix.mkdir mirrors 0o700; + let mirror = Filename.concat mirrors (G.Uuid.to_string graph_id) in + Unix.mkdir mirror 0o700; + let sqlite = Sqlite3.db_open (Filename.concat mirror "db.sqlite") in + Fun.protect ~finally:(fun () -> ignore (Sqlite3.db_close sqlite)) (fun () -> + Sqlite3.Rc.check (Sqlite3.exec sqlite "CREATE TABLE overlay_mutations (record TEXT); INSERT INTO overlay_mutations VALUES ('not decoded by presence')"); + if mode <> "corrupt" then ( + let checkpoint = Logseq_db_types.Sync_checkpoint.create ~graph_id + ~schema:{ major = 65; minor = 33 } + ~applied_server_t:(Logseq_db_types.Server_cursor.of_int64 0L |> Result.get_ok) + ~checksum:"0123456789abcdef" |> Result.get_ok in + Logseq_db_storage.Sync_checkpoint_store.initialize_database sqlite checkpoint |> Result.get_ok))); + let mirror_directory = Filename.concat (Filename.concat directory "synced-graphs") (G.Uuid.to_string graph_id) in + if List.mem mode ["delete"; "busy"; "owner"] then + Out_channel.with_open_bin (Filename.concat mirror_directory ".journal-local-owner.json") (fun output -> + output_string output (Yojson.Safe.to_string (`Assoc + ["user_id", `String (if mode = "owner" then "another-account" else user) + ; "origin", `String "https://api.logseq.io"; "graph_id", `String (G.Uuid.to_string graph_id)]))); + + with_client fixture.config (fun client -> + ignore (Worker.send client (Service.Client_command (Restore_local_account { user_id = user })) |> accepted); + ignore (Worker.send client (Service.Client_command (Reconcile_authenticated_user { user_id = Some user })) |> accepted); + let observed = ref None in + control_wait "synthetic catalog available" (fun () -> + Worker.For_testing.drain_events client ~max_events:64 |> List.iter (function + | Worker.Push { payload = Service.Client_state_changed state; _ } + when state.snapshot.startup.authenticated && state.snapshot.catalog <> [] -> + observed := Some state.snapshot + | _ -> ()); + Option.is_some !observed); + let snapshot = Option.get !observed in + let request : Service.local_copy_request = + { graph_id = (if mode = "unauthorized" then G.Uuid.of_string "94000000-0000-4000-8000-000000000102" |> Result.get_ok else graph_id) + ; account_generation = snapshot.startup.account_generation + (if mode = "stale" then 1 else 0) } in + let id = Worker.send client (Service.Inspect_local_copy request) |> accepted in + match await_response client id with + | Service.Local_copy_inspected (actual, result) -> + T.require (actual = request) "presence result retains exact identity"; + (match mode, result with + | "absent", Ok Service.Not_downloaded + | ("downloaded" | "delete" | "busy" | "owner"), Ok Service.Downloaded -> () + | ("stale" | "unauthorized" | "corrupt"), Error _ -> () + | _ -> T.fail "unexpected presence result for %s" mode); + if List.mem mode ["delete"; "busy"; "owner"] then ( + let lock = if mode <> "busy" then None else ( + let db = Sqlite3.db_open (Filename.concat mirror_directory ".logseq-overlay-owner.sqlite") in + Sqlite3.Rc.check (Sqlite3.exec db "CREATE TABLE owner_primitive (protocol INTEGER NOT NULL); BEGIN IMMEDIATE"); + Some db) in + let remove () = + let id = Worker.send client (Service.Delete_local_copy request) |> accepted in + match await_response client id with + | Service.Local_copy_deleted (actual, result) -> + T.require (actual = request) "delete result retains exact request"; result + | _ -> T.fail "delete returned another response" in + let result = remove () in + T.require (Result.is_ok result = (mode = "delete")) "delete success matches ownership"; + T.require (Sys.file_exists (Filename.concat mirror_directory "db.sqlite") = (mode <> "delete")) + "failure preserves the local mirror"; + Option.iter (fun db -> + Sqlite3.Rc.check (Sqlite3.exec db "ROLLBACK"); ignore (Sqlite3.db_close db); + T.require (Result.is_ok (remove ())) "busy deletion can be retried") lock) + | _ -> T.fail "presence request returned another response")) +;; + let () = T.run "lui worker service" - [ T.case + [ T.case "local copy confirmed deletion awaits physical removal" (test_local_copy_presence "delete") + ; T.case "local copy busy failure recovers" (test_local_copy_presence "busy") + ; T.case "local copy refuses another account mirror" (test_local_copy_presence "owner") + ; T.case "local copy absent metadata" (test_local_copy_presence "absent") + ; T.case "local copy downloaded without body decoding" (test_local_copy_presence "downloaded") + ; T.case "local copy stale account is rejected" (test_local_copy_presence "stale") + ; T.case "local copy unauthorized graph is rejected" (test_local_copy_presence "unauthorized") + ; T.case "local copy corrupt metadata reports failure" (test_local_copy_presence "corrupt") + ; T.case "control bypasses full data and handler waits" test_control_bypasses_full_data_and_waiting_handlers ; T.case diff --git a/logseq_overlay_db/lib/database.ml b/logseq_overlay_db/lib/database.ml index 36ba153..18951d6 100644 --- a/logseq_overlay_db/lib/database.ml +++ b/logseq_overlay_db/lib/database.ml @@ -269,9 +269,18 @@ let mirror_generation value = let mirror_generation_for_path path = let stat = Unix.stat path in + let owner_path = Filename.concat (Filename.dirname path) ".journal-local-owner.json" in + let owner_identity = + try + let owner = Unix.lstat owner_path in + if owner.st_kind <> Unix.S_REG || owner.st_nlink <> 1 || owner.st_size > 4096 + then "invalid-owner" + else In_channel.with_open_bin owner_path In_channel.input_all + |> Digestif.SHA256.digest_string |> Digestif.SHA256.to_hex + with Unix.Unix_error (Unix.ENOENT, _, _) -> "no-owner" in mirror_generation (Digestif.SHA256.digest_string - (Printf.sprintf "%s:%d:%d" path stat.st_dev stat.st_ino) + (Printf.sprintf "%s:%d:%d:%s" path stat.st_dev stat.st_ino owner_identity) |> Digestif.SHA256.to_hex) ;; @@ -698,34 +707,40 @@ let rec remove_mirror_tree path = | _ -> Unix.unlink path ;; +let deletion_sequence = Atomic.make 0 + let delete_mirror inspection = let path = inspection.location.database_path in match inspection.presence with | Types.Absent _ -> Error Types.Mirror_delete_stale | Available { generation = expected; _ } -> - (match inspect_durable_mirror inspection.location with - | Error _ | Ok { presence = Absent _; _ } -> Error Types.Mirror_delete_stale - | Ok { presence = Available { generation = current; _ }; _ } - when not (Types.Mirror_generation.equal expected current) -> - Error Types.Mirror_delete_stale - | Ok { presence = Available _; _ } -> - let graph_directory = Filename.dirname path in - (match Ownership.acquire ~graph_directory with - | Error Ownership.Already_owned -> Error Types.Mirror_delete_busy - | Error (Unavailable message) -> Error (Types.Mirror_delete_failed message) - | Ok ownership -> - (try - remove_mirror_tree graph_directory; - ignore (Ownership.release ownership); - Ok - { Types.graph_uuid = Some inspection.location.graph_id - ; previous_generation = expected - ; absent_generation = absent_mirror_generation path - } - with - | exn -> - ignore (Ownership.release ownership); - Error (Types.Mirror_delete_failed (Printexc.to_string exn))))) + let graph_directory = Filename.dirname path in + let ownership = + try Ownership.acquire ~graph_directory with + | exn -> Error (Ownership.Unavailable (Printexc.to_string exn)) in + (match ownership with + | Error Ownership.Already_owned -> Error Types.Mirror_delete_busy + | Error (Unavailable message) -> + if not (Sys.file_exists graph_directory) then Error Types.Mirror_delete_stale + else Error (Types.Mirror_delete_failed message) + | Ok ownership -> + Fun.protect ~finally:(fun () -> ignore (Ownership.release ownership)) (fun () -> + match inspect_durable_mirror inspection.location with + | Error _ | Ok { presence = Absent _; _ } -> Error Types.Mirror_delete_stale + | Ok { presence = Available { generation = current; _ }; _ } + when not (Types.Mirror_generation.equal expected current) -> Error Types.Mirror_delete_stale + | Ok _ -> + try + let retired = Printf.sprintf "%s.deleted-%d-%d" graph_directory (Unix.getpid ()) + (Atomic.fetch_and_add deletion_sequence 1) in + if Sys.file_exists retired then Error (Types.Mirror_delete_failed "retired location is occupied") + else ( + Unix.rename graph_directory retired; + ignore (Ownership.release ownership); + remove_mirror_tree retired; + Ok { Types.graph_uuid = Some inspection.location.graph_id + ; previous_generation = expected; absent_generation = absent_mirror_generation path }) + with exn -> Error (Types.Mirror_delete_failed (Printexc.to_string exn)))) ;; let validate_receipts sqlite = diff --git a/logseq_overlay_db/test/test_overlay_storage.ml b/logseq_overlay_db/test/test_overlay_storage.ml index a460f41..cb145c6 100644 --- a/logseq_overlay_db/test/test_overlay_storage.ml +++ b/logseq_overlay_db/test/test_overlay_storage.ml @@ -683,6 +683,20 @@ let stale_snapshot_commit_can_be_canceled () = | Ok _ -> Alcotest.fail "canceled persisted preparation was committed") ;; +let stale_delete_cannot_remove_rebound_owner () = + let behavior = "mirror owner identity changed after inspection" in + T.with_temp_directory "overlay-owner-rebind-" (fun support -> + let database_path = T.seed_mirror support in + let owner = Filename.concat (Filename.dirname database_path) ".journal-local-owner.json" in + Out_channel.with_open_bin owner (fun output -> output_string output "account-A"); + let _, _, inspection = location_and_inspection support behavior in + Out_channel.with_open_bin owner (fun output -> output_string output "account-B"); + (match Database.delete_mirror inspection with + | Error Mirror_delete_stale -> () + | _ -> Alcotest.fail "an old observation deleted the rebound account's mirror"); + Alcotest.(check bool) "new owner's mirror is retained" true (Sys.file_exists database_path)) +;; + let stale_delete_cannot_remove_replacement () = let behavior = "stale mirror delete is rejected" in T.with_temp_directory "overlay-stale-delete-" (fun support -> @@ -2832,6 +2846,7 @@ let () = `Quick sqlite_full_int64_checkpoint ] ) - ; "mirror and durability", cases + ; "mirror and durability", cases @ + [Alcotest.test_case "owner rebind invalidates deletion" `Quick stale_delete_cannot_remove_rebound_owner] ] ;; diff --git a/logseq_sync/lib/effect_runner/effect_runner.ml b/logseq_sync/lib/effect_runner/effect_runner.ml index 1d3ca25..b79f93b 100644 --- a/logseq_sync/lib/effect_runner/effect_runner.ml +++ b/logseq_sync/lib/effect_runner/effect_runner.ml @@ -373,6 +373,7 @@ type t = ; asset_byte_reservation_lock : Eio.Semaphore.t ; secret_lock : Eio.Mutex.t ; asset_caches : (Core.graph_scope, Asset_cache.t) Hashtbl.t + ; asset_leases : ((Core.graph_scope * string), unit) Hashtbl.t ; mutable closed : bool } @@ -394,6 +395,7 @@ let create ~sw dependencies ~post = ; asset_byte_reservation_lock = Eio.Semaphore.make 1 ; secret_lock = Eio.Mutex.create () ; asset_caches = Hashtbl.create 4 + ; asset_leases = Hashtbl.create 16 ; closed = false } ;; @@ -1512,19 +1514,20 @@ let retain_asset_file t ~scope ~handle = Option.bind (Hashtbl.find_opt t.asset_caches scope) (fun cache -> Option.bind (Asset_cache.retain cache handle) (fun lease -> match Asset_cache.path cache lease with - | Some path -> Some (lease, path) + | Some path -> Hashtbl.replace t.asset_leases (scope, lease) (); Some (lease, path) | None -> Asset_cache.release cache lease; None)) ;; let release_asset_file t ~scope ~handle = + Hashtbl.remove t.asset_leases (scope, handle); Option.iter (fun cache -> Asset_cache.release cache handle) (Hashtbl.find_opt t.asset_caches scope) ;; -let delete_graph_assets ?except_id t (request : Core.mirror_deletion) = +let cancel_graph_asset_operations ?except_id t (request : Core.mirror_deletion) = Hashtbl.iter (fun id (operation : operation) -> match operation.asset_identity with @@ -1538,7 +1541,25 @@ let delete_graph_assets ?except_id t (request : Core.mirror_deletion) = operation.cancelled <- true; operation.cancel () | Some _ | None -> ()) - t.operations; + t.operations +;; + +let delete_graph_assets ?except_id t (request : Core.mirror_deletion) ~current = + cancel_graph_asset_operations ?except_id t request; + let same_target (scope : Core.graph_scope) = + scope.graph_id = request.graph_id && scope.account.user_id = request.account.user_id + && Uri.equal scope.account.managed_sync_origin request.account.managed_sync_origin in + let busy () = + Hashtbl.fold (fun id (operation : operation) busy -> + busy || (Some id <> except_id && Option.fold ~none:false + ~some:(fun (scope, _) -> same_target scope) operation.asset_identity)) t.operations false + || Hashtbl.fold (fun (scope, _) () busy -> busy || same_target scope) t.asset_leases false in + let rec await_retirement attempts = + if not (current ()) then Error "The graph cleanup was cancelled." + else if not (busy ()) then Ok () + else if attempts = 0 then Error "Attachment resources are still in use." + else (t.dependencies.runtime.sleep 0.005; await_retirement (attempts - 1)) in + Result.bind (await_retirement 1000) (fun () -> let scopes = Hashtbl.fold (fun (scope : Core.graph_scope) _ acc -> @@ -1558,7 +1579,7 @@ let delete_graph_assets ?except_id t (request : Core.mirror_deletion) = ~root:(asset_root t) ~account:request.account ~graph_id:request.graph_id - |> Result.map_error (fun _ -> "The graph asset cache could not be removed.") + |> Result.map_error (fun _ -> "The graph asset cache could not be removed.")) ;; let read_upload_source ~maximum_plaintext_bytes source_file = @@ -1752,7 +1773,7 @@ let retain_staged_file t ~scope ~file = | Ok cache -> Option.bind (Asset_cache.retain_staged cache ~file) (fun lease -> match Asset_cache.path cache lease with - | Some path -> Some (lease, path) + | Some path -> Hashtbl.replace t.asset_leases (scope, lease) (); Some (lease, path) | None -> Asset_cache.release cache lease; None)) @@ -1840,7 +1861,7 @@ let execute_asset ?except_id t (request : Core.asset_io_request) ~current = ; scope = Core.effect_scope_of_graph scope } in - delete_graph_assets ?except_id t deletion |> storage_result |> map_unit + delete_graph_assets ?except_id t deletion ~current |> storage_result |> map_unit | Delete_account_assets -> delete_account_assets ?except_id t scope.account |> Result.map_error asset_cache_failure @@ -1887,6 +1908,12 @@ let submit_asset_io t instruction ticket (request : Core.asset_io_request) = in if claim_operation t instruction then ( + (match request.action with + | Core.Delete_graph_assets -> + let scope = request.context.scope in + cancel_graph_asset_operations t + { Core.account = scope.account; graph_id = scope.graph_id; scope = Core.effect_scope_of_graph scope } + | _ -> ()); let cancelled, resolve_cancelled = Eio.Promise.create () in let operation = { scope = Core.effect_scope_of_graph request.context.scope @@ -1993,6 +2020,12 @@ let cleanup_asset_action = function let submit t instruction = match instruction with + | Core.Asset_io (ticket, ({ action = Core.Delete_graph_assets; _ } as request)) -> + (* Deletion may wait for native leases. It must not block the event consumer + that delivers their release, and shutdown must revoke its execution. *) + if t.closed + then t.post (Core.Runner_completed (Core.Asset_completion (ticket, Error Core.Asset_cancelled))) + else submit_asset_io t instruction ticket request | Core.Asset_io (ticket, request) -> if cleanup_asset_action request.action then ( diff --git a/logseq_sync/lib/pure_reducer/core.ml b/logseq_sync/lib/pure_reducer/core.ml index 886150e..ee7c658 100644 --- a/logseq_sync/lib/pure_reducer/core.ml +++ b/logseq_sync/lib/pure_reducer/core.ml @@ -1066,7 +1066,8 @@ let authenticate_new_account core user_id = in let snapshot = { core.public_state.snapshot with - catalog = [] + local_deletion = None + ; catalog = [] ; selected_graph = None ; applied_server_t = None ; startup @@ -1080,6 +1081,7 @@ let authenticate_new_account core user_id = ; pending_effects = [] ; catalog_cache_loading = false ; deferred_catalog_reconciliation = false + ; deletion_scope = None ; cached_selected_graph = None ; current_graph_scope = None ; graph_key = None @@ -1162,6 +1164,7 @@ let restore_local core user_id = ; pending_effects = [] ; catalog_cache_loading = true ; deferred_catalog_reconciliation = false + ; deletion_scope = None ; cached_selected_graph = None ; selected_graph_value = None ; current_graph_scope = None @@ -1186,7 +1189,7 @@ let restore_local core user_id = ; deferred_authoritative_owner = None ; websocket_live = false } - { core.public_state.snapshot with startup } + { core.public_state.snapshot with local_deletion = None; startup } in let next, request = issue_request core (Load_catalog (account_scope core user_id)) in { next; effects = [ publish next; Run request ] } @@ -1198,7 +1201,8 @@ let sign_out core = let current_startup = core.public_state.snapshot.startup in let snapshot = { core.public_state.snapshot with - sync_phase = Offline + local_deletion = None + ; sync_phase = Offline ; catalog = [] ; selected_graph = None ; applied_server_t = None @@ -1218,6 +1222,7 @@ let sign_out core = ; pending_effects = [] ; catalog_cache_loading = false ; deferred_catalog_reconciliation = false + ; deletion_scope = None ; cached_selected_graph = None ; current_graph_scope = None ; graph_key = None @@ -2366,15 +2371,14 @@ let deletion_failed core stage = let message = match stage with | Closing_graph -> "Local graph close failed." - | Deleting_mirror -> "Local graph deletion failed." + | Deleting_mirror -> "Local graph deletion failed. Try again." | Clearing_selection -> "Clearing the saved graph selection failed." in let next = set_snapshot - { core with pending_effects = [] } + { core with deletion_scope = None } { core.public_state.snapshot with - local_deletion = Some (Deletion_failed stage) - ; sync_phase = Failed + local_deletion = None ; last_error = Some message } in @@ -2669,50 +2673,27 @@ let inspect_sync core scope = let begin_local_deletion core graph_id = let snapshot = core.public_state.snapshot in - match core.current_graph_scope, core.sync_view with - | Some scope, Some _ - when scope.graph_id = graph_id - && snapshot.selected_graph = Some graph_id - && snapshot.startup.authenticated - && snapshot.startup.failure = None - && (not snapshot.startup.restoring_local) - && (not snapshot.startup.bootstrapping) - && (not snapshot.startup.awaiting_selection) - && (not snapshot.startup.awaiting_e2ee_password) - && core.pending_attachment = None - && core.active_snapshot_download = None -> - let closing = return_to_graph_picker core in - let cleared = closing.next.public_state.snapshot in - let next = - set_snapshot - { closing.next with - deletion_scope = Some scope - ; lifecycle_generation = Int64.succ core.lifecycle_generation - ; connection_generation = core.connection_generation + 1 - ; catalog_cache_loading = false - ; deferred_catalog_reconciliation = false - } - { cleared with - selected_graph = snapshot.selected_graph - ; local_deletion = Some (Deletion_in_progress Closing_graph) - ; startup = { cleared.startup with awaiting_selection = false } - } - in - { next - ; effects = - [ publish next - ; Run - (Close_websocket - { graph = scope; connection_generation = core.connection_generation }) - ; Run - (Cancel_effects - { (effect_scope_of_account scope.account) with - presentation_generation = None - ; lifecycle_generation = None - }) - ; Delegate (Detach_graph scope) - ] - } + match core.user_id, graph_in_catalog snapshot.catalog graph_id with + | Some user_id, Some _ + when snapshot.startup.authenticated + && snapshot.selected_graph <> Some graph_id + && snapshot.local_deletion = None + && not snapshot.startup.restoring_local + && not snapshot.startup.bootstrapping + && not snapshot.startup.awaiting_e2ee_password + && core.pending_attachment = None + && core.active_snapshot_download = None -> + let account = account_scope core user_id in + (* A private flight identity leaves the live graph's generations untouched. *) + let account = { account with lifecycle_generation = Int64.of_int core.next_effect_id } in + let scope = { account; graph_id; graph_generation = snapshot.startup.graph_generation } in + let next = set_snapshot + { core with deletion_scope = Some scope + ; next_effect_id = core.next_effect_id + 1 + } + { snapshot with local_deletion = Some (Deletion_in_progress Deleting_mirror); last_error = None } in + { next; effects = [publish next; Delegate (Delete_mirror + { account; graph_id; scope = effect_scope_of_graph scope })] } | _ -> unchanged core ;; @@ -2746,38 +2727,33 @@ let graph_detached core scope result = let mirror_deleted core (request : mirror_deletion) result = match core.deletion_scope, core.public_state.snapshot.local_deletion with | Some scope, Some (Deletion_in_progress Deleting_mirror) - when request.account = scope.account - && request.graph_id = scope.graph_id - && request.scope = effect_scope_of_graph scope -> + when request.account = scope.account && request.graph_id = scope.graph_id + && request.scope = effect_scope_of_graph scope -> (match result with - | Error _ -> deletion_failed core Deleting_mirror + | Error message -> + let failed = deletion_failed core Deleting_mirror in + if List.mem message + [ "This local copy belongs to another account." + ; "Open this graph once, then switch to another graph before deleting." + ; "The local copy ownership could not be verified." + ; "Switch to another graph before deleting." + ; "The local copy is still in use. Close attachment previews and try again." ] + then + let next = set_snapshot failed.next { failed.next.public_state.snapshot with last_error = Some message } in + { next; effects = [publish next] } + else failed | Ok () -> - let snapshot = core.public_state.snapshot in - let next = - set_snapshot - { core with cached_selected_graph = None } - { snapshot with - selected_graph = None - ; local_deletion = Some (Deletion_in_progress Clearing_selection) - } - in - let account = account_scope next scope.account.user_id in - let cache = - catalog_cache - ~user_id:account.user_id - ~graphs:snapshot.catalog - ~selected_graph:None - in - let next, save = issue_request next (Save_catalog { account; cache }) in - { next; effects = [ publish next; Run save ] }) + let next = set_snapshot { core with deletion_scope = None } + { core.public_state.snapshot with local_deletion = None; last_error = None } in + { next; effects = [publish next] }) | _ -> unchanged core ;; let deletion_blocks_event core event = match core.public_state.snapshot.local_deletion, event with - | None, _ -> false - | Some _, (Graph_detached _ | Mirror_deleted _ | Runner_completed _ | Shutdown) -> false - | Some _, _ -> true + | Some (Deletion_in_progress _), + (Local_cache_deletion_requested _ | Graph_selected _ | Graph_picker_requested) -> true + | _ -> false ;; let step core event = @@ -3181,7 +3157,9 @@ let request_asset core (request : asset_request) = (fun (scope : graph_scope) -> scope.account = request.scope.account) core.asset_known_scopes | Delete_graph_assets -> - List.exists + Option.fold ~none:false ~some:(fun (scope : graph_scope) -> + scope.account = request.scope.account && scope.graph_id = request.scope.graph_id) core.deletion_scope + || List.exists (fun (scope : graph_scope) -> scope.account = request.scope.account && scope.graph_id = request.scope.graph_id) diff --git a/logseq_sync/test/core_contract.ml b/logseq_sync/test/core_contract.ml index b7ab164..83c1ef1 100644 --- a/logseq_sync/test/core_contract.ml +++ b/logseq_sync/test/core_contract.ml @@ -2439,12 +2439,14 @@ let deletion_save_completion effects (result : (unit, Core.effect_error) result) ;; let deletion_mirror effects = - List.find_map + let request = List.find_map (function | Core.Delegate (Core.Delete_mirror request) -> Some request | _ -> None) - effects - |> Option.get + effects in + match request with + | Some request -> request + | None -> Alcotest.fail "non-current deletion must delegate mirror removal" ;; let check_deletion_stage stage core = @@ -2494,254 +2496,117 @@ let deletion_blocked_events scope sync = ] ;; +let deletion_noncurrent core = + let target = List.find (fun (candidate : Core.graph) -> candidate.graph_id = graph_id) + (Core.state core).snapshot.catalog in + let catalog = refresh_catalog core [ target; other_graph ] in + let selected = Core.step catalog.next (Core.Graph_selected other_graph_id) in + let request = inspect_mirror_request selected.effects in + let inspected = Core.step selected.next (Core.Mirror_inspected (Core.Mirror_available request)) in + let token = Overlay.sync_token_of_string "sync-token:v1:noncurrent" |> Result.get_ok in + let sync = Overlay.sync_view ~token ~checkpoint:(server_cursor_of_int 0) ~submissions:[] in + let attached = Core.step inspected.next (Core.Graph_attached { scope = request.scope; sync }) in + let ready = Core.step attached.next Core.Timeline_presented in + ready.next, request.scope, sync, selected.effects +;; + let local_deletion_orders_cleanup encrypted () = - let ready, scope, sync, old_effects = deletion_ready_graph encrypted in - let refreshing = Core.step ready Core.Catalog_refresh_requested in - let late_catalog = fetch_catalog_completion refreshing.effects [] in - let ready = refreshing.next in + let original, old_scope, _, _ = deletion_ready_graph encrypted in + let ready, scope, sync, selection_effects = deletion_noncurrent original in check_no_work ready [ Core.Local_cache_deletion_requested other_graph_id ]; let deleting = Core.step ready (Core.Local_cache_deletion_requested graph_id) in - check_deletion_stage Closing_graph deleting.next; - Alcotest.(check bool) - "no graph work admitted" - true - (Core.admitted_graph_scope deleting.next = None); - Alcotest.(check bool) - "selection retained until mirror removed" - true - ((Core.state deleting.next).snapshot.selected_graph = Some graph_id); - let worker = - List.filter_map - (function - | Core.Delegate instruction -> Some instruction - | _ -> None) - deleting.effects - in - Alcotest.(check bool) - "only close is delegated initially" - true - (worker = [ Core.Detach_graph scope ]); - Alcotest.(check bool) - "old transport is cancelled" - true - (List.exists - (function - | Core.Run (Core.Cancel_effects _) -> true - | _ -> false) - deleting.effects); - Alcotest.(check bool) - "socket explicitly closes" - true - (List.exists - (function - | Core.Run (Core.Close_websocket connection) -> connection.graph = scope - | _ -> false) - deleting.effects); - let snapshot = (Core.state deleting.next).snapshot in - Alcotest.(check bool) - "graph and presentation fences advance" - true - (snapshot.startup.graph_generation > scope.graph_generation - && snapshot.startup.presentation_generation > scope.account.presentation_generation); - let blocked = late_catalog :: deletion_blocked_events scope sync in - check_no_work deleting.next blocked; - let stale_scope = { scope with graph_generation = scope.graph_generation + 99 } in - check_no_work - deleting.next - [ Core.Graph_detached (stale_scope, Ok ()) - ; Core.Mirror_deleted - ( { account = scope.account; graph_id; scope = Core.effect_scope_of_graph scope } - , Ok () ) - ]; - let closed = Core.step deleting.next (Core.Graph_detached (scope, Ok ())) in - check_deletion_stage Deleting_mirror closed.next; - let request = deletion_mirror closed.effects in - Alcotest.(check bool) - "mirror carries captured scope" - true - (request.graph_id = graph_id && request.scope = Core.effect_scope_of_graph scope); - check_no_work closed.next (Core.Graph_detached (scope, Ok ()) :: blocked); - let removed = Core.step closed.next (Core.Mirror_deleted (request, Ok ())) in - check_deletion_stage Clearing_selection removed.next; - let cache = save_catalog_cache removed.effects in - Alcotest.(check bool) - "saved selection cleared; catalog retained" - true - (Core.catalog_cache_selected_graph cache = None - && Core.catalog_cache_graphs cache = [ { graph with encrypted } ]); - Alcotest.(check bool) - "active selection cleared after cleanup" - true - ((Core.state removed.next).snapshot.selected_graph = None); - check_no_work removed.next (Core.Mirror_deleted (request, Ok ()) :: blocked); - let saved = Core.step removed.next (deletion_save_completion removed.effects (Ok ())) in - let snapshot = (Core.state saved.next).snapshot in - Alcotest.(check bool) - "success reaches graph selection" - true - (snapshot.startup.awaiting_selection - && snapshot.selected_graph = None - && snapshot.local_deletion = None - && snapshot.last_error = None); - Alcotest.(check bool) - "completion only publishes" - true - (List.for_all - (function - | Core.Publish _ -> true - | _ -> false) - saved.effects); - List.iter - (fun transition -> - Alcotest.(check bool) - "cleanup performs no key or bootstrap requests" - true - (List.for_all - (function - | Core.Run (Core.Request (_, Core.Save_catalog _)) -> true - | Core.Run (Core.Request _) -> false - | _ -> true) - transition.Core.effects)) - [ deleting; closed; removed; saved ]; - check_no_work - saved.next - [ late_catalog - ; Core.Graph_detached (scope, Ok ()) - ; Core.Mirror_deleted (request, Ok ()) - ; deletion_save_completion old_effects (Error (Core.Effect_failed "stale")) - ]; - let restarting = - Core.step (initial ()) (Core.Restore_local_account { user_id = "user" }) - in - let restored = - Core.step restarting.next (load_catalog_completion restarting.effects (Some cache)) - in - Alcotest.(check bool) - "restart has no selected graph or opening work" - true - ((Core.state restored.next).snapshot.selected_graph = None - && (Core.state restored.next).snapshot.startup.awaiting_selection - && not (has_graph_open_work restored.effects)); - let catalog = refresh_catalog saved.next [ { graph with encrypted }; other_graph ] in - let other_selected = Core.step catalog.next (Core.Graph_selected other_graph_id) in - let other_mirror = inspect_mirror_request other_selected.effects in - Alcotest.(check bool) - "another explicit selection follows normal opening" - true - (other_mirror.graph = other_graph && other_mirror.scope.graph_id = other_graph_id); - check_no_work - other_selected.next - [ Core.Graph_detached (scope, Ok ()) - ; Core.Mirror_deleted (request, Ok ()) - ; Core.Graph_attached { scope; sync } - ; Core.Snapshot_activated { scope } - ]; - let selected = Core.step saved.next (Core.Graph_selected graph_id) in + check_deletion_stage Deleting_mirror deleting.next; + let request = deletion_mirror deleting.effects in + Alcotest.(check bool) "target differs from current" true + (request.graph_id = graph_id && scope.graph_id = other_graph_id); + Alcotest.(check bool) "selected graph and admitted session remain unchanged" true + ((Core.state deleting.next).snapshot.selected_graph = Some other_graph_id + && Core.admitted_graph_scope deleting.next = Some scope); + Alcotest.(check bool) "no active graph close or remote request" true + (List.for_all (function Core.Delegate (Core.Delete_mirror _) | Core.Publish _ -> true | _ -> false) deleting.effects); + check_no_work deleting.next + [ Core.Local_cache_deletion_requested graph_id; Core.Local_cache_deletion_requested other_graph_id ]; + let stale = { request with scope = { request.scope with lifecycle_generation = Some 99999L } } in + check_no_work deleting.next [ Core.Mirror_deleted (stale, Ok ()); Core.Graph_detached (old_scope, Ok ()) ]; + let removed = Core.step deleting.next (Core.Mirror_deleted (request, Ok ())) in + let snapshot = (Core.state removed.next).snapshot in + Alcotest.(check bool) "success retains current and catalog" true + (snapshot.selected_graph = Some other_graph_id && snapshot.local_deletion = None + && snapshot.catalog = (Core.state ready).snapshot.catalog + && Core.admitted_graph_scope removed.next = Some scope && snapshot.last_error = None); + check_no_work removed.next [ Core.Mirror_deleted (request, Ok ()); Core.Graph_detached (old_scope, Ok ()) ]; + let cache = save_catalog_cache selection_effects in + Alcotest.(check bool) "saved selection remains the other graph" true + (Core.catalog_cache_selected_graph cache = Some other_graph_id); + let restarting = Core.step (initial ()) (Core.Restore_local_account { user_id = "user" }) in + let restored = Core.step restarting.next (load_catalog_completion restarting.effects (Some cache)) in + Alcotest.(check bool) "restart restores unchanged selection" true + ((Core.state restored.next).snapshot.selected_graph = Some other_graph_id); + let selected = Core.step removed.next (Core.Graph_selected graph_id) in let mirror = inspect_mirror_request selected.effects in - Alcotest.(check bool) - "explicit selection uses a fresh scope" - true - (mirror.scope <> scope); - let absent = - Core.step selected.next (Core.Mirror_inspected (Core.Mirror_absent mirror.scope)) - in - Alcotest.(check bool) - "explicit selection starts normal absent-mirror flow" - true - (List.exists - (function - | Core.Run (Core.Request (_, Core.Load_and_unlock_graph_key _)) -> encrypted - | Core.Run (Core.Request (_, Core.Fetch_snapshot_baseline _)) -> not encrypted - | _ -> false) - absent.effects); - check_no_work - selected.next - [ Core.Graph_detached (scope, Ok ()) - ; Core.Mirror_deleted (request, Ok ()) - ; Core.Graph_attached { scope; sync } - ; Core.Snapshot_activated { scope } - ] + let absent = Core.step selected.next (Core.Mirror_inspected (Core.Mirror_absent mirror.scope)) in + Alcotest.(check bool) "deleted graph uses normal absent-mirror download" true + (List.exists (function + | Core.Run (Core.Request (_, Core.Fetch_snapshot_baseline _)) -> not encrypted + | Core.Run (Core.Request (_, Core.Load_and_unlock_graph_key _)) -> encrypted + | _ -> false) absent.effects); + let _ = sync in + () ;; let local_deletion_discards_unsynchronized_work () = - let core, connection = submitted_core () in - Alcotest.(check bool) - "fixture has an unacknowledged submission" - true - ((Core.state core).snapshot.sync_phase = Core.Submitting); - let deleting = Core.step core (Core.Local_cache_deletion_requested graph_id) in - check_deletion_stage Closing_graph deleting.next; - Alcotest.(check bool) - "deletion neither flushes nor waits for the remote submission" - true - (List.for_all - (function - | Core.Publish _ - | Core.Run (Core.Close_websocket _ | Core.Cancel_effects _) - | Core.Delegate (Core.Detach_graph _) -> true - | _ -> false) - deleting.effects); - check_no_work - deleting.next - [ Core.Websocket_message - ( connection - , Protocol.Server.Tx_batch_ok - { t = server_cursor_of_int 1; checksum = Some "0123456789abcdef" } ) - ]; - let closed = Core.step deleting.next (Core.Graph_detached (connection.graph, Ok ())) in - ignore (deletion_mirror closed.effects) + let submitted, connection = submitted_core () in + Alcotest.(check bool) "fixture has unacknowledged work" true + ((Core.state submitted).snapshot.sync_phase = Core.Submitting); + let ready, scope, _, _ = deletion_noncurrent submitted in + let deleting = Core.step ready (Core.Local_cache_deletion_requested graph_id) in + let request = deletion_mirror deleting.effects in + Alcotest.(check bool) "confirmed deletion does not wait for upload" true + (request.graph_id = graph_id + && Core.admitted_graph_scope deleting.next = Some scope + && List.for_all (function Core.Publish _ | Core.Delegate (Core.Delete_mirror _) -> true | _ -> false) deleting.effects); + check_no_work deleting.next + [ Core.Websocket_message (connection, Protocol.Server.Tx_batch_ok + { t = server_cursor_of_int 1; checksum = Some "0123456789abcdef" }) ]; + let removed = Core.step deleting.next (Core.Mirror_deleted (request, Ok ())) in + Alcotest.(check bool) "unsynced discard completes locally" true + ((Core.state removed.next).snapshot.local_deletion = None) ;; let local_deletion_failures_stop encrypted () = - let ready, scope, sync, _ = deletion_ready_graph encrypted in + let original, _, _, _ = deletion_ready_graph encrypted in + let ready, scope, _, _ = deletion_noncurrent original in let deleting = Core.step ready (Core.Local_cache_deletion_requested graph_id) in - let closed = Core.step deleting.next (Core.Graph_detached (scope, Ok ())) in - let request = deletion_mirror closed.effects in - let removed = Core.step closed.next (Core.Mirror_deleted (request, Ok ())) in - List.iter - (fun (stage, core, event, message) -> - let failed = Core.step core event in - let snapshot = (Core.state failed.next).snapshot in - Alcotest.(check bool) - "stage failure is terminal" - true - (snapshot.local_deletion = Some (Core.Deletion_failed stage) - && snapshot.sync_phase = Core.Failed - && not snapshot.startup.awaiting_selection); - Alcotest.(check (option string)) - "sanitized stage error" - (Some message) - snapshot.last_error; - Alcotest.(check bool) - "failure only publishes" - true - (List.for_all - (function - | Core.Publish _ -> true - | _ -> false) - failed.effects); - check_no_work - failed.next - (deletion_blocked_events scope sync - @ [ Core.Graph_detached (scope, Ok ()) - ; Core.Mirror_deleted (request, Ok ()) - ; deletion_save_completion removed.effects (Ok ()) - ])) - [ ( Core.Closing_graph - , deleting.next - , Core.Graph_detached (scope, Error "secret path") - , "Local graph close failed." ) - ; ( Core.Deleting_mirror - , closed.next - , Core.Mirror_deleted (request, Error "secret path") - , "Local graph deletion failed." ) - ; ( Core.Clearing_selection - , removed.next - , deletion_save_completion - removed.effects - (Error (Core.Effect_failed "secret path")) - , "Clearing the saved graph selection failed." ) - ] + let request = deletion_mirror deleting.effects in + let failed = Core.step deleting.next (Core.Mirror_deleted (request, Error "secret path")) in + let snapshot = (Core.state failed.next).snapshot in + Alcotest.(check bool) "failure releases the flight and retains current session" true + (snapshot.local_deletion = None && snapshot.selected_graph = Some other_graph_id + && Core.admitted_graph_scope failed.next = Some scope); + Alcotest.(check (option string)) "failure is sanitized" + (Some "Local graph deletion failed. Try again.") snapshot.last_error; + let retry = Core.step failed.next (Core.Local_cache_deletion_requested graph_id) in + let retry_request = deletion_mirror retry.effects in + Alcotest.(check bool) "retry has a distinct identity" true (retry_request <> request); + check_no_work retry.next [ Core.Mirror_deleted (request, Ok ()); Core.Mirror_deleted (request, Error "late") ]; + let recovered = Core.step retry.next (Core.Mirror_deleted (retry_request, Ok ())) in + Alcotest.(check bool) "retry recovers" true ((Core.state recovered.next).snapshot.last_error = None); + let reopened = Core.step failed.next (Core.Graph_selected graph_id) in + Alcotest.(check bool) "failed deletion can reopen intact graph" true + ((inspect_mirror_request reopened.effects).graph.graph_id = graph_id) +;; + +let local_deletion_account_change_fences_completion () = + let original, _, _, _ = deletion_ready_graph false in + let ready, _, _, _ = deletion_noncurrent original in + let deleting = Core.step ready (Core.Local_cache_deletion_requested graph_id) in + let request = deletion_mirror deleting.effects in + let changed = Core.step deleting.next (Core.Account_authenticated { user_id = Some "another-user" }) in + Alcotest.(check bool) "account change retires deletion" true + ((Core.state changed.next).snapshot.startup.account_generation + > (Core.state deleting.next).snapshot.startup.account_generation + && (Core.state changed.next).snapshot.local_deletion = None); + check_no_work changed.next [ Core.Mirror_deleted (request, Ok ()); Core.Mirror_deleted (request, Error "late") ] ;; let recovery_connection effects = @@ -4236,6 +4101,10 @@ let scenarios = "encrypted local deletion failures" `Quick (local_deletion_failures_stop true) + ; Alcotest.test_case + "local deletion account change fences completion" + `Quick + local_deletion_account_change_fences_completion ; Alcotest.test_case "cold start absent mirror reaches pull after snapshot activation" `Quick diff --git a/logseq_sync/test/runner_contract.ml b/logseq_sync/test/runner_contract.ml index 2fbafa2..515e51c 100644 --- a/logseq_sync/test/runner_contract.ml +++ b/logseq_sync/test/runner_contract.ml @@ -1484,6 +1484,24 @@ let test_cache_durable_staging () = (request (Core.Retain_staged_file file) = Ok (Core.Asset_retained None))) ;; +let test_cache_deletion_waits_for_native_lease () = + with_local_asset_cache (fun support request -> + let source = Filename.concat support "preview.bin" in + Out_channel.with_open_bin source (fun output -> output_string output "preview"); + let file, _, _ = cache_stage request source in + let lease, path = request (Core.Retain_staged_file file) |> retained_value in + cache_bool "deletion waits or reports resources in use" true + (Result.is_error (request Core.Delete_graph_assets)); + cache_bool "live preview keeps its file" true (Sys.file_exists path); + cache_unit (request (Core.Release_asset_file lease)); + (match request Core.Delete_graph_assets with + | Ok Core.Asset_unit -> () + | Error (Core.Asset_invalid_content message) -> Alcotest.fail message + | Error Core.Asset_cancelled -> Alcotest.fail "cleanup unexpectedly cancelled" + | _ -> Alcotest.fail "cleanup returned another result"); + cache_bool "retry removes released file" false (Sys.file_exists path)) +;; + let test_cache_staging_limits () = with_local_asset_cache ~pending_budget:4L (fun support request -> let source = Filename.concat support "picker.bin" in @@ -1638,6 +1656,7 @@ let test_cache_staged_preview () = let scenarios = scenarios + @ [ Alcotest.test_case "graph deletion waits for native preview lease" `Quick test_cache_deletion_waits_for_native_lease ] @ List.map (fun (name, test) -> Alcotest.test_case name `Quick test) [ "cache durable staging via submit", test_cache_durable_staging diff --git a/swift/JournalChrome.swift b/swift/JournalChrome.swift index 9b543bd..0d2702e 100644 --- a/swift/JournalChrome.swift +++ b/swift/JournalChrome.swift @@ -25,6 +25,7 @@ struct JournalSheetDismissButton: SwiftUI.View { @MainActor enum JournalChrome { private enum Mode: String, Decodable { case page, header, detail, unlock, startup + case graphPicker = "graph-picker" case toolbarControl = "toolbar-control" case detailText = "detail-text" case boundedText = "bounded-text" @@ -136,12 +137,13 @@ struct JournalSheetDismissButton: SwiftUI.View { let cluster: AnyView let progress: AnyView let properties: Properties + var leading: AnyView? = nil @Environment(\.journalContainerTopInset) private var containerTopInset @State private var controlsSize = CGSize.zero var body: some SwiftUI.View { GeometryReader { bounds in - content.frame(width: bounds.size.width, height: bounds.size.height) + scrollingContent.frame(width: bounds.size.width, height: bounds.size.height) .scrollContentBackground(.hidden) #if os(iOS) // Journals has no section title; named sections retain native indicators. @@ -161,6 +163,11 @@ struct JournalSheetDismissButton: SwiftUI.View { controls.padding(.trailing, 16) } } + .overlay(alignment: .topLeading) { + if let leading { + leading.glassEffect(.regular, in: .capsule).padding(.leading, 16) + } + } .safeAreaBar(edge: .top, spacing: 0) { if let title = properties.title { HStack(alignment: .center, spacing: 12) { @@ -187,6 +194,14 @@ struct JournalSheetDismissButton: SwiftUI.View { #endif } + @ViewBuilder private var scrollingContent: some SwiftUI.View { + if properties.mode == .graphPicker { + content.contentMargins(.top, 52, for: .scrollContent) + } else { + content + } + } + private var controls: some SwiftUI.View { HStack(spacing: 8) { if properties.connecting! { progress.controlSize(.small).fixedSize() } @@ -237,6 +252,12 @@ struct JournalSheetDismissButton: SwiftUI.View { FloatingChrome(content: child(0), cluster: child(1), progress: child(2), properties: properties) } + case .graphPicker: + if let properties, context.childIDs.count == 4, + properties.connecting != nil, properties.controls != nil { + FloatingChrome(content: child(0), cluster: child(1), + progress: child(2), properties: properties, leading: child(3)) + } case .startup: if let properties, context.childIDs.count == 1 { GeometryReader { bounds in diff --git a/swift/JournalIcons.swift b/swift/JournalIcons.swift index a03362c..9cca2a6 100644 --- a/swift/JournalIcons.swift +++ b/swift/JournalIcons.swift @@ -25,6 +25,7 @@ let journalIconNames: [String] = [ "exclamationmark.circle", "exclamationmark.triangle", "folder", + "icloud.and.arrow.down", "lock.doc", "lock.shield", "minus.circle", diff --git a/test/application_view_test.ml b/test/application_view_test.ml index 6e0f7bb..626af8b 100644 --- a/test/application_view_test.ml +++ b/test/application_view_test.ml @@ -610,6 +610,7 @@ let run_favorites_native_visibility Ok ()) ~handle:(fun context () request -> match request with + | Service.Inspect_local_copy _ | Service.Delete_local_copy _ -> Error "Unused fixture local-copy query" | Service.Get_graph_state -> if Atomic.get import_probe then Atomic.set import_probed true; if Atomic.exchange publish_block false @@ -3661,6 +3662,7 @@ let test_reference_outer_terminals cancel = })) in match request with + | Service.Inspect_local_copy _ | Service.Delete_local_copy _ -> Error "Unused fixture local-copy query" | Service.Get_graph_state -> if Atomic.exchange refresh false then @@ -4312,9 +4314,9 @@ let test_reactive_header_current_controls () = ignore (Lui_app.flush app); Alcotest.(check int) "Capture consults current disabled gate" 0 !capture_count; Alcotest.(check bool) - "current cache capability exposes menu action" + "current graph cannot be deleted from the account menu" true - (Option.is_some (find TextValue "Delete local graph copy")); + (Option.is_none (find TextValue "Delete local graph copy")); ignore (Lui_app.dispatch_event app @@ -4456,6 +4458,7 @@ let unlock_fixture ?(worker_failure = false) ?(queue_full = false) () = (Service.Client_state_changed value) in match request with + | Service.Inspect_local_copy _ | Service.Delete_local_copy _ -> Error "Unused fixture local-copy query" | Service.Get_graph_state -> if queue_full && Atomic.fetch_and_add graph_reads 1 > 0 then ( while not (Atomic.get release) do @@ -4902,6 +4905,9 @@ let test_graph_picker_controls scenario () = ; last_error = None; local_deletion = None } ; diagnostics = { groups = [] } } in let current = Atomic.make manager and commands = Atomic.make [] in + let inspections = Atomic.make 0 and deletions = Atomic.make [] in + let graph_opens = Atomic.make 0 in + let root_renders = ref 0 in let client = ref None in let service = W.Service.create ~push_topic_count:6 ~concurrency:Serial ~init:(fun context _ -> @@ -4912,6 +4918,14 @@ let test_graph_picker_controls scenario () = Atomic.set current value; W.Request_context.emit context ~topic:Service.manager_topic (Service.Client_state_changed value) in match request with + | Service.Inspect_local_copy request -> + Atomic.incr inspections; + Ok (Service.Local_copy_inspected (request, + if scenario = `Presence_failure then Error "Synthetic presence read failed. Try again." + else Ok (if scenario = `Visible then Service.Not_downloaded else Service.Downloaded))) + | Service.Delete_local_copy request -> + Atomic.set deletions (request :: Atomic.get deletions); + Ok (Service.Local_copy_deleted (request, if scenario = `Failure_select then Error "Synthetic delete failed. Try again." else Ok ())) | Service.Get_graph_state -> let value = Atomic.get current in Ok (Service.Graph_state { generation = value.snapshot.startup.graph_generation @@ -4935,7 +4949,7 @@ let test_graph_picker_controls scenario () = | Acquire_asset_file _ | Acquire_imported_file _ -> Ok (Service.Asset_file None) | Graph_request request -> let outcome = match request.P.command with - | V2_graph_info -> P.V2_graph_info_outcome + | V2_graph_info -> Atomic.incr graph_opens; P.V2_graph_info_outcome { graph_uuid = Option.get (Atomic.get current).snapshot.selected_graph ; graph_name = "Synthetic"; schema = { major = 65; minor = 33 } ; admission_facts = []; journal_title_format = None @@ -4952,16 +4966,22 @@ let test_graph_picker_controls scenario () = { api_version = P.api_version; request_id = request.request_id; outcome })) | Import_asset _ -> Error "Unused graph-picker fixture import") ~shutdown:(fun () -> ()) () in - let hooks = Application.For_testing.app_with_service ~on_client:(fun value -> client := Some value) service in + let hooks = Application.For_testing.app_with_service + ~on_view_region:(fun region -> if region = "root" then incr root_renders) + ~on_client:(fun value -> client := Some value) service in let props = Hashtbl.create 128 in - let teardown = track_json_wire_teardown (Hashtbl.remove props) in + let retained_select = ref None and select_detached = ref false in + let teardown = track_json_wire_teardown (fun id -> + if !retained_select = Some id then select_detached := true; + Hashtbl.remove props id) in let consume source = if source <> "" then let open Yojson.Safe.Util in Yojson.Safe.from_string source |> member "ops" |> to_list |> List.iter (fun op -> teardown op; match member "op" op |> to_string with - | "create-node" | "create-extension" -> Hashtbl.replace props (member "id" op |> to_int) [] + | "create-node" | "create-extension" -> + Hashtbl.replace props (member "id" op |> to_int) ["__kind", member "kind" op; "_extension", member "identifier" op] | "set-prop" | "set-extension-prop" -> let id = member "id" op |> to_int in let key = member "property" op |> to_string in @@ -4993,6 +5013,17 @@ let test_graph_picker_controls scenario () = press (Option.get (find "text" "Switch graph")); wait "graph list mounts" (fun () -> Option.is_some (find "accessibility-identifier" "graph-picker-list")); match scenario with + | `Visible -> + let before = !root_renders in + let list = Option.get (find "_extension" "journal-list") in + let range () = consume (hooks.extension_event list "event" + {|{"id":1,"payload":"{\"type\":\"visible_range\",\"first\":0,\"last\":9223372036854775807}"}|}) in + range (); + wait "visible rows are inspected" (fun () -> Atomic.get inspections = 2 + && Option.is_some (find "text" "Not downloaded")); + for _ = 1 to 10 do range (); consume (hooks.pump ()) done; + Alcotest.(check int) "same viewport reuses presence observations" 2 (Atomic.get inspections); + Alcotest.(check int) "presence does not reconstruct the graph catalog" before !root_renders | `Current -> Alcotest.(check bool) "current marker stays in catalog row" true (Option.is_some (find "text" "Current graph")); @@ -5009,20 +5040,63 @@ let test_graph_picker_controls scenario () = press (node ("graph-picker:" ^ Logseq_db_types.Graph_types.Uuid.to_string second)); wait "name selection reaches worker" (fun () -> List.exists (function Service.Select_graph id -> id = second | _ -> false) (Atomic.get commands)) - | `Delete_menu -> + | (`Delete_menu | `Cancel | `Confirm | `Reopen | `Failure_select | `Presence_failure) -> + let stable_select = node ("graph-picker:" ^ Logseq_db_types.Graph_types.Uuid.to_string second) in + if scenario = `Failure_select then retained_select := Some stable_select; ignore (node ("graph-picker-options:" ^ Logseq_db_types.Graph_types.Uuid.to_string second)); wait "row action is exposed" (fun () -> Option.is_some (find "text" "Delete local copy")); press (node ("graph-picker-delete:" ^ Logseq_db_types.Graph_types.Uuid.to_string second)); + if scenario = `Presence_failure then ( + wait "presence failure is visible" (fun () -> Option.is_some (find "text" "Synthetic presence read failed. Try again.")); + Alcotest.(check int) "one bounded presence request" 1 (Atomic.get inspections)) + else ( wait "confirm names data loss" (fun () -> Hashtbl.fold (fun _ values found -> found || List.exists (fun (_, value) -> match value with `String text -> String.starts_with ~prefix:"Remove “" text && String.contains text '\n' | _ -> false) values) props false); Alcotest.(check bool) "requesting confirmation does not delete" false - (List.exists (function Service.Delete_local_cache _ -> true | _ -> false) (Atomic.get commands)) + (List.exists (function Service.Delete_local_cache _ -> true | _ -> false) (Atomic.get commands)); + wait "single presence request completes" (fun () -> Atomic.get inspections = 1); + for _ = 1 to 8 do consume (hooks.pump ()) done; + Alcotest.(check int) "rendering never repeats presence IO" 1 (Atomic.get inspections); + if scenario = `Cancel then ( + press (Option.get (find "text" "Cancel")); + Alcotest.(check int) "Cancel sends no deletion" 0 (List.length (Atomic.get deletions))) + else if scenario = `Confirm || scenario = `Reopen || scenario = `Failure_select then ( + let confirm = Hashtbl.fold (fun id values found -> + if List.assoc_opt "text" values = Some (`String "Delete local copy") + && List.assoc_opt "__kind" values <> Some (`String "menu-item") + then Some id else found) props None |> Option.get in + press confirm; + wait "confirmed delete reaches account-scoped worker request" (fun () -> Atomic.get deletions <> []); + let deleted = List.hd (Atomic.get deletions) in + Alcotest.(check bool) "deletion identity is exact" true + (deleted.graph_id = second && deleted.account_generation = 1); + if scenario = `Failure_select then ( + wait "delete failure is visible" (fun () -> Option.is_some (find "text" "Synthetic delete failed. Try again.")); + Alcotest.(check bool) "native List retains live interactive row during operation status changes" false !select_detached; + press stable_select; + wait "existing row callback retains its graph after failure" (fun () -> + List.exists (function Service.Select_graph id -> id = second | _ -> false) (Atomic.get commands))) + else wait "deleted graph stays available to download" (fun () -> Option.is_some (find "text" "Not downloaded")); + if scenario = `Reopen then ( + let switch id = + let before = Atomic.get graph_opens in + press (node ("graph-picker:" ^ Logseq_db_types.Graph_types.Uuid.to_string id)); + wait "reopened graph becomes current" (fun () -> (Atomic.get current).snapshot.selected_graph = Some id); + wait "reopened graph handshake runs" (fun () -> Atomic.get graph_opens > before); + wait "reopened graph presents timeline" (fun () -> Option.is_none (find "accessibility-identifier" "graph-picker-list") + && Option.is_some (find "text" "Switch graph")); + press (Option.get (find "text" "Switch graph")); + wait "picker returns after reopening" (fun () -> Option.is_some (find "accessibility-identifier" "graph-picker-list")) in + switch second; + switch first; + Alcotest.(check bool) "reopened copy does not keep the stale not-downloaded label" true + (Option.is_none (find "text" "Not downloaded"))))) | `Current_menu -> ignore (node ("graph-picker-options:" ^ Logseq_db_types.Graph_types.Uuid.to_string first)); Alcotest.(check bool) "current protection explains switching first" true - (Option.is_some (find "text" "Switch to another graph before deleting."))) + (Option.is_some (find "text" "Switch graphs first."))) ;; let () = @@ -5037,7 +5111,13 @@ let () = Alcotest.test_case "close retains current session" `Quick (test_graph_picker_controls `Close); Alcotest.test_case "name switches graph" `Quick (test_graph_picker_controls `Select); Alcotest.test_case "row deletion confirmation" `Quick (test_graph_picker_controls `Delete_menu); - Alcotest.test_case "current row deletion protection" `Quick (test_graph_picker_controls `Current_menu)]) + Alcotest.test_case "current row deletion protection" `Quick (test_graph_picker_controls `Current_menu); + Alcotest.test_case "cancel deletion keeps local copy" `Quick (test_graph_picker_controls `Cancel); + Alcotest.test_case "confirmed deletion preserves downloadable row" `Quick (test_graph_picker_controls `Confirm); + Alcotest.test_case "presence failure is bounded and visible" `Quick (test_graph_picker_controls `Presence_failure); + Alcotest.test_case "visible presence is bounded and cached" `Quick (test_graph_picker_controls `Visible); + Alcotest.test_case "reopened copy invalidates absence observation" `Quick (test_graph_picker_controls `Reopen); + Alcotest.test_case "deletion failure keeps live row callback identity" `Quick (test_graph_picker_controls `Failure_select)]) ; ( "relative date chrome", [ Alcotest.test_case "calendar boundaries and date updates" `Quick test_timeline_relative_date_headers; Alcotest.test_case "empty Detail navigation title" `Quick test_detail_empty_navigation_title; diff --git a/test/journal_semantics_test.ml b/test/journal_semantics_test.ml index 9b11e4b..2fa9727 100644 --- a/test/journal_semantics_test.ml +++ b/test/journal_semantics_test.ml @@ -822,9 +822,9 @@ let test_timeline_options_menu () = | _ -> None) (ops ()) |> Option.get in ignore (Lui_app.dispatch_event app (Lui_protocol.Press node)); ignore (Lui_app.flush app)) - [ "Attachment settings"; "Diagnostics"; "Switch graph"; "Delete local graph copy"; "Sign out" ]; + [ "Attachment settings"; "Diagnostics"; "Switch graph"; "Sign out" ]; require (List.rev !actions = Journal_header.[Open_asset_settings; Open_diagnostics; - Switch_graph; Request_local_cache_reset; Sign_out]) "Options lost an existing action") + Switch_graph; Sign_out]) "Options lost an existing action") ;; let media_item n file_type presentation size = diff --git a/test/source_boundary_test.ml b/test/source_boundary_test.ml index 145cdd4..a82c312 100644 --- a/test/source_boundary_test.ml +++ b/test/source_boundary_test.ml @@ -1539,14 +1539,14 @@ let () = ; "Request_local_cache_reset" ; "Action \"cancel\"" ; "V.Confirmation.alert" - ; "Graph_service.Delete_local_cache" + ; "Graph_service.Delete_local_copy" ; "Graph_service.Return_to_graph_picker" ; "Presentation.list" ; "graph-picker-list" ; "e2ee-password-cancel" ; "Refresh the authorized graph catalog" - ; "pending local" - ; "then returns to graph selection" + ; "Unsynced changes, drafts" + ; "Your cloud copy will be" ; "Lui_app.create_with_extensions" ; "application_theme" ; "V.Sheet.create" diff --git a/test/startup_test.ml b/test/startup_test.ml index c1493ca..95b8da0 100644 --- a/test/startup_test.ml +++ b/test/startup_test.ml @@ -215,23 +215,13 @@ let test_stale_graph_generation_and_structured_failures () = | None | Some _ -> fail "graph failure did not expose structured recovery" ;; -let test_deletion_failure_has_no_recovery () = - let snapshot = - { (startup_snapshot ()) with - local_deletion = Some (Graph_service.Deletion_failed Closing_graph) - ; last_error = Some "Local graph close failed." - } - in - let derived = Journal_startup.derive ~snapshot ~graph:(graph_state ()) in - require - (derived.phase = Journal_startup.Failed) - "deletion failure returned to ready timeline"; - require - (Option.fold - ~none:false - ~some:(fun error -> error.Journal_startup.recovery = None) - derived.error) - "deletion failure offered recovery" +let test_noncurrent_deletion_keeps_active_graph_ready () = + List.iter (fun local_deletion -> + let snapshot = { (startup_snapshot ()) with local_deletion = Some local_deletion } in + let derived = Journal_startup.derive ~snapshot ~graph:(graph_state ()) in + require (derived.phase = Journal_startup.Ready) + "non-current deletion displaced the active graph") + [ Graph_service.Deletion_in_progress Deleting_mirror; Deletion_failed Deleting_mirror ] ;; let test_stable_startup_presentation () = @@ -366,7 +356,7 @@ let test_e2ee_recovery_requires_a_prepared_challenge () = let () = test_e2ee_recovery_requires_a_prepared_challenge (); test_stable_startup_presentation (); - test_deletion_failure_has_no_recovery (); + test_noncurrent_deletion_keeps_active_graph_ready (); test_exact_codec_and_round_trip (); test_bounded_rejection (); test_startup_phase_is_owned_by_ui_domain (); From a6bfadfb6e8f2040486bec88f2575e8f477bc439 Mon Sep 17 00:00:00 2001 From: rcmerci Date: Thu, 15 Oct 2026 18:06:34 +0800 Subject: [PATCH 3/8] Keep current graph responsive during local-copy deletion --- app/application.ml | 18 ++------- .../2026-10-15-switch-graph-native-picker.md | 2 + test/application_view_test.ml | 38 ++++++++++++++++--- 3 files changed, 39 insertions(+), 19 deletions(-) diff --git a/app/application.ml b/app/application.ml index a5b1e00..264d57a 100644 --- a/app/application.ml +++ b/app/application.ml @@ -3852,7 +3852,6 @@ let start && !state_ref.graph_state.generation = fst owner && !state_ref.graph_state.phase = Graph_open && !state_ref.graph_ready - && not (local_deletion_active !state_ref) in let copy_error owner message = set_state (fun state -> @@ -4464,7 +4463,7 @@ let start let result = match outcome with | Worker.Completed (Graph_service.Local_copy_deleted (actual, result)) when actual = request -> result | _ -> Error "Local copy deletion was interrupted. Check the copy and try again." in - let state = { state with local_copy_deleting = None; modal = No_modal } in + let state = { state with local_copy_deleting = None } in match result with | Error message -> { state with local_copy_message = Some message } | Ok () -> { (discard_graph_copy_draft state request.graph_id) with @@ -4482,7 +4481,9 @@ let start result state.local_copies } in if not confirm || not visible || manager.selected_graph = Some request.graph_id then state else match result with - | Ok Graph_service.Downloaded -> { state with modal = Cache_reset_confirmation request.graph_id; local_copy_message = None } + | Ok Graph_service.Downloaded -> { state with modal = Cache_reset_confirmation request.graph_id + ; confirmation_sequence = Int64.succ state.confirmation_sequence + ; local_copy_message = None } | Ok Not_downloaded -> { state with local_copy_message = Some "No local copy on this device." } | Error message -> { state with local_copy_message = Some message }))) ~f:(fun () -> !drain_visible_copies ()) @@ -5630,14 +5631,6 @@ let start | action -> action in match payload with - | payload - when local_deletion_active snapshot - && - match payload with - | Open_diagnostics | Close_diagnostics | Close_graph_picker | Sign_out - | Native_payload (Ui.Event.Payload.Navigation_path_changed _) -> false - | Asset_settings _ -> false - | _ -> true -> Effect.ignore | Startup_retry -> Effect.bind (update (fun state -> {state with startup_presentation= Journal_startup.Presentation.step state.startup_presentation Retry})) ~f:(fun () -> managed_startup) @@ -5649,9 +5642,6 @@ let start | Some Dismissed -> update (fun state -> { state with asset_settings_open = false }) | Some (Retry_upload operation) -> - if local_deletion_active snapshot - then Effect.ignore - else Effect.of_thunk (fun () -> let current = !state_ref in let uploads = diff --git a/docs/agent-guide/proposed/feature/2026-10-15-switch-graph-native-picker.md b/docs/agent-guide/proposed/feature/2026-10-15-switch-graph-native-picker.md index dd172de..cb0f052 100644 --- a/docs/agent-guide/proposed/feature/2026-10-15-switch-graph-native-picker.md +++ b/docs/agent-guide/proposed/feature/2026-10-15-switch-graph-native-picker.md @@ -38,6 +38,8 @@ Presence observations are cached by account and invalidated for a graph lifecycl Deletion uses a single account-scoped flight and exact terminal result. It preserves the selected graph and catalog, validates persisted local ownership, cancels target work, waits for native leases, and revalidates before mirror removal under the existing exclusive ownership primitive. Unsynchronized local content is intentionally discarded after the warning. Unbound legacy mirrors require opening once and switching away before removal. +The active graph remains usable when the picker closes during deletion of another graph. Application owns the manager snapshot and action gate; the public Root_navigation reducer does not own either, so its events cannot reproduce that guard. A regression at the public mounted Application boundary holds deletion in flight and exercises the current graph's Capture action. Each new confirmation also advances its request token so an older response cannot authorize a newer dialog. + ## Questions - Design A, Simulator verification, discard semantics and PR publication are already answered by the user's recorded instructions. No unanswered UI/presence question blocks this scope. diff --git a/test/application_view_test.ml b/test/application_view_test.ml index 626af8b..55b6ebb 100644 --- a/test/application_view_test.ml +++ b/test/application_view_test.ml @@ -4906,6 +4906,8 @@ let test_graph_picker_controls scenario () = ; diagnostics = { groups = [] } } in let current = Atomic.make manager and commands = Atomic.make [] in let inspections = Atomic.make 0 and deletions = Atomic.make [] in + let finish_deletion = Atomic.make false in + let deletion_terminal = ref false in let graph_opens = Atomic.make 0 in let root_renders = ref 0 in let client = ref None in @@ -4925,6 +4927,11 @@ let test_graph_picker_controls scenario () = else Ok (if scenario = `Visible then Service.Not_downloaded else Service.Downloaded))) | Service.Delete_local_copy request -> Atomic.set deletions (request :: Atomic.get deletions); + if scenario = `During_delete then ( + let previous = Atomic.get current in + publish { previous with snapshot = { previous.snapshot with + local_deletion = Some (Deletion_in_progress Deleting_mirror) } }; + while not (Atomic.get finish_deletion) do Unix.sleepf 0.001 done); Ok (Service.Local_copy_deleted (request, if scenario = `Failure_select then Error "Synthetic delete failed. Try again." else Ok ())) | Service.Get_graph_state -> let value = Atomic.get current in @@ -4968,7 +4975,12 @@ let test_graph_picker_controls scenario () = ~shutdown:(fun () -> ()) () in let hooks = Application.For_testing.app_with_service ~on_view_region:(fun region -> if region = "root" then incr root_renders) - ~on_client:(fun value -> client := Some value) service in + ~on_client:(fun value -> + client := Some value; + W.on_event value (function + | W.Response { outcome = Completed (Service.Local_copy_deleted _); _ } -> + deletion_terminal := true + | _ -> ())) service in let props = Hashtbl.create 128 in let retained_select = ref None and select_detached = ref false in let teardown = track_json_wire_teardown (fun id -> @@ -5006,6 +5018,7 @@ let test_graph_picker_controls scenario () = ~response_budget_bytes:P.maximum_response_bytes ~default_page_size:P.default_page_size |> Result.get_ok |> Journal_startup.encode |> Result.get_ok |> Bytes.to_string in Fun.protect ~finally:(fun () -> + Atomic.set finish_deletion true; ignore (hooks.dispose ()); Option.iter Logseq_db_worker_lui.Journal_worker_runtime.stop !client) (fun () -> consume (hooks.init 2 2 config); @@ -5040,7 +5053,7 @@ let test_graph_picker_controls scenario () = press (node ("graph-picker:" ^ Logseq_db_types.Graph_types.Uuid.to_string second)); wait "name selection reaches worker" (fun () -> List.exists (function Service.Select_graph id -> id = second | _ -> false) (Atomic.get commands)) - | (`Delete_menu | `Cancel | `Confirm | `Reopen | `Failure_select | `Presence_failure) -> + | (`Delete_menu | `Cancel | `Confirm | `Reopen | `Failure_select | `Presence_failure | `During_delete) -> let stable_select = node ("graph-picker:" ^ Logseq_db_types.Graph_types.Uuid.to_string second) in if scenario = `Failure_select then retained_select := Some stable_select; ignore (node ("graph-picker-options:" ^ Logseq_db_types.Graph_types.Uuid.to_string second)); @@ -5062,7 +5075,7 @@ let test_graph_picker_controls scenario () = if scenario = `Cancel then ( press (Option.get (find "text" "Cancel")); Alcotest.(check int) "Cancel sends no deletion" 0 (List.length (Atomic.get deletions))) - else if scenario = `Confirm || scenario = `Reopen || scenario = `Failure_select then ( + else if scenario = `Confirm || scenario = `Reopen || scenario = `Failure_select || scenario = `During_delete then ( let confirm = Hashtbl.fold (fun id values found -> if List.assoc_opt "text" values = Some (`String "Delete local copy") && List.assoc_opt "__kind" values <> Some (`String "menu-item") @@ -5072,7 +5085,21 @@ let test_graph_picker_controls scenario () = let deleted = List.hd (Atomic.get deletions) in Alcotest.(check bool) "deletion identity is exact" true (deleted.graph_id = second && deleted.account_generation = 1); - if scenario = `Failure_select then ( + if scenario = `During_delete then ( + wait "worker publishes deletion in flight" (fun () -> + (Atomic.get current).snapshot.local_deletion <> None); + for _ = 1 to 8 do consume (hooks.pump ()) done; + press (node "graph-picker-close"); + wait "Timeline is reachable while another graph deletes" (fun () -> + Option.is_some (find "accessibility-label" "Capture")); + press (Option.get (find "accessibility-label" "Capture")); + wait "current graph Capture responds during noncurrent deletion" (fun () -> + Option.is_some (find "accessibility-identifier" "journal-capture-composer")); + Atomic.set finish_deletion true; + wait "noncurrent deletion reaches its terminal" (fun () -> !deletion_terminal); + Alcotest.(check bool) "deletion completion preserves current graph composer" true + (Option.is_some (find "accessibility-identifier" "journal-capture-composer"))) + else if scenario = `Failure_select then ( wait "delete failure is visible" (fun () -> Option.is_some (find "text" "Synthetic delete failed. Try again.")); Alcotest.(check bool) "native List retains live interactive row during operation status changes" false !select_detached; press stable_select; @@ -5117,7 +5144,8 @@ let () = Alcotest.test_case "presence failure is bounded and visible" `Quick (test_graph_picker_controls `Presence_failure); Alcotest.test_case "visible presence is bounded and cached" `Quick (test_graph_picker_controls `Visible); Alcotest.test_case "reopened copy invalidates absence observation" `Quick (test_graph_picker_controls `Reopen); - Alcotest.test_case "deletion failure keeps live row callback identity" `Quick (test_graph_picker_controls `Failure_select)]) + Alcotest.test_case "deletion failure keeps live row callback identity" `Quick (test_graph_picker_controls `Failure_select); + Alcotest.test_case "noncurrent deletion keeps current Timeline interactive" `Quick (test_graph_picker_controls `During_delete)]) ; ( "relative date chrome", [ Alcotest.test_case "calendar boundaries and date updates" `Quick test_timeline_relative_date_headers; Alcotest.test_case "empty Detail navigation title" `Quick test_detail_empty_navigation_title; From 0e3075c6145e5d9e3e8e0f1e1ecf753a5bf6ce6a Mon Sep 17 00:00:00 2001 From: rcmerci Date: Thu, 15 Oct 2026 18:18:54 +0800 Subject: [PATCH 4/8] Record verified Switch Graph implementation --- .../feature/2026-10-15-switch-graph-native-picker.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) rename docs/agent-guide/{proposed => implemented}/feature/2026-10-15-switch-graph-native-picker.md (75%) diff --git a/docs/agent-guide/proposed/feature/2026-10-15-switch-graph-native-picker.md b/docs/agent-guide/implemented/feature/2026-10-15-switch-graph-native-picker.md similarity index 75% rename from docs/agent-guide/proposed/feature/2026-10-15-switch-graph-native-picker.md rename to docs/agent-guide/implemented/feature/2026-10-15-switch-graph-native-picker.md index cb0f052..f6b31f8 100644 --- a/docs/agent-guide/proposed/feature/2026-10-15-switch-graph-native-picker.md +++ b/docs/agent-guide/implemented/feature/2026-10-15-switch-graph-native-picker.md @@ -4,7 +4,7 @@ The graph picker uses uneven capsule buttons and loses the active graph when opened. It has no per-row local-copy management or durable presence observation. -## Proposal +## Decision Implement the user's selected concept A with native equal-width list rows, a full-canvas List with Timeline-style floating close/refresh/existing account controls, an inline current marker and a separate row menu. Keep picker and request ownership in OCaml. Query one graph's mirror presence on demand with an account-generation fence; cache observations outside rendering. @@ -18,14 +18,17 @@ The user subsequently requested removing the header entirely and making the whol It detaches the current graph and clears its identity. Keep an application-owned picker presentation instead, and use the existing Select_graph lifecycle only for an actual switch. -## Acceptance criteria +## Validation - Uniform native rows preserve catalog order, expose the current marker and support long names and Dynamic Type. - Close returns to the retained current graph; name activation switches through the worker. - Row menus are independently reachable; current graph deletion is disabled with an explanation. - Presence is bounded and requested only for the relevant graph, with stale and cancelled results ignored. - Confirmation names the graph, warns that unsynchronized changes/drafts/attachments will be lost and says the cloud copy remains downloadable. -- Isolated synthetic Simulator acceptance covers cancellation, deletion, failure recovery and redownload before PR publication. +- Two native XCTest cases pass on the isolated iPhone 13 / iOS 26.1 Simulator: actual menu and row taps, cancellation, current-graph protection, ownership-busy failure and retry, retained Not downloaded rows, real-worker snapshot redownload through a controlled loopback TLS fixture, close/session retention and accessibility XXXL scrolling/menu reachability. The fixture does not implement a live remote sync service. +- Real UI submission also produced an unsent text capture and photo attachment; inspection found three durable outbox records and one account/graph-specific upload intent before confirmed local deletion removed that copy and its intent. +- Application regression: 98 tests pass, including deletion-in-flight Capture and composer retention. Sync and overlay storage suites pass 261 and 50 cases respectively; worker service, startup, semantics and source-boundary checks pass. +- Independent read-only journal-code-review found and prompted fixes for action gating and confirmation identity; the code-fix commit was re-reviewed with no remaining evidenced findings. All temporary reports, builds, hosts, network fixtures and screenshots remain outside Git. ## Risks @@ -40,6 +43,3 @@ Deletion uses a single account-scoped flight and exact terminal result. It prese The active graph remains usable when the picker closes during deletion of another graph. Application owns the manager snapshot and action gate; the public Root_navigation reducer does not own either, so its events cannot reproduce that guard. A regression at the public mounted Application boundary holds deletion in flight and exercises the current graph's Capture action. Each new confirmation also advances its request token so an older response cannot authorize a newer dialog. -## Questions - -- Design A, Simulator verification, discard semantics and PR publication are already answered by the user's recorded instructions. No unanswered UI/presence question blocks this scope. From 6a8e3174ae3c24077bd4892054856221a6104f0b Mon Sep 17 00:00:00 2001 From: rcmerci Date: Thu, 15 Oct 2026 18:39:18 +0800 Subject: [PATCH 5/8] Handle local-copy responses in managed-sync test harness --- logseq_db_worker/test/test_managed_sync_e2e.ml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/logseq_db_worker/test/test_managed_sync_e2e.ml b/logseq_db_worker/test/test_managed_sync_e2e.ml index a4804f0..3f3db35 100644 --- a/logseq_db_worker/test/test_managed_sync_e2e.ml +++ b/logseq_db_worker/test/test_managed_sync_e2e.ml @@ -304,6 +304,9 @@ let handle_event context = function | Response { outcome = Completed (Service.Asset_imported _); _ } | Response { outcome = Completed (Service.Asset_file _); _ } -> fail_worker context "assets" "unexpected file response" + | Response { outcome = Completed (Service.Local_copy_inspected _); _ } + | Response { outcome = Completed (Service.Local_copy_deleted _); _ } -> + fail_worker context "local-copy" "unexpected local-copy response" | Response { outcome = Completed Service.Client_command_completed; _ } -> None | Response { outcome = Completed (Service.Graph_state state); _ } -> handle_graph_state context state; From 4ad4681c9ecd7d04d099e779e96b7172cf6ef64e Mon Sep 17 00:00:00 2001 From: rcmerci Date: Fri, 16 Oct 2026 09:57:57 +0800 Subject: [PATCH 6/8] Make local graph removal recoverable and retire legacy APIs --- app/application.ml | 8 + .../2026-10-15-switch-graph-native-picker.md | 61 ++-- logseq_db_storage/lib/asset_upload_store.ml | 83 ++++- logseq_db_storage/lib/asset_upload_store.mli | 17 +- .../test/test_asset_upload_store.ml | 44 ++- logseq_db_types/lib/graph_types.ml | 6 + logseq_db_types/lib/graph_types.mli | 7 + .../lib/effect_runner/effect_runner.ml | 197 ++++++++--- logseq_db_worker/lib/logseq_db_worker.ml | 19 +- logseq_db_worker/test/test_effect_runner.ml | 147 +++++++++ logseq_db_worker/test/test_pure_reducer.ml | 31 +- logseq_overlay_db/lib/database.ml | 239 ++++++++++++-- logseq_overlay_db/spec/database.mli | 30 +- .../test/test_overlay_storage.ml | 98 +++++- .../lib/effect_runner/effect_runner.ml | 140 +++++--- .../platform/platform_crypto_stubs.c | 22 ++ .../lib/effect_runner/storage/bootstrap.ml | 309 +++++++++++++----- .../lib/effect_runner/storage/bootstrap.mli | 19 +- logseq_sync/lib/pure_reducer/core.ml | 84 ++++- logseq_sync/spec/pure_reducer/core.mli | 23 +- logseq_sync/test/core_contract.ml | 63 +++- logseq_sync/test/runner_contract.ml | 82 ++++- logseq_sync/test/transport_contract.ml | 6 +- test/application_view_test.ml | 38 ++- test/logseq_overlay_db_boundary_test.ml | 4 +- 25 files changed, 1475 insertions(+), 302 deletions(-) diff --git a/app/application.ml b/app/application.ml index 264d57a..7492607 100644 --- a/app/application.ml +++ b/app/application.ml @@ -4109,6 +4109,14 @@ let start in let local_copy_request ?(confirm = true) ~deleting (request : Graph_service.local_copy_request) = Effect.of_thunk (fun () -> + if confirm then ( + let superseded = Hashtbl.fold (fun id (previous, pending_delete, _) ids -> + if not pending_delete && previous.Graph_service.graph_id = request.graph_id + && previous.account_generation = request.account_generation + then id :: ids else ids) local_copy_requests [] in + List.iter (fun id -> + Hashtbl.remove local_copy_requests id; + Worker.cancel client ~request_id:id) superseded); let command = if deleting then Graph_service.Delete_local_copy request else Inspect_local_copy request in match Worker.send client command with | Worker.Accepted id -> Hashtbl.replace local_copy_requests id (request, deleting, confirm) diff --git a/docs/agent-guide/implemented/feature/2026-10-15-switch-graph-native-picker.md b/docs/agent-guide/implemented/feature/2026-10-15-switch-graph-native-picker.md index f6b31f8..3b5f0ad 100644 --- a/docs/agent-guide/implemented/feature/2026-10-15-switch-graph-native-picker.md +++ b/docs/agent-guide/implemented/feature/2026-10-15-switch-graph-native-picker.md @@ -2,44 +2,61 @@ ## Problem -The graph picker uses uneven capsule buttons and loses the active graph when opened. It has no per-row local-copy management or durable presence observation. +The graph picker used uneven capsule buttons and lost the active graph when opened. It had no per-row local-copy management or durable presence observation. Deleting several local resources in sequence could lose attachment sources before mirror ownership was checked, or strand partially collected retired bytes after a failed purge. ## Decision -Implement the user's selected concept A with native equal-width list rows, a full-canvas List with Timeline-style floating close/refresh/existing account controls, an inline current marker and a separate row menu. Keep picker and request ownership in OCaml. Query one graph's mirror presence on demand with an account-generation fence; cache observations outside rendering. +Use the selected concept A with native equal-width list rows and no header, following Timeline's full-canvas List and floating controls. Retain close, refresh and existing account controls. Keep catalog order, mark the current graph in place, activate names to switch, and expose Delete local copy only through the separate row menu. Do not add search, graph creation or remote deletion. OCaml owns presentation and request state; Swift provides native-specific list controls. -The user explicitly permits discarding unsynchronized local content after confirmation (Sentinel_3c2ff0ee96f88191ab349fca7f7befe9). The confirmation must explain this loss, device-only deletion and the retained cloud copy. The active graph requires switching first. The user separately approved the minimal existing protected Core contract comment change (Sentinel_752ae13c79888191aeb7123ba1d9fd25). No other protected spec, Dune or lock edit is part of this change. +The user explicitly permits discarding unsynchronized content after confirmation (Sentinel_3c2ff0ee96f88191ab349fca7f7befe9). The confirmation explains that unsynchronized changes, drafts and attachments are discarded on this device, while the cloud copy remains downloadable. The active graph must be switched away from first. Unbound legacy mirrors require opening once and switching away before removal. -The user subsequently requested removing the header entirely and making the whole UI a List like Timeline. Reuse the existing floating chrome; retain the native controls and per-row actions without a title bar. +The user authorized the two protected deletion interface changes (Sentinel_098717acd4dc8191baeaae8a9cdbee4b), migration without compatibility paths (Sentinel_17788ec75690819198347944f824cd8c and Sentinel_8265ed5fca0c8191852d23c144887053), and retaining Delete_account_assets. Remove Delete_graph_assets, Database.delete_mirror and the obsolete graph-wide upload-checkpoint delete. Do not add wrappers or fallbacks. Dune, lockfiles and shared opam/pins remain unchanged. + +Use an independent Retirement_id value type, distinct from a graph UUID. Share that storage-independent type through the existing value library, avoiding a runtime dependency from the pure reducer into the database I/O implementation. + +Keep a single worker business deletion flight. Its internal resource protocol is: + +- Acquire the inspected mirror's existing exclusive ownership primitive before retiring any asset source. Revalidate the physical generation and persisted account owner immediately before the namespace commit. +- Cancel and drain target uploads, then freeze staging/checkpoint writers while capturing exact operation/payload pairs. Persist a coordinator journal before preparing assets. +- The asset owner blocks new work for the target origin/account/graph across generations, cancels existing operations and asynchronously waits for native leases. Release and cancellation controls remain available; destructive staging cleanup waits. Move the captured cache into a unique retirement namespace without unlinking its bytes. +- Mirror commit atomically renames that one canonical mirror namespace. This is not an atomic transaction over all directories. Synchronize the directory chain and persist this retirement's committed receipt before destructive cleanup in another producer. An unrelated missing or changed canonical mirror is never commit evidence. +- After durable commit, conditionally remove only captured upload rows and collect only the captured retired asset namespace. Preserve new same-graph operations and replacement canonical files. Physical purge failures stay in producer-owned pending queues; committed collection failure must not prevent redownload. +- Before commit, cancel restores only the captured asset directory with an atomic no-replace rename. A conflict preserves bytes, journal and the barrier for recovery. Aborted/account-deleted tombstones reject late preparation or restoration. Before a mirror is inspected or attached, replay its journal using exact durable commit evidence; unknown outcomes fail closed. + +The producer APIs expose prepare/commit/cancel because the worker coordinates independently owned resources between those steps. Durable receipts and cleanup queues remain private. Completed receipts are looked up by ID; ordinary collection visits pending namespaces rather than all historical receipts. ## Alternatives considered -### Reuse the existing graph-picker transition unchanged +### Reuse the original picker transition + +It detached the current graph and cleared its identity. Keep an application-owned picker presentation, using the existing graph selection lifecycle only for an actual switch. + +### Delete attachments and then delete the mirror -It detaches the current graph and clears its identity. Keep an application-owned picker presentation instead, and use the existing Select_graph lifecycle only for an actual switch. +A busy or stale mirror could reject deletion after its attachment sources were already gone. Merely reversing the calls would still leave post-commit cleanup failures and retries unsafe. Explicitly capture each resource and retain durable recovery evidence. + +### Retain the old single-graph deletion APIs + +Their production callers belong to this workflow. Migrate those callers and their contract tests, then remove the obsolete entry points rather than maintain parallel destructive paths. Account cleanup retains its separate existing purpose. ## Validation -- Uniform native rows preserve catalog order, expose the current marker and support long names and Dynamic Type. -- Close returns to the retained current graph; name activation switches through the worker. -- Row menus are independently reachable; current graph deletion is disabled with an explanation. -- Presence is bounded and requested only for the relevant graph, with stale and cancelled results ignored. -- Confirmation names the graph, warns that unsynchronized changes/drafts/attachments will be lost and says the cloud copy remains downloadable. -- Two native XCTest cases pass on the isolated iPhone 13 / iOS 26.1 Simulator: actual menu and row taps, cancellation, current-graph protection, ownership-busy failure and retry, retained Not downloaded rows, real-worker snapshot redownload through a controlled loopback TLS fixture, close/session retention and accessibility XXXL scrolling/menu reachability. The fixture does not implement a live remote sync service. -- Real UI submission also produced an unsent text capture and photo attachment; inspection found three durable outbox records and one account/graph-specific upload intent before confirmed local deletion removed that copy and its intent. -- Application regression: 98 tests pass, including deletion-in-flight Capture and composer retention. Sync and overlay storage suites pass 261 and 50 cases respectively; worker service, startup, semantics and source-boundary checks pass. -- Independent read-only journal-code-review found and prompted fixes for action gating and confirmation identity; the code-fix commit was re-reviewed with no remaining evidenced findings. All temporary reports, builds, hosts, network fixtures and screenshots remain outside Git. +- Public mounted Application tests cover current marking, close/session retention, name switching, menu confirmation, action usability while another graph is deleting, and cancellation of superseded presence checks. +- Public worker actor tests use real SQLite ownership contention to prove that busy admission precedes asset retirement. They also hold an asset preparation completion while catalog reconciliation retires another graph, proving only the exact deletion ticket/request can resolve its waiter. +- Public Database tests cover stale generations, rebound account ownership, preparation ownership, cancellation, durable commit replay, partial-purge retry and replacement-mirror isolation. A pure reducer cannot execute filesystem permission failures or ownership acquisition; injecting a failed completion there would not reproduce either bug. +- Public asset reducer events cover token scope validation and replay during pending work, after commit and after account cleanup. Public runner requests exercise native leases, staged-file restoration, atomic restore conflicts and replacement-cache isolation. +- Upload checkpoint tests cover durable capture roundtrips, changed-row refusal with no partial deletion, restart/replay and preservation of new same-graph rows. +- Run full repository compilation and regression/source-boundary checks before building the native probe. Simulator acceptance uses only an isolated iPhone 13 / iOS 26.1 device and rebuildable synthetic graphs, with actual list/menu taps, long names and accessibility Dynamic Type, cancellation, current-graph protection, busy failure/retry, retained Not downloaded rows and real-worker redownload through a controlled loopback TLS fixture. Also exercise intentional discard of an unsent text capture and photo attachment. The fixture is not a live remote sync service. +- Keep build manifests, exact source hashes, test results, screenshots and independent review reports outside Git. Review the final PR head with journal-code-review and wait for that head's CI terminal result. ## Risks -- Presence can be unknown or fail; expose a clear retry/status rather than assuming a downloaded copy. -- Host ARM64 OCaml restamping is an experimental Simulator compatibility path; actual runtime acceptance does not establish full iOS cross-compilation or ABI validity. +Unknown presence or an unresolved recovery conflict must expose an actionable error rather than assume a safe downloaded state. Restoring a prepared directory can fail if another namespace has appeared; retain the exclusive captured bytes for retry instead of overwriting the new copy. -## Consequences +Host ARM64 OCaml restamping is an experimental Simulator compatibility route. Runtime acceptance does not establish full iOS cross-compilation or ABI validity. -Presence observations are cached by account and invalidated for a graph lifecycle change. Native row controls stay mounted while property signals update text and enabled state: real Simulator testing showed that replacing interactive subtrees could leave the retained List cell with an obsolete callback binding. Headless final-state assertions alone did not reproduce that native retention boundary, so the acceptance check is the real Simulator flow. - -Deletion uses a single account-scoped flight and exact terminal result. It preserves the selected graph and catalog, validates persisted local ownership, cancels target work, waits for native leases, and revalidates before mirror removal under the existing exclusive ownership primitive. Unsynchronized local content is intentionally discarded after the warning. Unbound legacy mirrors require opening once and switching away before removal. +## Consequences -The active graph remains usable when the picker closes during deletion of another graph. Application owns the manager snapshot and action gate; the public Root_navigation reducer does not own either, so its events cannot reproduce that guard. A regression at the public mounted Application boundary holds deletion in flight and exercises the current graph's Capture action. Each new confirmation also advances its request token so an older response cannot authorize a newer dialog. +Presence observations are cached by account and invalidated for graph lifecycle changes. Native row controls stay mounted while property signals update text and enabled state: real Simulator testing showed that replacing interactive subtrees could retain obsolete callbacks. The active graph remains usable when the picker closes while another graph's copy is deleting. +Application owns the manager snapshot and action gate; Root_navigation does not, so its events cannot reproduce those UI guards. The worker runtime owns exact promise completion; a reducer supplied with an already incorrect completion cannot reproduce its wrapper inference bug. Resource producers own filesystem transitions, durable receipts and native leases. Each regression lives at the narrowest public owner that executes its defect. diff --git a/logseq_db_storage/lib/asset_upload_store.ml b/logseq_db_storage/lib/asset_upload_store.ml index 4268c2f..76949a0 100644 --- a/logseq_db_storage/lib/asset_upload_store.ml +++ b/logseq_db_storage/lib/asset_upload_store.ml @@ -241,15 +241,6 @@ let list db ~origin ~account ~graph ~after ~limit = rows []) ;; -let delete_graph db ~origin ~account ~graph = - statement - db - "DELETE FROM asset_upload_intents WHERE origin=? AND account=? AND graph=?" - (fun stmt -> - bind stmt [ TEXT origin; TEXT account; TEXT (Uuid.to_string graph) ]; - check db (Sqlite3.step stmt)) -;; - let delete_account db ~origin ~account = statement db @@ -258,3 +249,77 @@ let delete_account db ~origin ~account = bind stmt [ TEXT origin; TEXT account ]; check db (Sqlite3.step stmt)) ;; + +module Captured_rows = Map.Make (struct type t = Uuid.t let compare = Uuid.compare end) + +type captured_graph = + { captured_origin : string; captured_account : string; captured_graph : Uuid.t + ; captured_rows : string Captured_rows.t } + +let captured_graph_matches capture ~origin ~account ~graph = + capture.captured_origin = origin && capture.captured_account = account && Uuid.equal capture.captured_graph graph +;; + +let capture_graph db ~origin ~account ~graph = + statement db "SELECT operation,payload FROM asset_upload_intents WHERE origin=? AND account=? AND graph=? ORDER BY operation" + (fun stmt -> + bind stmt [TEXT origin; TEXT account; TEXT (Uuid.to_string graph)]; + let rec rows acc = match Sqlite3.step stmt with + | DONE -> Ok {captured_origin=origin; captured_account=account; captured_graph=graph; captured_rows=acc} + | ROW -> + let payload = Sqlite3.column_text stmt 1 in + let* intent = decode payload in + if intent.origin <> origin || intent.account <> account || not (Uuid.equal intent.graph graph) + || Uuid.to_string intent.operation_id <> Sqlite3.column_text stmt 0 + then Error "Retirement checkpoint scope mismatch" + else rows (Captured_rows.add intent.operation_id payload acc) + | rc -> let* () = check db rc in Error "Unexpected retirement capture result" in + rows Captured_rows.empty) +;; + +let captured_graph_to_yojson capture = + `Assoc ["version", `Int 1; "origin", `String capture.captured_origin + ; "account", `String capture.captured_account; "graph", Uuid.to_yojson capture.captured_graph + ; "rows", `List (Captured_rows.bindings capture.captured_rows |> List.map (fun (id,payload) -> + `List [Uuid.to_yojson id; `String payload]))] +;; + +let captured_graph_of_yojson json = + try + let open Yojson.Safe.Util in + if json |> member "version" |> to_int <> 1 then failwith "Unsupported retirement capture"; + let origin = json |> member "origin" |> to_string in + let account = json |> member "account" |> to_string in + let* graph = json |> member "graph" |> Uuid.of_yojson in + let rec decode_rows acc = function + | [] -> Ok acc + | `List [id; `String payload] :: rest -> + let* id = Uuid.of_yojson id in let* intent = decode payload in + if intent.origin <> origin || intent.account <> account || not (Uuid.equal graph intent.graph) + || not (Uuid.equal id intent.operation_id) then Error "Retirement capture scope mismatch" + else if Captured_rows.mem id acc then Error "Duplicate retirement checkpoint" + else decode_rows (Captured_rows.add id payload acc) rest + | _ -> Error "Invalid retirement capture" in + let* rows = decode_rows Captured_rows.empty (json |> member "rows" |> to_list) in + Ok {captured_origin=origin;captured_account=account;captured_graph=graph;captured_rows=rows} + with _ -> Error "Invalid retirement capture" +;; + +let delete_captured_graph db capture = + let* () = check db (Sqlite3.exec db "BEGIN IMMEDIATE") in + let result = + let* () = Captured_rows.fold (fun id payload result -> + let* () = result in + statement db "SELECT payload FROM asset_upload_intents WHERE operation=?" (fun stmt -> + bind stmt [TEXT (Uuid.to_string id)]; match Sqlite3.step stmt with + | DONE -> Ok () | ROW when Sqlite3.column_text stmt 0 = payload -> Ok () + | ROW -> Error "A captured upload changed; retirement cleanup remains pending." + | rc -> check db rc)) capture.captured_rows (Ok ()) in + Captured_rows.fold (fun id payload result -> let* () = result in + statement db "DELETE FROM asset_upload_intents WHERE operation=? AND payload=?" (fun stmt -> + bind stmt [TEXT (Uuid.to_string id); TEXT payload]; check db (Sqlite3.step stmt))) + capture.captured_rows (Ok ()) in + match result with + | Ok () -> check db (Sqlite3.exec db "COMMIT") + | Error _ as error -> ignore (Sqlite3.exec db "ROLLBACK"); error +;; diff --git a/logseq_db_storage/lib/asset_upload_store.mli b/logseq_db_storage/lib/asset_upload_store.mli index 18c6f35..5c11f44 100644 --- a/logseq_db_storage/lib/asset_upload_store.mli +++ b/logseq_db_storage/lib/asset_upload_store.mli @@ -21,15 +21,18 @@ val list -> limit:int -> (Logseq_db_types.Asset_upload_intent.t list, string) result -val delete_graph - : Sqlite3.db - -> origin:string - -> account:string - -> graph:Logseq_db_types.Graph_types.Uuid.t - -> (unit, string) result - val delete_account : Sqlite3.db -> origin:string -> account:string -> (unit, string) result + +(** A frozen set of exact checkpoint payloads for coordinated local retirement. *) +type captured_graph +val capture_graph : Sqlite3.db -> origin:string -> account:string -> graph:Logseq_db_types.Graph_types.Uuid.t -> (captured_graph, string) result +val captured_graph_to_yojson : captured_graph -> Yojson.Safe.t +val captured_graph_of_yojson : Yojson.Safe.t -> (captured_graph, string) result +val captured_graph_matches : captured_graph -> origin:string -> account:string -> graph:Logseq_db_types.Graph_types.Uuid.t -> bool +(** Fails without deleting if any captured operation acquired a different payload. + Missing rows are already retired. Uncaptured operations are never removed. *) +val delete_captured_graph : Sqlite3.db -> captured_graph -> (unit, string) result diff --git a/logseq_db_storage/test/test_asset_upload_store.ml b/logseq_db_storage/test/test_asset_upload_store.ml index 572b9c6..4aecb25 100644 --- a/logseq_db_storage/test/test_asset_upload_store.ml +++ b/logseq_db_storage/test/test_asset_upload_store.ml @@ -110,8 +110,9 @@ let scoped_cleanup () = and d = intent ~origin:"https://other.example" 5 in with_db path (fun db -> List.iter (fun i -> get (S.save db ~expected:None i)) [ a; b; c; d ]; - get (S.delete_graph db ~origin:a.origin ~account:a.account ~graph:a.graph); - get (S.delete_graph db ~origin:a.origin ~account:a.account ~graph:a.graph); + let captured = get (S.capture_graph db ~origin:a.origin ~account:a.account ~graph:a.graph) in + get (S.delete_captured_graph db captured); + get (S.delete_captured_graph db captured); Alcotest.(check bool) "selected graph removed" true @@ -197,13 +198,50 @@ let generated_checkpoint_validation () = (get (S.read db ~operation:original.operation_id) = Some original))) ;; +let exact_retirement_preserves_new_and_changed_operations () = + fixture (fun path -> + let first = intent 2 and second = intent 3 and replacement = intent 8 in + let captured = with_db path (fun db -> + get (S.save db ~expected:None first); get (S.save db ~expected:None second); + get (S.capture_graph db ~origin:first.origin ~account:first.account ~graph:first.graph)) in + let captured = S.captured_graph_to_yojson captured |> S.captured_graph_of_yojson |> get in + with_db path (fun db -> + get (S.save db ~expected:None replacement); + let changed = get (I.advance second Local_committed) in + get (S.save db ~expected:(Some second.revision) changed); + Alcotest.(check bool) "changed captured checkpoint defers cleanup" true + (Result.is_error (S.delete_captured_graph db captured)); + Alcotest.(check bool) "failed cleanup retains every captured row" true + (get (S.read db ~operation:first.operation_id) = Some first); + Alcotest.(check bool) "new operation survives failed old cleanup" true + (get (S.read db ~operation:replacement.operation_id) = Some replacement))) +;; + +let exact_retirement_survives_restart_and_replacement () = + fixture (fun path -> + let old = intent 2 and fresh = intent 8 in + let captured = with_db path (fun db -> + get (S.save db ~expected:None old); + get (S.capture_graph db ~origin:old.origin ~account:old.account ~graph:old.graph)) in + with_db path (fun db -> + get (S.save db ~expected:None fresh); + get (S.delete_captured_graph db captured)); + with_db path (fun db -> + get (S.delete_captured_graph db captured); + Alcotest.(check bool) "old captured row absent" true (get (S.read db ~operation:old.operation_id) = None); + Alcotest.(check bool) "new same-graph row retained after replay" true + (get (S.read db ~operation:fresh.operation_id) = Some fresh))) +;; + let () = Alcotest.run "asset upload storage" [ ( "SQLite" , List.map (fun (n, f) -> Alcotest.test_case n `Quick f) - [ "generated checkpoint validation", generated_checkpoint_validation + [ "retirement preserves changed operations", exact_retirement_preserves_new_and_changed_operations + ; "retirement restart and replacement isolation", exact_retirement_survives_restart_and_replacement + ; "generated checkpoint validation", generated_checkpoint_validation ; "scoped cleanup", scoped_cleanup ; "restart", restart ; "atomic revision", atomic_revision diff --git a/logseq_db_types/lib/graph_types.ml b/logseq_db_types/lib/graph_types.ml index 2b9cca0..2e35819 100644 --- a/logseq_db_types/lib/graph_types.ml +++ b/logseq_db_types/lib/graph_types.ml @@ -30,6 +30,12 @@ module Uuid = struct let of_yojson json = Result.bind (of_yojson json) of_string end +module Retirement_id = struct + type t = Retirement_id of Uuid.t + let of_string value = Result.map (fun uuid -> Retirement_id uuid) (Uuid.of_string value) + let to_string (Retirement_id uuid) = Uuid.to_string uuid +end + module Cursor = struct type t = string [@@deriving yojson] diff --git a/logseq_db_types/lib/graph_types.mli b/logseq_db_types/lib/graph_types.mli index bad08dd..f126864 100644 --- a/logseq_db_types/lib/graph_types.mli +++ b/logseq_db_types/lib/graph_types.mli @@ -13,6 +13,13 @@ module Uuid : sig val of_yojson : Yojson.Safe.t -> (t, string) result end +(** Identity of one local resource retirement, never a graph identity. *) +module Retirement_id : sig + type t + val of_string : string -> (t, string) result + val to_string : t -> string +end + module Cursor : sig type t diff --git a/logseq_db_worker/lib/effect_runner/effect_runner.ml b/logseq_db_worker/lib/effect_runner/effect_runner.ml index 535a26b..5bd96ce 100644 --- a/logseq_db_worker/lib/effect_runner/effect_runner.ml +++ b/logseq_db_worker/lib/effect_runner/effect_runner.ml @@ -454,6 +454,7 @@ let reclaim_asset_output t (output : Sync.asset_output) = let action = match output.result with | Ok (Sync.Asset_staged { file; _ }) -> Some (Sync.Release_staged_file file) + | Ok (Sync.Asset_deletion_prepared deletion) -> Some (Sync.Cancel_graph_asset_deletion deletion) | Ok ( Asset_retained (Some (handle, _)) | Asset_cached (Some handle) @@ -1495,9 +1496,102 @@ let deletion_current t request = not t.stopped && t.deletion_owner = Some request && deletion_authorized t request ;; +let sync_retirement_directory path = + let fd = Unix.openfile path [Unix.O_RDONLY] 0 in + Fun.protect ~finally:(fun () -> Unix.close fd) (fun () -> Unix.fsync fd) +;; + +let rec make_retirement_directory path = + if not (Sys.file_exists path) then ( + make_retirement_directory (Filename.dirname path); + Unix.mkdir path 0o700; sync_retirement_directory (Filename.dirname path)) +;; + +let local_retirement_directory t (scope : Sync.graph_scope) = + let identity = Digest.to_hex (Digest.string + (Uri.to_string scope.account.managed_sync_origin ^ "\000" ^ scope.account.user_id)) in + Filename.concat t.dependencies.config.application_support_directory + (Filename.concat "logseq-db-worker/local-copy-retirements" + (Filename.concat identity (Graph.Uuid.to_string scope.graph_id))) +;; + +let fresh_retirement () = + let bytes = In_channel.with_open_bin "/dev/urandom" (fun input -> really_input_string input 16) in + let hex = String.concat "" (List.init 16 (fun n -> Printf.sprintf "%02x" (Char.code bytes.[n]))) in + Sync.retirement_id_of_string (Printf.sprintf "%s-%s-4%s-8%s-%s" + (String.sub hex 0 8) (String.sub hex 8 4) (String.sub hex 13 3) + (String.sub hex 17 3) (String.sub hex 20 12)) |> Result.get_ok +;; + +let write_local_retirement path expected captured = + let json = `Assoc ["version",`Int 1 + ; "expected", `String (Overlay.Mirror_generation.to_string expected) + ; "uploads", Logseq_db_storage.Asset_upload_store.captured_graph_to_yojson captured] |> Yojson.Safe.to_string in + let temporary = path ^ ".part" in + let fd = Unix.openfile temporary [Unix.O_WRONLY;Unix.O_CREAT;Unix.O_TRUNC] 0o600 in + Fun.protect ~finally:(fun () -> Unix.close fd) (fun () -> + let rec write offset = if offset < String.length json then + let n = Unix.write_substring fd json offset (String.length json-offset) in + if n=0 then failwith "retirement journal write stalled" else write (offset+n) in + write 0; Unix.fsync fd); + Unix.rename temporary path; sync_retirement_directory (Filename.dirname path) +;; + +let remove_local_retirement path = + Unix.unlink path; sync_retirement_directory (Filename.dirname path) +;; + +let finish_local_retirement t scope deletion captured ~commit = + let ( let* ) = Result.bind in + let* () = if commit then with_upload_store t (fun db -> Logseq_db_storage.Asset_upload_store.delete_captured_graph db captured) else Ok () in + asset_unit t ~scope + (if commit then Sync.Commit_graph_asset_deletion deletion else Sync.Cancel_graph_asset_deletion deletion) +;; + +let recover_local_retirements t (scope : Sync.graph_scope) = + let ( let* ) = Result.bind in + let directory = local_retirement_directory t scope in + if not (Sys.file_exists directory) then Ok () else + Array.fold_left (fun result name -> + let* () = result in + if not (Filename.check_suffix name ".json") then Ok () else + try + let journal_path = Filename.concat directory name in + let retirement = String.sub name 0 (String.length name - 5) |> Sync.retirement_id_of_string |> Result.get_ok in + let open Yojson.Safe.Util in + let json = Yojson.Safe.from_file journal_path in + if json |> member "version" |> to_int <> 1 then failwith "Unsupported retirement journal"; + let expected = json |> member "expected" |> to_string |> Overlay.Mirror_generation.of_string |> Result.get_ok in + let captured = json |> member "uploads" |> Logseq_db_storage.Asset_upload_store.captured_graph_of_yojson |> Result.get_ok in + if not (Logseq_db_storage.Asset_upload_store.captured_graph_matches captured + ~origin:(Uri.to_string scope.account.managed_sync_origin) ~account:scope.account.user_id ~graph:scope.graph_id) + then failwith "Retirement journal account mismatch"; + let* inspection = Database.inspect_mirror + ~application_support_directory:t.dependencies.config.application_support_directory ~graph_id:scope.graph_id + |> Result.map_error (fun _ -> "Retirement recovery could not inspect the mirror.") in + let* prepared = Database.prepare_mirror_deletion inspection ~retirement + |> Result.map_error (fun _ -> "Retirement recovery is blocked by mirror ownership or an unknown commit state.") in + Fun.protect ~finally:(fun () -> Database.cancel_mirror_deletion prepared) (fun () -> + let committed = Database.mirror_deletion_committed prepared in + let original = match Database.mirror_presence inspection with + | Overlay.Available {generation;_} -> Overlay.Mirror_generation.equal generation expected + | Absent _ -> false in + if not committed && not original then Error "Retirement recovery conflict; the captured resources were preserved." + else + let deletion = Sync.asset_deletion_reference ~scope ~retirement in + let* () = finish_local_retirement t scope deletion captured ~commit:committed in + (if committed then (try remove_local_retirement journal_path with _ -> ()) + else remove_local_retirement journal_path); + Ok ()) + with _ -> Error "Local copy recovery is incomplete; its resources were preserved.") (Ok ()) (Sys.readdir directory) +;; + let handle_sync_worker_effect t = function | Sync.Inspect_mirror request -> - (match + let recovery = with_staging_lock t (fun () -> recover_local_retirements t request.scope) in + (match recovery with + | Error message -> Error (effect_error message) + | Ok () -> match Database.inspect_mirror ~application_support_directory: t.dependencies.config.application_support_directory @@ -1579,52 +1673,73 @@ let handle_sync_worker_effect t = function | Delete_mirror request -> let ( let* ) = Result.bind in let current () = deletion_current t request in + let scope : Sync.graph_scope = {account=request.account;graph_id=request.graph_id;graph_generation=0} in let result = let* () = if current () then Ok () else Error "The graph account is no longer current." in let* () = match t.attached with - | Some (_, scope) when scope.graph_id = request.graph_id -> - Error "Switch to another graph before deleting." + | Some (_, actual) when actual.graph_id = request.graph_id -> Error "Switch to another graph before deleting." | _ -> Ok () in + let* () = with_staging_lock t (fun () -> recover_local_retirements t scope) in let* inspection = Database.inspect_mirror - ~application_support_directory:t.dependencies.config.application_support_directory - ~graph_id:request.graph_id + ~application_support_directory:t.dependencies.config.application_support_directory ~graph_id:request.graph_id |> Result.map_error (fun _ -> "The local copy could not be inspected.") in - let present = match Database.mirror_presence inspection with Overlay.Absent _ -> false | Available _ -> true in - let* () = if present then check_local_owner t request.account request.graph_id ~bind:false else Ok () in - let same_target (scope : Sync.graph_scope) = - scope.graph_id = request.graph_id - && scope.account.user_id = request.account.user_id - && Uri.equal scope.account.managed_sync_origin request.account.managed_sync_origin in - Hashtbl.iter (fun (ticket : Upload.ticket) resolve -> - if same_target ticket.scope then ignore (Eio.Promise.try_resolve resolve () : bool)) t.upload_operations; - let rec drain attempts = - if not (current ()) then Error "The graph account is no longer current." - else if not (Hashtbl.fold (fun (ticket : Upload.ticket) _ busy -> busy || same_target ticket.scope) - t.upload_operations false) then Ok () - else if attempts = 0 then Error "The local copy is still in use. Close attachment previews and try again." - else (t.dependencies.runtime.sleep 0.005; drain (attempts - 1)) in - let* () = drain 1000 in - with_staging_lock t (fun () -> - let* () = if current () then Ok () else Error "The graph account is no longer current." in - let* () = asset_unit t - ~scope:{ Sync.account = request.account; graph_id = request.graph_id; graph_generation = 0 } - Sync.Delete_graph_assets |> Result.map_error (fun _ -> - "The local copy is still in use. Close attachment previews and try again.") in - let* () = if current () then Ok () else Error "The graph account is no longer current." in - let* () = if present then check_local_owner t request.account request.graph_id ~bind:false else Ok () in - let* () = - if not present then Ok () - else Database.delete_mirror inspection |> Result.map (fun _ -> ()) - |> Result.map_error (function - | Overlay.Mirror_delete_busy -> "The local copy is still in use. Close attachment previews and try again." - | _ -> "Local graph deletion failed. Try again.") in - let* () = with_upload_store t (fun db -> - Logseq_db_storage.Asset_upload_store.delete_graph db - ~origin:(Uri.to_string request.account.managed_sync_origin) - ~account:request.account.user_id ~graph:request.graph_id) - |> Result.map_error (fun _ -> "Local graph deletion failed. Try again.") in - Hashtbl.remove t.inspections (Graph.Uuid.to_string request.graph_id); - Ok ()) + let* () = check_local_owner t request.account request.graph_id ~bind:false in + let retirement = fresh_retirement () in + (* Ownership is acquired before any asset retirement or intent mutation. *) + let* prepared = Database.prepare_mirror_deletion inspection ~retirement + |> Result.map_error (function + | Overlay.Mirror_delete_busy -> "The local copy is still in use. Close attachment previews and try again." + | _ -> "The local copy changed. Refresh and try again.") in + Fun.protect ~finally:(fun () -> Database.cancel_mirror_deletion prepared) (fun () -> + let same_target (actual : Sync.graph_scope) = actual.graph_id=scope.graph_id + && actual.account.user_id=scope.account.user_id + && Uri.equal actual.account.managed_sync_origin scope.account.managed_sync_origin in + Hashtbl.iter (fun (ticket : Upload.ticket) resolve -> + if same_target ticket.scope then ignore (Eio.Promise.try_resolve resolve () : bool)) t.upload_operations; + let rec drain attempts = + if not (current ()) then Error "The graph account is no longer current." + else if not (Hashtbl.fold (fun (ticket : Upload.ticket) _ busy -> busy || same_target ticket.scope) t.upload_operations false) + then Ok () + else if attempts=0 then Error "The local copy is still in use. Try again." + else (t.dependencies.runtime.sleep 0.005; drain (attempts-1)) in + let* () = drain 1000 in + with_staging_lock t (fun () -> + let* () = if current () then Ok () else Error "The graph account is no longer current." in + let* captured = with_upload_store t (fun db -> Logseq_db_storage.Asset_upload_store.capture_graph db + ~origin:(Uri.to_string request.account.managed_sync_origin) ~account:request.account.user_id ~graph:request.graph_id) in + let expected = match Database.mirror_presence inspection with + | Overlay.Available {generation;_} -> generation | Absent _ -> assert false in + let directory = local_retirement_directory t scope in + make_retirement_directory directory; + let path = Filename.concat directory (Sync.retirement_id_to_string retirement ^ ".json") in + write_local_retirement path expected captured; + let deletion = Sync.asset_deletion_reference ~scope ~retirement in + let committed = ref false in + let abort () = + if not !committed then + match finish_local_retirement t scope deletion captured ~commit:false with + | Ok () -> remove_local_retirement path + | Error _ -> () in + Fun.protect ~finally:abort (fun () -> + let* () = match asset_call t ~scope (Sync.Prepare_graph_asset_deletion retirement) with + | Ok (Sync.Asset_deletion_prepared actual) when actual=deletion -> Ok () + | Error error -> Error (asset_failure_message error) + | _ -> Error "Unexpected attachment retirement response." in + let* () = if current () then Ok () else Error "The graph account is no longer current." in + let* () = check_local_owner t request.account request.graph_id ~bind:false in + let* _ = Database.commit_mirror_deletion prepared + |> Result.map_error (fun _ -> "Local deletion did not commit; its resources were preserved.") in + committed := true; + (* The mirror receipt is the durable commit authority. Cleanup failure + retains this coordinator journal for exact replay, not a broad sweep. *) + (try + if Database.mirror_deletion_committed prepared then + match finish_local_retirement t scope deletion captured ~commit:true with + | Ok () -> remove_local_retirement path + | Error _ -> () + with _ -> ()); + Hashtbl.remove t.inspections (Graph.Uuid.to_string request.graph_id); + Ok ()))) in Ok (sync_result ~event:(Sync.Mirror_deleted (request, result)) ()) | Attach_graph request -> diff --git a/logseq_db_worker/lib/logseq_db_worker.ml b/logseq_db_worker/lib/logseq_db_worker.ml index 3198c8d..d51f943 100644 --- a/logseq_db_worker/lib/logseq_db_worker.ml +++ b/logseq_db_worker/lib/logseq_db_worker.ml @@ -70,7 +70,7 @@ type graph_state = Pure_reducer.graph_state = type local_deletion_waiter = { graph_id : Logseq_db_types.Graph_types.Uuid.t ; account_generation : int - ; mutable request : Sync.mirror_deletion option + ; mutable request : (Pure_reducer.ticket * Sync.mirror_deletion) option ; resolve : (unit, string) result Eio.Promise.u } @@ -108,13 +108,22 @@ let dispatch t event = | None, Pure_reducer.Sync_event (Sync.Local_cache_deletion_requested graph_id) when graph_id = waiter.graph_id -> waiter.request <- List.find_map (function - | Pure_reducer.Run_worker (Pure_reducer.Request (_, Pure_reducer.Handle_sync_worker_effect (Sync.Delete_mirror request))) -> - Some request + | Pure_reducer.Run_worker (Pure_reducer.Request (ticket, Pure_reducer.Handle_sync_worker_effect (Sync.Delete_mirror request))) -> + Some (ticket, request) | _ -> None) transition.effects; if waiter.request = None then finish (Error "This local copy cannot be deleted now.") | _ -> ()); - if waiter.request <> None && snapshot.local_deletion = None - then finish (match snapshot.last_error with None -> Ok () | Some message -> Error message))); + match waiter.request, event with + | Some (expected_ticket, expected), + Pure_reducer.Runner_completed + (Pure_reducer.Sync_worker_effect_completed + (ticket, Ok { event = Some (Sync.Mirror_deleted (actual, result)); _ })) + when ticket = expected_ticket && actual = expected -> finish result + | Some (expected_ticket, _), + Pure_reducer.Runner_completed + (Pure_reducer.Sync_worker_effect_completed (ticket, Error _)) + when ticket = expected_ticket -> finish (Error "Mirror deletion failed.") + | _ -> ())); List.iter (Effect_runner.submit t.runner) transition.effects ;; diff --git a/logseq_db_worker/test/test_effect_runner.ml b/logseq_db_worker/test/test_effect_runner.ml index 861810e..45fd55f 100644 --- a/logseq_db_worker/test/test_effect_runner.ml +++ b/logseq_db_worker/test/test_effect_runner.ml @@ -1347,6 +1347,147 @@ let test_upload_checkpoint_io () = Runner.shutdown restarted))) ;; +let deletion_runtime_fixture ~busy () = + let module Db = Logseq_db_worker in + let module G = Logseq_db_types.Graph_types in + T.with_managed (fun fixture -> + Eio_main.run (fun env -> Eio.Switch.run (fun sw -> + let graph_id suffix = G.Uuid.of_string ("94000000-0000-4000-8000-" ^ suffix) |> Result.get_ok in + let first = graph_id "000000000201" and second = graph_id "000000000202" in + let graph graph_id : Sync.graph = + { graph_id; name = "Deletion waiter fixture"; encrypted = false + ; schema = { major = 65; minor = 33; exact = true } } in + let root = fixture.config.application_support_directory in + let worker_root = Filename.concat root "logseq-db-worker" in + Unix.mkdir worker_root 0o700; + let mirrors = Filename.concat worker_root "synced-graphs" in + Unix.mkdir mirrors 0o700; + let directory = Filename.concat mirrors (G.Uuid.to_string first) in + Unix.mkdir directory 0o700; + let first_database_path = Filename.concat directory "db.sqlite" in + let sqlite = Sqlite3.db_open first_database_path in + let checkpoint = Logseq_db_types.Sync_checkpoint.create ~graph_id:first + ~schema:{ major = 65; minor = 33 } + ~applied_server_t:(Logseq_db_types.Server_cursor.of_int64 0L |> Result.get_ok) + ~checksum:"0123456789abcdef" |> Result.get_ok in + Logseq_db_storage.Sync_checkpoint_store.initialize_database sqlite checkpoint |> Result.get_ok; + ignore (Sqlite3.db_close sqlite); + Out_channel.with_open_bin (Filename.concat directory ".journal-local-owner.json") (fun output -> + Printf.fprintf output {|{"origin":"https://api.logseq.io","user_id":"user-1","graph_id":%S}|} + (G.Uuid.to_string first)); + let second_directory = Filename.concat mirrors (G.Uuid.to_string second) in + Unix.mkdir second_directory 0o700; + let sqlite = Sqlite3.db_open (Filename.concat second_directory "db.sqlite") in + let open Yojson.Safe.Util in + let storage = T.read_json (T.fixture "storage/logseq-65.33-create-page.json") in + Sqlite3.Rc.check (Sqlite3.exec sqlite (storage |> member "tableSql" |> to_string)); + let statement = Sqlite3.prepare sqlite "INSERT INTO kvs(addr, content, addresses) VALUES(?, ?, ?)" in + storage |> member "rows" |> to_list |> List.iter (fun row -> + Sqlite3.Rc.check (Sqlite3.reset statement); + Sqlite3.Rc.check (Sqlite3.bind_int statement 1 (row |> member "addr" |> to_int)); + Sqlite3.Rc.check (Sqlite3.bind_text statement 2 (row |> member "content" |> to_string)); + Sqlite3.Rc.check (Sqlite3.bind statement 3 (match member "addresses" row with + | `Null -> Sqlite3.Data.NULL | `String value -> TEXT value | _ -> assert false)); + Sqlite3.Rc.check (Sqlite3.step statement)); + Sqlite3.Rc.check (Sqlite3.finalize statement); + let checkpoint = Logseq_db_types.Sync_checkpoint.create ~graph_id:second + ~schema:{ major = 65; minor = 33 } + ~applied_server_t:(Logseq_db_types.Server_cursor.of_int64 0L |> Result.get_ok) + ~checksum:"0123456789abcdef" |> Result.get_ok in + Logseq_db_storage.Sync_checkpoint_store.initialize_database sqlite checkpoint |> Result.get_ok; + Logseq_db_storage.Sync_outbox_store.initialize_database sqlite |> Result.get_ok; + ignore (Sqlite3.db_close sqlite); + let worker = ref None and refreshed = ref false and held_cleanup = ref None in + let destructive_requests = ref 0 in + let post event = Db.post (Option.get !worker) (Core.Sync_event event) in + let sync_runner = Runner.sync_runner ~submit:(function + | Sync.Request (ticket, Sync.Load_catalog _) -> + post (Sync.Runner_completed (Sync.Completion (ticket, Ok None))) + | Sync.Request (ticket, Sync.Save_catalog _) -> + post (Sync.Runner_completed (Sync.Completion (ticket, Ok ()))) + | Sync.Request (ticket, Sync.Fetch_catalog _) -> + post (Sync.Runner_completed (Sync.Completion + (ticket, Ok (if !refreshed then [graph first] else [graph first; graph second])))) + | Sync.Request (ticket, Sync.Fetch_snapshot_baseline _) -> + post (Sync.Runner_completed (Sync.Completion + (ticket, Error (Sync.Effect_failed "Synthetic bootstrap unavailable.")))) + | Sync.Request (ticket, Sync.Fetch_snapshot_metadata _) -> + post (Sync.Runner_completed (Sync.Completion + (ticket, Error (Sync.Effect_failed "Synthetic bootstrap unavailable.")))) + | Sync.Asset_io (ticket, { context; action = Sync.Prepare_graph_asset_deletion retirement; _ }) -> + incr destructive_requests; + let deletion = Sync.asset_deletion_reference ~scope:context.scope ~retirement in + if busy then post (Sync.Runner_completed (Sync.Asset_completion (ticket, Ok (Sync.Asset_deletion_prepared deletion)))) + else held_cleanup := Some (ticket, deletion) + | Sync.Asset_io (ticket, _) -> + post (Sync.Runner_completed (Sync.Asset_completion (ticket, Ok Sync.Asset_unit))) + | _ -> ()) ~shutdown:(fun () -> ()) () in + let dependencies = Runner.dependencies + ~runtime:(Runner.runtime ~fork:(fun ~sw task -> Eio.Fiber.fork ~sw task) + ~sleep:(Eio.Time.sleep (Eio.Stdenv.clock env)) |> Result.get_ok) + ~config:fixture.config ~overlay:fixture.overlay ~sync_runner ~publish:(fun _ -> ()) + |> Result.get_ok in + let limits = Sync.limits ~maximum_response_bytes:1024 ~maximum_artifact_bytes:1024 + ~submission_batch_size:1 |> Result.get_ok in + let sync = Sync.config ~managed_sync_origin:(Uri.of_string "https://api.logseq.io") ~limits + |> Result.get_ok in + let db = Db.create ~sw ~config:(Core.config ~worker:fixture.config ~sync) + ~runner_dependencies:dependencies |> Result.get_ok in + worker := Some db; + let wait predicate = + match Eio.Time.with_timeout_exn (Eio.Stdenv.clock env) 3. (fun () -> + while not (predicate ()) do Eio.Fiber.yield () done) with + | () -> () + | exception error -> + let snapshot = (Db.view db).sync.snapshot in + Alcotest.failf "Wait failed: %s selected=%s bootstrap=%b restore=%b failure=%b error=%s" + (Printexc.to_string error) + (Option.fold ~none:"none" ~some:G.Uuid.to_string snapshot.selected_graph) + snapshot.startup.bootstrapping snapshot.startup.restoring_local + (Option.is_some snapshot.startup.failure) + (Option.value snapshot.last_error ~default:"none") in + Fun.protect ~finally:(fun () -> + Option.iter (fun (ticket, _) -> post (Sync.Runner_completed + (Sync.Asset_completion (ticket, Error Sync.Asset_cancelled)))) !held_cleanup; + Db.shutdown db) + (fun () -> + post (Sync.Account_authenticated { user_id = Some "user-1" }); + wait (fun () -> (Db.view db).sync.snapshot.catalog <> []); + post (Sync.Graph_selected second); + wait (fun () -> let snapshot = (Db.view db).sync.snapshot in + snapshot.selected_graph = Some second && (Db.graph_state db).phase = Db.Graph_open + && not snapshot.startup.bootstrapping && not snapshot.startup.restoring_local); + let generation = (Db.view db).sync.snapshot.startup.account_generation in + if busy then ( + let lock = Sqlite3.db_open (Filename.concat directory ".logseq-overlay-owner.sqlite") in + Fun.protect ~finally:(fun () -> ignore (Sqlite3.exec lock "ROLLBACK"); ignore (Sqlite3.db_close lock)) (fun () -> + Sqlite3.Rc.check (Sqlite3.exec lock "BEGIN IMMEDIATE"); + Alcotest.(check bool) "busy mirror reports failure" true + (Result.is_error (Db.delete_local_copy db ~account_generation:generation ~graph_id:first)); + Alcotest.(check int) "busy mirror never starts destructive asset work" 0 !destructive_requests; + Alcotest.(check bool) "busy mirror remains usable" true (Sys.file_exists first_database_path))) + else ( + let result, resolve = Eio.Promise.create () in + Eio.Fiber.fork ~sw (fun () -> Eio.Promise.resolve resolve + (Db.delete_local_copy db ~account_generation:generation ~graph_id:first)); + wait (fun () -> Option.is_some !held_cleanup); + refreshed := true; + post Sync.Catalog_refresh_requested; + wait (fun () -> (Db.view db).sync.snapshot.selected_graph = None); + Alcotest.(check bool) "catalog refresh cannot report deletion success" true + (match Eio.Promise.peek result with Some (Ok ()) -> false | _ -> true); + Option.iter (fun (ticket, deletion) -> post (Sync.Runner_completed + (Sync.Asset_completion (ticket, Ok (Sync.Asset_deletion_prepared deletion))))) !held_cleanup; + held_cleanup := None; + Alcotest.(check bool) "matching deletion finishes with cancellation failure" true + (match Eio.Promise.await result with Error _ -> true | Ok () -> false); + Alcotest.(check bool) "unauthorized deletion preserves its mirror" true + (Sys.file_exists first_database_path)))))) +;; + +let test_catalog_refresh_cannot_complete_another_deletion () = deletion_runtime_fixture ~busy:false () +let test_deletion_checks_ownership_before_assets () = deletion_runtime_fixture ~busy:true () + let () = Alcotest.run "logseq db worker effect runner" @@ -1418,6 +1559,12 @@ let () = "sync and publish delegation" `Quick test_sync_and_publish_instructions_are_not_reduced_recursively + ; Alcotest.test_case + "catalog refresh cannot complete another deletion" + `Quick + test_catalog_refresh_cannot_complete_another_deletion + ; Alcotest.test_case "busy mirror preserves attachment sources" `Quick + test_deletion_checks_ownership_before_assets ] ) ] ;; diff --git a/logseq_db_worker/test/test_pure_reducer.ml b/logseq_db_worker/test/test_pure_reducer.ml index f2772bd..ca1586c 100644 --- a/logseq_db_worker/test/test_pure_reducer.ml +++ b/logseq_db_worker/test/test_pure_reducer.ml @@ -202,7 +202,7 @@ let worker_open_graph () = ready.Core.next, scope ;; -let test_local_deletion_drains_worker_operations () = +let test_current_deletion_preserves_work_until_retirement () = let ready, scope = worker_open_graph () in let request = graph_info_request () in let executing = @@ -245,15 +245,24 @@ let test_local_deletion_drains_worker_operations () = inspecting.effects |> Option.get in - let deleting = + let protected = Core.step inspecting.next (Sync_event (Sync.Local_cache_deletion_requested scope.graph_id)) in Alcotest.(check bool) - "graph closes admission immediately" + "current-copy deletion preserves the open graph and executing work" true - ((Core.view deleting.next).graph.phase = Core.Graph_closing); + ((Core.view protected.next).graph.phase = Core.Graph_open + && (Core.view protected.next).sync.snapshot.local_deletion = None + && protected.effects = []); + let retiring = + Core.step protected.next (Sync_event Sync.Graph_picker_requested) + in + Alcotest.(check bool) + "actual graph retirement closes admission immediately" + true + ((Core.view retiring.next).graph.phase = Core.Graph_closing); let has_close effects = List.exists (function @@ -266,21 +275,21 @@ let test_local_deletion_drains_worker_operations () = Alcotest.(check bool) "close waits for executing work" false - (has_close deleting.effects); + (has_close retiring.effects); let edit = { request with Protocol.command = Protocol.V2_save_block { mutation_id = uuid "20000000-0000-4000-8000-000000000001" ; block = uuid "30000000-0000-4000-8000-000000000001" - ; title = "Must not be saved after deletion admission" + ; title = "Must not be saved after graph retirement admission" ; preconditions = { blocks = []; pages = []; scopes = [] } } } in let rejected = Core.step - deleting.next + retiring.next (Core.Graph_request { id = Core.request_id_of_int64 12L; request = edit }) in Alcotest.(check bool) @@ -334,10 +343,10 @@ let test_local_deletion_drains_worker_operations () = in let failed = Core.step drained.next close_failure in Alcotest.(check bool) - "worker forwards close failure to owner" + "ordinary retirement failure cannot authorize local deletion" true ((Core.view failed.next).sync.snapshot.local_deletion - = Some (Sync.Deletion_failed Sync.Closing_graph)); + = None); Alcotest.(check bool) "close failure never deletes" false @@ -1021,9 +1030,9 @@ let () = `Quick test_sync_instructions_are_translated_in_order ; Alcotest.test_case - "deletion drains executing work" + "current deletion preserves work until retirement" `Quick - test_local_deletion_drains_worker_operations + test_current_deletion_preserves_work_until_retirement ; Alcotest.test_case "deterministic replay" `Quick test_replay_is_deterministic ] ) ] diff --git a/logseq_overlay_db/lib/database.ml b/logseq_overlay_db/lib/database.ml index 18951d6..b9f31f2 100644 --- a/logseq_overlay_db/lib/database.ml +++ b/logseq_overlay_db/lib/database.ml @@ -23,6 +23,19 @@ type mirror_inspection = ; presence : Types.mirror_presence } +type retirement_id = Graph.Retirement_id.t +let retirement_id_of_string = Graph.Retirement_id.of_string +let retirement_id_to_string = Graph.Retirement_id.to_string + +type prepared_mirror_deletion = + { deletion_inspection : mirror_inspection + ; deletion_retirement : retirement_id + ; mutable deletion_ownership : Ownership.t option + ; mutable deletion_committed : Types.mirror_delete option + ; mutable deletion_cancelled : bool + ; mutable deletion_durable : bool + } + type prepared_snapshot_activation = { snapshot_dependencies : dependencies ; snapshot_inspection : mirror_inspection @@ -267,7 +280,7 @@ let mirror_generation value = token Types.Mirror_generation.of_string ("mirror-generation:v1:" ^ value) ;; -let mirror_generation_for_path path = +let mirror_generation_for_path_as ~identity_path path = let stat = Unix.stat path in let owner_path = Filename.concat (Filename.dirname path) ".journal-local-owner.json" in let owner_identity = @@ -280,10 +293,13 @@ let mirror_generation_for_path path = with Unix.Unix_error (Unix.ENOENT, _, _) -> "no-owner" in mirror_generation (Digestif.SHA256.digest_string - (Printf.sprintf "%s:%d:%d:%s" path stat.st_dev stat.st_ino owner_identity) + (Printf.sprintf "%s:%d:%d:%s" identity_path stat.st_dev stat.st_ino owner_identity) |> Digestif.SHA256.to_hex) ;; +let mirror_generation_for_path path = mirror_generation_for_path_as ~identity_path:path path +;; + let absent_mirror_generation path = mirror_generation (Digestif.SHA256.digest_string ("absent:" ^ path) |> Digestif.SHA256.to_hex) @@ -707,40 +723,192 @@ let rec remove_mirror_tree path = | _ -> Unix.unlink path ;; -let deletion_sequence = Atomic.make 0 +let sync_deletion_directory directory = + let fd = Unix.openfile directory [ Unix.O_RDONLY ] 0 in + Fun.protect ~finally:(fun () -> Unix.close fd) (fun () -> Unix.fsync fd) +;; + +let rec ensure_retirement_directory path = + if not (Sys.file_exists path) then ( + ensure_retirement_directory (Filename.dirname path); + Unix.mkdir path 0o700; + sync_deletion_directory (Filename.dirname path)) +;; + +let mirror_deletion_directory inspection = + Filename.concat + (Filename.concat (Filename.dirname (Filename.dirname inspection.location.database_path)) ".mirror-deletions") + (Graph.Uuid.to_string inspection.location.graph_id) +;; + +let mirror_deletion_paths inspection retirement = + let root = mirror_deletion_directory inspection in + let id = retirement_id_to_string retirement in + root, Filename.concat (Filename.concat root "states") (id ^ ".json"), + Filename.concat (Filename.concat root "pending") (id ^ ".data") +;; + +let write_deletion_receipt path (result : Types.mirror_delete) committed = + let data = Yojson.Safe.to_string (`Assoc + [ "version", `Int 1 + ; "graph", Graph.Uuid.to_yojson (Option.get result.graph_uuid) + ; "retirement", `String (Filename.chop_suffix (Filename.basename path) ".json") + ; "previous", `String (Types.Mirror_generation.to_string result.previous_generation) + ; "absent", `String (Types.Mirror_generation.to_string result.absent_generation) + ; "committed", `Bool committed ]) in + let temporary = path ^ ".part" in + let fd = Unix.openfile temporary [Unix.O_WRONLY; Unix.O_CREAT; Unix.O_TRUNC] 0o600 in + Fun.protect ~finally:(fun () -> Unix.close fd) (fun () -> + let rec write offset = if offset < String.length data then + let count = Unix.write_substring fd data offset (String.length data - offset) in + if count = 0 then failwith "receipt write stalled" else write (offset + count) in + write 0; Unix.fsync fd); + Unix.rename temporary path; + sync_deletion_directory (Filename.dirname path) +;; + +let read_deletion_receipt inspection receipt_path = + let open Yojson.Safe.Util in + if not (is_regular_single_link receipt_path) || (Unix.stat receipt_path).st_size > 4096 + then failwith "invalid retirement receipt"; + let json = Yojson.Safe.from_file receipt_path in + if json |> member "version" |> to_int <> 1 then failwith "unsupported retirement receipt"; + if json |> member "graph" |> to_string <> Graph.Uuid.to_string inspection.location.graph_id + || json |> member "retirement" |> to_string <> Filename.chop_suffix (Filename.basename receipt_path) ".json" + then failwith "retirement receipt identity mismatch"; + let generation name = json |> member name |> to_string + |> Types.Mirror_generation.of_string |> Result.get_ok in + ({ Types.graph_uuid = Some inspection.location.graph_id + ; previous_generation = generation "previous"; absent_generation = generation "absent" }, + json |> member "committed" |> to_bool) +;; + +let collect_retired_mirrors inspection = + let root = Filename.concat (mirror_deletion_directory inspection) "pending" in + if Sys.file_exists root then + Sys.readdir root |> Array.iter (fun name -> + if Filename.check_suffix name ".data" then + try + let id = String.sub name 0 (String.length name - 5) |> retirement_id_of_string |> Result.get_ok in + let _, receipt, data = mirror_deletion_paths inspection id in + let _, committed = read_deletion_receipt inspection receipt in + if committed && Sys.file_exists data then ( + match Ownership.acquire ~graph_directory:data with + | Error _ -> () + | Ok owner -> Fun.protect ~finally:(fun () -> ignore (Ownership.release owner)) + (fun () -> remove_mirror_tree data; sync_deletion_directory root)) + with _ -> ()) +;; + +let prepare_mirror_deletion inspection ~retirement = + let _root, receipt, data = mirror_deletion_paths inspection retirement in + try + let existing = if Sys.file_exists receipt then Some (read_deletion_receipt inspection receipt) else None in + match existing with + | Some (result, committed) when committed || Sys.file_exists data -> + (* The exact retired destination is commit evidence even if the process died + before persisting its committed flag. Never infer this from canonical. *) + if not committed then ( + let database = Filename.concat data "db.sqlite" in + if (Unix.lstat data).st_kind <> Unix.S_DIR || not (is_regular_single_link database) + || not (Types.Mirror_generation.equal result.previous_generation + (mirror_generation_for_path_as ~identity_path:inspection.location.database_path database)) + then failwith "retired mirror identity mismatch"); + sync_deletion_directory (Filename.dirname data); + sync_deletion_directory (Filename.dirname (Filename.dirname inspection.location.database_path)); + write_deletion_receipt receipt result true; + Ok { deletion_inspection = inspection; deletion_retirement = retirement + ; deletion_ownership = None; deletion_committed = Some result; deletion_cancelled = false; deletion_durable = true } + | _ -> + (match inspection.presence with + | Types.Absent _ -> Error Types.Mirror_delete_stale + | Available { generation = expected; _ } -> + if not (Sys.file_exists inspection.location.database_path) then Error Types.Mirror_delete_stale else + let graph_directory = Filename.dirname inspection.location.database_path in + match Ownership.acquire ~graph_directory with + | Error Ownership.Already_owned -> Error Types.Mirror_delete_busy + | Error (Unavailable _) when not (Sys.file_exists graph_directory) -> Error Types.Mirror_delete_stale + | Error (Unavailable message) -> Error (Types.Mirror_delete_failed message) + | Ok owner -> + let fail error = ignore (Ownership.release owner); Error error in + match inspect_durable_mirror inspection.location with + | Ok { presence = Available { generation = actual; _ }; _ } + when Types.Mirror_generation.equal actual expected -> + (try + ensure_retirement_directory (Filename.dirname receipt); + ensure_retirement_directory (Filename.dirname data); + let result = { Types.graph_uuid = Some inspection.location.graph_id + ; previous_generation = expected + ; absent_generation = absent_mirror_generation inspection.location.database_path } in + (match existing with + | Some (old, _) when not (Types.Mirror_generation.equal old.previous_generation expected) -> + fail Types.Mirror_delete_stale + | _ -> + write_deletion_receipt receipt result false; + Ok { deletion_inspection = inspection; deletion_retirement = retirement + ; deletion_ownership = Some owner; deletion_committed = None; deletion_cancelled = false; deletion_durable = false }) + with exn -> fail (Types.Mirror_delete_failed (Printexc.to_string exn))) + | _ -> fail Types.Mirror_delete_stale) + with exn -> Error (Types.Mirror_delete_failed (Printexc.to_string exn)) +;; -let delete_mirror inspection = - let path = inspection.location.database_path in - match inspection.presence with - | Types.Absent _ -> Error Types.Mirror_delete_stale - | Available { generation = expected; _ } -> - let graph_directory = Filename.dirname path in - let ownership = - try Ownership.acquire ~graph_directory with - | exn -> Error (Ownership.Unavailable (Printexc.to_string exn)) in - (match ownership with - | Error Ownership.Already_owned -> Error Types.Mirror_delete_busy - | Error (Unavailable message) -> - if not (Sys.file_exists graph_directory) then Error Types.Mirror_delete_stale - else Error (Types.Mirror_delete_failed message) - | Ok ownership -> - Fun.protect ~finally:(fun () -> ignore (Ownership.release ownership)) (fun () -> - match inspect_durable_mirror inspection.location with - | Error _ | Ok { presence = Absent _; _ } -> Error Types.Mirror_delete_stale - | Ok { presence = Available { generation = current; _ }; _ } - when not (Types.Mirror_generation.equal expected current) -> Error Types.Mirror_delete_stale - | Ok _ -> - try - let retired = Printf.sprintf "%s.deleted-%d-%d" graph_directory (Unix.getpid ()) - (Atomic.fetch_and_add deletion_sequence 1) in - if Sys.file_exists retired then Error (Types.Mirror_delete_failed "retired location is occupied") - else ( - Unix.rename graph_directory retired; - ignore (Ownership.release ownership); - remove_mirror_tree retired; - Ok { Types.graph_uuid = Some inspection.location.graph_id - ; previous_generation = expected; absent_generation = absent_mirror_generation path }) - with exn -> Error (Types.Mirror_delete_failed (Printexc.to_string exn)))) +let mirror_deletion_committed preparation = preparation.deletion_durable + +let cancel_mirror_deletion preparation = + Option.iter (fun owner -> ignore (Ownership.release owner)) preparation.deletion_ownership; + preparation.deletion_ownership <- None; + if preparation.deletion_committed = None then preparation.deletion_cancelled <- true +;; + +let commit_mirror_deletion preparation = + let inspection = preparation.deletion_inspection in + let _root, receipt, data = mirror_deletion_paths inspection preparation.deletion_retirement in + let durable = + try + let result, committed = read_deletion_receipt inspection receipt in + if committed then Some result else None + with _ -> None in + match preparation.deletion_committed, durable with + | Some result, _ | _, Some result -> + preparation.deletion_committed <- Some result; + (try + sync_deletion_directory (Filename.dirname data); + sync_deletion_directory (Filename.dirname (Filename.dirname inspection.location.database_path)); + write_deletion_receipt receipt result true; + preparation.deletion_durable <- true; + collect_retired_mirrors inspection + with _ -> ()); + Ok result + | None, None when preparation.deletion_cancelled -> Error Types.Mirror_delete_stale + | None, None -> + match inspection.presence, inspect_durable_mirror inspection.location with + | Types.Available { generation = expected; _ }, + Ok { presence = Available { generation = actual; _ }; _ } + when preparation.deletion_ownership <> None && Types.Mirror_generation.equal expected actual -> + let renamed = + try + if Sys.file_exists data then failwith "retirement destination is occupied"; + let result, _ = read_deletion_receipt inspection receipt in + Unix.rename (Filename.dirname inspection.location.database_path) data; + Ok result + with exn -> Error (Types.Mirror_delete_failed (Printexc.to_string exn)) in + (match renamed with + | Error _ as error -> error + | Ok result -> + (* Every operation after rename is post-commit maintenance. An error may + retain bytes, but can never authorize restoring partially purged data. *) + preparation.deletion_committed <- Some result; + (try Option.iter (fun owner -> ignore (Ownership.release owner)) preparation.deletion_ownership with _ -> ()); + preparation.deletion_ownership <- None; + (try + sync_deletion_directory (Filename.dirname data); + sync_deletion_directory (Filename.dirname (Filename.dirname inspection.location.database_path)); + write_deletion_receipt receipt result true; + preparation.deletion_durable <- true; + collect_retired_mirrors inspection + with _ -> ()); + Ok result) + | _ -> Error Types.Mirror_delete_stale ;; let validate_receipts sqlite = @@ -5285,6 +5453,7 @@ let open_owned ~sw dependencies inspection ~graph_name ownership = ;; let open_ ~sw dependencies inspection ~graph_name = + (try collect_retired_mirrors inspection with _ -> ()); if String.trim graph_name = "" then Error Types.Invalid_graph_name else ( diff --git a/logseq_overlay_db/spec/database.mli b/logseq_overlay_db/spec/database.mli index b68b400..a4ea855 100644 --- a/logseq_overlay_db/spec/database.mli +++ b/logseq_overlay_db/spec/database.mli @@ -164,12 +164,36 @@ val cancel_snapshot_activation : prepared_snapshot_activation -> unit It therefore applies only while the mirror is closed and still denotes the filesystem object that was inspected. *) -(** Deletes the inspected mirror if its generation is current and no process owns - it. *) -val delete_mirror +(** A deletion transaction identity, distinct from a graph UUID. *) +type retirement_id = Graph.Retirement_id.t +val retirement_id_of_string : string -> (retirement_id, string) result +val retirement_id_to_string : retirement_id -> string + +(** Holds exclusive ownership of the inspected generation until commit/cancel. + Preparing does not delete data. Replaying the same identity recovers its + durable commit receipt, including after a replacement mirror was created. *) +type prepared_mirror_deletion +val prepare_mirror_deletion : mirror_inspection + -> retirement:retirement_id + -> (prepared_mirror_deletion, Types.mirror_delete_error) result + +(** Whether this exact retirement has durable commit evidence. An unrelated + missing or replaced canonical mirror is never commit evidence. *) +val mirror_deletion_committed : prepared_mirror_deletion -> bool + +(** Revalidates the generation and atomically retires this mirror namespace. + Success denotes the namespace commit, not completion of physical collection. + A durable receipt precedes collection; failed collection is retried by the + producer against only the exclusive retired namespace. Replay is idempotent. *) +val commit_mirror_deletion + : prepared_mirror_deletion -> (Types.mirror_delete, Types.mirror_delete_error) result +(** Releases an uncommitted preparation. Idempotent; a committed retirement is + never restored, and its durable receipt is retained for coordinated recovery. *) +val cancel_mirror_deletion : prepared_mirror_deletion -> unit + (** Collects eligible durable garbage without removing mutation receipts needed to classify late synchronization responses. *) val collect_garbage diff --git a/logseq_overlay_db/test/test_overlay_storage.ml b/logseq_overlay_db/test/test_overlay_storage.ml index cb145c6..c45ffaa 100644 --- a/logseq_overlay_db/test/test_overlay_storage.ml +++ b/logseq_overlay_db/test/test_overlay_storage.ml @@ -3,6 +3,13 @@ let server_cursor_of_int value = ;; module Database = Logseq_overlay_db.Database + +let retirement_sequence = Atomic.make 0 +let next_retirement () = + Database.retirement_id_of_string + (Printf.sprintf "97000000-0000-4000-8000-%012d" (Atomic.fetch_and_add retirement_sequence 1)) + |> Result.get_ok +;; module T = Test_support module Types = Logseq_overlay_db.Types open Types @@ -691,7 +698,10 @@ let stale_delete_cannot_remove_rebound_owner () = Out_channel.with_open_bin owner (fun output -> output_string output "account-A"); let _, _, inspection = location_and_inspection support behavior in Out_channel.with_open_bin owner (fun output -> output_string output "account-B"); - (match Database.delete_mirror inspection with + (match (match Database.prepare_mirror_deletion inspection ~retirement:(next_retirement ()) with + | Error _ as error -> error + | Ok preparation -> Fun.protect ~finally:(fun () -> Database.cancel_mirror_deletion preparation) + (fun () -> Database.commit_mirror_deletion preparation)) with | Error Mirror_delete_stale -> () | _ -> Alcotest.fail "an old observation deleted the rebound account's mirror"); Alcotest.(check bool) "new owner's mirror is retained" true (Sys.file_exists database_path)) @@ -702,13 +712,91 @@ let stale_delete_cannot_remove_replacement () = T.with_temp_directory "overlay-stale-delete-" (fun support -> ignore (T.seed_mirror support); let _dependencies, _location, inspection = location_and_inspection support behavior in - ignore (Database.delete_mirror inspection |> T.require_ok ~behavior); - match Database.delete_mirror inspection with + ignore ((match Database.prepare_mirror_deletion inspection ~retirement:(next_retirement ()) with + | Error _ as error -> error + | Ok preparation -> Fun.protect ~finally:(fun () -> Database.cancel_mirror_deletion preparation) + (fun () -> Database.commit_mirror_deletion preparation)) |> T.require_ok ~behavior); + match (match Database.prepare_mirror_deletion inspection ~retirement:(next_retirement ()) with + | Error _ as error -> error + | Ok preparation -> Fun.protect ~finally:(fun () -> Database.cancel_mirror_deletion preparation) + (fun () -> Database.commit_mirror_deletion preparation)) with | Error Mirror_delete_stale -> () | Error _ -> Alcotest.fail "stale delete returned the wrong typed error" | Ok _ -> Alcotest.fail "stale inspection deleted a mirror twice") ;; +let retired_cleanup_retries_without_touching_a_new_mirror () = + let behavior = "committed retirement survives cleanup failure" in + T.with_temp_directory "overlay-retired-cleanup-" (fun support -> + let database_path = T.seed_mirror support in + let blocked_name = "synthetic-cleanup-blocked" in + let blocked = Filename.concat (Filename.dirname database_path) blocked_name in + Unix.mkdir blocked 0o700; + Out_channel.with_open_bin (Filename.concat blocked "pending") (fun out -> output_string out "synthetic"); + let rec blocked_paths path = + match Unix.lstat path with + | { Unix.st_kind = Unix.S_DIR; _ } -> + let children = Sys.readdir path |> Array.to_list + |> List.concat_map (fun name -> blocked_paths (Filename.concat path name)) in + if Filename.basename path = blocked_name then path :: children else children + | _ -> [] + | exception Unix.Unix_error (Unix.ENOENT, _, _) -> [] in + let unblock () = List.iter (fun path -> Unix.chmod path 0o700) (blocked_paths support) in + Fun.protect ~finally:unblock (fun () -> + let dependencies, _, inspection = location_and_inspection support behavior in + Unix.chmod blocked 0o500; + ignore ((match Database.prepare_mirror_deletion inspection ~retirement:(next_retirement ()) with + | Error _ as error -> error + | Ok preparation -> Fun.protect ~finally:(fun () -> Database.cancel_mirror_deletion preparation) + (fun () -> Database.commit_mirror_deletion preparation)) |> T.require_ok ~behavior); + Alcotest.(check bool) "canonical copy was committed absent" false (Sys.file_exists database_path); + Alcotest.(check bool) "failed purge remains durably retryable" true (blocked_paths support <> []); + ignore (T.seed_mirror support); + Out_channel.with_open_bin (Filename.concat (Filename.dirname database_path) ".journal-local-owner.json") + (fun out -> output_string out "new synthetic owner"); + unblock (); + let fresh = inspect_mirror support behavior in + Eio_main.run (fun _ -> Eio.Switch.run (fun sw -> + let database = Database.open_ ~sw dependencies fresh ~graph_name:"New synthetic copy" + |> T.require_ok ~behavior in + ignore (Database.close database |> T.require_ok ~behavior))); + Alcotest.(check bool) "new live copy survives retired cleanup" true (Sys.file_exists database_path); + Alcotest.(check bool) "ordinary producer lifecycle retries retired purge" true (blocked_paths support = []))) +;; + +let mirror_preparation_cancel_and_commit_replay () = + let behavior = "mirror preparation ownership and durable replay" in + T.with_temp_directory "overlay-deletion-protocol-" (fun support -> + let database_path = T.seed_mirror support in + let dependencies, _, inspection = location_and_inspection support behavior in + let retirement = next_retirement () in + let prepared = Database.prepare_mirror_deletion inspection ~retirement |> T.require_ok ~behavior in + Eio_main.run (fun _ -> Eio.Switch.run (fun sw -> + match Database.open_ ~sw dependencies inspection ~graph_name:"Busy synthetic mirror" with + | Error Ownership_conflict -> () + | _ -> Alcotest.fail "preparation did not hold exclusive ownership")); + Database.cancel_mirror_deletion prepared; + Database.cancel_mirror_deletion prepared; + Alcotest.(check bool) "cancel retains canonical mirror" true (Sys.file_exists database_path); + Alcotest.(check bool) "cancelled preparation cannot commit" true + (Result.is_error (Database.commit_mirror_deletion prepared)); + let committed_retirement = next_retirement () in + let prepared = Database.prepare_mirror_deletion inspection ~retirement:committed_retirement |> T.require_ok ~behavior in + let committed_id = prepared in + ignore (Database.commit_mirror_deletion prepared |> T.require_ok ~behavior); + Database.cancel_mirror_deletion prepared; + Alcotest.(check bool) "commit has durable evidence" true (Database.mirror_deletion_committed prepared); + ignore (T.seed_mirror support); + let _, _, fresh = location_and_inspection support behavior in + let replay = Database.prepare_mirror_deletion fresh ~retirement:committed_retirement |> T.require_ok ~behavior in + Alcotest.(check bool) "fresh handle recovers durable commit" true (Database.mirror_deletion_committed replay); + ignore (Database.commit_mirror_deletion replay |> T.require_ok ~behavior); + Database.cancel_mirror_deletion replay; + ignore (Database.commit_mirror_deletion committed_id |> T.require_ok ~behavior); + Database.cancel_mirror_deletion committed_id; + Alcotest.(check bool) "late commit/cancel preserves new canonical mirror" true (Sys.file_exists database_path)) +;; + let corrupt_outbox_fails_closed () = let behavior = "corrupt durable outbox fails closed" in T.with_temp_directory "overlay-corrupt-outbox-" (fun support -> @@ -2847,6 +2935,8 @@ let () = sqlite_full_int64_checkpoint ] ) ; "mirror and durability", cases @ - [Alcotest.test_case "owner rebind invalidates deletion" `Quick stale_delete_cannot_remove_rebound_owner] + [Alcotest.test_case "owner rebind invalidates deletion" `Quick stale_delete_cannot_remove_rebound_owner; + Alcotest.test_case "retired cleanup preserves new mirror" `Quick retired_cleanup_retries_without_touching_a_new_mirror; + Alcotest.test_case "mirror preparation cancel and commit replay" `Quick mirror_preparation_cancel_and_commit_replay] ] ;; diff --git a/logseq_sync/lib/effect_runner/effect_runner.ml b/logseq_sync/lib/effect_runner/effect_runner.ml index b79f93b..a5e9b4c 100644 --- a/logseq_sync/lib/effect_runner/effect_runner.ml +++ b/logseq_sync/lib/effect_runner/effect_runner.ml @@ -374,6 +374,8 @@ type t = ; secret_lock : Eio.Mutex.t ; asset_caches : (Core.graph_scope, Asset_cache.t) Hashtbl.t ; asset_leases : ((Core.graph_scope * string), unit) Hashtbl.t + ; asset_retirements : ((string * string * string), Core.retirement_id) Hashtbl.t + ; preparing_retirements : ((string * string * string), Core.retirement_id) Hashtbl.t ; mutable closed : bool } @@ -396,6 +398,8 @@ let create ~sw dependencies ~post = ; secret_lock = Eio.Mutex.create () ; asset_caches = Hashtbl.create 4 ; asset_leases = Hashtbl.create 16 + ; asset_retirements = Hashtbl.create 4 + ; preparing_retirements = Hashtbl.create 4 ; closed = false } ;; @@ -406,6 +410,13 @@ let asset_root t = "logseq-db-worker/assets" ;; +let asset_target (scope : Core.graph_scope) = + Uri.to_string scope.account.managed_sync_origin, scope.account.user_id, + Graph_types.Uuid.to_string scope.graph_id +;; +let asset_blocked t scope = Hashtbl.mem t.asset_retirements (asset_target scope) +;; + let delete_account_assets ?except_id t (account : Core.account_scope) = Hashtbl.iter (fun id (operation : operation) -> @@ -429,6 +440,13 @@ let delete_account_assets ?except_id t (account : Core.account_scope) = else Some cache) t.asset_caches; Asset_cache.delete_account ~root:(asset_root t) ~account + |> Result.map (fun () -> + let origin = Uri.to_string account.managed_sync_origin in + Hashtbl.filter_map_inplace (fun (actual_origin, user, _) retirement -> + if actual_origin=origin && user=account.user_id then None else Some retirement) t.asset_retirements; + Hashtbl.filter_map_inplace (fun ((scope : Core.graph_scope), _) () -> + if Uri.equal scope.account.managed_sync_origin account.managed_sync_origin + && scope.account.user_id=account.user_id then None else Some ()) t.asset_leases) ;; let catalog_root local_store = @@ -1508,7 +1526,7 @@ let scoped_asset_cache t scope = ;; let retain_asset_file t ~scope ~handle = - if t.closed + if t.closed || asset_blocked t scope then None else Option.bind (Hashtbl.find_opt t.asset_caches scope) (fun cache -> @@ -1544,42 +1562,47 @@ let cancel_graph_asset_operations ?except_id t (request : Core.mirror_deletion) t.operations ;; -let delete_graph_assets ?except_id t (request : Core.mirror_deletion) ~current = +let prepare_graph_asset_deletion ?except_id t (scope : Core.graph_scope) retirement ~current = + match Asset_cache.graph_deletion_terminal ~root:(asset_root t) ~scope ~retirement with + | Error error -> Error (asset_cache_failure error) + | Ok (Some `Committed) -> Ok (Core.Asset_deletion_prepared (Core.asset_deletion_reference ~scope ~retirement)) + | Ok (Some `Aborted) -> Error Core.Asset_cancelled + | Ok None -> + let request : Core.mirror_deletion = + { account = scope.account; graph_id = scope.graph_id; scope = Core.effect_scope_of_graph scope } in cancel_graph_asset_operations ?except_id t request; - let same_target (scope : Core.graph_scope) = - scope.graph_id = request.graph_id && scope.account.user_id = request.account.user_id - && Uri.equal scope.account.managed_sync_origin request.account.managed_sync_origin in + let same_target scope' = asset_target scope' = asset_target scope in let busy () = Hashtbl.fold (fun id (operation : operation) busy -> busy || (Some id <> except_id && Option.fold ~none:false ~some:(fun (scope, _) -> same_target scope) operation.asset_identity)) t.operations false || Hashtbl.fold (fun (scope, _) () busy -> busy || same_target scope) t.asset_leases false in - let rec await_retirement attempts = - if not (current ()) then Error "The graph cleanup was cancelled." + let rec wait attempts = + if not (current ()) || Hashtbl.find_opt t.asset_retirements (asset_target scope) <> Some retirement + then Error Core.Asset_cancelled else if not (busy ()) then Ok () - else if attempts = 0 then Error "Attachment resources are still in use." - else (t.dependencies.runtime.sleep 0.005; await_retirement (attempts - 1)) in - Result.bind (await_retirement 1000) (fun () -> - let scopes = - Hashtbl.fold - (fun (scope : Core.graph_scope) _ acc -> - if - scope.graph_id = request.graph_id - && scope.account.user_id = request.account.user_id - && Uri.equal - scope.account.managed_sync_origin - request.account.managed_sync_origin - then scope :: acc - else acc) - t.asset_caches - [] - in - List.iter (close_asset_scope ?except_id t) scopes; - Asset_cache.delete_graph - ~root:(asset_root t) - ~account:request.account - ~graph_id:request.graph_id - |> Result.map_error (fun _ -> "The graph asset cache could not be removed.")) + else if attempts = 0 then Error (Core.Asset_invalid_content "Attachment resources are still in use.") + else (t.dependencies.runtime.sleep 0.005; wait (attempts - 1)) in + let result = Result.bind (wait 1000) (fun () -> + Hashtbl.fold (fun scope _ scopes -> if same_target scope then scope :: scopes else scopes) + t.asset_caches [] |> List.iter (close_asset_scope ?except_id t); + Asset_cache.prepare_graph_deletion ~root:(asset_root t) ~scope ~retirement + |> Result.map_error asset_cache_failure + |> Result.map (fun () -> Core.Asset_deletion_prepared (Core.asset_deletion_reference ~scope ~retirement))) in + (* A failed preparation may already own a durable rename. Its coordinating + journal will cancel it; keep the barrier until that exact terminal action. *) + result +;; + +let finish_graph_asset_deletion t deletion ~commit = + let scope = Core.asset_deletion_scope deletion in + let retirement = Core.asset_deletion_retirement deletion in + Asset_cache.finish_graph_deletion ~root:(asset_root t) ~deletion ~commit + |> Result.map_error asset_cache_failure + |> Result.map (fun () -> + if Hashtbl.find_opt t.asset_retirements (asset_target scope) = Some retirement + then Hashtbl.remove t.asset_retirements (asset_target scope); + Core.Asset_unit) ;; let read_upload_source ~maximum_plaintext_bytes source_file = @@ -1790,6 +1813,11 @@ let execute_asset ?except_id t (request : Core.asset_io_request) ~current = (scoped_asset_cache t scope |> Result.map_error asset_cache_failure) action in + let permitted = match request.action with + | Core.Release_asset_file _ | Cancel_asset_operation _ | Prepare_graph_asset_deletion _ + | Commit_graph_asset_deletion _ | Cancel_graph_asset_deletion _ | Delete_account_assets -> true + | _ -> not (asset_blocked t scope) in + if not permitted then Error Core.Asset_cancelled else match request.action with | Core.Check_asset_cache (asset, version) -> cache (fun cache -> @@ -1854,14 +1882,10 @@ let execute_asset ?except_id t (request : Core.asset_io_request) ~current = | Close_asset_scope -> close_asset_scope ?except_id t scope; Ok Core.Asset_unit - | Delete_graph_assets -> - let deletion : Core.mirror_deletion = - { account = scope.account - ; graph_id = scope.graph_id - ; scope = Core.effect_scope_of_graph scope - } - in - delete_graph_assets ?except_id t deletion ~current |> storage_result |> map_unit + | Prepare_graph_asset_deletion retirement -> + prepare_graph_asset_deletion ?except_id t scope retirement ~current + | Commit_graph_asset_deletion deletion -> finish_graph_asset_deletion t deletion ~commit:true + | Cancel_graph_asset_deletion deletion -> finish_graph_asset_deletion t deletion ~commit:false | Delete_account_assets -> delete_account_assets ?except_id t scope.account |> Result.map_error asset_cache_failure @@ -1908,11 +1932,28 @@ let submit_asset_io t instruction ticket (request : Core.asset_io_request) = in if claim_operation t instruction then ( + let busy = match request.action with + | Core.Prepare_graph_asset_deletion retirement -> + let key = asset_target request.context.scope in + Hashtbl.mem t.preparing_retirements key + || (match Hashtbl.find_opt t.asset_retirements key with Some other -> other <> retirement | None -> false) + | _ -> false in + if busy then t.post (Core.Runner_completed (Core.Asset_completion (ticket, + Error (Core.Asset_invalid_content "Attachment retirement is already in progress.")))) + else ( (match request.action with - | Core.Delete_graph_assets -> + | Core.Prepare_graph_asset_deletion retirement -> let scope = request.context.scope in - cancel_graph_asset_operations t - { Core.account = scope.account; graph_id = scope.graph_id; scope = Core.effect_scope_of_graph scope } + (match Asset_cache.graph_deletion_terminal ~root:(asset_root t) ~scope ~retirement with + | Ok None -> + Hashtbl.replace t.preparing_retirements (asset_target scope) retirement; + (match Hashtbl.find_opt t.asset_retirements (asset_target scope) with + | None -> Hashtbl.add t.asset_retirements (asset_target scope) retirement + | Some _ -> ()); + if Hashtbl.find_opt t.asset_retirements (asset_target scope) = Some retirement then + cancel_graph_asset_operations t + { Core.account = scope.account; graph_id = scope.graph_id; scope = Core.effect_scope_of_graph scope } + | _ -> ()) | _ -> ()); let cancelled, resolve_cancelled = Eio.Promise.create () in let operation = @@ -1927,7 +1968,14 @@ let submit_asset_io t instruction ticket (request : Core.asset_io_request) = let current () = (not t.closed) && not operation.cancelled in let result = Fun.protect - ~finally:(fun () -> Hashtbl.remove t.operations id) + ~finally:(fun () -> + Hashtbl.remove t.operations id; + match request.action with + | Core.Prepare_graph_asset_deletion retirement -> + let key = asset_target request.context.scope in + if Hashtbl.find_opt t.preparing_retirements key = Some retirement + then Hashtbl.remove t.preparing_retirements key + | _ -> ()) (fun () -> try Eio.Fiber.first @@ -1943,7 +1991,7 @@ let submit_asset_io t instruction ticket (request : Core.asset_io_request) = in (* A completed resource must reach the reducer even after cancellation. The reducer owns acceptance and issues cleanup for stale resources. *) - t.post (Core.Runner_completed (Core.Asset_completion (ticket, result))))) + t.post (Core.Runner_completed (Core.Asset_completion (ticket, result)))))) ;; let execute_protected t (request : Core.protected_request) = @@ -2005,7 +2053,9 @@ let cleanup_asset_action = function | Core.Release_asset_file _ | Release_staged_file _ | Close_asset_scope - | Delete_graph_assets + | Prepare_graph_asset_deletion _ + | Commit_graph_asset_deletion _ + | Cancel_graph_asset_deletion _ | Delete_account_assets | Cancel_asset_operation _ -> true | Check_asset_cache _ @@ -2020,7 +2070,7 @@ let cleanup_asset_action = function let submit t instruction = match instruction with - | Core.Asset_io (ticket, ({ action = Core.Delete_graph_assets; _ } as request)) -> + | Core.Asset_io (ticket, ({ action = Core.Prepare_graph_asset_deletion _; _ } as request)) -> (* Deletion may wait for native leases. It must not block the event consumer that delivers their release, and shutdown must revoke its execution. *) if t.closed diff --git a/logseq_sync/lib/effect_runner/platform/platform_crypto_stubs.c b/logseq_sync/lib/effect_runner/platform/platform_crypto_stubs.c index c23fa41..a069ee8 100644 --- a/logseq_sync/lib/effect_runner/platform/platform_crypto_stubs.c +++ b/logseq_sync/lib/effect_runner/platform/platform_crypto_stubs.c @@ -90,3 +90,25 @@ CAMLprim value logseq_journal_crypto_binary_call( } #include "transport_dns_stubs.h" + +/* Restore a retired directory without overwriting a concurrently created copy. */ +#include +#include +#include +#ifdef __linux__ +#include +#include +#endif +CAMLprim value logseq_journal_rename_exclusive(value source, value target) { + CAMLparam2(source, target); + int rc; +#ifdef __APPLE__ + rc = renameatx_np(AT_FDCWD, String_val(source), AT_FDCWD, String_val(target), RENAME_EXCL); +#elif defined(__linux__) && defined(SYS_renameat2) + rc = syscall(SYS_renameat2, AT_FDCWD, String_val(source), AT_FDCWD, String_val(target), 1); +#else + errno = ENOTSUP; + rc = -1; +#endif + CAMLreturn(Val_int(rc == 0 ? 0 : errno)); +} diff --git a/logseq_sync/lib/effect_runner/storage/bootstrap.ml b/logseq_sync/lib/effect_runner/storage/bootstrap.ml index e79a102..4ac0f8f 100644 --- a/logseq_sync/lib/effect_runner/storage/bootstrap.ml +++ b/logseq_sync/lib/effect_runner/storage/bootstrap.ml @@ -196,7 +196,9 @@ module Asset_cache = struct let protect f = try f () with - | Sys_error message -> Error (Io message) + | Sys_error message | Failure message -> Error (Io message) + | Yojson.Json_error message -> Error (Invalid message) + | Yojson.Safe.Util.Type_error (message, _) -> Error (Invalid message) | Unix.Unix_error (error, operation, _) -> Error (Io (operation ^ ": " ^ Unix.error_message error)) ;; @@ -325,76 +327,6 @@ module Asset_cache = struct Filename.concat root name ;; - let create ~root ~(scope : Core.graph_scope) ~budget_bytes ~maximum_file_bytes = - protect (fun () -> - if - budget_bytes < 0L - || maximum_file_bytes < 0 - || maximum_file_bytes > 100 * 1024 * 1024 - then Error (Invalid "Invalid cache size limits") - else ( - let directory = - Filename.concat - (account_directory ~root scope.account) - (Uuid.to_string scope.graph_id) - in - mkdir directory; - (* Validate all persisted JSON before cleaning anything. Unsupported - internal formats require an explicit caller-visible failure. *) - let rec load_manifests records = function - | [] -> Ok (List.rev records) - | file :: rest when Filename.check_suffix file ".json" -> - let* record = read_manifest directory file in - load_manifests ((file, record) :: records) rest - | _ :: rest -> load_manifests records rest - in - let* manifests = load_manifests [] (Array.to_list (Sys.readdir directory)) in - let pending = Filename.concat directory "pending" in - if Sys.file_exists pending - then - Array.iter - (fun file -> - if Filename.check_suffix file ".part" - then unlink (Filename.concat pending file)) - (Sys.readdir pending); - let t = - { directory - ; budget = budget_bytes - ; maximum_file_bytes - ; records = Hashtbl.create 64 - ; handles = Hashtbl.create 64 - ; staged_records = Hashtbl.create 8 - ; staged_handles = Hashtbl.create 8 - ; closed = false - } - in - List.iter - (fun (file, (record : record)) -> - if - record.size <= Int64.of_int maximum_file_bytes - && Sys.file_exists (data_path t record) - then Hashtbl.replace t.records record.name record - else unlink (Filename.concat directory file)) - manifests; - Array.iter - (fun file -> - if Filename.check_suffix file ".part" - then unlink (Filename.concat directory file)) - (Sys.readdir directory); - let kept = Hashtbl.create (2 * Hashtbl.length t.records) in - Hashtbl.iter - (fun _ record -> - Hashtbl.replace kept (record.name ^ ".json") (); - Hashtbl.replace kept (record.name ^ "." ^ record.file_type) ()) - t.records; - Array.iter - (fun file -> - let path = Filename.concat directory file in - if (not (Hashtbl.mem kept file)) && not (Sys.is_directory path) - then unlink path) - (Sys.readdir directory); - Ok t)) - ;; let lookup t ~asset ~version = protect (fun () -> @@ -587,11 +519,238 @@ module Asset_cache = struct Ok ()) ;; - let delete_account ~root ~account = remove_tree (account_directory ~root account) + external rename_exclusive : string -> string -> int = "logseq_journal_rename_exclusive" + + let graph_deletion_paths ~root (scope : Core.graph_scope) retirement = + let account = account_directory ~root scope.account in + let graph = Uuid.to_string scope.graph_id in + let directory = Filename.concat + (Filename.concat (Filename.concat root ".asset-retirements") (Filename.basename account)) graph in + Filename.concat account graph, directory, + Filename.concat (Filename.concat directory "states") (Core.retirement_id_to_string retirement ^ ".json"), + Filename.concat (Filename.concat directory "pending") (Core.retirement_id_to_string retirement ^ ".data") + ;; + + let sync_path directory = + let fd = Unix.openfile directory [Unix.O_RDONLY] 0 in + Fun.protect ~finally:(fun () -> Unix.close fd) (fun () -> Unix.fsync fd) + ;; + + let write_retirement path json = + let temporary = path ^ ".part" in + write temporary (Yojson.Safe.to_string json); + Unix.rename temporary path; + sync_path (Filename.dirname path) + ;; + + let identity path = + match Unix.lstat path with + | { Unix.st_kind = Unix.S_DIR; st_dev; st_ino; _ } -> Printf.sprintf "%d:%d" st_dev st_ino + | _ -> failwith "asset namespace is not a directory" + ;; - let delete_graph ~root ~account ~graph_id = - remove_tree - (Filename.concat (account_directory ~root account) (Uuid.to_string graph_id)) + let retirement_state manifest_path = + let open Yojson.Safe.Util in + let json = Yojson.Safe.from_file manifest_path in + if json |> member "version" |> to_int <> 1 then failwith "unsupported asset retirement"; + json |> member "identity" |> to_string, json |> member "state" |> to_string + ;; + + let retirement_json captured state = + `Assoc ["version", `Int 1; "identity", `String captured; "state", `String state] + ;; + + let graph_deletion_terminal ~root ~scope ~retirement = + protect (fun () -> + let _, _, manifest, _ = graph_deletion_paths ~root scope retirement in + if not (Sys.file_exists manifest) then Ok None else + let _, state = retirement_state manifest in + Ok (match state with + | "complete" | "committed" -> + let fd = Unix.openfile manifest [Unix.O_RDONLY] 0 in + Fun.protect ~finally:(fun () -> Unix.close fd) (fun () -> Unix.fsync fd); + sync_path (Filename.dirname manifest); + Some `Committed + | "aborted" | "account-deleted" -> Some `Aborted + | _ -> None)) + ;; + + let prepare_graph_deletion ~root ~(scope : Core.graph_scope) ~retirement = + protect (fun () -> + let canonical, directory, manifest, data = graph_deletion_paths ~root scope retirement in + mkdir directory; + mkdir (Filename.dirname manifest); mkdir (Filename.dirname data); + let captured, state = + if Sys.file_exists manifest then retirement_state manifest + else ( + let captured = if Sys.file_exists canonical then identity canonical else "absent" in + write_retirement manifest (retirement_json captured "preparing"); captured, "preparing") in + if state <> "preparing" && state <> "prepared" then Error Stale + else ( + if Sys.file_exists data then ( + if identity data <> captured then failwith "retired asset identity changed") + else if captured <> "absent" then ( + if not (Sys.file_exists canonical) || identity canonical <> captured + then failwith "canonical asset identity changed"; + let error = rename_exclusive canonical data in + if error <> 0 then failwith ("asset retirement rename failed: " ^ string_of_int error); + sync_path (Filename.dirname canonical); sync_path (Filename.dirname data)); + write_retirement manifest (retirement_json captured "prepared"); + Ok ())) + ;; + + let finish_graph_deletion ~root ~deletion ~commit = + protect (fun () -> + let scope = Core.asset_deletion_scope deletion in + let canonical, _directory, manifest, data = graph_deletion_paths ~root scope + (Core.asset_deletion_retirement deletion) in + if not (Sys.file_exists manifest) then ( + if Sys.file_exists data then Error Stale + else if commit then Error Stale + else ( + mkdir (Filename.dirname manifest); + write_retirement manifest (retirement_json "absent" "aborted"); Ok ())) + else + let captured, state = retirement_state manifest in + if state = "account-deleted" then ( + ignore (remove_tree data); Ok ()) + else if state = "aborted" || state = "complete" then Ok () + else if commit then ( + if Sys.file_exists data && identity data <> captured then failwith "retired asset identity changed"; + write_retirement manifest (retirement_json captured "committed"); + (* The producer owns deferred collection. A purge failure must not + block a replacement cache or undo the durable commit decision. *) + (match remove_tree data with + | Error _ -> () + | Ok () -> + (try sync_path (Filename.dirname data); + write_retirement manifest (retirement_json captured "complete") with _ -> ())); + Ok ()) + else if state = "committed" then Ok () + else ( + if Sys.file_exists data then ( + if identity data <> captured then failwith "retired asset identity changed"; + mkdir (Filename.dirname canonical); + let error = rename_exclusive data canonical in + if error <> 0 then failwith ("asset restore conflict; retirement retained: " ^ string_of_int error); + sync_path (Filename.dirname canonical); sync_path (Filename.dirname data)) + else if captured <> "absent" && + (not (Sys.file_exists canonical) || identity canonical <> captured) + then failwith "asset restore is incomplete; retirement retained"; + if Sys.file_exists (Filename.dirname canonical) then sync_path (Filename.dirname canonical); + if Sys.file_exists (Filename.dirname data) then sync_path (Filename.dirname data); + write_retirement manifest (retirement_json captured "aborted"); + Ok ())) + ;; + + let collect_retired_assets ~root ~(scope : Core.graph_scope) = + let _, directory, _, _ = graph_deletion_paths ~root scope + (Core.retirement_id_of_string "00000000-0000-4000-8000-000000000000" |> Result.get_ok) in + let pending = Filename.concat directory "pending" in + if Sys.file_exists pending then + Array.iter (fun name -> + if Filename.check_suffix name ".data" then + try + let retirement = Filename.chop_suffix name ".data" |> Core.retirement_id_of_string |> Result.get_ok in + let _, _, manifest, _ = graph_deletion_paths ~root scope retirement in + let _, state = retirement_state manifest in + if state = "committed" || state = "complete" || state = "account-deleted" then + ignore (finish_graph_deletion ~root + ~deletion:(Core.asset_deletion_reference ~scope ~retirement) ~commit:true) + with _ -> ()) (Sys.readdir pending) + ;; + + let delete_account ~root ~account = + protect (fun () -> + let directory = account_directory ~root account in + let retired = Filename.concat (Filename.concat root ".asset-retirements") (Filename.basename directory) in + if Sys.file_exists retired then + Array.iter (fun graph -> + match Uuid.of_string graph with + | Error _ -> () + | Ok _ -> + let folder = Filename.concat retired graph in + let states = Filename.concat folder "states" in + if Sys.file_exists states then Array.iter (fun file -> + if Filename.check_suffix file ".json" then ( + let manifest = Filename.concat states file in + let captured, _ = retirement_state manifest in + write_retirement manifest (retirement_json captured "account-deleted"))) (Sys.readdir states); + match remove_tree (Filename.concat folder "pending") with + | Ok () -> () | Error _ -> failwith "retired account assets remain") (Sys.readdir retired); + remove_tree directory) + ;; + + let create ~root ~(scope : Core.graph_scope) ~budget_bytes ~maximum_file_bytes = + (try collect_retired_assets ~root ~scope with _ -> ()); + protect (fun () -> + if + budget_bytes < 0L + || maximum_file_bytes < 0 + || maximum_file_bytes > 100 * 1024 * 1024 + then Error (Invalid "Invalid cache size limits") + else ( + let directory = + Filename.concat + (account_directory ~root scope.account) + (Uuid.to_string scope.graph_id) + in + mkdir directory; + (* Validate all persisted JSON before cleaning anything. Unsupported + internal formats require an explicit caller-visible failure. *) + let rec load_manifests records = function + | [] -> Ok (List.rev records) + | file :: rest when Filename.check_suffix file ".json" -> + let* record = read_manifest directory file in + load_manifests ((file, record) :: records) rest + | _ :: rest -> load_manifests records rest + in + let* manifests = load_manifests [] (Array.to_list (Sys.readdir directory)) in + let pending = Filename.concat directory "pending" in + if Sys.file_exists pending + then + Array.iter + (fun file -> + if Filename.check_suffix file ".part" + then unlink (Filename.concat pending file)) + (Sys.readdir pending); + let t = + { directory + ; budget = budget_bytes + ; maximum_file_bytes + ; records = Hashtbl.create 64 + ; handles = Hashtbl.create 64 + ; staged_records = Hashtbl.create 8 + ; staged_handles = Hashtbl.create 8 + ; closed = false + } + in + List.iter + (fun (file, (record : record)) -> + if + record.size <= Int64.of_int maximum_file_bytes + && Sys.file_exists (data_path t record) + then Hashtbl.replace t.records record.name record + else unlink (Filename.concat directory file)) + manifests; + Array.iter + (fun file -> + if Filename.check_suffix file ".part" + then unlink (Filename.concat directory file)) + (Sys.readdir directory); + let kept = Hashtbl.create (2 * Hashtbl.length t.records) in + Hashtbl.iter + (fun _ record -> + Hashtbl.replace kept (record.name ^ ".json") (); + Hashtbl.replace kept (record.name ^ "." ^ record.file_type) ()) + t.records; + Array.iter + (fun file -> + let path = Filename.concat directory file in + if (not (Hashtbl.mem kept file)) && not (Sys.is_directory path) + then unlink path) + (Sys.readdir directory); + Ok t)) ;; type staged = diff --git a/logseq_sync/lib/effect_runner/storage/bootstrap.mli b/logseq_sync/lib/effect_runner/storage/bootstrap.mli index df38bd2..241a27b 100644 --- a/logseq_sync/lib/effect_runner/storage/bootstrap.mli +++ b/logseq_sync/lib/effect_runner/storage/bootstrap.mli @@ -63,10 +63,21 @@ module Asset_cache : sig -> account:Logseq_sync_pure_reducer.Core.account_scope -> (unit, error) result - val delete_graph - : root:string - -> account:Logseq_sync_pure_reducer.Core.account_scope - -> graph_id:Logseq_db_types.Graph_types.Uuid.t + val graph_deletion_terminal + : root:string -> scope:Logseq_sync_pure_reducer.Core.graph_scope + -> retirement:Logseq_sync_pure_reducer.Core.retirement_id + -> ([ `Committed | `Aborted ] option, error) result + + val prepare_graph_deletion + : root:string + -> scope:Logseq_sync_pure_reducer.Core.graph_scope + -> retirement:Logseq_sync_pure_reducer.Core.retirement_id + -> (unit, error) result + + val finish_graph_deletion + : root:string + -> deletion:Logseq_sync_pure_reducer.Core.asset_deletion + -> commit:bool -> (unit, error) result type staged = private diff --git a/logseq_sync/lib/pure_reducer/core.ml b/logseq_sync/lib/pure_reducer/core.ml index ee7c658..a86f9a4 100644 --- a/logseq_sync/lib/pure_reducer/core.ml +++ b/logseq_sync/lib/pure_reducer/core.ml @@ -301,6 +301,15 @@ type private_key_unlock = ; private_key_package : string } +type retirement_id = Logseq_db_types.Graph_types.Retirement_id.t +let retirement_id_of_string = Logseq_db_types.Graph_types.Retirement_id.of_string +let retirement_id_to_string = Logseq_db_types.Graph_types.Retirement_id.to_string + +type asset_deletion = { deletion_scope : graph_scope; retirement : retirement_id } +let asset_deletion_reference ~scope ~retirement = { deletion_scope = scope; retirement } +let asset_deletion_scope deletion = deletion.deletion_scope +let asset_deletion_retirement deletion = deletion.retirement + type asset_context = { scope : graph_scope ; encrypted : bool @@ -331,7 +340,9 @@ type asset_action = | Release_staged_file of string | Prune_asset_staging of string list | Close_asset_scope - | Delete_graph_assets + | Prepare_graph_asset_deletion of retirement_id + | Commit_graph_asset_deletion of asset_deletion + | Cancel_graph_asset_deletion of asset_deletion | Delete_account_assets | Asset_retry_after of float | Cancel_asset_operation of string @@ -359,6 +370,7 @@ type asset_value = ; size : int64 } | Asset_retained of (string * string) option + | Asset_deletion_prepared of asset_deletion | Asset_pruned of int | Asset_retry_elapsed @@ -720,8 +732,16 @@ type websocket_owner = ; phase : websocket_phase } +module Asset_retirement_set = Map.Make (String) +let asset_retirement_key (scope : graph_scope) retirement = + Printf.sprintf "%S:%S:%S:%S" (Uri.to_string scope.account.managed_sync_origin) + scope.account.user_id (Logseq_db_types.Graph_types.Uuid.to_string scope.graph_id) + (retirement_id_to_string retirement) +;; + type t = - { asset_known_scopes : graph_scope list + { asset_retirement_observed : unit Asset_retirement_set.t + ; asset_known_scopes : graph_scope list ; protected_pending : protected_ticket list ; assets_pending : pending_asset list ; config : config @@ -796,7 +816,8 @@ let initial config = } in Ok - { asset_known_scopes = [] + { asset_retirement_observed = Asset_retirement_set.empty + ; asset_known_scopes = [] ; protected_pending = [] ; assets_pending = [] ; config @@ -3024,7 +3045,9 @@ let asset_cleanup_action = function | Release_asset_file _ | Release_staged_file _ | Close_asset_scope - | Delete_graph_assets + | Prepare_graph_asset_deletion _ + | Commit_graph_asset_deletion _ + | Cancel_graph_asset_deletion _ | Delete_account_assets | Cancel_asset_operation _ -> true | Check_asset_cache _ @@ -3044,6 +3067,7 @@ let asset_cleanup core (request : asset_io_request) result = Some (Release_asset_file handle) | Ok (Asset_retained (Some (lease, _))) -> Some (Release_asset_file lease) | Ok (Asset_staged { file; _ }) -> Some (Release_staged_file file) + | Ok (Asset_deletion_prepared deletion) -> Some (Cancel_graph_asset_deletion deletion) | Ok ( Asset_cached None | Asset_retained None @@ -3059,10 +3083,10 @@ let asset_cleanup core (request : asset_io_request) result = { request with operation = "cleanup:" ^ string_of_int core.next_effect_id; action } ;; -let asset_result_matches action = function +let asset_result_matches (request : asset_io_request) = function | Error _ -> true | Ok value -> - (match action, value with + (match request.action, value with | Check_asset_cache _, Asset_cached _ | Fetch_asset _, Asset_downloaded _ | Stage_asset_file _, Asset_staged _ @@ -3073,7 +3097,11 @@ let asset_result_matches action = function | Release_staged_file _, Asset_unit | Prune_asset_staging _, Asset_pruned _ | Close_asset_scope, Asset_unit - | Delete_graph_assets, Asset_unit + -> true + | Prepare_graph_asset_deletion retirement, Asset_deletion_prepared deletion -> + retirement = deletion.retirement && request.context.scope = deletion.deletion_scope + | Commit_graph_asset_deletion _, Asset_unit + | Cancel_graph_asset_deletion _, Asset_unit | Delete_account_assets, Asset_unit | Asset_retry_after _, Asset_retry_elapsed | Cancel_asset_operation _, Asset_unit -> true @@ -3093,9 +3121,15 @@ let complete_asset core ticket result = } in let request = ticket.asset_request in + let core = match request.action, result with + | (Commit_graph_asset_deletion deletion | Cancel_graph_asset_deletion deletion), Ok Asset_unit -> + {core with asset_retirement_observed=Asset_retirement_set.add + (asset_retirement_key (asset_deletion_scope deletion) (asset_deletion_retirement deletion)) () + core.asset_retirement_observed} + | _ -> core in if pending.asset_cancelled then asset_cleanup core request result - else if not (asset_result_matches request.action result) + else if not (asset_result_matches request result) then ( let cleaned = asset_cleanup core request result in { cleaned with @@ -3156,14 +3190,15 @@ let request_asset core (request : asset_request) = List.exists (fun (scope : graph_scope) -> scope.account = request.scope.account) core.asset_known_scopes - | Delete_graph_assets -> - Option.fold ~none:false ~some:(fun (scope : graph_scope) -> - scope.account = request.scope.account && scope.graph_id = request.scope.graph_id) core.deletion_scope - || List.exists - (fun (scope : graph_scope) -> - scope.account = request.scope.account - && scope.graph_id = request.scope.graph_id) - core.asset_known_scopes + | Commit_graph_asset_deletion deletion | Cancel_graph_asset_deletion deletion -> + asset_deletion_scope deletion = request.scope + && (List.mem request.scope core.asset_known_scopes + || Option.fold ~none:false ~some:(fun user -> request.scope.account = account_scope core user) core.user_id) + | Prepare_graph_asset_deletion _ -> + let same_target (scope : graph_scope) = scope.account = request.scope.account + && scope.graph_id = request.scope.graph_id in + Option.fold ~none:false ~some:same_target core.deletion_scope + || Option.fold ~none:false ~some:same_target core.current_graph_scope | _ -> List.mem request.scope core.asset_known_scopes in if known @@ -3188,11 +3223,15 @@ let request_asset core (request : asset_request) = | Put_asset_file { maximum_plaintext_bytes; _ } -> maximum_plaintext_bytes >= 0 && maximum_plaintext_bytes <= 100 * 1024 * 1024 | Asset_retry_after seconds -> Float.is_finite seconds && seconds >= 0. + | Commit_graph_asset_deletion deletion | Cancel_graph_asset_deletion deletion -> + asset_deletion_scope deletion = request.scope | _ -> true in if duplicate then unchanged core - else if (core.closed && not cleanup) || context = None || not valid + else if (core.closed && not cleanup) + || (match request.action with Prepare_graph_asset_deletion _ -> core.closed || core.user_id=None | _ -> false) + || context = None || not valid then { next = core; effects = [ asset_output request (Error Asset_cancelled) ] } else ( let core, cancelled = @@ -3202,7 +3241,8 @@ let request_asset core (request : asset_request) = previous.context.scope = request.scope && previous.operation = operation) | Close_asset_scope -> cancel_asset_pending core (fun previous -> previous.context.scope = request.scope) - | Delete_graph_assets -> + | Prepare_graph_asset_deletion retirement + when not (Asset_retirement_set.mem (asset_retirement_key request.scope retirement) core.asset_retirement_observed) -> cancel_asset_pending core (fun previous -> same_asset_account previous.context.scope.account request.scope.account && previous.context.scope.graph_id = request.scope.graph_id) @@ -3211,6 +3251,14 @@ let request_asset core (request : asset_request) = same_asset_account previous.context.scope.account request.scope.account) | _ -> core, [] in + let core = match request.action with + | Prepare_graph_asset_deletion retirement -> + {core with + asset_retirement_observed=Asset_retirement_set.add (asset_retirement_key request.scope retirement) () + core.asset_retirement_observed + ; asset_known_scopes=if List.mem request.scope core.asset_known_scopes then core.asset_known_scopes + else request.scope :: core.asset_known_scopes} + | _ -> core in let issued = issue_asset core diff --git a/logseq_sync/spec/pure_reducer/core.mli b/logseq_sync/spec/pure_reducer/core.mli index 0a5d8ee..1697a07 100644 --- a/logseq_sync/spec/pure_reducer/core.mli +++ b/logseq_sync/spec/pure_reducer/core.mli @@ -208,6 +208,18 @@ type asset_context = ; key : graph_key_handle option } +(** A transaction identity, never the target graph UUID. *) +type retirement_id = Logseq_db_types.Graph_types.Retirement_id.t +val retirement_id_of_string : string -> (retirement_id, string) result +val retirement_id_to_string : retirement_id -> string + +(** A scoped reference to one durable, exclusive retired asset namespace. + Construction grants no access; execution validates the producer journal. *) +type asset_deletion +val asset_deletion_reference : scope:graph_scope -> retirement:retirement_id -> asset_deletion +val asset_deletion_scope : asset_deletion -> graph_scope +val asset_deletion_retirement : asset_deletion -> retirement_id + (** Business inputs carry identities and resource references, never asset bytes. *) type asset_action = | Check_asset_cache of graph_id * Logseq_db_types.Asset_descriptor.version @@ -233,7 +245,15 @@ type asset_action = | Release_staged_file of string | Prune_asset_staging of string list | Close_asset_scope - | Delete_graph_assets + (** Cancels every target generation and waits for operations/native leases. + New target work remains blocked until its terminal action; lease release + remains available. Preparing retires files without unlinking their bytes. + The worker journals the mirror identity and frozen upload intents first. *) + | Prepare_graph_asset_deletion of retirement_id + (** Collects only this captured namespace. Both terminal actions are replayable; + committed work is never restored, and a replacement cache is never swept. *) + | Commit_graph_asset_deletion of asset_deletion + | Cancel_graph_asset_deletion of asset_deletion | Delete_account_assets | Asset_retry_after of float | Cancel_asset_operation of string @@ -261,6 +281,7 @@ type asset_value = ; size : int64 } | Asset_retained of (string * string) option + | Asset_deletion_prepared of asset_deletion | Asset_pruned of int | Asset_retry_elapsed diff --git a/logseq_sync/test/core_contract.ml b/logseq_sync/test/core_contract.ml index 83c1ef1..c2bc18f 100644 --- a/logseq_sync/test/core_contract.ml +++ b/logseq_sync/test/core_contract.ml @@ -4478,7 +4478,7 @@ let asset_cleanup_requires_admitted_scope () = [ Core.Release_asset_file "resource" ; Core.Release_staged_file "stage" ; Core.Close_asset_scope - ; Core.Delete_graph_assets + ; (Core.Prepare_graph_asset_deletion (Core.retirement_id_of_string "96000000-0000-4000-8000-000000000001" |> Result.get_ok)) ; Core.Delete_account_assets ; Core.Cancel_asset_operation "pending" ]; @@ -4559,7 +4559,7 @@ let asset_graph_deletion_cancels_other_generation () = core scope { scope with graph_generation = 0 } - Core.Delete_graph_assets + (Core.Prepare_graph_asset_deletion (Core.retirement_id_of_string "96000000-0000-4000-8000-000000000001" |> Result.get_ok)) ;; let asset_account_deletion_cancels_other_known_graph () = @@ -4583,8 +4583,65 @@ let asset_account_deletion_cancels_other_known_graph () = Core.Delete_account_assets ;; +let completed_retirement_cannot_cancel_replacement_work () = + let core, scope = asset_fixture () in + let retirement = Core.retirement_id_of_string "96000000-0000-4000-8000-000000000003" |> Result.get_ok in + let deletion = Core.asset_deletion_reference ~scope ~retirement in + let preparing, ticket, _ = asset_issue core scope "prepare-old" (Core.Prepare_graph_asset_deletion retirement) in + let prepared = asset_complete preparing.next ticket (Ok (Core.Asset_deletion_prepared deletion)) in + let committing, ticket, _ = asset_issue prepared.next scope "commit-old" (Core.Commit_graph_asset_deletion deletion) in + let committed = asset_complete committing.next ticket (Ok Core.Asset_unit) in + let pending, _, fresh = asset_issue committed.next scope "replacement-fetch" + (Core.Fetch_asset {asset=graph_id;version=asset_version;maximum_plaintext_bytes=8}) in + let replay = Core.step pending.next (Core.Asset_requested + {scope;operation="replay-old-prepare";action=Core.Prepare_graph_asset_deletion retirement}) in + Alcotest.(check bool) "old retirement cannot cancel replacement work" false + (List.exists (fun (output : Core.asset_output) -> output.request = fresh && output.result = Error Core.Asset_cancelled) + (asset_outputs replay.effects)) +;; + +let retirement_replay_preserves_inflight_and_account_cleanup () = + List.iter (fun after_account_cleanup -> + let core, scope = asset_fixture () in + let retirement = Core.retirement_id_of_string "96000000-0000-4000-8000-000000000014" |> Result.get_ok in + let deletion = Core.asset_deletion_reference ~scope ~retirement in + let preparing, ticket, original = asset_issue core scope "initial-prepare" (Core.Prepare_graph_asset_deletion retirement) in + let owner, pending = + if not after_account_cleanup then preparing.next, original + else ( + let prepared = asset_complete preparing.next ticket (Ok (Core.Asset_deletion_prepared deletion)) in + let clearing, ticket, _ = asset_issue prepared.next scope "account-cleanup" Core.Delete_account_assets in + let cleared = asset_complete clearing.next ticket (Ok Core.Asset_unit) in + let fresh, _, request = asset_issue cleared.next scope "new-account-fetch" + (Core.Fetch_asset {asset=graph_id;version=asset_version;maximum_plaintext_bytes=8}) in + fresh.next, request) in + let replay = Core.step owner (Core.Asset_requested + {scope;operation="replay-initial-prepare";action=Core.Prepare_graph_asset_deletion retirement}) in + Alcotest.(check bool) "replay does not cancel the existing owner" false + (List.exists (fun (output : Core.asset_output) -> output.request=pending && output.result=Error Core.Asset_cancelled) + (asset_outputs replay.effects))) [false;true] +;; + +let retirement_terminal_rejects_mismatched_token_scope () = + let core, scope = asset_fixture () in + let foreign = {scope with account={scope.account with user_id="foreign-token-account"}} in + let retirement = Core.retirement_id_of_string "96000000-0000-4000-8000-000000000015" |> Result.get_ok in + let deletion = Core.asset_deletion_reference ~scope:foreign ~retirement in + List.iter (fun action -> + let transition = Core.step core (Core.Asset_requested {scope;operation="foreign-token";action}) in + Alcotest.(check bool) "current scope cannot execute another token's cleanup" false + (List.exists (function Core.Run (Core.Asset_io _) -> true | _ -> false) transition.effects)) + [Core.Commit_graph_asset_deletion deletion;Core.Cancel_graph_asset_deletion deletion] +;; + let asset_scenarios = - [ Alcotest.test_case + [ Alcotest.test_case "retirement replay preserves inflight and account cleanup" `Quick + retirement_replay_preserves_inflight_and_account_cleanup + ; Alcotest.test_case "retirement token scope is checked for active context" `Quick + retirement_terminal_rejects_mismatched_token_scope + ; Alcotest.test_case "completed retirement preserves replacement work" `Quick + completed_retirement_cannot_cancel_replacement_work + ; Alcotest.test_case "graph asset deletion cancels pending work across generations" `Quick asset_graph_deletion_cancels_other_generation diff --git a/logseq_sync/test/runner_contract.ml b/logseq_sync/test/runner_contract.ml index 515e51c..0361c7e 100644 --- a/logseq_sync/test/runner_contract.ml +++ b/logseq_sync/test/runner_contract.ml @@ -1,4 +1,12 @@ module Core = Logseq_sync_pure_reducer.Core + +let retire_cache request = + let retirement = Core.retirement_id_of_string "96000000-0000-4000-8000-000000000001" |> Result.get_ok in + match request (Core.Prepare_graph_asset_deletion retirement) with + | Ok (Core.Asset_deletion_prepared deletion) -> request (Core.Commit_graph_asset_deletion deletion) + | Error _ as error -> error + | _ -> Error (Core.Asset_invalid_content "Expected prepared retirement") +;; module Runner = Logseq_sync_effect_runner.Effect_runner let fail format = Printf.ksprintf (fun message -> Alcotest.fail message) format @@ -1064,7 +1072,7 @@ let test_asset_local_lifecycle_uses_completion_outputs () = Alcotest.(check bool) "graph deletion completes" true - (request "delete-graph" Core.Delete_graph_assets = Ok Core.Asset_unit); + (retire_cache (fun action -> request "delete-graph" action) = Ok Core.Asset_unit); Alcotest.(check bool) "account deletion completes" true @@ -1491,10 +1499,10 @@ let test_cache_deletion_waits_for_native_lease () = let file, _, _ = cache_stage request source in let lease, path = request (Core.Retain_staged_file file) |> retained_value in cache_bool "deletion waits or reports resources in use" true - (Result.is_error (request Core.Delete_graph_assets)); + (Result.is_error (retire_cache request)); cache_bool "live preview keeps its file" true (Sys.file_exists path); cache_unit (request (Core.Release_asset_file lease)); - (match request Core.Delete_graph_assets with + (match retire_cache request with | Ok Core.Asset_unit -> () | Error (Core.Asset_invalid_content message) -> Alcotest.fail message | Error Core.Asset_cancelled -> Alcotest.fail "cleanup unexpectedly cancelled" @@ -1502,6 +1510,64 @@ let test_cache_deletion_waits_for_native_lease () = cache_bool "retry removes released file" false (Sys.file_exists path)) ;; +let test_asset_retirement_cancel_and_replacement () = + with_local_asset_cache (fun support request -> + let source = Filename.concat support "retirement-source.bin" in + Out_channel.with_open_bin source (fun output -> output_string output "source"); + let file, _, _ = cache_stage request source in + let lease, original = request (Core.Retain_staged_file file) |> retained_value in + cache_unit (request (Core.Release_asset_file lease)); + let id text = Core.retirement_id_of_string text |> Result.get_ok in + let retirement = id "96000000-0000-4000-8000-000000000011" in + let deletion = match request (Core.Prepare_graph_asset_deletion retirement) with + | Ok (Core.Asset_deletion_prepared deletion) -> deletion + | _ -> Alcotest.fail "retirement did not prepare" in + cache_bool "prepared source is only retired" false (Sys.file_exists original); + cache_unit (request (Core.Cancel_graph_asset_deletion deletion)); + cache_bool "cancel restores exact source" true (Sys.file_exists original); + cache_unit (request (Core.Commit_graph_asset_deletion deletion)); + cache_bool "late commit cannot delete aborted source" true (Sys.file_exists original); + let retirement = id "96000000-0000-4000-8000-000000000012" in + let deletion = match request (Core.Prepare_graph_asset_deletion retirement) with + | Ok (Core.Asset_deletion_prepared deletion) -> deletion + | _ -> Alcotest.fail "second retirement did not prepare" in + cache_unit (request (Core.Commit_graph_asset_deletion deletion)); + cache_bool "committed old source removed" false (Sys.file_exists original); + let fresh_file, _, _ = cache_stage request source in + let lease, fresh_path = request (Core.Retain_staged_file fresh_file) |> retained_value in + cache_unit (request (Core.Cancel_graph_asset_deletion deletion)); + cache_unit (request (Core.Commit_graph_asset_deletion deletion)); + cache_bool "old terminal replay preserves replacement source" true (Sys.file_exists fresh_path); + cache_unit (request (Core.Release_asset_file lease))) +;; + +let test_asset_retirement_restore_conflict () = + with_local_asset_cache (fun support request -> + let source = Filename.concat support "restore-source.bin" in + Out_channel.with_open_bin source (fun output -> output_string output "source"); + let file, _, _ = cache_stage request source in + let lease, original = request (Core.Retain_staged_file file) |> retained_value in + cache_unit (request (Core.Release_asset_file lease)); + let retirement = Core.retirement_id_of_string "96000000-0000-4000-8000-000000000013" |> Result.get_ok in + let deletion = match request (Core.Prepare_graph_asset_deletion retirement) with + | Ok (Core.Asset_deletion_prepared deletion) -> deletion + | _ -> Alcotest.fail "retirement did not prepare" in + let rec first_missing_ancestor location = + let parent = Filename.dirname location in + if Sys.file_exists parent then location else first_missing_ancestor parent in + let canonical = first_missing_ancestor (Filename.dirname original) in + Unix.mkdir canonical 0o700; + let sentinel = Filename.concat canonical "new-synthetic-copy" in + Out_channel.with_open_bin sentinel (fun output -> output_string output "replacement"); + cache_bool "restore conflict is reported" true + (Result.is_error (request (Core.Cancel_graph_asset_deletion deletion))); + cache_bool "restore does not overwrite replacement" true (Sys.file_exists sentinel); + Sys.remove sentinel; Unix.rmdir canonical; + cache_unit (request (Core.Cancel_graph_asset_deletion deletion)); + cache_bool "retry restores original bytes" true + (In_channel.with_open_bin original In_channel.input_all = "source")) +;; + let test_cache_staging_limits () = with_local_asset_cache ~pending_budget:4L (fun support request -> let source = Filename.concat support "picker.bin" in @@ -1544,7 +1610,7 @@ let test_cache_staging_limits () = let lease, path = request (Core.Retain_staged_file file) |> retained_value in cache_bool "failed staging leaves original intact" true (Sys.file_exists path); cache_unit (request (Core.Release_asset_file lease)); - cache_unit (request Core.Delete_graph_assets); + cache_unit (retire_cache request); cache_bool "graph deletion removes staging" false (Sys.file_exists path)) ;; @@ -1902,7 +1968,7 @@ let test_graph_asset_deletion_cancels_queued_fetch_before_cache_open () = (Core.Asset_requested { scope ; operation = "delete-before-cache" - ; action = Core.Delete_graph_assets + ; action = Core.Prepare_graph_asset_deletion (Core.retirement_id_of_string "96000000-0000-4000-8000-000000000002" |> Result.get_ok) }) in List.iter @@ -2118,3 +2184,9 @@ let scenarios = test_legacy_manifest_is_rejected_without_cleanup ] ;; + +let scenarios = scenarios @ [Alcotest.test_case + "asset retirement cancel and replacement isolation" `Quick test_asset_retirement_cancel_and_replacement] + +let scenarios = scenarios @ [Alcotest.test_case + "asset retirement restore conflict is retryable" `Quick test_asset_retirement_restore_conflict] diff --git a/logseq_sync/test/transport_contract.ml b/logseq_sync/test/transport_contract.ml index 78406a2..0ec3b5d 100644 --- a/logseq_sync/test/transport_contract.ml +++ b/logseq_sync/test/transport_contract.ml @@ -2877,7 +2877,7 @@ let test_downloaded_cache_isolation () = true (cache_lookup request second second_scope "file" = None); ignore (cache_download request second second_scope "next"); - Runner_contract.cache_unit (request first first_scope Core.Delete_graph_assets); + Runner_contract.cache_unit (Runner_contract.retire_cache (request first first_scope)); Alcotest.(check bool) "deletion leaves other account" true @@ -2942,8 +2942,8 @@ let test_downloaded_cache_graph_cleanup () = List.iter (fun (core, scope) -> ignore (cache_download request core scope "file")) [ first, scope; another, another_scope ]; - Runner_contract.cache_unit (request first scope Core.Delete_graph_assets); - Runner_contract.cache_unit (request first scope Core.Delete_graph_assets); + Runner_contract.cache_unit (Runner_contract.retire_cache (request first scope)); + Runner_contract.cache_unit (Runner_contract.retire_cache (request first scope)); Alcotest.(check bool) "selected graph removed" true diff --git a/test/application_view_test.ml b/test/application_view_test.ml index 55b6ebb..c922cc7 100644 --- a/test/application_view_test.ml +++ b/test/application_view_test.ml @@ -4908,10 +4908,14 @@ let test_graph_picker_controls scenario () = let inspections = Atomic.make 0 and deletions = Atomic.make [] in let finish_deletion = Atomic.make false in let deletion_terminal = ref false in + let release_automatic_inspection = Atomic.make false in + let automatic_inspection_returned = Atomic.make false in + let inspection_terminals = ref 0 in let graph_opens = Atomic.make 0 in let root_renders = ref 0 in let client = ref None in - let service = W.Service.create ~push_topic_count:6 ~concurrency:Serial + let service = W.Service.create ~push_topic_count:6 + ~concurrency:(if scenario = `Late_presence then Concurrent { max_in_flight = 2 } else Serial) ~init:(fun context _ -> W.Session_context.emit context ~topic:Service.manager_topic (Service.Client_state_changed manager); Ok ()) @@ -4921,7 +4925,11 @@ let test_graph_picker_controls scenario () = W.Request_context.emit context ~topic:Service.manager_topic (Service.Client_state_changed value) in match request with | Service.Inspect_local_copy request -> - Atomic.incr inspections; + let index = Atomic.fetch_and_add inspections 1 in + if scenario = `Late_presence && index = 0 then ( + Fun.protect + ~finally:(fun () -> Atomic.set automatic_inspection_returned true) + (fun () -> while not (Atomic.get release_automatic_inspection) do Eio.Fiber.yield () done)); Ok (Service.Local_copy_inspected (request, if scenario = `Presence_failure then Error "Synthetic presence read failed. Try again." else Ok (if scenario = `Visible then Service.Not_downloaded else Service.Downloaded))) @@ -4980,6 +4988,8 @@ let test_graph_picker_controls scenario () = W.on_event value (function | W.Response { outcome = Completed (Service.Local_copy_deleted _); _ } -> deletion_terminal := true + | W.Response { outcome = Completed (Service.Local_copy_inspected _) | Cancelled; _ } -> + incr inspection_terminals | _ -> ())) service in let props = Hashtbl.create 128 in let retained_select = ref None and select_detached = ref false in @@ -5019,12 +5029,18 @@ let test_graph_picker_controls scenario () = |> Result.get_ok |> Journal_startup.encode |> Result.get_ok |> Bytes.to_string in Fun.protect ~finally:(fun () -> Atomic.set finish_deletion true; + Atomic.set release_automatic_inspection true; ignore (hooks.dispose ()); Option.iter Logseq_db_worker_lui.Journal_worker_runtime.stop !client) (fun () -> consume (hooks.init 2 2 config); wait "Switch graph action mounts" (fun () -> Option.is_some (find "text" "Switch graph")); press (Option.get (find "text" "Switch graph")); wait "graph list mounts" (fun () -> Option.is_some (find "accessibility-identifier" "graph-picker-list")); + if scenario = `Late_presence then ( + let list = Option.get (find "_extension" "journal-list") in + consume (hooks.extension_event list "event" + {|{"id":1,"payload":"{\"type\":\"visible_range\",\"first\":1,\"last\":2}"}|}); + wait "automatic inspection is held in flight" (fun () -> Atomic.get inspections = 1)); match scenario with | `Visible -> let before = !root_renders in @@ -5053,7 +5069,7 @@ let test_graph_picker_controls scenario () = press (node ("graph-picker:" ^ Logseq_db_types.Graph_types.Uuid.to_string second)); wait "name selection reaches worker" (fun () -> List.exists (function Service.Select_graph id -> id = second | _ -> false) (Atomic.get commands)) - | (`Delete_menu | `Cancel | `Confirm | `Reopen | `Failure_select | `Presence_failure | `During_delete) -> + | (`Delete_menu | `Cancel | `Confirm | `Reopen | `Failure_select | `Presence_failure | `During_delete | `Late_presence) -> let stable_select = node ("graph-picker:" ^ Logseq_db_types.Graph_types.Uuid.to_string second) in if scenario = `Failure_select then retained_select := Some stable_select; ignore (node ("graph-picker-options:" ^ Logseq_db_types.Graph_types.Uuid.to_string second)); @@ -5069,13 +5085,14 @@ let test_graph_picker_controls scenario () = && String.contains text '\n' | _ -> false) values) props false); Alcotest.(check bool) "requesting confirmation does not delete" false (List.exists (function Service.Delete_local_cache _ -> true | _ -> false) (Atomic.get commands)); - wait "single presence request completes" (fun () -> Atomic.get inspections = 1); + let expected_inspections = if scenario = `Late_presence then 2 else 1 in + wait "bounded presence request completes" (fun () -> Atomic.get inspections = expected_inspections); for _ = 1 to 8 do consume (hooks.pump ()) done; - Alcotest.(check int) "rendering never repeats presence IO" 1 (Atomic.get inspections); + Alcotest.(check int) "rendering never repeats presence IO" expected_inspections (Atomic.get inspections); if scenario = `Cancel then ( press (Option.get (find "text" "Cancel")); Alcotest.(check int) "Cancel sends no deletion" 0 (List.length (Atomic.get deletions))) - else if scenario = `Confirm || scenario = `Reopen || scenario = `Failure_select || scenario = `During_delete then ( + else if scenario = `Confirm || scenario = `Reopen || scenario = `Failure_select || scenario = `During_delete || scenario = `Late_presence then ( let confirm = Hashtbl.fold (fun id values found -> if List.assoc_opt "text" values = Some (`String "Delete local copy") && List.assoc_opt "__kind" values <> Some (`String "menu-item") @@ -5106,6 +5123,12 @@ let test_graph_picker_controls scenario () = wait "existing row callback retains its graph after failure" (fun () -> List.exists (function Service.Select_graph id -> id = second | _ -> false) (Atomic.get commands))) else wait "deleted graph stays available to download" (fun () -> Option.is_some (find "text" "Not downloaded")); + if scenario = `Late_presence then ( + Atomic.set release_automatic_inspection true; + wait "automatic inspection retires" (fun () -> + Atomic.get automatic_inspection_returned && !inspection_terminals = 2); + Alcotest.(check bool) "late pre-delete inspection cannot restore Downloaded" true + (Option.is_some (find "text" "Not downloaded"))); if scenario = `Reopen then ( let switch id = let before = Atomic.get graph_opens in @@ -5145,7 +5168,8 @@ let () = Alcotest.test_case "visible presence is bounded and cached" `Quick (test_graph_picker_controls `Visible); Alcotest.test_case "reopened copy invalidates absence observation" `Quick (test_graph_picker_controls `Reopen); Alcotest.test_case "deletion failure keeps live row callback identity" `Quick (test_graph_picker_controls `Failure_select); - Alcotest.test_case "noncurrent deletion keeps current Timeline interactive" `Quick (test_graph_picker_controls `During_delete)]) + Alcotest.test_case "noncurrent deletion keeps current Timeline interactive" `Quick (test_graph_picker_controls `During_delete); + Alcotest.test_case "late inspection cannot overwrite deletion" `Quick (test_graph_picker_controls `Late_presence)]) ; ( "relative date chrome", [ Alcotest.test_case "calendar boundaries and date updates" `Quick test_timeline_relative_date_headers; Alcotest.test_case "empty Detail navigation title" `Quick test_detail_empty_navigation_title; diff --git a/test/logseq_overlay_db_boundary_test.ml b/test/logseq_overlay_db_boundary_test.ml index 67296dc..8b9100a 100644 --- a/test/logseq_overlay_db_boundary_test.ml +++ b/test/logseq_overlay_db_boundary_test.ml @@ -149,7 +149,9 @@ let test_canonical_modules root = ; "type authoritative_application =\n\ \ | Authoritative_applied of Types.authoritative_commit\n\ \ | Authoritative_deferred of Types.authoritative_defer" - ; "val delete_mirror\n : mirror_inspection" + ; "val prepare_mirror_deletion\n : mirror_inspection" + ; "val commit_mirror_deletion" + ; "val cancel_mirror_deletion" ; "val collect_garbage\n : mirror_inspection" ; "val inspect_mirror\n\ \ : application_support_directory:string\n\ From 01028571038214961fa760515c4765f03ecf0398 Mon Sep 17 00:00:00 2001 From: rcmerci Date: Fri, 16 Oct 2026 10:13:55 +0800 Subject: [PATCH 7/8] Contain local deletion IO failures and show catalog refresh errors --- app/application.ml | 11 ++++++++++- .../2026-10-15-switch-graph-native-picker.md | 3 ++- .../lib/effect_runner/effect_runner.ml | 5 ++++- logseq_db_worker/test/test_effect_runner.ml | 17 +++++++++++++++-- test/application_view_test.ml | 10 ++++++++++ 5 files changed, 41 insertions(+), 5 deletions(-) diff --git a/app/application.ml b/app/application.ml index 7492607..e237576 100644 --- a/app/application.ml +++ b/app/application.ml @@ -3311,7 +3311,14 @@ let manager_page ?picker_signal state dispatch = | Some _, _, _ -> "Deleting local copy…" | _, Some _, _ -> "Checking local copy…" | _, _, Some message -> message - | _ -> "" in + | _ -> + (match current.manager with + | Some manager -> + (match manager.last_error with + | Some message -> message + | None when manager.startup.catalog_loading -> "Refreshing graphs…" + | None -> "") + | None -> "") in let status = V.of_lui (fun context parent -> Lui_elements.text ~value:(status_text state) ?value_signal:(projected context status_text) @@ -6647,6 +6654,8 @@ let start | Some left, Some right -> left.catalog == right.catalog && left.selected_graph = right.selected_graph && left.startup.account_generation = right.startup.account_generation + && left.startup.catalog_loading = right.startup.catalog_loading + && left.last_error = right.last_error | _ -> false) else if (not left.graph_ready) || not right.graph_ready then left == right diff --git a/docs/agent-guide/implemented/feature/2026-10-15-switch-graph-native-picker.md b/docs/agent-guide/implemented/feature/2026-10-15-switch-graph-native-picker.md index 3b5f0ad..5811325 100644 --- a/docs/agent-guide/implemented/feature/2026-10-15-switch-graph-native-picker.md +++ b/docs/agent-guide/implemented/feature/2026-10-15-switch-graph-native-picker.md @@ -41,7 +41,8 @@ Their production callers belong to this workflow. Migrate those callers and thei ## Validation -- Public mounted Application tests cover current marking, close/session retention, name switching, menu confirmation, action usability while another graph is deleting, and cancellation of superseded presence checks. +- Public mounted Application tests reproduce catalog-refresh error visibility through the existing service and mounted property subscriptions; the reducer cannot execute the missing presentation dependency. Refresh loading/errors remain visible in the list. They also cover current marking, close/session retention, name switching, menu confirmation, action usability while another graph is deleting, and cancellation of superseded presence checks. +- A public worker actor regression faults the coordinator-journal directory with a real file, proving that mkdir/write/fsync exceptions return a retryable deletion error rather than escaping into the shared session switch. The mirror and current graph session remain available. Pure reducer events cannot execute filesystem exceptions. - Public worker actor tests use real SQLite ownership contention to prove that busy admission precedes asset retirement. They also hold an asset preparation completion while catalog reconciliation retires another graph, proving only the exact deletion ticket/request can resolve its waiter. - Public Database tests cover stale generations, rebound account ownership, preparation ownership, cancellation, durable commit replay, partial-purge retry and replacement-mirror isolation. A pure reducer cannot execute filesystem permission failures or ownership acquisition; injecting a failed completion there would not reproduce either bug. - Public asset reducer events cover token scope validation and replay during pending work, after commit and after account cleanup. Public runner requests exercise native leases, staged-file restoration, atomic restore conflicts and replacement-cache isolation. diff --git a/logseq_db_worker/lib/effect_runner/effect_runner.ml b/logseq_db_worker/lib/effect_runner/effect_runner.ml index 5bd96ce..704f9ed 100644 --- a/logseq_db_worker/lib/effect_runner/effect_runner.ml +++ b/logseq_db_worker/lib/effect_runner/effect_runner.ml @@ -1674,7 +1674,7 @@ let handle_sync_worker_effect t = function let ( let* ) = Result.bind in let current () = deletion_current t request in let scope : Sync.graph_scope = {account=request.account;graph_id=request.graph_id;graph_generation=0} in - let result = + let result = try let* () = if current () then Ok () else Error "The graph account is no longer current." in let* () = match t.attached with | Some (_, actual) when actual.graph_id = request.graph_id -> Error "Switch to another graph before deleting." @@ -1740,6 +1740,9 @@ let handle_sync_worker_effect t = function with _ -> ()); Hashtbl.remove t.inspections (Graph.Uuid.to_string request.graph_id); Ok ()))) + with + | Eio.Cancel.Cancelled _ as cancelled -> raise cancelled + | _ -> Error "The local copy could not be deleted. Check available storage and try again." in Ok (sync_result ~event:(Sync.Mirror_deleted (request, result)) ()) | Attach_graph request -> diff --git a/logseq_db_worker/test/test_effect_runner.ml b/logseq_db_worker/test/test_effect_runner.ml index 45fd55f..4cddc50 100644 --- a/logseq_db_worker/test/test_effect_runner.ml +++ b/logseq_db_worker/test/test_effect_runner.ml @@ -1347,7 +1347,7 @@ let test_upload_checkpoint_io () = Runner.shutdown restarted))) ;; -let deletion_runtime_fixture ~busy () = +let deletion_runtime_fixture ?(journal_failure = false) ~busy () = let module Db = Logseq_db_worker in let module G = Logseq_db_types.Graph_types in T.with_managed (fun fixture -> @@ -1458,7 +1458,17 @@ let deletion_runtime_fixture ~busy () = snapshot.selected_graph = Some second && (Db.graph_state db).phase = Db.Graph_open && not snapshot.startup.bootstrapping && not snapshot.startup.restoring_local); let generation = (Db.view db).sync.snapshot.startup.account_generation in - if busy then ( + if journal_failure then ( + let blocked = Filename.concat worker_root "local-copy-retirements" in + Out_channel.with_open_bin blocked (fun output -> output_string output "synthetic mkdir fault"); + Alcotest.(check bool) "journal IO failure returns a deletion error" true + (Result.is_error (Db.delete_local_copy db ~account_generation:generation ~graph_id:first)); + Alcotest.(check int) "journal failure never retires assets" 0 !destructive_requests; + Alcotest.(check bool) "journal failure preserves target mirror" true (Sys.file_exists first_database_path); + Alcotest.(check bool) "current graph Worker stays open" true ((Db.graph_state db).phase = Db.Graph_open); + post Sync.Catalog_refresh_requested; + wait (fun () -> (Db.view db).sync.snapshot.catalog <> [])) + else if busy then ( let lock = Sqlite3.db_open (Filename.concat directory ".logseq-overlay-owner.sqlite") in Fun.protect ~finally:(fun () -> ignore (Sqlite3.exec lock "ROLLBACK"); ignore (Sqlite3.db_close lock)) (fun () -> Sqlite3.Rc.check (Sqlite3.exec lock "BEGIN IMMEDIATE"); @@ -1487,6 +1497,7 @@ let deletion_runtime_fixture ~busy () = let test_catalog_refresh_cannot_complete_another_deletion () = deletion_runtime_fixture ~busy:false () let test_deletion_checks_ownership_before_assets () = deletion_runtime_fixture ~busy:true () +let test_deletion_journal_failure_preserves_worker () = deletion_runtime_fixture ~journal_failure:true ~busy:false () let () = Alcotest.run @@ -1565,6 +1576,8 @@ let () = test_catalog_refresh_cannot_complete_another_deletion ; Alcotest.test_case "busy mirror preserves attachment sources" `Quick test_deletion_checks_ownership_before_assets + ; Alcotest.test_case "journal IO failure preserves current Worker" `Quick + test_deletion_journal_failure_preserves_worker ] ) ] ;; diff --git a/test/application_view_test.ml b/test/application_view_test.ml index c922cc7..40f71f1 100644 --- a/test/application_view_test.ml +++ b/test/application_view_test.ml @@ -4952,6 +4952,9 @@ let test_graph_picker_controls scenario () = | Return_to_graph_picker -> publish { previous with snapshot = { previous.snapshot with selected_graph = None ; startup = { previous.snapshot.startup with awaiting_selection = true } } } + | Refresh_catalog when scenario = `Refresh_failure -> + publish { previous with snapshot = { previous.snapshot with + last_error = Some "Synthetic catalog refresh failed. Try again." } } | Select_graph id -> let generation = previous.snapshot.startup.graph_generation + 1 in publish { previous with snapshot = { previous.snapshot with selected_graph = Some id @@ -5042,6 +5045,12 @@ let test_graph_picker_controls scenario () = {|{"id":1,"payload":"{\"type\":\"visible_range\",\"first\":1,\"last\":2}"}|}); wait "automatic inspection is held in flight" (fun () -> Atomic.get inspections = 1)); match scenario with + | `Refresh_failure -> + press (node "graph-picker-refresh"); + wait "catalog refresh failure is visible" (fun () -> + Option.is_some (find "text" "Synthetic catalog refresh failed. Try again.")); + Alcotest.(check bool) "catalog rows remain available after refresh failure" true + (Option.is_some (find "text" long_name)) | `Visible -> let before = !root_renders in let list = Option.get (find "_extension" "journal-list") in @@ -5157,6 +5166,7 @@ let () = else Alcotest.run "application view" [ ( "graph picker", [ + Alcotest.test_case "catalog refresh failure is visible" `Quick (test_graph_picker_controls `Refresh_failure); Alcotest.test_case "current marker and long names" `Quick (test_graph_picker_controls `Current); Alcotest.test_case "close retains current session" `Quick (test_graph_picker_controls `Close); Alcotest.test_case "name switches graph" `Quick (test_graph_picker_controls `Select); From daf90732a1a65f97114d5186888f5e3013a47738 Mon Sep 17 00:00:00 2001 From: rcmerci Date: Fri, 16 Oct 2026 10:20:36 +0800 Subject: [PATCH 8/8] Clear stale local copy feedback when refreshing graph catalog --- app/application.ml | 4 +++- test/application_view_test.ml | 15 ++++++++++----- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/app/application.ml b/app/application.ml index e237576..d521484 100644 --- a/app/application.ml +++ b/app/application.ml @@ -5955,7 +5955,9 @@ let start if Option.bind snapshot.manager (fun value -> value.selected_graph) = Some graph_id then Effect.ignore else send_manager (Graph_service.Select_graph graph_id))) - | Refresh_catalog -> send_manager Graph_service.Refresh_catalog + | Refresh_catalog -> + Effect.bind (update (fun state -> { state with local_copy_message = None })) + ~f:(fun () -> send_manager Graph_service.Refresh_catalog) | Begin_online_recovery -> send_manager Graph_service.Begin_online_recovery | Open_capture when snapshot.write_enabled diff --git a/test/application_view_test.ml b/test/application_view_test.ml index 40f71f1..b77f3e9 100644 --- a/test/application_view_test.ml +++ b/test/application_view_test.ml @@ -4931,7 +4931,7 @@ let test_graph_picker_controls scenario () = ~finally:(fun () -> Atomic.set automatic_inspection_returned true) (fun () -> while not (Atomic.get release_automatic_inspection) do Eio.Fiber.yield () done)); Ok (Service.Local_copy_inspected (request, - if scenario = `Presence_failure then Error "Synthetic presence read failed. Try again." + if scenario = `Presence_failure || scenario = `Refresh_after_presence_failure then Error "Synthetic presence read failed. Try again." else Ok (if scenario = `Visible then Service.Not_downloaded else Service.Downloaded))) | Service.Delete_local_copy request -> Atomic.set deletions (request :: Atomic.get deletions); @@ -4952,7 +4952,7 @@ let test_graph_picker_controls scenario () = | Return_to_graph_picker -> publish { previous with snapshot = { previous.snapshot with selected_graph = None ; startup = { previous.snapshot.startup with awaiting_selection = true } } } - | Refresh_catalog when scenario = `Refresh_failure -> + | Refresh_catalog when scenario = `Refresh_failure || scenario = `Refresh_after_presence_failure -> publish { previous with snapshot = { previous.snapshot with last_error = Some "Synthetic catalog refresh failed. Try again." } } | Select_graph id -> @@ -5078,15 +5078,19 @@ let test_graph_picker_controls scenario () = press (node ("graph-picker:" ^ Logseq_db_types.Graph_types.Uuid.to_string second)); wait "name selection reaches worker" (fun () -> List.exists (function Service.Select_graph id -> id = second | _ -> false) (Atomic.get commands)) - | (`Delete_menu | `Cancel | `Confirm | `Reopen | `Failure_select | `Presence_failure | `During_delete | `Late_presence) -> + | (`Delete_menu | `Cancel | `Confirm | `Reopen | `Failure_select | `Presence_failure | `Refresh_after_presence_failure | `During_delete | `Late_presence) -> let stable_select = node ("graph-picker:" ^ Logseq_db_types.Graph_types.Uuid.to_string second) in if scenario = `Failure_select then retained_select := Some stable_select; ignore (node ("graph-picker-options:" ^ Logseq_db_types.Graph_types.Uuid.to_string second)); wait "row action is exposed" (fun () -> Option.is_some (find "text" "Delete local copy")); press (node ("graph-picker-delete:" ^ Logseq_db_types.Graph_types.Uuid.to_string second)); - if scenario = `Presence_failure then ( + if scenario = `Presence_failure || scenario = `Refresh_after_presence_failure then ( wait "presence failure is visible" (fun () -> Option.is_some (find "text" "Synthetic presence read failed. Try again.")); - Alcotest.(check int) "one bounded presence request" 1 (Atomic.get inspections)) + Alcotest.(check int) "one bounded presence request" 1 (Atomic.get inspections); + if scenario = `Refresh_after_presence_failure then ( + press (node "graph-picker-refresh"); + wait "refresh failure replaces previous local-copy message" (fun () -> + Option.is_some (find "text" "Synthetic catalog refresh failed. Try again.")))) else ( wait "confirm names data loss" (fun () -> Hashtbl.fold (fun _ values found -> found || List.exists (fun (_, value) -> @@ -5166,6 +5170,7 @@ let () = else Alcotest.run "application view" [ ( "graph picker", [ + Alcotest.test_case "refresh failure replaces old presence failure" `Quick (test_graph_picker_controls `Refresh_after_presence_failure); Alcotest.test_case "catalog refresh failure is visible" `Quick (test_graph_picker_controls `Refresh_failure); Alcotest.test_case "current marker and long names" `Quick (test_graph_picker_controls `Current); Alcotest.test_case "close retains current session" `Quick (test_graph_picker_controls `Close);