diff --git a/.serena/memories/frontend/workspace-state-persistence-subtleties.md b/.serena/memories/frontend/workspace-state-persistence-subtleties.md index 90237fb83c..0cb9e7bbc5 100644 --- a/.serena/memories/frontend/workspace-state-persistence-subtleties.md +++ b/.serena/memories/frontend/workspace-state-persistence-subtleties.md @@ -27,8 +27,9 @@ - Save answer `{:revn (pre-update) :lagged [rows revn > sent revn, created-at order, own row last] :rejected?}`. `settle-saved-commit` replaces a direct `commit-persisted`: for the current file, when the last `:lagged` row is this session's (`:session-id state`) and a row of another session (a peer, or the backend repair session, whose `:file-change` `:repaired` message also reaches the saver) has revn > `:remote-revn`, the head entry gets `::awaiting` (attempt-state `:in-flight`, no resend) and stays pending until a remote commit with that revn arrives, so the normal remote rebase puts that row under the saved commits, as the server ordered them; then `commit-persisted`. Bounded: `unseen-changes-timeout-ms` (10 s, `rx/timer`) acknowledges and emits `::dw/reload-current-file`; `finalize-workspace` acknowledges at once. Repair changes are never applied from the HTTP answer; ws messages per file arrive in publish order, so max received revn is the seen test. - A reload to another version (peer restore; `pending-file-changes` already skips other-vern edits): `bundle-fetched` calls `dps/forget-other-versions` → `pcm/without-other-versions`: other-vern buffered and `:saved` commits dropped, other-vern queue entries kept but emptied (`:commits []`), remote changes pruned, a `:pending` status with nothing left turns `:saved`. So remote rebases and rollbacks never replay them on the reloaded data. `stale-commit?` (entry/commit `:file-vern` ≠ loaded file `:vern`, never when the file has no `:vern`): attempt-state `:stale` → acknowledged unsent; `rollback-rejected` of a stale entry does nothing (no rebase, toast or `::rolled-back`); `save-attempt-failed` drops a stale commit's failure (e.g. `:vern-conflict`) instead of halting. `merge-commit` groups the buffer by file and vern (one save carries one vern). - Persistence is skipped in version preview/read-only mode or without edit permission. -- Failing saves lock the workspace (`app.main.data.persistence.lock`, alias `plk`): `plk/failing?` (status `:error`, or `:recovering` while a slow-cycle attempt is in flight as `:saving`) drives `sync-failure-lock` (also on `change-team-role`), which locks only while `[:permissions :can-edit]`: a viewer, already read-only, gets the `:save-permission-denied` toast and never the save-lock banner; `lock-on-failure` stores the previous read-only value in `[:workspace-global :persistence-lock]` and sets `:read-only?`; `unlock-after-failure` restores it (into `[:render-state :pre-context-lost-read-only?]` instead while the WebGL context is lost, which keeps its own save/restore). `set-workspace-read-only` writes into the lock while it is held, so an Inspect/Design switch is not lost. `view-only-bar*` (`ui/workspace/viewport/top_bar.cljs`, shared by both renderers) shows the save error while the lock exists (text per `dps/save-retrying?`), hides its Done/exit button and takes priority over the context-loss message. No save-failure popup: page navigation, pan, zoom and inspection stay available. Once unlocked, the usual Inspect/context-loss message and action return. Plugins still write while read-only (never as a viewer: `commit-changes` drops commits without `:can-edit`). A peer's `:file-restored` (backend adds `:profile-id`, `:created-at` and, for a version a user named, `:label`) reloads the file and opens the `:file-restored` modal: who restored (profile, else the session's presence, else "A teammate"), the version by label, else date (`PPp`), and the "latest changes couldn't be saved" line when `dps/pending-file-changes` held edits of the replaced version, which the reload drops. -- A terminal failure halts the queue on purpose: the edits stay queued so `wait-persisted-or-error` keeps rejecting, and `workspace/versions.cljs` relies on that to avoid snapshotting a file whose edits were never saved. `recover-persistence` lifts the halt when a file is opened again or the role changes. A transport failure still inside its retry window (`slow-retry-pending?`) is resent once (`resume-persistence true`), never released; any other halt emits `release-failed-file` (drops every queued commit of the file that failed) then `resume-persistence`. Opening a file also replays the queued, then buffered, edits of its version onto the loaded data (`pending-file-changes` → `restore-pending-changes` in `bundle-fetched`); a replay that throws keeps the server data and the queue. That is what unblocks the editor without a reload; because the queue is shared, dropping the failed file is also what lets other files' pending edits drain. `release-failed-file` runs at recovery time, never at failure time — clearing the error eagerly would make waiters resolve. +- The copy the backend keeps before a repair is a system snapshot labeled `internal/repair/` (`dwv/repair-snapshot?`). The History panel lists it as its own per-day group, "Before automatic repair, " (`workspace.versions.repair.version`), above that day's autosaves, and the preview banner uses the same label, so the repair notice's pointer to the version history can be followed. +- Failing saves lock the workspace (`app.main.data.persistence.lock`, alias `plk`): `plk/failing?` (status `:error`, or `:recovering` while a slow-cycle attempt is in flight as `:saving`) drives `sync-failure-lock` (also on `change-team-role`), which locks only while `[:permissions :can-edit]`: a viewer, already read-only, gets the `:save-permission-denied` toast and never the save-lock banner; `lock-on-failure` stores the previous read-only value in `[:workspace-global :persistence-lock]` and sets `:read-only?`, then emits `::plk/cancel-gestures` before `:interrupt`: drags built on `plk/gesture-stopper` (transforms: move, resize, rotate, line endpoint; drawing: box, curve, line) see `cancelled?` and drop their preview (`dwt/cancel-transform`, `dwdc/cancel-drawing`) instead of committing; the pen tool and path edition rely on the lock's `clear-edition-mode` (drops the drawing object; `finalize-path-content` is refused); component gestures key on read-only: guides (`use-guide`) and flex padding/margin/gap (`fcc/use-read-only-cancel`) drop the drag when read-only turns on and ignore pointer-down while read-only, the grid editor's `use-drag` cancels on unmount mid-drag; the v2 text editor resets its DOM and WASM layout to the committed content on `::plk/cancel-gestures` (its typing commits debounced); `options-content*` shows only the read-only Inspect panel while the lock exists (it keys on the lock, never on `:read-only?`, which the Inspect tab sets too); `unlock-after-failure` restores it (into `[:render-state :pre-context-lost-read-only?]` instead while the WebGL context is lost, which keeps its own save/restore). `set-workspace-read-only` writes into the lock while it is held, so an Inspect/Design switch is not lost. `view-only-bar*` (`ui/workspace/viewport/top_bar.cljs`, shared by both renderers) shows the save error while the lock exists (text per `dps/save-retrying?`), hides its Done/exit button and takes priority over the context-loss message. No save-failure popup: page navigation, pan, zoom and inspection stay available. Once unlocked, the usual Inspect/context-loss message and action return. Plugin writes throw (`register/check-writable`) while locked, as a viewer, in a version preview and with the render context lost, but not for `:read-only?` alone (Inspect); async writes (`createShapeFromSvgWithImages`, `uploadMediaData`/`uploadMediaUrl`, `connectLibrary`) call `r/check-writable` again after their server await and reject with the read-only error (an image already uploaded stays on the server, unused); any other commit made while locked is refused at `commit-changes`. A peer's `:file-restored` (backend adds `:profile-id`, `:created-at` and, for a version a user named, `:label`) reloads the file and opens the `:file-restored` modal: who restored (profile, else the session's presence, else "A teammate"), the version by label, else date (`PPp`), and the "latest changes couldn't be saved" line when `dps/pending-file-changes` held edits of the replaced version, which the reload drops. Reloads (`::dw/reload-current-file`: restore, rebase fallback, `errors.cljs`) re-run `initialize-workspace` without `finalize-workspace` (that would drop file data, the save lock, modals and toasts). Every stream it starts (undo `append-undo`, position-data, stale-text resize, comment positions) also stops on a newer `::initialize-workspace`, and `dwsl/initialize-shape-layout`/`dwtxt/initialize-text-reflow` on a newer instance of themselves, so a reload never doubles undo entries or handlers. It emits a new `dwn/initialize`: a new `::dwn/initialize` ends the previous subscription; for the same file it sends no `unsubscribe-file` (the server keeps one file subscription per connection, so a late unsubscribe would end the new one), for another file it does. Without it each reload doubled message handling. +- A terminal failure halts the queue on purpose: the edits stay queued so `wait-persisted-or-error` keeps rejecting, and `workspace/versions.cljs` relies on that to avoid snapshotting a file whose edits were never saved. Its rejection carries `::errors/handled? true` (the failure toast already showed), so `create-version` catches `:type :persistence` and shows `workspace.versions.create-failed-unsaved` instead of failing silently; the plugin variant rejects its promise. `recover-persistence` lifts the halt when a file is opened again or the role changes. A transport failure still inside its retry window (`slow-retry-pending?`) is resent once (`resume-persistence true`), never released; any other halt emits `release-failed-file` (drops every queued commit of the file that failed) then `resume-persistence`. Opening a file also replays the queued, then buffered, edits of its version onto the loaded data (`pending-file-changes` → `restore-pending-changes` in `bundle-fetched`); a replay that throws keeps the server data and the queue. That is what unblocks the editor without a reload; because the queue is shared, dropping the failed file is also what lets other files' pending edits drain. `release-failed-file` runs at recovery time, never at failure time — clearing the error eagerly would make waiters resolve. - The split between `initialize-persistence-state` and `recover-persistence` is deliberate: initialization deliberately does NOT lift a halt, so undeliverable edits are released when the user acts, not when the view mounts. - **Every non-transport terminal save failure is permanent** (a backend without commit-id groups still answers an invalid batch with a terminal `:validation`). There is no "stale but valid" category: permission lost, file gone, and server-rejected changes all fail again on a resend. Losing edit permission is not temporary — the backend raises `:not-found` for it (`check-edition-permissions!` → `perms/make-check-fn`), and a save's `:not-found` is a terminal save error that keeps the workspace open (`errors/delegated-persistence-types` is only `#{:authentication}`; a missing data fragment answers it too). So `:save-permission-denied` (a client-side `can-edit` check) must never be resumed. A role change that leaves `can-edit` false keeps it halted (the edits stay queued and on screen; `notify-kept-failure` shows the count toast again, since the single-toast store lets the role-change info toast replace it); the role change that brings edit rights back releases it. A release on a role change (`release-failed-file` with `revert?`, i.e. not on opening a file, whose data comes fresh from the server) also rolls the released commits back locally with `pcm/release-plan` (a `rollback-plan` rejecting every pending commit of the file) through `apply-rollback-plan`: rebase, compensation of saved commits of the same groups, undo-history cut, `prune-removed-shapes`, `::rolled-back`. Without it the canvas keeps showing edits the server never got. Its toast `errors.save-permission-lost` is plural over `:unsaved-commits` (`count-unsaved-commits`); plural keys need `(tr k (i18n/c n))`, a bare number renders the fallback text. - Save failures split transient vs terminal (`transient-error?`: the repo retryable types `:network`/`:offline`/`:bad-gateway`/`:service-unavailable`/`:gateway-error`, plus `:invalid-save-response`, plus save-only `busy-server-error?`: `:rate-limit` (429 from rlimit/climit — a save parked on another save's file lock holds the profile's single `:update-file/by-profile` permit, so the user's saves to other files 429 after 60 s) and backend-written 504 `:server-error` with `:statement-timeout`/`:idle-in-transaction-timeout` (Postgres cancels the lock wait at 300 s, before nginx; rolled back, so resend is safe). Kept out of `rp/retryable-types` on purpose so GETs do not retry them; everything else is terminal). Terminal keeps the `:error` halt + `flash-persistence` path; transient enters a `:retrying` episode: the head commit stays queued and resends with backoff 2s/8s/20s (3 retries, then the `:error` path). A transient failure that outlasts the backoff arms `slow-retry-cycle`: one silent resend (`:recovering`) every `slow-retry-delay-ms` (30s) until a save lands, `retry-give-up-ms` (24h since `:failing-since`) passes, or the head commit collects `max-slow-failures` (3) attempts that failed after `slow-failure-ms` (90s) or more (`:slow-failures`, counted in `persistence-transient-failure` from the attempt's elapsed time; the cap is checked there before any resend, fast backoff included, so a commit at the cap goes to `persistence-failed` at once; when the cycle stops, `end-stopped-slow-cycle` clears `:recovering`, so `save-retrying?` turns false and the banner shows the stopped copy): a gateway timeout may leave the backend applying the save, so a save that keeps failing slowly must not be replayed for a day, while fast failures (offline) never count; the cycle stops on `::resume-persistence`, `::initialize-persistence` and `finalize-workspace`. Leaving the workspace also emits `suspend-retries` (`:suspended` in `:persistence`, drops `:retry-token` so a pending backoff timer goes stale): no backoff, slow cycle or `online` resend runs outside the workspace; only `workspace-initialized` (not a role change) lifts it via `recover-persistence` (`opened-file?`), which also resends a `:retrying` episode whose backoff was cut short. Its toast is `errors.save-still-retrying` without a reload link; a landing save hides the failure toast (`errors/persistence-failed-tag`). Replaying a save is safe because changes are idempotent, so there is no unknown-outcome guard: a commit whose earlier request left `active-requests` is simply sent again under the same `:commit-id`; only a request still in flight blocks a resend (`:in-flight` stays silent). Tests hold the slow cycle by stubbing `rx/timer` to never fire for `slow-retry-delay-ms`, or instant timers loop forever. Retry timers carry the episode token; a superseded token stays silent. Status stays `:retrying` through re-entries (`next-status` refuses `:pending`/`:saving` from it); waiters (`wait-persisted-or-error`) wait through it and reject only on `:error`. `terminal-status?` releases waiters only with an empty queue and an empty buffer: `run-persistence-task` sets `:saved` as soon as the queue empties, even with commits still buffered (a slow save landing while later edits wait for the 3 s flush). diff --git a/frontend/src/app/main/data/workspace/versions.cljs b/frontend/src/app/main/data/workspace/versions.cljs index 71b85abf58..30fab0de28 100644 --- a/frontend/src/app/main/data/workspace/versions.cljs +++ b/frontend/src/app/main/data/workspace/versions.cljs @@ -22,8 +22,16 @@ [app.main.repo :as rp] [app.util.i18n :refer [tr]] [beicon.v2.core :as rx] + [cuerdas.core :as str] [potok.v2.core :as ptk])) +(defn repair-snapshot? + "True when `snapshot` is the copy the backend keeps of a file before it + repairs it." + [{:keys [label]}] + (and (string? label) + (str/starts-with? label "internal/repair/"))) + (defonce default-state {:status :loading :data nil @@ -303,6 +311,10 @@ ;; localized date, instead of the internal snapshot ;; label (e.g. `internal/snapshot/20`). label (cond + (repair-snapshot? snapshot) + (tr "workspace.versions.repair.version" + (ct/format-inst (:created-at snapshot) :localized-date)) + (= "system" (:created-by snapshot)) (tr "workspace.versions.autosaved.version" (ct/format-inst (:created-at snapshot) :localized-date)) diff --git a/frontend/src/app/main/ui/workspace/sidebar/versions.cljs b/frontend/src/app/main/ui/workspace/sidebar/versions.cljs index 107c4d0885..2f5a4dcf5a 100644 --- a/frontend/src/app/main/ui/workspace/sidebar/versions.cljs +++ b/frontend/src/app/main/ui/workspace/sidebar/versions.cljs @@ -65,6 +65,9 @@ (tr "workspace.versions.warning.subtext" support-email)))) (defn- group-snapshots + "History entries of `data`: each pinned version, and the system + snapshots grouped by day, the copies kept before a repair apart from + the autosaves." [data] (->> (concat (->> data @@ -72,12 +75,15 @@ (map #(assoc % :type :version))) (->> data (filter #(= "system" (:created-by %))) - (group-by #(ct/format-inst (:created-at %) :iso-date)) - (map (fn [[day entries]] + (group-by (juxt #(ct/format-inst (:created-at %) :iso-date) + #(if (dwv/repair-snapshot? %) :repair :autosave))) + (map (fn [[[day kind] entries]] {:type :snapshot + :kind kind :created-at (ct/inst day) :snapshots entries})))) - (sort-by :created-at) + ;; On the same day the copies before a repair come above the autosaves + (sort-by (juxt :created-at #(if (= :repair (:kind %)) 1 0))) (map-indexed (fn [index item] (assoc item :index index))) (reverse))) @@ -297,8 +303,10 @@ [:li {:ref entry-ref :class (stl/css :version-entry-wrap)} [:> milestone-group* - {:label (tr "workspace.versions.autosaved.version" - (ct/format-inst (:created-at entry) :localized-date)) + {:label (let [day (ct/format-inst (:created-at entry) :localized-date)] + (if (= :repair (:kind entry)) + (tr "workspace.versions.repair.version" day) + (tr "workspace.versions.autosaved.version" day))) :snapshots (mapv :created-at (:snapshots entry)) :on-menu-click on-open-snapshot-menu}] diff --git a/frontend/test/frontend_tests/data/workspace_versions_test.cljs b/frontend/test/frontend_tests/data/workspace_versions_test.cljs index c715f2dc02..7753ba4164 100644 --- a/frontend/test/frontend_tests/data/workspace_versions_test.cljs +++ b/frontend/test/frontend_tests/data/workspace_versions_test.cljs @@ -6,6 +6,7 @@ (ns frontend-tests.data.workspace-versions-test (:require + [app.common.time :as ct] [app.common.uuid :as uuid] [app.main.data.changes :as dch] [app.main.data.notifications :as ntf] @@ -15,6 +16,7 @@ [app.main.data.workspace.versions :as versions] [app.main.refs :as refs] [app.main.repo :as rp] + [app.main.ui.workspace.sidebar.versions :as versions-ui] [app.util.timers :as tm] [beicon.v2.core :as rx] [cljs.test :as t :include-macros true] @@ -61,6 +63,25 @@ (t/is (= (if (= failure :error) :save-permission-denied :save-timeout) (:code (ex-data (first @rejected)))))))))))) +(t/deftest repair-snapshots-are-told-apart-from-autosaves + (t/is (versions/repair-snapshot? {:label "internal/repair/5" :created-by "system"})) + (t/is (not (versions/repair-snapshot? {:label "internal/snapshot/20" :created-by "system"}))) + (t/is (not (versions/repair-snapshot? {:label "My version" :created-by "user"}))) + (t/is (not (versions/repair-snapshot? {:created-by "user"})))) + +;; Scenario: on one day the backend kept two autosaves and the copy from +;; before a repair. Proves: the history shows the repair copy as its own +;; entry, above the day's autosaves, so it can be found by its label. +(t/deftest the-history-lists-the-copy-before-a-repair-apart + (let [at (fn [hh] (ct/inst (str "2026-11-03T" hh ":00:00Z"))) + data [{:id (uuid/next) :created-by "system" :label "internal/snapshot/0" :created-at (at "10")} + {:id (uuid/next) :created-by "system" :label "internal/repair/5" :created-at (at "11")} + {:id (uuid/next) :created-by "system" :label "internal/snapshot/20" :created-at (at "12")}] + entries (#'versions-ui/group-snapshots data)] + (t/is (= [:repair :autosave] (mapv :kind entries))) + (t/is (= [["internal/repair/5"] ["internal/snapshot/0" "internal/snapshot/20"]] + (mapv #(mapv :label (:snapshots %)) entries))))) + (t/deftest leaving-the-workspace-clears-the-preview-flags (let [team-id (uuid/next) file-id (uuid/next) diff --git a/frontend/translations/en.po b/frontend/translations/en.po index 1244293995..5e1f3c57e0 100644 --- a/frontend/translations/en.po +++ b/frontend/translations/en.po @@ -9886,6 +9886,10 @@ msgstr "Previewing version: %s" msgid "workspace.versions.preview.unnamed" msgstr "Unnamed version" +#: src/app/main/data/workspace/versions.cljs, src/app/main/ui/workspace/sidebar/versions.cljs +msgid "workspace.versions.repair.version" +msgstr "Before automatic repair, %s" + #: src/app/main/data/workspace/versions.cljs:239 msgid "workspace.versions.restore-warning" msgstr "Do you want to restore this version?" diff --git a/frontend/translations/es.po b/frontend/translations/es.po index db017dc84e..04c8c71b1b 100644 --- a/frontend/translations/es.po +++ b/frontend/translations/es.po @@ -10048,6 +10048,10 @@ msgstr "Previsualizando version: %s" msgid "workspace.versions.preview.unnamed" msgstr "Versión sin nombre" +#: src/app/main/data/workspace/versions.cljs, src/app/main/ui/workspace/sidebar/versions.cljs +msgid "workspace.versions.repair.version" +msgstr "Antes de la reparación automática, %s" + #: src/app/main/data/workspace/versions.cljs:239 msgid "workspace.versions.restore-warning" msgstr "¿Quieres restaurar esta versión?"