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>
Jake
20ef6c459b