From 2185a5aecd653219f61df01bdfee777d7de354fd Mon Sep 17 00:00:00 2001
From: Tienson Qin <tiensonqin@gmail.com>
Date: Thu, 2 Apr 2026 12:47:24 +0800
Subject: [PATCH] fix: undo doesn't handle retracting property history block

---
 .../src/logseq/db/frontend/malli_schema.cljs  |   2 +-
 .../src/logseq/outliner/op/construct.cljc     | 151 ++++++++++++++----
 .../logseq/outliner/op_construct_test.cljs    |  88 ++++++++++
 src/main/frontend/worker/commands.cljs        |  23 +--
 src/main/frontend/worker/pipeline.cljs        |   4 +-
 src/main/frontend/worker/sync/apply_txs.cljs  |   1 +
 src/test/frontend/worker/db_sync_test.cljs    |  80 ++++++++++
 7 files changed, 304 insertions(+), 45 deletions(-)

diff --git a/deps/db/src/logseq/db/frontend/malli_schema.cljs b/deps/db/src/logseq/db/frontend/malli_schema.cljs
index adecac1943..83392a166c 100644
--- a/deps/db/src/logseq/db/frontend/malli_schema.cljs
+++ b/deps/db/src/logseq/db/frontend/malli_schema.cljs
@@ -444,7 +444,7 @@
     [:logseq.property.reaction/target :int]
     [:block/created-at :int]
     [:block/tx-id {:optional true} :int]
-    [:logseq.property/created-by-ref {:optional true} :int]
+    [:block/properties {:optional true} block-properties]
     [:block/refs {:optional true} [:set :int]]]))
 
 (def property-history-block*
diff --git a/deps/outliner/src/logseq/outliner/op/construct.cljc b/deps/outliner/src/logseq/outliner/op/construct.cljc
index 3f64e31485..e43cfb79c0 100644
--- a/deps/outliner/src/logseq/outliner/op/construct.cljc
+++ b/deps/outliner/src/logseq/outliner/op/construct.cljc
@@ -568,46 +568,135 @@
                            (get property-id))]
     (property-ref-value db property-id value)))
 
+(defn- property-history-refs-from-tx-data
+  [db-before db-after tx-data block-ids property-id]
+  (let [target-block-ids (->> block-ids
+                              (keep (fn [block-id]
+                                      (:db/id (block-entity db-before block-id))))
+                              set)
+        target-property-id (some-> (d/entity db-before property-id) :db/id)
+        history-eid->block-id (reduce (fn [acc d]
+                                        (if (and (:added d)
+                                                 (= :logseq.property.history/block (:a d)))
+                                          (assoc acc (:e d) (:v d))
+                                          acc))
+                                      {}
+                                      tx-data)
+        history-eid->property-id (reduce (fn [acc d]
+                                           (if (and (:added d)
+                                                    (= :logseq.property.history/property (:a d)))
+                                             (assoc acc (:e d) (:v d))
+                                             acc))
+                                         {}
+                                         tx-data)]
+    (->> history-eid->block-id
+         (keep (fn [[history-eid history-block-id]]
+                 (when (and (contains? target-block-ids history-block-id)
+                            (= target-property-id (get history-eid->property-id history-eid))
+                            (nil? (d/entity db-before history-eid)))
+                   (stable-entity-ref db-after history-eid))))
+         distinct
+         vec
+         seq)))
+
+(defn- normalize-op-or-ops
+  [op-or-ops]
+  (cond
+    (nil? op-or-ops)
+    []
+
+    (and (sequential? op-or-ops)
+         (seq op-or-ops)
+         (sequential? (first op-or-ops)))
+    (vec op-or-ops)
+
+    :else
+    [op-or-ops]))
+
+(defn- prepend-history-cleanup-op
+  [cleanup-op op-or-ops]
+  (let [ops (normalize-op-or-ops op-or-ops)
+        ops' (if cleanup-op
+               (into [cleanup-op] ops)
+               ops)]
+    (seq ops')))
+
 (defn- inverse-property-op
-  [db-before op args]
+  [db-before db-after tx-data op args]
   (case op
     :set-block-property
     (let [[block-id property-id _value] args
           before-value (block-property-value db-before block-id property-id)
-          block-ref (stable-entity-ref db-before block-id)]
-      (if (nil? before-value)
-        [:remove-block-property [block-ref property-id]]
-        [:set-block-property [block-ref property-id before-value]]))
+          block-ref (stable-entity-ref db-before block-id)
+          cleanup-op (when-let [history-refs (property-history-refs-from-tx-data
+                                              db-before
+                                              db-after
+                                              tx-data
+                                              [block-id]
+                                              property-id)]
+                       [:delete-blocks [history-refs {}]])]
+      (prepend-history-cleanup-op
+       cleanup-op
+       (if (nil? before-value)
+         [:remove-block-property [block-ref property-id]]
+         [:set-block-property [block-ref property-id before-value]])))
 
     :remove-block-property
     (let [[block-id property-id] args
           before-value (block-property-value db-before block-id property-id)
-          block-ref (stable-entity-ref db-before block-id)]
-      (when (some? before-value)
-        [:set-block-property [block-ref property-id before-value]]))
+          block-ref (stable-entity-ref db-before block-id)
+          cleanup-op (when-let [history-refs (property-history-refs-from-tx-data
+                                              db-before
+                                              db-after
+                                              tx-data
+                                              [block-id]
+                                              property-id)]
+                       [:delete-blocks [history-refs {}]])]
+      (prepend-history-cleanup-op
+       cleanup-op
+       (when (some? before-value)
+         [:set-block-property [block-ref property-id before-value]])))
 
     :batch-set-property
-    (let [[block-ids property-id _value _opts] args]
-      (->> block-ids
-           (keep (fn [block-id]
-                   (let [before-value (block-property-value db-before block-id property-id)
-                         block-ref (stable-entity-ref db-before block-id)]
-                     (if (nil? before-value)
-                       [:remove-block-property [block-ref property-id]]
-                       [:set-block-property [block-ref property-id before-value]]))))
-           vec
-           seq))
+    (let [[block-ids property-id _value _opts] args
+          cleanup-op (when-let [history-refs (property-history-refs-from-tx-data
+                                              db-before
+                                              db-after
+                                              tx-data
+                                              block-ids
+                                              property-id)]
+                       [:delete-blocks [history-refs {}]])]
+      (prepend-history-cleanup-op
+       cleanup-op
+       (->> block-ids
+            (keep (fn [block-id]
+                    (let [before-value (block-property-value db-before block-id property-id)
+                          block-ref (stable-entity-ref db-before block-id)]
+                      (if (nil? before-value)
+                        [:remove-block-property [block-ref property-id]]
+                        [:set-block-property [block-ref property-id before-value]]))))
+            vec
+            seq)))
 
     :batch-remove-property
-    (let [[block-ids property-id _opts] args]
-      (->> block-ids
-           (keep (fn [block-id]
-                   (let [before-value (block-property-value db-before block-id property-id)
-                         block-ref (stable-entity-ref db-before block-id)]
-                     (when (some? before-value)
-                       [:set-block-property [block-ref property-id before-value]]))))
-           vec
-           seq))
+    (let [[block-ids property-id _opts] args
+          cleanup-op (when-let [history-refs (property-history-refs-from-tx-data
+                                              db-before
+                                              db-after
+                                              tx-data
+                                              block-ids
+                                              property-id)]
+                       [:delete-blocks [history-refs {}]])]
+      (prepend-history-cleanup-op
+       cleanup-op
+       (->> block-ids
+            (keep (fn [block-id]
+                    (let [before-value (block-property-value db-before block-id property-id)
+                          block-ref (stable-entity-ref db-before block-id)]
+                      (when (some? before-value)
+                        [:set-block-property [block-ref property-id before-value]]))))
+            vec
+            seq)))
 
     nil))
 
@@ -853,16 +942,16 @@
                             (build-inverse-delete-page db-before page-uuid))
 
                           :set-block-property
-                          (inverse-property-op db-before op args)
+                          (inverse-property-op db-before db-after tx-data op args)
 
                           :remove-block-property
-                          (inverse-property-op db-before op args)
+                          (inverse-property-op db-before db-after tx-data op args)
 
                           :batch-set-property
-                          (inverse-property-op db-before op args)
+                          (inverse-property-op db-before db-after tx-data op args)
 
                           :batch-remove-property
-                          (inverse-property-op db-before op args)
+                          (inverse-property-op db-before db-after tx-data op args)
 
                           :create-property-text-block
                           (let [[_block-id _property-id _value opts] args
diff --git a/deps/outliner/test/logseq/outliner/op_construct_test.cljs b/deps/outliner/test/logseq/outliner/op_construct_test.cljs
index 4fddf4d8a9..18cbf252dd 100644
--- a/deps/outliner/test/logseq/outliner/op_construct_test.cljs
+++ b/deps/outliner/test/logseq/outliner/op_construct_test.cljs
@@ -364,6 +364,94 @@
                                         :logical-outdenting? nil}]]]
              inverse-outliner-ops)))))
 
+(deftest derive-history-outliner-ops-property-history-blocks-undo-cleanup-test
+  (testing ":set-block-property inverse deletes newly created property history blocks"
+    (let [conn (db-test/create-conn-with-blocks
+                {:properties {:pnum {:logseq.property/type :number
+                                     :db/cardinality :db.cardinality/one}}
+                 :pages-and-blocks
+                 [{:page {:block/title "page"}
+                   :blocks [{:block/title "task"
+                             :build/properties {:pnum 1}}]}]})
+          block (db-test/find-block-by-content @conn "task")
+          block-id (:db/id block)
+          block-ref [:block/uuid (:block/uuid block)]
+          property-id (:db/id (d/entity @conn :user.property/pnum))
+          history-uuid (random-uuid)
+          {:keys [db-after tx-data]}
+          (d/with @conn
+                  [[:db/add block-id :user.property/pnum 2]
+                   {:db/id -1
+                    :block/uuid history-uuid
+                    :logseq.property.history/block block-id
+                    :logseq.property.history/property property-id
+                    :logseq.property.history/scalar-value 2}]
+                  {})
+          {:keys [inverse-outliner-ops]}
+          (op-construct/derive-history-outliner-ops
+           @conn
+           db-after
+           tx-data
+           {:outliner-op :set-block-property
+            :outliner-ops [[:set-block-property [block-id :user.property/pnum 2]]]})]
+      (is (= :delete-blocks (ffirst inverse-outliner-ops)))
+      (is (= #{[:block/uuid history-uuid]}
+             (set (get-in inverse-outliner-ops [0 1 0]))))
+      (is (= [:set-block-property [block-ref :user.property/pnum 1]]
+             (second inverse-outliner-ops)))))
+
+  (testing ":batch-set-property inverse deletes all newly created property history blocks"
+    (let [conn (db-test/create-conn-with-blocks
+                {:properties {:pnum {:logseq.property/type :number
+                                     :db/cardinality :db.cardinality/one}}
+                 :pages-and-blocks
+                 [{:page {:block/title "page"}
+                   :blocks [{:block/title "task-1"
+                             :build/properties {:pnum 1}}
+                            {:block/title "task-2"}]}]})
+          block-1 (db-test/find-block-by-content @conn "task-1")
+          block-2 (db-test/find-block-by-content @conn "task-2")
+          block-1-id (:db/id block-1)
+          block-2-id (:db/id block-2)
+          block-1-ref [:block/uuid (:block/uuid block-1)]
+          block-2-ref [:block/uuid (:block/uuid block-2)]
+          property-id (:db/id (d/entity @conn :user.property/pnum))
+          history-uuid-1 (random-uuid)
+          history-uuid-2 (random-uuid)
+          {:keys [db-after tx-data]}
+          (d/with @conn
+                  [[:db/add block-1-id :user.property/pnum 2]
+                   [:db/add block-2-id :user.property/pnum 2]
+                   {:db/id -1
+                    :block/uuid history-uuid-1
+                    :logseq.property.history/block block-1-id
+                    :logseq.property.history/property property-id
+                    :logseq.property.history/scalar-value 2}
+                   {:db/id -2
+                    :block/uuid history-uuid-2
+                    :logseq.property.history/block block-2-id
+                    :logseq.property.history/property property-id
+                    :logseq.property.history/scalar-value 2}]
+                  {})
+          {:keys [inverse-outliner-ops]}
+          (op-construct/derive-history-outliner-ops
+           @conn
+           db-after
+           tx-data
+           {:outliner-op :batch-set-property
+            :outliner-ops [[:batch-set-property [[block-1-id block-2-id]
+                                                 :user.property/pnum
+                                                 2
+                                                 {}]]]})]
+      (is (= :delete-blocks (ffirst inverse-outliner-ops)))
+      (is (= #{[:block/uuid history-uuid-1]
+               [:block/uuid history-uuid-2]}
+             (set (get-in inverse-outliner-ops [0 1 0]))))
+      (is (= [:set-block-property [block-1-ref :user.property/pnum 1]]
+             (second inverse-outliner-ops)))
+      (is (= [:remove-block-property [block-2-ref :user.property/pnum]]
+             (nth inverse-outliner-ops 2))))))
+
 (deftest derive-history-outliner-ops-direct-outdent-with-extra-moved-blocks-keeps-semantic-ops-test
   (testing "direct outdent keeps semantic indent-outdent op and inverse"
     (let [conn (db-test/create-conn-with-blocks
diff --git a/src/main/frontend/worker/commands.cljs b/src/main/frontend/worker/commands.cljs
index 0c7f9c8d0d..7f58285701 100644
--- a/src/main/frontend/worker/commands.cljs
+++ b/src/main/frontend/worker/commands.cljs
@@ -208,17 +208,18 @@
                           (when (and (true? (get property :logseq.property/enable-history?))
                                      (:added d))
                             {:property property
-                             :value (:v d)}))) datoms)]
-    (map
-     (fn [{:keys [property value]}]
-       (let [ref? (= :db.type/ref (:db/valueType property))
-             value-key (if ref? :logseq.property.history/ref-value :logseq.property.history/scalar-value)]
-         (sqlite-util/block-with-timestamps
-          {:block/uuid (ldb/new-block-id)
-           value-key value
-           :logseq.property.history/block (:db/id entity)
-           :logseq.property.history/property (:db/id property)})))
-     changes)))
+                             :value (:v d)}))) datoms)
+        data (map
+              (fn [{:keys [property value]}]
+                (let [ref? (= :db.type/ref (:db/valueType property))
+                      value-key (if ref? :logseq.property.history/ref-value :logseq.property.history/scalar-value)]
+                  (sqlite-util/block-with-timestamps
+                   {:block/uuid (ldb/new-block-id)
+                    value-key value
+                    :logseq.property.history/block (:db/id entity)
+                    :logseq.property.history/property (:db/id property)})))
+              changes)]
+    data))
 
 (defmethod handle-command :default [command _db entity datoms]
   (throw (ex-info "Unhandled command"
diff --git a/src/main/frontend/worker/pipeline.cljs b/src/main/frontend/worker/pipeline.cljs
index 91d50db936..37129b183b 100644
--- a/src/main/frontend/worker/pipeline.cljs
+++ b/src/main/frontend/worker/pipeline.cljs
@@ -23,7 +23,7 @@
             [logseq.outliner.pipeline :as outliner-pipeline]))
 
 (def ^:private rtc-tx-or-download-graph?
-  (let [p (some-fn :rtc-op? :rtc-tx? :rtc-download-graph?)]
+  (let [p (some-fn :rtc-op? :rtc-tx? :rtc-download-graph? :transact-remote?)]
     (fn [tx-meta]
       (p tx-meta))))
 
@@ -437,7 +437,7 @@
                                         (toggle-page-and-block db tx-report))
         display-blocks-tx-data (add-missing-properties-to-typed-display-blocks db-after tx-data tx-meta)
         ensure-query-tx-data (ensure-query-property-on-tag-additions tx-report)
-        commands-tx (when-not (rtc-tx-or-download-graph? tx-meta)
+        commands-tx (when-not (or (:undo? tx-meta) (rtc-tx-or-download-graph? tx-meta))
                       (commands/run-commands tx-report))
         insert-templates-tx (when-not (rtc-tx-or-download-graph? tx-meta)
                               (insert-tag-templates tx-report))
diff --git a/src/main/frontend/worker/sync/apply_txs.cljs b/src/main/frontend/worker/sync/apply_txs.cljs
index f6eb97983b..d0515b0d14 100644
--- a/src/main/frontend/worker/sync/apply_txs.cljs
+++ b/src/main/frontend/worker/sync/apply_txs.cljs
@@ -342,6 +342,7 @@
                                 :gen-undo-ops? false
                                 :persist-op? true
                                 :undo? undo?
+                                :redo? (:redo? tx-meta)
                                 :db-sync/tx-id history-tx-id
                                 :db-sync/source-tx-id (or (:db-sync/source-tx-id tx-meta)
                                                           tx-id)}
diff --git a/src/test/frontend/worker/db_sync_test.cljs b/src/test/frontend/worker/db_sync_test.cljs
index bd7c8ddeb7..3ffd610b13 100644
--- a/src/test/frontend/worker/db_sync_test.cljs
+++ b/src/test/frontend/worker/db_sync_test.cljs
@@ -35,6 +35,7 @@
             [logseq.db.test.helper :as db-test]
             [logseq.outliner.core :as outliner-core]
             [logseq.outliner.op :as outliner-op]
+            [logseq.outliner.op.construct :as op-construct]
             [logseq.outliner.page :as outliner-page]
             [logseq.outliner.property :as outliner-property]
             [promesa.core :as p]))
@@ -3096,6 +3097,39 @@
                      (set (map :db/ident (:block/tags block-restored)))))
               (is (= base-history-count restored-history-count)))))))))
 
+(deftest derive-history-set-block-property-inverse-includes-property-history-cleanup-test
+  (testing "derive-history-outliner-ops should delete created property-history block for set-block-property"
+    (let [conn (db-test/create-conn-with-blocks
+                {:properties {:pnum {:logseq.property/type :number
+                                     :db/cardinality :db.cardinality/one}}
+                 :pages-and-blocks
+                 [{:page {:block/title "page1"}
+                   :blocks [{:block/title "task"
+                             :build/properties {:pnum 1}}]}]})
+          block-before (db-test/find-block-by-content @conn "task")
+          block-id (:db/id block-before)
+          property-id (:db/id (d/entity @conn :user.property/pnum))
+          history-uuid (random-uuid)
+          {:keys [db-after tx-data]}
+          (d/with @conn
+                  [[:db/add block-id :user.property/pnum 2]
+                   {:db/id -1
+                    :block/uuid history-uuid
+                    :logseq.property.history/block block-id
+                    :logseq.property.history/property property-id
+                    :logseq.property.history/scalar-value 2}]
+                  {})
+          {:keys [inverse-outliner-ops]}
+          (op-construct/derive-history-outliner-ops
+           @conn
+           db-after
+           tx-data
+           {:outliner-op :set-block-property
+            :outliner-ops [[:set-block-property [block-id :user.property/pnum 2]]]})]
+      (is (= :delete-blocks (ffirst inverse-outliner-ops)))
+      (is (= #{[:block/uuid history-uuid]}
+             (set (get-in inverse-outliner-ops [0 1 0])))))))
+
 (deftest pending-reversed-txs-for-batch-status-changes-restore-base-db-test
   (testing "fresh persisted reversed tx rows from repeated batch status changes should restore the base db"
     (let [conn (db-test/create-conn-with-blocks
@@ -3139,6 +3173,52 @@
                      (set (map :db/ident (:block/tags block-restored)))))
               (is (= base-history-count restored-history-count)))))))))
 
+(deftest derive-history-batch-set-property-inverse-includes-property-history-cleanup-test
+  (testing "derive-history-outliner-ops should delete created property-history blocks for batch-set-property"
+    (let [conn (db-test/create-conn-with-blocks
+                {:properties {:pnum {:logseq.property/type :number
+                                     :db/cardinality :db.cardinality/one}}
+                 :pages-and-blocks
+                 [{:page {:block/title "page1"}
+                   :blocks [{:block/title "task-1"
+                             :build/properties {:pnum 1}}
+                            {:block/title "task-2"}]}]})
+          block-1 (db-test/find-block-by-content @conn "task-1")
+          block-2 (db-test/find-block-by-content @conn "task-2")
+          property-id (:db/id (d/entity @conn :user.property/pnum))
+          history-uuid-1 (random-uuid)
+          history-uuid-2 (random-uuid)
+          {:keys [db-after tx-data]}
+          (d/with @conn
+                  [[:db/add (:db/id block-1) :user.property/pnum 2]
+                   [:db/add (:db/id block-2) :user.property/pnum 2]
+                   {:db/id -1
+                    :block/uuid history-uuid-1
+                    :logseq.property.history/block (:db/id block-1)
+                    :logseq.property.history/property property-id
+                    :logseq.property.history/scalar-value 2}
+                   {:db/id -2
+                    :block/uuid history-uuid-2
+                    :logseq.property.history/block (:db/id block-2)
+                    :logseq.property.history/property property-id
+                    :logseq.property.history/scalar-value 2}]
+                  {})
+          {:keys [inverse-outliner-ops]}
+          (op-construct/derive-history-outliner-ops
+           @conn
+           db-after
+           tx-data
+           {:outliner-op :batch-set-property
+            :outliner-ops [[:batch-set-property [[(:db/id block-1)
+                                                  (:db/id block-2)]
+                                                 :user.property/pnum
+                                                 2
+                                                 {}]]]})]
+      (is (= :delete-blocks (ffirst inverse-outliner-ops)))
+      (is (= #{[:block/uuid history-uuid-1]
+               [:block/uuid history-uuid-2]}
+             (set (get-in inverse-outliner-ops [0 1 0])))))))
+
 (deftest normalize-rebased-pending-tx-keeps-reconstructive-reverse-for-retract-entity-test
   (testing "rebased pending tx should keep non-empty reverse datoms even when forward tx collapses to retractEntity"
     (let [conn (db-test/create-conn-with-blocks