Skip to main content

max / audiofiles

Refuse to open a vault written by a newer audiofiles `migrate()` only ever asked whether the database was behind. A vault ahead of the build matched no `version < target`, applied nothing, and returned Ok, after which every query ran against a schema this code has never seen. An added column survives that; a dropped one, a rename, or a NOT NULL the older code never populates is silent data loss with nothing surfaced. Nothing exotic reaches it. A rollback to an older release after a bad update, a restored backup, or one vault opened from two machines on different versions -- which is the shape the vault-per-detachable-drive workflow is made of, so it wants fixing before such a setup exists rather than after. The guard is one comparison, and what it needed was a number to compare against. MIGRATIONS moves to module scope so SCHEMA_VERSION derives from its length: the runner and the guard have to agree on the version this build produces, and deriving it is how they cannot drift. DbError::VaultTooNew carries both numbers and says which side is old, since the message is the whole of what the user can act on. It crosses into CoreError as its own variant rather than folding into Db -- it is not a SQLite failure and flattening it would lose the sentence. The recovery screen already renders it and already offers "Choose a different location", which is the one action that helps. Three tests: the refusal leaves the database untouched (an older build must not half-write a newer vault on its way to giving up), the boundary is `>` so a current vault still opens, and SCHEMA_VERSION tracks the list.
Co-Authored-By
Claude Opus 5 (1M context) <noreply@anthropic.com>
Author: Max Johnson <me@maxj.phd> · 2026-08-16 17:18 UTC
Signed with PGP, not checked
Commit: 0e80b19fe9667fc220b765e10bb82d31a089f67b
Parent: c43f6aa
2 files changed, +165 insertions, -40 deletions
M crates/audiofiles-core/src/db.rs +157 -40
@@ -14,6 +14,21 @@
14 14 pub enum DbError {
15 15 #[error("SQLite error: {0}")]
16 16 Sqlite(#[from] rusqlite::Error),
17 +
18 + /// The vault's schema is ahead of what this build knows how to read, so
19 + /// opening it would mean querying a shape this code has never seen. See
20 + /// [`Database::migrate`] for why that is refused rather than tolerated.
21 + #[error(
22 + "this vault was written by a newer version of audiofiles \
23 + (vault schema {found}, this build understands {supported}). \
24 + Update audiofiles to open it."
25 + )]
26 + VaultTooNew {
27 + /// `PRAGMA user_version` read off the vault.
28 + found: i32,
29 + /// The highest version this build can produce, [`SCHEMA_VERSION`].
30 + supported: i32,
31 + },
17 32 }
18 33
19 34 /// The config store wraps rusqlite; its one failure mode is the database, so it
@@ -1842,6 +1857,61 @@
1842 1857 Ok(())
1843 1858 }
1844 1859
1860 + /// Every migration, in order. Index + 1 is the `PRAGMA user_version` a database
1861 + /// carries once that migration has been applied, so the list's length is the
1862 + /// schema version this build produces.
1863 + ///
1864 + /// At module scope rather than inside [`Database::migrate`] so [`SCHEMA_VERSION`]
1865 + /// can be derived from it: the guard against opening a newer vault and the
1866 + /// migration runner have to agree on one number, and deriving it is how they
1867 + /// cannot drift.
1868 + const MIGRATIONS: &[&str] = &[
1869 + MIGRATION_001,
1870 + MIGRATION_002,
1871 + MIGRATION_003,
1872 + MIGRATION_004,
1873 + MIGRATION_005,
1874 + MIGRATION_006,
1875 + MIGRATION_007,
1876 + MIGRATION_008,
1877 + MIGRATION_009,
1878 + MIGRATION_010,
1879 + MIGRATION_011,
1880 + MIGRATION_012,
1881 + MIGRATION_013,
1882 + MIGRATION_014,
1883 + MIGRATION_015,
1884 + MIGRATION_016,
1885 + MIGRATION_017,
1886 + MIGRATION_018,
1887 + MIGRATION_019,
1888 + MIGRATION_020,
1889 + MIGRATION_021,
1890 + MIGRATION_022,
1891 + MIGRATION_023,
1892 + MIGRATION_024,
1893 + MIGRATION_025,
1894 + MIGRATION_026,
1895 + MIGRATION_027,
1896 + MIGRATION_028,
1897 + MIGRATION_029,
1898 + MIGRATION_030,
1899 + MIGRATION_031,
1900 + MIGRATION_032,
1901 + MIGRATION_033,
1902 + MIGRATION_034,
1903 + MIGRATION_035,
1904 + MIGRATION_036,
1905 + MIGRATION_037,
1906 + MIGRATION_038,
1907 + ];
1908 +
1909 + /// The schema version this build produces, and the highest one it can read.
1910 + ///
1911 + /// A vault reporting more than this was written by a newer audiofiles and is
1912 + /// refused; see [`DbError::VaultTooNew`].
1913 + pub const SCHEMA_VERSION: i32 = MIGRATIONS.len() as i32;
1914 +
1845 1915 /// Compile-time proof that a write transaction is open on the connection.
1846 1916 ///
1847 1917 /// Constructed only by [`Database::transaction`], and required by the row-write
@@ -1959,52 +2029,32 @@
1959 2029 /// Each migration step runs inside a transaction so the schema change and
1960 2030 /// version bump are atomic, a crash between the two can no longer leave the
1961 2031 /// database in an inconsistent state.
2032 + ///
2033 + /// The runner is bounded on both sides. Forward is the ordinary case.
2034 + /// Backward is not possible and must not be attempted silently: a vault
2035 + /// carrying a version this build has never heard of was written by a newer
2036 + /// audiofiles, and every query past this point assumes a schema this code
2037 + /// has seen. Applying nothing and returning `Ok` reads as success and then
2038 + /// queries a shape it does not understand, survivable for an added column
2039 + /// and silent data loss for a dropped one, a rename, or a NOT NULL the
2040 + /// older code never populates.
2041 + ///
2042 + /// Nothing exotic is needed to reach it: a rollback to an older release
2043 + /// after a bad update, a restored backup, or one vault opened from two
2044 + /// machines running different versions, which is the shape the
2045 + /// vault-per-detachable-drive workflow is made of.
1962 2046 #[instrument(skip_all)]
1963 2047 fn migrate(&mut self) -> Result<(), DbError> {
1964 2048 let version: i32 = self
1965 2049 .conn
1966 2050 .query_row("PRAGMA user_version", [], |row| row.get(0))?;
1967 2051
1968 - const MIGRATIONS: &[&str] = &[
1969 - MIGRATION_001,
1970 - MIGRATION_002,
1971 - MIGRATION_003,
1972 - MIGRATION_004,
1973 - MIGRATION_005,
1974 - MIGRATION_006,
1975 - MIGRATION_007,
1976 - MIGRATION_008,
1977 - MIGRATION_009,
1978 - MIGRATION_010,
1979 - MIGRATION_011,
1980 - MIGRATION_012,
1981 - MIGRATION_013,
1982 - MIGRATION_014,
1983 - MIGRATION_015,
1984 - MIGRATION_016,
1985 - MIGRATION_017,
1986 - MIGRATION_018,
1987 - MIGRATION_019,
1988 - MIGRATION_020,
1989 - MIGRATION_021,
1990 - MIGRATION_022,
1991 - MIGRATION_023,
1992 - MIGRATION_024,
1993 - MIGRATION_025,
1994 - MIGRATION_026,
1995 - MIGRATION_027,
1996 - MIGRATION_028,
1997 - MIGRATION_029,
1998 - MIGRATION_030,
1999 - MIGRATION_031,
2000 - MIGRATION_032,
2001 - MIGRATION_033,
2002 - MIGRATION_034,
2003 - MIGRATION_035,
2004 - MIGRATION_036,
2005 - MIGRATION_037,
2006 - MIGRATION_038,
2007 - ];
2052 + if version > SCHEMA_VERSION {
2053 + return Err(DbError::VaultTooNew {
2054 + found: version,
2055 + supported: SCHEMA_VERSION,
2056 + });
2057 + }
2008 2058
2009 2059 for (i, sql) in MIGRATIONS.iter().enumerate() {
2010 2060 let target = (i + 1) as i32;
@@ -2675,6 +2725,73 @@
2675 2725 assert_eq!(version, 38);
2676 2726 }
2677 2727
2728 + /// A vault written by a newer audiofiles is refused, not opened.
2729 + ///
2730 + /// The bug this pins: `migrate()` only ever compared `version < target`, so
2731 + /// a vault ahead of the build applied nothing, returned `Ok`, and left every
2732 + /// query below running against a schema this code has never seen. Two things
2733 + /// are asserted, and the second is the one that matters: the open fails, AND
2734 + /// it fails without having touched the database, so an older build cannot
2735 + /// half-write a newer vault on its way to giving up.
2736 + #[test]
2737 + fn open_refuses_a_vault_from_a_newer_audiofiles() {
2738 + let dir = tempfile::tempdir().unwrap();
2739 + let path = dir.path().join("audiofiles.db");
2740 +
2741 + Database::open(&path).unwrap();
2742 + let ahead = SCHEMA_VERSION + 1;
2743 + {
2744 + let conn = Connection::open(&path).unwrap();
2745 + conn.execute_batch(&format!("PRAGMA user_version = {ahead}"))
2746 + .unwrap();
2747 + }
2748 +
2749 + let Err(err) = Database::open(&path) else {
2750 + panic!("a newer vault must not open");
2751 + };
2752 + let DbError::VaultTooNew { found, supported } = err else {
2753 + panic!("expected VaultTooNew, got {err:?}");
2754 + };
2755 + assert_eq!(found, ahead);
2756 + assert_eq!(supported, SCHEMA_VERSION);
2757 + // The message is the whole remedy the user gets, so it has to say which
2758 + // side is old rather than printing two bare numbers.
2759 + let text = err.to_string();
2760 + assert!(text.contains("newer version of audiofiles"), "{text}");
2761 +
2762 + let after: i32 = Connection::open(&path)
2763 + .unwrap()
2764 + .query_row("PRAGMA user_version", [], |row| row.get(0))
2765 + .unwrap();
2766 + assert_eq!(after, ahead, "a refused open must not rewrite the vault");
2767 + }
2768 +
2769 + /// The boundary is `>`, not `>=`: a vault at exactly this build's version is
2770 + /// the ordinary case and opens with no migration run. Guards against a
2771 + /// one-off that would refuse every up-to-date vault.
2772 + #[test]
2773 + fn open_accepts_a_vault_at_the_current_version() {
2774 + let dir = tempfile::tempdir().unwrap();
2775 + let path = dir.path().join("audiofiles.db");
2776 +
2777 + Database::open(&path).unwrap();
2778 + let db = Database::open(&path).expect("a current vault opens");
2779 + let version: i32 = db
2780 + .conn()
2781 + .query_row("PRAGMA user_version", [], |row| row.get(0))
2782 + .unwrap();
2783 + assert_eq!(version, SCHEMA_VERSION);
2784 + }
2785 +
2786 + /// `SCHEMA_VERSION` is derived from `MIGRATIONS`, and the migration runner
2787 + /// keys the version it writes off the same list. Pinning the number here
2788 + /// means adding a migration without meaning to shows up as a failure.
2789 + #[test]
2790 + fn schema_version_matches_the_migration_list() {
2791 + assert_eq!(SCHEMA_VERSION, 38);
2792 + assert_eq!(SCHEMA_VERSION as usize, MIGRATIONS.len());
2793 + }
2794 +
2678 2795 /// M037 contract: the browse-list sort must not build a temp B-tree.
2679 2796 ///
2680 2797 /// Asserting the index exists would be the weaker test, because an index
M crates/audiofiles-core/src/error.rs +8
@@ -18,6 +18,13 @@
18 18 #[error("database error: {0}")]
19 19 Db(#[from] rusqlite::Error),
20 20
21 + /// The vault's schema is ahead of this build. Carried across from
22 + /// [`DbError::VaultTooNew`](crate::db::DbError::VaultTooNew) rather than
23 + /// folded into [`CoreError::Db`]: it is not a SQLite failure, and its
24 + /// message is the whole of what the user can act on.
25 + #[error(transparent)]
26 + VaultTooNew(crate::db::DbError),
27 +
21 28 /// Filesystem I/O error with the originating path.
22 29 #[error("I/O error at {path}: {source}")]
23 30 Io {
@@ -141,6 +148,7 @@
141 148 fn from(e: crate::db::DbError) -> Self {
142 149 match e {
143 150 crate::db::DbError::Sqlite(e) => CoreError::Db(e),
151 + other @ crate::db::DbError::VaultTooNew { .. } => CoreError::VaultTooNew(other),
144 152 }
145 153 }
146 154 }