max / audiofiles
1 file changed,
+0 insertions,
-49 deletions
| @@ -12,54 +12,5 @@ Launch shipped 2026-06-01 (see `/Users/max/Code/launchplan_final.md`). Post-laun | |||
| 12 | 12 | ||
| 13 | 13 | ## Run #9 deferrals (Phase 4) | |
| 14 | 14 | ||
| 15 | - | ### Trust / data integrity (creator-fuzz) | |
| 16 | - | - [x] **Export silently strips BWF / iXML / smpl / cue / ID3 chunks on conversion.** Added a one-line warning under the Format radio group in `ui/export_screens.rs` when format != Original. Long-term (still open): round-trip BWF `bext`, `smpl` loop points, `cue ` markers in the WAV encoder. | |
| 17 | - | - [x] **Format support gaps in `AUDIO_EXTENSIONS`.** Added `m4a`, `alac`, `caf`, `bwf` to `util.rs`; enabled symphonia `aac`, `alac`, `isomp4`, `caf` features in the workspace `Cargo.toml`. Skipped: `opus` (no symphonia codec) and `w64` (no symphonia format) — documented in the extension list rustdoc. | |
| 18 | - | - [x] **Export not atomic.** Added `write_atomic` helper in `export/runner.rs`: each format branch (Original copy / Wav encode / Aiff encode) now writes to `dest.audiofiles_tmp` and `fs::rename`s into place. On any error the tmp is removed, so a killed export leaves no partial files in the user's export dir. | |
| 19 | - | - [x] **Edit-result temp leaked on import failure** — `state/import_workflow.rs:1288-1297` only ran `remove_file` on the success branch, leaking the temp on every `import_file` error. Restructured to capture `import_result`, unconditionally remove the temp, then match the result. (The "VFS link not confirmed" concern in the original note is a non-issue: once `import_file` copies the bytes into the content-addressed store, the temp is redundant; the worst failure downstream of that is an orphan blob in the store, recoverable via `remove_orphaned_samples`.) | |
| 20 | - | - [x] **`SampleStore::remove` deletes DB row first, then file** — inverted to file-first. ENOENT on the file is now tolerated (idempotent). Failure mode flipped from silent orphan-blob accumulation to a visible dangling row the user can retry. Comment + test name + assertions all updated; added `remove_tolerates_missing_file` test. | |
| 21 | - | ||
| 22 | 15 | ### UX polish (use-fuzz) | |
| 23 | 16 | - [ ] **Preserve form fields on signup-style error swaps.** (Not applicable to audiofiles per se, but the pattern — re-render the wizard step with user input — would apply if any wizard step ever fails inline.) | |
| 24 | - | - [x] **Sort-arrow glyphs U+25B2 / U+25BC** — kept as documented exception per user sign-off 2026-06-02. | |
| 25 | - | - [x] **First-launch welcome cannot be dismissed inline** — added a small "Dismiss" link at the bottom of the welcome (`ui/file_list.rs`); reuses the existing `state.dismiss_first_launch_hint()` which already persists the pref. | |
| 26 | - | - [x] **Toolbar Import emphasis when library is empty** — `ui/toolbar.rs` now bolds the Import label when contents+search+filters are all empty. Single conditional, no extra widget. | |
| 27 | - | - [x] **Error toast / status copy nits** — `activation.rs:119` `Activating...` → `Activating…` (U+2026); `library.rs:100` `Locate failed: {e}` → `Could not locate sample on disk — {e}` (U+2014). | |
| 28 | - | ||
| 29 | - | ### Rust quality (rust-fuzz) | |
| 30 | - | - [x] **`Result<_, String>` leaks past the typed-error wall** — folded into `PreviewError` (new `InvalidHash`/`FileNotFound` variants), new `MidiError` (with `#[from] InitError`/`ConnectError`), new `DeactivationError` for `license::deactivate_key`. | |
| 31 | - | - [x] **`unwrap()` after `is_none()` check** at `direct.rs:571, 595` — extracted to `let built = idx.as_ref().expect("set on the line above");` in both `find_similar` and `find_near_duplicates`. The original todo proposed `get_or_insert_with` but that can't propagate the `?` from `load_data`; the match-style refactor preserves error propagation and makes the lazy-init invariant explicit. | |
| 32 | - | - [x] **Hand-rolled `synckit.toml` parser** — replaced with `toml::from_str::<HashMap<String, String>>()`; added `toml` dep to `audiofiles-app`. | |
| 33 | - | - [x] **Dependency duplication** — bumped eframe 0.31→0.34, egui 0.31→0.34, cpal 0.15→0.17, midir 0.10→0.11, tray-icon 0.21→0.22. `windows-core` dups fully eliminated (4→0 on win target). `objc2-foundation` still 2× (0.2.2 via winit 0.30, 0.3.2 via arboard/glutin/muda) — blocked on winit bumping objc2; floor reached for our codebase. | |
| 34 | - | - [x] **egui 0.34 deprecation cleanup** — full sweep: `App::update` → `App::ui` (new trait shape, `ui: &mut Ui` instead of `ctx`); `SidePanel`/`TopBottomPanel` → `Panel::left`/`Panel::right`/`Panel::top`/`Panel::bottom`; `Panel::show(ctx,...)` → `show_inside(ui,...)` across 26 sites; `Memory::toggle_popup`/`close_popup` → `Popup::toggle_id`/`close_id`; `popup_below_widget` → `Popup::from_response`; `show_tooltip_at_pointer` → `Tooltip::always_open`; `Context::style`/`set_style` → `global_style`/`set_global_style`; `Context::screen_rect` → `content_rect`; `Ui::close_menu` → `Ui::close`; `cpal DeviceTrait::name` → `description().name()`. Required signature changes to `draw_browser` and all leaf `draw_*` screen fns (configure/progress/tagging/export/activation/vault_setup/db_error) from `ctx: &Context` → `ui: &mut Ui`. 0 deprecation warnings remaining; 269 tests pass. | |
| 35 | - | ||
| 36 | - | ### Repo hygiene (launchplan §2.3) | |
| 37 | - | - [x] `crates/audiofiles-app/tests/harness/mod.rs.bak` confirmed absent 2026-06-02. | |
| 38 | - | - [x] **Audit `docs/` for stale plans.** Subagent-graded 2026-06-02: `database_schema.md` rewritten for the 12→18 migration count + M013/M015/M018 schema (source_path, filter_json, smart_folders dropped, row_id hashing, row_id_salt). `architecture.md` updated for the same plus the m4a/alac/caf/bwf extension list and the atomic-write helper in §Export System. `description.md` swapped "Smart Folders" for "Dynamic Collections" + added Cmd+, About entry. `troubleshooting.md` fixed migration count, sync table list, row_id_salt note, and a wrong mirror path (`~/.audiofiles-mirror/` → `~/audiofiles-mirror/`). `design-system.md`, `loose-files-mode.md`, `ml_classifier.md`, `plugin_authoring.md`, `trial-mode.md` were graded CURRENT and left alone. | |
| 39 | - | - [x] **CONTRIBUTING.md walkthrough.** Tables-synced list updated (smart_folders out, edit_history + user_config in). Added a Sync Changelog Triggers paragraph on M018 row_id hashing + canonical-PK-in-data, with guidance for new synced tables. Added a replay-safety paragraph in the Database section (`IF NOT EXISTS` / `DROP IF EXISTS` / `INSERT OR IGNORE`, exempting M001/M002, pointing at the regression test) and a note on the `hash_row_id` SQLite function. Build commands table at the end was already accurate. | |
| 40 | - | ||
| 41 | - | ## Audit deltas to revisit | |
| 42 | - | ||
| 43 | - | - [x] **In-app updater toggle now takes effect at runtime.** Wired `tokio::sync::watch<bool>` into the background loop: the task is always spawned and parked on `watch.changed()` when the pref is off, no network calls. Toggling the About-modal checkbox calls `update_checker.set_enabled(new)` which signals through the channel; enabling triggers an immediate check via the loop's `tokio::select!` between the 6h sleep and the watch. `UpdateChecker::disabled()` replaced by `inert()` (kept for tests / no-runtime contexts). Tests added for the watch-channel propagation and the inert no-op. | |
| 44 | - | ||
| 45 | - | ## Future enhancements (not blocking) | |
| 46 | - | ||
| 47 | - | - [x] **About menu + Cmd+, shortcut.** Toolbar Help is now a popup with "Keyboard shortcuts" and "About audiofiles". Browser→app signal via `BrowserState::about_requested` mirrors the MidiAction pattern. `Cmd+,` joins `Cmd+I` as the About toggle. | |
| 48 | - | - [x] **SyncKit upload contract audit + row_id hashing (M018).** Subagent-audited 2026-06-02. Findings: cleartext leaks in `sync_changelog.row_id` (tag strings as `sample_hash:tag`, raw sample SHA-256s as content fingerprints), plus a false privacy claim in `sync_panel.rs:402`. Landed: | |
| 49 | - | - Registered `hash_row_id(salt, key) -> TEXT` SQLite scalar function in `Database::open` / `open_in_memory`, using SHA-256 over a per-user salt + canonical key. Enabled rusqlite `functions` feature. | |
| 50 | - | - M018 migration: generates `row_id_salt` in `sync_state` (32-byte `randomblob` hex, INSERT OR IGNORE), drops and recreates every sync trigger with `hash_row_id` wrapping for sensitive tables (samples, audio_analysis, tags, collection_members). DELETE triggers now emit canonical-key JSON in `data` so pull-side replay doesn't depend on row_id semantics. | |
| 51 | - | - Backfills unpushed `sync_changelog` rows: hashes existing cleartext row_ids; reconstructs canonical PKs into `data` for DELETE rows on composite-PK tables before hashing. | |
| 52 | - | - `resolve::apply_delete` now reads composite PK from decrypted `data` JSON first, falls back to splitting `row_id` for pre-M018 rows already on the server. | |
| 53 | - | - Privacy copy at `sync_panel.rs:402` rewritten to match reality. | |
| 54 | - | - smart_folders triggers from M007 not recreated (table dropped in M015). | |
| 55 | - | - Tests: added `m018_hashes_sensitive_row_ids` (asserts 64-hex hash, no cleartext leak, salt differs across DBs) and `m018_delete_triggers_emit_canonical_key_in_data`. Updated 3 sync trigger tests to read from `data` rather than asserting cleartext `row_id`. | |
| 56 | - | - **Deferred:** server-side blob `hash` is still a content fingerprint per the audit's Risk 5; per-user blob namespace is a server change, out of scope for this client-side fix. Risk 4 (no tag UPDATE trigger) still applies but with hashing in place its impact is reduced to "one extra hashed-row push per rename" rather than "cleartext tag exposure twice". | |
| 57 | - | - [x] **Database migration safety review** (2026-06-02). Subagent-audited all 17 migrations; per-migration risk grade in session transcript. Landed: | |
| 58 | - | - Made M004, M005, M006, M007, M012 fully idempotent (`CREATE TABLE/INDEX/TRIGGER IF NOT EXISTS`; M007 seed `INSERT OR IGNORE`). M008–M011, M016, M017 were already replay-safe via `DROP IF EXISTS + CREATE`. M014 already idempotent. | |
| 59 | - | - Added `migration_replay_from_file_no_op` and `migration_replay_from_version_two_against_full_schema` tests. The replay test rolls `PRAGMA user_version=2` against a populated schema and re-runs every migration from M003 onward; future non-idempotent CREATEs will fail this test loudly. | |
| 60 | - | - Documented why M001 (initial schema) and M002 (table-rebuild dance with `DROP TABLE tags; ALTER tags_v2 RENAME TO tags`) are inherently one-shot and not replay-safe. | |
| 61 | - | - **Recovery branch validator landed 2026-06-02.** Replaced the `tracing::warn!` + silent-bump path with fail-fast: on any non-ALTER recovery failure that isn't "already exists", roll back and surface the error. This immediately surfaced a real latent bug — M007 created sync triggers on `smart_folders`, which M015 drops, so post-M015 replay parsed-failed on M007. Fixed by removing the smart_folders triggers from M007 (M015's `DROP TRIGGER IF EXISTS` stays for already-installed DBs). M015 itself is now documented as inherently one-shot alongside M001/M002 (the backfill `SELECT FROM smart_folders` can't parse on replay against a populated post-M015 schema; SQLite has no conditional-execute). Replay test rolls `user_version` back to 15 (post-M015) rather than 2; the realistic recovery scenario is "re-apply the one migration that crashed", not "re-apply every migration from scratch". Added `migrate_recovery_branch_fails_fast_on_non_alter_error` and `migrate_recovery_branch_tolerates_already_exists`. | |
| 62 | - | - **Design landed 2026-06-02:** sample-deletion semantics for multi-device sync are designed in `docs/design-sample-deletion.md` (F: tombstone column on `samples`, 30-day retention, sync-replicated, Trash UI). Single-device delete was already correct (placement-only); the load-bearing concern was sync-pull `DELETE samples` cascading globally on the receiving device. Phasing in the doc (M019 + read-path filter → delete+undelete ops → Trash UI → sweep → notification). | |
| 63 | - | - **Phase 1 landed 2026-06-02:** M019 adds `samples.deleted_at INTEGER`, a partial `idx_samples_deleted_at` index, and seeds `user_config.sample_tombstone_retain_days=30` (suppressed from sync_changelog during migration). Samples triggers recreated to flow `deleted_at` through the wire JSON. Read-path filter (`WHERE deleted_at IS NULL` or `s.deleted_at IS NULL` on joins) applied across `store.rs` (orphan query, field lookup, source_path lookup, loose-files integrity, loose-files file_size, purge_missing_loose_files), `vfs.rs` (enriched contents + by-hash queries), `search.rs` (search_in_folder + search_global), `export/mod.rs` (collect items, both paths), `cleanup.rs` (orphan worker), `backend/direct.rs` (blob file_size lookup). `m019_tombstone_column_and_read_filter` test proves column + index + seed + read filter. Three sync snapshot test counts bumped (+1 for the user_config seed). Phase 2 (delete/undelete ops) next. | |
| 64 | - | - **Local-only orphan cleanup shipped 2026-06-02:** Settings → Storage → "Cleanup orphans" button calls a new `Backend::cleanup_orphans_local` that wraps `remove_orphaned_samples` in `applying_remote='1'` trigger suppression. Forward-compatible with the tombstone design (Phase 2 will replace the underlying call with `tombstone_sample` + immediate hard-delete for orphans). | |
| 65 | - | - **Deferred, historical:** M015 backfill INSERT into `collections` fires `sync_collections_insert` from M007 → emits spurious `sync_changelog` rows on devices that ran M015. Has already shipped. Future migrations should wrap data backfills with `UPDATE sync_state SET value='1' WHERE key='applying_remote'` and reset on commit. |