🐛 Decode snapshot migrations as an ordered set

`file-snapshots/decode-snapshot` handed the `file_change.migrations`
pgarray as a vector. `set/difference` falls back to index lookups when
the first collection is shorter than the second, so the pending set was
right only by accident: a stored subset happens to work, while a
stored set larger than `available-migrations` (a snapshot taken by a
newer build) marks every available migration as pending and re-applies
it to already migrated data.

Decode it as a `d/ordered-set`, the type the rest of the code and
`schema:file` expect. This also covers `restore!` and the snapshot
list, and it lets `get-file-snapshot` drop the `(into #{} ...)`
normalization it was carrying. Re-wrap the union in `fmg/migrate` so
the invariant holds even when a caller hands a plain set.

A snapshot with NULL `migrations` predates SQL migration 0140 and
keeps the format version of its time. Pin that legacy path: the
preview regenerates the applied set from that version, migrates in
memory and leaves the stored row untouched. Update the file-data
memory with the invariant and the legacy path.

AI-assisted-by: deepseek-v4.1-flash

.serena/memories/backend/subtleties.md

@@ -94,7 +94,8 @@
 - Pointer-map fragments are persisted separately as type `fragment`, and only modified pointer maps are written. A pointer keeps its id while nothing changes; content (`assoc`/`without`) and metadata (`with-meta`) changes produce a fresh id, so a fragment row is never rewritten after it is written. Re-assoc with an identical value returns the same map in array and hash maps alike, so a save keeps the fragment id of every page it does not change.
 - `enable-objects-map` skips unloaded pointer pages: their fragment was already wrapped when it was written, and loading it only to wrap it again costs one page read per untouched page on every save. A page with a plain fragment is wrapped when it is first loaded or edited; a data migration that enables `fdata/objects-map` must rewrite the data itself.
 - Snapshots share fragment rows with the live file (they are stored with pointers, not materialized). `file-gc` marks every fragment referenced neither by the current main file nor by an unpurged snapshot; the file is read unrealized and unchanged pages keep their ids via reattach, so a pass over an unchanged file writes no new fragments.
-- A save attaches its automatic snapshot before applying pending file migrations: the snapshot keeps the pre-migration pointers, data and migration set, and restoring it leaves the next read to migrate the file. This also holds for files without the feature. The preview (`get-file-snapshot`) applies the pending migrations in memory with the live file's libraries and never persists them: the stored version keeps its data, the payload carries the snapshot's `:migrations` (normalized to a set; the decoder keeps the pgarray as a vector and `set/difference` would compare indexes) and `fdata/objects-map` is realized for clients that do not declare it. The historical feature set is deliberately not checked against the enabled one: a version may predate a feature change. The window is narrow: the `get-file` RPC reads with `migrate?` on and its ETag hashes `available-migrations`, so a post-deploy read cannot be a 304 from the pre-deploy state and migrates before any save in the normal flow; the pre-migration snapshot only appears when the save is the first request to touch the file after the deploy (an open tab that saves without re-reading, or a non-UI writer). `create-file-snapshot` and the restore backup read with `migrate?` on, so those versions are stored already migrated.
+- A save attaches its automatic snapshot before applying pending file migrations: the snapshot keeps the pre-migration pointers, data and migration set, and restoring it leaves the next read to migrate the file. This also holds for files without the feature. The preview (`get-file-snapshot`) applies the pending migrations in memory with the live file's libraries and never persists them: the stored version keeps its data, the payload carries the snapshot's `:migrations` (an ordered set) and `fdata/objects-map` is realized for clients that do not declare it. The historical feature set is deliberately not checked against the enabled one: a version may predate a feature change. The window is narrow: the `get-file` RPC reads with `migrate?` on and its ETag hashes `available-migrations`, so a post-deploy read cannot be a 304 from the pre-deploy state and migrates before any save in the normal flow; the pre-migration snapshot only appears when the save is the first request to touch the file after the deploy (an open tab that saves without re-reading, or a non-UI writer). `create-file-snapshot` and the restore backup read with `migrate?` on, so those versions are stored already migrated.
+- `:migrations` is always a `d/ordered-set`: `file-migrations/resolve-applied-migrations` builds one, `fmg/migrate` re-wraps the union after applying, and `file-snapshots/decode-snapshot` converts the `file_change` pgarray (as a plain vector, `set/difference` and `contains?` would compare indexes instead of names). A snapshot with NULL `:migrations` and an old `:version` predates the `file_change.migrations` column (SQL migration 0140): the preview regenerates the set from the stored version and applies that full chain, while `restore!` uses a fixed 67 baseline, so preview and restore can disagree for very old snapshots.
 - The migration read path (`binfile.common/migrate-file`) captures `fdata/original-pointers` before realizing, reattaches unchanged positions after applying the migrations, and persists the reattached file; it returns the realized file because reads expose plain data. A migration that adds `fdata/objects-map` skips the reattach so the pages get wrapped.
 - `fdata/load-pointer` reads rows that a previous `file-gc` pass marked on purpose (explicit `::db/remove-deleted false`): the fragment is only gone once `objects-gc` purges it. The `get-file-fragment` RPC keeps rejecting marked rows.
 - Bind the fragment loader through `fdata/with-pointers` (reads), `fdata/with-tracked-pointers` (writes with a loader) or `fdata/with-tracked` (writes with nothing to load yet, e.g. file creation), and `fdata/with-untracked` (validation passes that must track nothing) instead of hand-rolled `binding` forms. Two direct bindings remain on purpose: the fine-grained tracked toggles of `app.rpc.commands.files-update` (validation/repair turn tracking off, restore the existing atom and read it before repairing), and `binfile.common/encode-file` (a fresh tracked set inheriting the current one). Converge them only if atom-scoped macros (`with-tracking`, `with-inherited-tracking`) are added; today the extra indirection buys nothing.

backend/src/app/features/file_snapshots.clj

@@ -48,11 +48,19 @@
                                    AND fd.type = 'snapshot')
     WHERE c.label IS NOT NULL")
 
+;; `:migrations` is an ordered set everywhere (see `common.types.file`),
+;; and `need-migration?`/`migrate` use set semantics on it: the pgarray
+;; must never surface as a vector, where `contains?`/`set/difference`
+;; would compare indexes instead of names.
+(defn- decode-migrations
+  [value]
+  (into (d/ordered-set) (db/decode-pgarray value [])))
+
 (defn- decode-snapshot
   [snapshot]
   (some-> snapshot
           (-> (d/update-when :metadata fdata/decode-metadata)
-              (d/update-when :migrations db/decode-pgarray [])
+              (d/update-when :migrations decode-migrations)
               (d/update-when :features db/decode-pgarray #{}))))
 
 (def ^:private sql:get-minimal-file

backend/src/app/rpc/commands/files_snapshot.clj

@@ -77,13 +77,11 @@
       ;; format. Apply the pending migrations in memory, the same pure
       ;; step a normal read runs, and never persist them: the stored
       ;; version keeps its data and restoring it re-migrates on the
-      ;; next read. `:migrations` is normalized to a set because the
-      ;; snapshot decoder keeps the pgarray as a vector and
-      ;; `set/difference` would compare indexes.
+      ;; next read. `decode-snapshot` hands an ordered set, which is
+      ;; what `need-migration?`/`migrate` require; a stored NULL is
+      ;; regenerated from the snapshot's version.
       (let [base-file (bfc/get-file cfg file-id :load-data? false)
-            snapshot  (-> snapshot
-                          (assoc :id file-id)
-                          (update :migrations #(into #{} %)))
+            snapshot  (assoc snapshot :id file-id)
             file      (if (fmg/need-migration? snapshot)
                         (let [file (fdata/realize cfg snapshot)]
                           (fmg/migrate-file

backend/test/backend_tests/rpc_file_snapshot_test.clj

@@ -7,7 +7,10 @@
 (ns backend-tests.rpc-file-snapshot-test
   (:require
    [app.binfile.common :as bfc]
+   [app.common.data :as d]
    [app.common.features :as cfeat]
+   [app.common.files.defaults :as cfd]
+   [app.common.files.migrations :as fmg]
    [app.common.pprint :as pp]
    [app.common.thumbnails :as thc]
    [app.common.time :as ct]
@@ -686,3 +689,53 @@
            (get-in with-feature [:result :data :pages-index page-id :objects])))
     (t/is (not (omap/objects-map?
                 (get-in without-feature [:result :data :pages-index page-id :objects]))))))
+
+(t/deftest snapshot-preview-migrates-legacy-snapshots-without-migrations
+  ;; Snapshots written before the `migrations` column existed (SQL
+  ;; migration 0140) store NULL and the format version of their time.
+  ;; The decoder must hand an ordered set (a vector would make
+  ;; `need-migration?` compare indexes), and the preview must
+  ;; regenerate the applied set from the stored version, migrate the
+  ;; data in memory and leave the row untouched.
+  (let [profile (th/create-profile* 1)
+        file    (th/create-file* 1 {:profile-id (:id profile)
+                                    :project-id (:default-project-id profile)
+                                    :is-shared false})
+        file-id (:id file)
+        snap-id (:id (:result (th/command! {::th/type :create-file-snapshot
+                                            ::rpc/profile-id (:id profile)
+                                            :file-id file-id
+                                            :label "legacy"})))]
+
+    (t/is (d/ordered-set? (:migrations (fsnap/get-snapshot th/*system* file-id snap-id))))
+
+    ;; Turn the version into a pre-0140 snapshot: no applied set and an
+    ;; old format version.
+    (th/db-exec! ["UPDATE file_change SET migrations = NULL, version = 60 WHERE id = ?"
+                  snap-id])
+
+    (let [stored (fsnap/get-snapshot th/*system* file-id snap-id)
+          out    (th/command! {::th/type :get-file-snapshot
+                               ::rpc/profile-id (:id profile)
+                               :file-id file-id
+                               :id snap-id
+                               :features cfeat/supported-features})
+          result (:result out)]
+
+      (t/is (nil? (:migrations stored)))
+      (t/is (= 60 (:version stored)))
+
+      (t/is (nil? (:error out)))
+      (t/is (d/ordered-set? (:migrations result)))
+      ;; The preview regenerates everything the stored version lacks
+      ;; instead of trusting the absent applied set.
+      (t/is (= cfd/version (:version result)))
+      (t/is (= (into #{} fmg/available-migrations)
+               (set (:migrations result))))
+      (t/is (set/subset? (into #{} (fmg/generate-migrations-from-version cfd/version))
+                         (set (:migrations result))))
+
+      ;; Nothing is persisted.
+      (let [stored (fsnap/get-snapshot th/*system* file-id snap-id)]
+        (t/is (nil? (:migrations stored)))
+        (t/is (= 60 (:version stored)))))))

common/src/app/common/files/migrations.cljc

@@ -88,7 +88,9 @@
 
     (-> file
         (assoc :data data)
-        (update :migrations set/union diff)
+        ;; `:migrations` is an ordered set everywhere; keep it that way
+        ;; even if a caller hands a plain set.
+        (update :migrations #(into (d/ordered-set) (set/union % diff)))
         (vary-meta assoc ::migrated (not-empty diff)))))
 
 (defn generate-migrations-from-version