agent: lumotia — Phase B.4 close restore-during-purge race via atomic DELETE RETURNING

Phase B.4 audit of commits 15b74db, 87e6248, 50d0715, 99f4ecd (the
soft-delete / trash / restore wave — Rev-2, Rev-3). Existing backend
coverage is solid: migration_v16_adds_deleted_at_column_and_index,
delete_transcript_soft_deletes, delete_transcript_removes_audio_file,
list_transcripts_excludes_soft_deleted (with a restore round-trip),
and purge_deleted_transcripts_hard_deletes_old.

The Svelte UI components added by 87e6248 (Trash view + restore) and
50d0715 (type-the-word DELETE modal) carry TODO(test) notes saying
"vitest not installed". That comment is stale — vitest landed in
Phase A.5 (206ac62). Adding Svelte component tests is real follow-up
work but outside the per-item methodology for B.4; calling it out
here for the Phase-B finishing pass to triage.

One real residual found.

Surface: `purge_deleted_transcripts` in `crates/storage/src/database.rs`.
The prior form was a two-statement SELECT-then-DELETE pair:

  1. SELECT id, audio_path FROM transcripts WHERE deleted_at IS NOT NULL
     AND deleted_at < datetime('now', '-30 days');
  2. DELETE FROM transcripts WHERE id IN (chunk_of_ids);

A `restore_transcript(id)` between (1) and (2) clears `deleted_at` on a
row whose id is in the chunk, but the DELETE has no `deleted_at IS NOT
NULL` filter — so the now-LIVE row is hard-deleted alongside its audio
file. That bypasses the entire Rev-2 soft-delete safety contract: the
user can lose data without the 30-day retention window the contract
promised. In the current code the purge runs once at startup before
the user can issue a restore, so the race window is narrow in
practice. The safety should be structural, not operational —
especially if a future change moves the purge to a daily cron.

Fix: collapse the SELECT + DELETE into a single
`DELETE … RETURNING audio_path`. SQLite (3.35+, well within the
sqlx 0.8 amalgam) evaluates the WHERE clause and the row removal
atomically; the returned `audio_path`s are guaranteed to belong to
rows that THIS call hard-deleted. The audio cleanup loop then operates
on those returned paths, never on rows that survived the WHERE clause.
The chunking concern (IN-clause near SQLITE_MAX_VARIABLE_NUMBER)
disappears too — there is no IN-clause.

Behavioural diff for the non-racing path: identical (same WHERE clause,
same NotFound-tolerant best-effort fs::remove_file).

Behavioural diff for the racing path: a row restored between SELECT and
DELETE survives the purge and keeps its audio file — which is the
contract Rev-2 was added to enforce.

Other surface notes (no fix needed):
  * `delete_transcript` is robust to its own concurrent restore — the
    UPDATE has `AND deleted_at IS NULL` and audio removal only fires
    when `rows_affected() > 0`.
  * `restore_transcript` is a single UPDATE — atomic.
  * FTS triggers on UPDATE preserve the row in transcripts_fts; the
    `t.deleted_at IS NULL` filter on `search_transcripts`'s JOIN keeps
    trashed rows out of search results.

New regression test: `purge_audio_cleanup_only_fires_for_hard_deleted_rows`
covers the structural property — an in-retention trashed row with its
audio file on disk survives purge with the audio intact, while a
past-retention trashed row is hard-deleted with audio removed.

Verification:
  * cargo test -p lumotia-storage --lib database::tests
      → 53/53 pass including the new test (old purge test still passes).
  * cargo fmt --check → clean (applied fmt after the test edit).
  * cargo clippy -p lumotia-storage --all-targets -- -D warnings → clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

crates/storage/src/database.rs

@@ -387,19 +387,30 @@ pub async fn delete_transcript(pool: &SqlitePool, id: &str) -> Result<()> {
 ///
 /// Audio files are also best-effort removed here in case the original
 /// soft-delete failed at the filesystem layer (Rev-3 belt-and-braces).
+///
+/// **Atomicity (Phase B.4 audit fix 2026-05-14):** the prior form was a
+/// two-statement SELECT-then-DELETE-WHERE-id-IN sequence. If a row was
+/// restored between the SELECT and the DELETE (`restore_transcript`
+/// clearing `deleted_at`), the DELETE still hard-deleted the now-live
+/// row and removed its audio file — bypassing the soft-delete safety
+/// contract Rev-2 was added to enforce. We now use a single
+/// `DELETE … RETURNING` so the row filter is re-evaluated atomically at
+/// DELETE time and the returned `audio_path`s are guaranteed to belong
+/// to rows that this call actually hard-deleted. This also removes the
+/// chunking concern (no `IN(…)` clause means no SQLITE_MAX_VARIABLE_NUMBER
+/// ceiling).
 pub async fn purge_deleted_transcripts(pool: &SqlitePool, older_than_days: i64) -> Result<u64> {
-    // Collect (id, audio_path) BEFORE the DELETE so we still have the
-    // paths to clean up after the row is gone.
     let rows = sqlx::query(
-        "SELECT id, audio_path FROM transcripts \
+        "DELETE FROM transcripts \
          WHERE deleted_at IS NOT NULL \
-           AND deleted_at < datetime('now', ?)",
+           AND deleted_at < datetime('now', ?) \
+         RETURNING audio_path",
     )
     .bind(format!("-{older_than_days} days"))
     .fetch_all(pool)
     .await
     .map_err(|source| Error::Query {
-        operation: "purge_deleted_transcripts_select".into(),
+        operation: "purge_deleted_transcripts".into(),
         source,
     })?;
 
@@ -407,37 +418,14 @@ pub async fn purge_deleted_transcripts(pool: &SqlitePool, older_than_days: i64)
         return Ok(0);
     }
 
-    let mut ids: Vec<String> = Vec::with_capacity(rows.len());
     let mut audio_paths: Vec<String> = Vec::new();
     for row in &rows {
-        let id: String = row.get("id");
         let audio: Option<String> = row.get("audio_path");
-        ids.push(id);
         if let Some(p) = audio {
             audio_paths.push(p);
         }
     }
 
-    // Build the IN clause manually because sqlx 0.8 doesn't expand Vec
-    // bindings; we batch into chunks of 200 to stay well clear of
-    // SQLITE_MAX_VARIABLE_NUMBER (default 999).
-    let mut total: u64 = 0;
-    for chunk in ids.chunks(200) {
-        let placeholders = std::iter::repeat_n("?", chunk.len())
-            .collect::<Vec<_>>()
-            .join(",");
-        let sql = format!("DELETE FROM transcripts WHERE id IN ({placeholders})");
-        let mut q = sqlx::query(&sql);
-        for id in chunk {
-            q = q.bind(id);
-        }
-        let res = q.execute(pool).await.map_err(|source| Error::Query {
-            operation: "purge_deleted_transcripts_delete".into(),
-            source,
-        })?;
-        total += res.rows_affected();
-    }
-
     // Best-effort audio cleanup. NotFound is the expected case for rows
     // whose audio was already removed at soft-delete time.
     for path in &audio_paths {
@@ -451,7 +439,7 @@ pub async fn purge_deleted_transcripts(pool: &SqlitePool, older_than_days: i64)
         }
     }
 
-    Ok(total)
+    Ok(rows.len() as u64)
 }
 
 /// List soft-deleted transcripts (the "trash" view), most-recently-deleted
@@ -3210,4 +3198,90 @@ mod tests {
             "t-new still inside the retention window"
         );
     }
+
+    /// Phase B.4 audit regression (2026-05-14). Asserts the
+    /// `DELETE … RETURNING` form correctly couples row-removal with the
+    /// audio-cleanup loop: only rows the DELETE actually affected get
+    /// their audio file removed, and rows outside the retention window
+    /// are untouched.
+    ///
+    /// The race we fixed (restore between SELECT and DELETE in the old
+    /// two-statement form) requires fault injection between the two
+    /// statements to exercise deterministically. The atomic single-
+    /// statement form makes that race structurally impossible. We test
+    /// the structural property: an in-retention trashed row with its
+    /// audio file on disk survives purge with the audio intact, while a
+    /// past-retention trashed row is hard-deleted with audio removed.
+    #[tokio::test]
+    async fn purge_audio_cleanup_only_fires_for_hard_deleted_rows() {
+        let pool = test_pool().await;
+
+        let tmpdir = tempfile::tempdir().expect("tmpdir");
+        let old_audio = tmpdir.path().join("old.wav");
+        let recent_audio = tmpdir.path().join("recent.wav");
+        std::fs::write(&old_audio, b"old wav").unwrap();
+        std::fs::write(&recent_audio, b"recent wav").unwrap();
+
+        let old_path_static: &'static str =
+            Box::leak(old_audio.to_string_lossy().to_string().into_boxed_str());
+        let recent_path_static: &'static str =
+            Box::leak(recent_audio.to_string_lossy().to_string().into_boxed_str());
+
+        insert_transcript(
+            &pool,
+            &minimal_transcript("t-purge-old", Some(old_path_static)),
+        )
+        .await
+        .unwrap();
+        insert_transcript(
+            &pool,
+            &minimal_transcript("t-purge-recent", Some(recent_path_static)),
+        )
+        .await
+        .unwrap();
+
+        // Manually mark both trashed without going through delete_transcript
+        // (which would best-effort-remove the audio files itself). Old row
+        // is backdated past the 30-day retention; recent row is fresh.
+        sqlx::query("UPDATE transcripts SET deleted_at = datetime('now', '-60 days') WHERE id = ?")
+            .bind("t-purge-old")
+            .execute(&pool)
+            .await
+            .unwrap();
+        sqlx::query("UPDATE transcripts SET deleted_at = datetime('now') WHERE id = ?")
+            .bind("t-purge-recent")
+            .execute(&pool)
+            .await
+            .unwrap();
+
+        let purged = purge_deleted_transcripts(&pool, 30).await.unwrap();
+        assert_eq!(purged, 1, "only t-purge-old is past retention");
+
+        let old_exists: Option<String> =
+            sqlx::query_scalar("SELECT id FROM transcripts WHERE id = ?")
+                .bind("t-purge-old")
+                .fetch_optional(&pool)
+                .await
+                .unwrap();
+        assert!(old_exists.is_none(), "t-purge-old must be hard-deleted");
+        assert!(
+            !old_audio.exists(),
+            "audio for hard-deleted row must be removed by purge"
+        );
+
+        let recent_exists: Option<String> =
+            sqlx::query_scalar("SELECT id FROM transcripts WHERE id = ?")
+                .bind("t-purge-recent")
+                .fetch_optional(&pool)
+                .await
+                .unwrap();
+        assert!(
+            recent_exists.is_some(),
+            "t-purge-recent within retention must survive"
+        );
+        assert!(
+            recent_audio.exists(),
+            "audio for surviving in-retention row must NOT be removed by purge"
+        );
+    }
 }