From 6259f2b375b89e83cad05ab3c333f586ab063e6e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=81lvaro=20Tejero=20Cantero?= Date: Thu, 17 Sep 2026 01:06:06 +0200 Subject: [PATCH] :bug: Carry the command's conflicts into the resolution modal The modal rebuilt its conflict list from a diff fetched separately, which is computed in another id frame than the one the update command resolves against, so an update could open a resolver showing 0 of 0 with an Apply that could never enable. The conflicts a command returns are authoritative, because they are what its own resolved? matches against: the modal now prefers them, keeps them in the store, and the bulk actions resolve that same set. Verified in a browser against preview-demo-loud without applying: 0 of 368 resolved with Apply disabled, then 368 of 368 with Apply enabled after Use main for all. Main stayed at revn 28453 and the branch row did not move. --- .../src/app/main/data/workspace/branches.cljs | 43 ++++++++++++++++--- .../main/ui/workspace/sidebar/branches.cljs | 9 +++- 2 files changed, 45 insertions(+), 7 deletions(-) diff --git a/frontend/src/app/main/data/workspace/branches.cljs b/frontend/src/app/main/data/workspace/branches.cljs index 8ef3383e15..320c553257 100644 --- a/frontend/src/app/main/data/workspace/branches.cljs +++ b/frontend/src/app/main/data/workspace/branches.cljs @@ -412,6 +412,30 @@ (rx/empty))))) (rx/catch (fn [_] (rx/of (set-branch-context file-id nil))))))))))) +(defn open-conflict-resolutions + "Open the conflict-resolution modal and seed the store with the conflict set + the command just returned. + + Those conflicts are authoritative for the whole modal: they are the very + ones the command's own `resolved?` matches against, while the diff the modal + can fetch on its own is computed in another id frame and may list a + different set, or none at all (`pp:vcs:tp-update-conflict-frame`). Keeping + them in the store also lets the bulk actions resolve the same set the modal + shows." + [{:keys [branch mode conflicts]}] + (ptk/reify ::open-conflict-resolutions + ptk/UpdateEvent + (update [_ state] + (assoc state :workspace-branch-conflicts + {:branch-id (:id branch) + :mode mode + :conflicts (vec conflicts)})) + ptk/WatchEvent + (watch [_ _ _] + (rx/of (modal/show :branch-conflicts {:branch branch + :mode mode + :conflicts conflicts}))))) + (defn update-branch-from-main "Bring main's changes into the branch (reverse of merge). `branch` is the branch row. With conflicts and no `resolutions`, opens the conflict @@ -426,13 +450,19 @@ (->> (rp/cmd! :update-branch-from-main (cond-> {:branch-id (:id branch)} (seq resolutions) (assoc :resolutions resolutions))) (rx/mapcat - (fn [{:keys [status]}] + (fn [{:keys [status conflicts]}] (case status ;; the open branch file just changed server-side: hard-reload ;; it so the pulled changes are shown :updated (rx/of (ntf/success (tr "workspace.branches.update.success")) (reload-file-window)) - :conflicts (rx/of (modal/show :branch-conflicts {:branch branch :mode :update})) + ;; the returned conflicts are authoritative: they are the very + ;; ones the command computes `resolved?` against, and the diff + ;; the modal could fetch on its own is computed in another id + ;; frame, which can disagree with them (vcs:tp-update-conflict-frame) + :conflicts (rx/of (open-conflict-resolutions {:branch branch + :mode :update + :conflicts conflicts})) :unsupported (rx/of (ntf/warn (tr "workspace.branches.update.unsupported"))) (rx/of (ntf/error (tr "workspace.branches.update.error")))))) (rx/catch (fn [cause] @@ -472,7 +502,8 @@ (ptk/reify ::set-all-resolutions ptk/UpdateEvent (update [_ state] - (let [conflicts (get-in state [:workspace-branch-diff :diff :conflicts]) + (let [conflicts (or (get-in state [:workspace-branch-conflicts :conflicts]) + (get-in state [:workspace-branch-diff :diff :conflicts])) res (into {} (map (fn [c] [(:id c) choice])) conflicts)] (assoc-in state [:workspace-branch-diff :resolutions] res))))) @@ -512,7 +543,7 @@ keep-branch (assoc :keep-branch true) (some? main-revn) (assoc :expected-main-revn main-revn))) (rx/mapcat - (fn [{:keys [status source-file-id]}] + (fn [{:keys [status source-file-id conflicts]}] (case status ;; the merged result lives in main: take the user there ;; to see it (navigate if elsewhere, hard-reload if @@ -521,7 +552,9 @@ ;; kept) already happened server-side. :merged (rx/of (ntf/success (tr "workspace.branches.merge.success")) (show-merge-result source-file-id)) - :conflicts (rx/of (modal/show :branch-conflicts {:branch branch :mode :merge})) + :conflicts (rx/of (open-conflict-resolutions {:branch branch + :mode :merge + :conflicts conflicts})) :unsupported (rx/of (ntf/warn (tr "workspace.branches.merge.unsupported"))) (rx/of (ntf/error (tr "workspace.branches.merge.error")))))) (rx/catch diff --git a/frontend/src/app/main/ui/workspace/sidebar/branches.cljs b/frontend/src/app/main/ui/workspace/sidebar/branches.cljs index 5cc6d8a850..1d0f0e2ef2 100644 --- a/frontend/src/app/main/ui/workspace/sidebar/branches.cljs +++ b/frontend/src/app/main/ui/workspace/sidebar/branches.cljs @@ -1531,7 +1531,7 @@ (mf/defc branch-conflicts-dialog* {::mf/register modal/components ::mf/register-as :branch-conflicts} - [{:keys [branch mode]}] + [{:keys [branch mode conflicts]}] (let [{:keys [diff selected resolutions status]} (mf/deref branch-diff) loading? (= status :loading) @@ -1551,7 +1551,12 @@ you (tr "workspace.branches.conflicts.card.you")] (if ago (str ago " ยท " you) you)) - conflicts (:conflicts diff) + ;; the command that opened this modal returned its own conflict set, + ;; computed in the frame it resolves against; the diff this dialog can + ;; fetch may have been computed in another frame and disagree, which + ;; left the dialog showing 0 of 0 with an Apply that could never + ;; enable (vcs:tp-update-conflict-frame). Prefer what the command said. + conflicts (if (seq conflicts) conflicts (:conflicts diff)) resolutions (or resolutions {}) total (count conflicts) resolved (count (filterv #(bm/conflict-resolved? % (get resolutions (:id %))) conflicts))