diff --git a/desktop/src-tauri/src/main.rs b/desktop/src-tauri/src/main.rs index e8af4f4..36b8037 100644 --- a/desktop/src-tauri/src/main.rs +++ b/desktop/src-tauri/src/main.rs @@ -2,6 +2,7 @@ use flate2::read::GzDecoder; use serde::{Deserialize, Serialize}; use sha2::{Digest, Sha256}; use std::{ + collections::HashMap, env, fs, io::{Read, Write}, net::{TcpListener, TcpStream}, @@ -67,7 +68,24 @@ struct BackendStateInner { starting: bool, /// PID of an in-progress pip subprocess; killed by stop_backend on window close (#140). pip_pid: Option, -} + /// Save destinations the user has picked but not yet downloaded to (#338). + /// + /// The export is two commands so the UI can tell "choosing a folder" apart + /// from "writing the file", but the second half must not take a path from + /// JS: that would hand a compromised WebView the ability to write any URL + /// to any location on disk. The path stays here and JS only ever holds an + /// opaque token. + pending_saves: HashMap, + /// Source of those tokens. A counter is enough -- the token is not a + /// secret. Every live token maps to a path the user chose in a native + /// dialog, so guessing one only ever yields another approved destination. + next_save_token: u64, +} + +/// Cap on unconsumed destinations. A pick whose download never runs (the user +/// closes the window mid-export) would otherwise sit here for the life of the +/// process. +const MAX_PENDING_SAVES: usize = 16; impl Default for BackendStateInner { fn default() -> Self { @@ -75,6 +93,8 @@ impl Default for BackendStateInner { handles: None, starting: false, pip_pid: None, + pending_saves: HashMap::new(), + next_save_token: 0, } } } @@ -269,6 +289,8 @@ fn main() { build_target, open_url, save_audio_file, + pick_export_destination, + download_to_path, pick_stems_folder, store_get, store_set, @@ -1641,23 +1663,71 @@ fn open_url(url: String) -> Result<(), String> { Ok(()) } -/// Prompts the user for a save path, then streams a localhost audio URL to disk. -#[tauri::command] -async fn save_audio_file( - app: tauri::AppHandle, - url: String, - filename: String, -) -> Result<(), String> { +/// Only localhost URLs, and only http(s). Guards against a compromised WebView +/// using the desktop shell as an SSRF proxy (#138). +fn validate_download_url(url: &str) -> Result<(), String> { if !url.starts_with("http://") && !url.starts_with("https://") { return Err("only http/https URLs are permitted".to_string()); } - // Restrict to localhost to prevent SSRF from a compromised WebView (#138). - let parsed_url = reqwest::Url::parse(&url).map_err(|_| "invalid URL".to_string())?; + let parsed_url = reqwest::Url::parse(url).map_err(|_| "invalid URL".to_string())?; let host = parsed_url.host_str().unwrap_or(""); if host != "127.0.0.1" && host != "localhost" { return Err("only localhost URLs are permitted".to_string()); } + Ok(()) +} +/// Records a picked destination and returns the token JS will hand back. +fn store_pending_save(state: &BackendState, dest: PathBuf) -> Result { + let mut guard = state + .inner + .lock() + .map_err(|_| "state poisoned".to_string())?; + if guard.pending_saves.len() >= MAX_PENDING_SAVES { + // Drop the oldest by token order; tokens are monotonic, so the smallest + // numeric key is the stalest pick. + if let Some(oldest) = guard + .pending_saves + .keys() + .min_by_key(|k| k.parse::().unwrap_or(u64::MAX)) + .cloned() + { + guard.pending_saves.remove(&oldest); + } + } + guard.next_save_token += 1; + let token = guard.next_save_token.to_string(); + guard.pending_saves.insert(token.clone(), dest); + Ok(token) +} + +/// Consumes a token. Single use: a failed transfer needs a fresh destination +/// rather than silently reusing one the user picked for an earlier attempt. +fn take_pending_save(state: &BackendState, token: &str) -> Result { + let mut guard = state + .inner + .lock() + .map_err(|_| "state poisoned".to_string())?; + guard + .pending_saves + .remove(token) + .ok_or_else(|| "no destination is pending for this export".to_string()) +} + +/// Shows the native save dialog and remembers where the user pointed it. +/// +/// Split from the transfer (#338) so the UI can show "Exporting..." for the +/// writing only. Awaiting one combined command meant the button claimed to be +/// exporting for however long the picker sat open, when nothing was happening. +/// +/// Returns None when the user cancels, which the caller treats as "do nothing" +/// -- no busy state is ever entered, so there is none to unwind. +#[tauri::command] +async fn pick_export_destination( + app: tauri::AppHandle, + state: tauri::State<'_, BackendState>, + filename: String, +) -> Result, String> { use tauri_plugin_dialog::DialogExt; let dest = app .dialog() @@ -1665,9 +1735,25 @@ async fn save_audio_file( .set_file_name(&filename) .blocking_save_file(); let Some(file_path) = dest else { - return Ok(()); // user cancelled + return Ok(None); // user cancelled }; let dest = file_path.into_path().map_err(|e| e.to_string())?; + store_pending_save(&state, dest).map(Some) +} + +/// Streams a localhost URL to the destination a previous pick recorded. +/// +/// Takes a token rather than a path on purpose: a path parameter would let +/// anything running in the WebView write an arbitrary URL to an arbitrary +/// location. The destination never leaves Rust. +#[tauri::command] +async fn download_to_path( + state: tauri::State<'_, BackendState>, + token: String, + url: String, +) -> Result<(), String> { + validate_download_url(&url)?; + let dest = take_pending_save(&state, &token)?; // Stream response to disk to avoid buffering a large audio file in memory (#139). // 5-minute timeout covers large WAV exports over a slow loopback. @@ -1700,6 +1786,25 @@ async fn save_audio_file( Ok(()) } +/// Pick-then-transfer in one call, for the lane download links. +/// +/// Those have no busy state to mislabel, so they want the convenience. The +/// export menu drives the two halves separately. +#[tauri::command] +async fn save_audio_file( + app: tauri::AppHandle, + state: tauri::State<'_, BackendState>, + url: String, + filename: String, +) -> Result<(), String> { + // Validate before showing a dialog the request could never satisfy. + validate_download_url(&url)?; + let Some(token) = pick_export_destination(app, state.clone(), filename).await? else { + return Ok(()); // user cancelled + }; + download_to_path(state, token, url).await +} + fn stop_backend(state: &BackendState) { let (handles, pip_pid) = match state.inner.lock() { Ok(mut guard) => (guard.handles.take(), guard.pip_pid.take()), @@ -3031,6 +3136,7 @@ fn hide_console_window(_command: &mut Command) {} #[cfg(test)] mod tests { use std::fs; + use std::path::PathBuf; use tempfile::TempDir; fn make_tmp() -> TempDir { @@ -3561,4 +3667,74 @@ b6052160df96b31c9b1e33854a4dcda3d4b57641b880270f31736fb9f445d384 ffmpeg-n7.1-la // Present but empty: also None. assert_eq!(super::find_driver_store_nvidia_smi(repo.path()), None); } + + // ── export destinations (#338) ─────────────────────────────────────────── + + #[test] + fn a_token_yields_the_path_that_was_picked() { + let state = super::BackendState::default(); + let token = super::store_pending_save(&state, PathBuf::from("/tmp/song.wav")).unwrap(); + assert_eq!( + super::take_pending_save(&state, &token).unwrap(), + PathBuf::from("/tmp/song.wav") + ); + } + + #[test] + fn a_token_works_only_once() { + // A failed transfer has to go back through the dialog rather than + // quietly reusing a destination the user chose for an earlier attempt. + let state = super::BackendState::default(); + let token = super::store_pending_save(&state, PathBuf::from("/tmp/song.wav")).unwrap(); + assert!(super::take_pending_save(&state, &token).is_ok()); + assert!(super::take_pending_save(&state, &token).is_err()); + } + + #[test] + fn an_unknown_token_is_refused() { + // This is the security property: without a matching pick there is no + // destination, so the WebView cannot name one of its own. + let state = super::BackendState::default(); + assert!(super::take_pending_save(&state, "nope").is_err()); + assert!(super::take_pending_save(&state, "1").is_err()); + } + + #[test] + fn tokens_are_distinct_per_pick() { + let state = super::BackendState::default(); + let a = super::store_pending_save(&state, PathBuf::from("/tmp/a.wav")).unwrap(); + let b = super::store_pending_save(&state, PathBuf::from("/tmp/b.wav")).unwrap(); + assert_ne!(a, b); + assert_eq!( + super::take_pending_save(&state, &b).unwrap(), + PathBuf::from("/tmp/b.wav") + ); + assert_eq!( + super::take_pending_save(&state, &a).unwrap(), + PathBuf::from("/tmp/a.wav") + ); + } + + #[test] + fn unconsumed_picks_do_not_accumulate_forever() { + let state = super::BackendState::default(); + let first = super::store_pending_save(&state, PathBuf::from("/tmp/first.wav")).unwrap(); + for i in 0..super::MAX_PENDING_SAVES { + super::store_pending_save(&state, PathBuf::from(format!("/tmp/{i}.wav"))).unwrap(); + } + let held = state.inner.lock().unwrap().pending_saves.len(); + assert!(held <= super::MAX_PENDING_SAVES, "held {held}"); + // The stalest pick is the one dropped. + assert!(super::take_pending_save(&state, &first).is_err()); + } + + #[test] + fn only_localhost_urls_are_downloadable() { + assert!(super::validate_download_url("http://127.0.0.1:8000/api/x.wav").is_ok()); + assert!(super::validate_download_url("http://localhost:8000/api/x.wav").is_ok()); + // The SSRF boundary from #138, still enforced after the split. + assert!(super::validate_download_url("http://example.com/x.wav").is_err()); + assert!(super::validate_download_url("file:///etc/passwd").is_err()); + assert!(super::validate_download_url("not a url").is_err()); + } } diff --git a/static/js/main.js b/static/js/main.js index 6a730e6..b9d493d 100644 --- a/static/js/main.js +++ b/static/js/main.js @@ -150,6 +150,8 @@ function wireFooterControls() { let format = "wav"; let busy = false; + // True from the click until the transfer starts or the dialog is cancelled. + let picking = false; const panelOpen = () => exportPanel && !exportPanel.classList.contains("hidden"); function openPanel() { @@ -194,12 +196,11 @@ function wireFooterControls() { busy = false; exportBtn?.classList.remove("is-busy"); if (exportLabel) exportLabel.textContent = "Export Mix"; - // Clear every row, not just the ones flashBusy could see: it disables via - // actionItems(), which filters on visibility, so a row hidden at reset time - // would keep the attribute forever. Under Tauri the panel is still open when - // flashBusy runs (invoke() returns without the synthetic click that - // closes it in a browser), so all three get disabled and only a symmetric - // clear brings them back. + // Clear every row, not just the ones enterBusy could see: it disables via + // actionItems(), which filters on visibility, and it closes the panel, so + // by the time this runs every row is hidden and a visibility-filtered clear + // would clear nothing at all -- leaving the menu dead for the rest of the + // session (#335). for (const it of [itemMix, itemStems, itemRegion]) it?.removeAttribute("aria-disabled"); applyFormatState(); // re-derives the region row's genuine disabled state } @@ -212,18 +213,32 @@ function wireFooterControls() { // Guards against a stale timer from a finished export resetting a later one. let busyToken = 0; - // `pending` is whatever the download helper returned: a promise on desktop, - // where save_audio_file resolves once the file is written, or `true` in a - // browser, where an is fire-and-forget and there is nothing to - // wait on. Only the guess needs a fixed duration. - function flashBusy(pending) { + // Show the busy state. Called when bytes actually start moving, which on + // desktop is after the user has picked a destination -- not on click, or the + // label would claim to be exporting for as long as the save dialog sat open + // (#338). + function enterBusy() { busy = true; - const token = ++busyToken; - const finish = () => { if (token === busyToken) resetBusy(); }; exportBtn?.classList.add("is-busy"); if (exportLabel) exportLabel.textContent = "Exporting…"; actionItems().forEach((it) => it?.setAttribute("aria-disabled", "true")); closePanel(); + } + + // `pending` is whatever the download helper returned: a promise on desktop, + // resolving once the file is written, or `true` in a browser, where an + // is fire-and-forget and there is nothing to wait on. Only the + // guess needs a fixed duration. + // + // `picking` covers the gap between the click and the transfer: the dialog is + // app-modal so the menu is unreachable anyway, but the flag keeps a second + // export from being queued behind it without lying about the label. + function settleBusy(pending) { + const token = ++busyToken; + const finish = () => { + picking = false; + if (token === busyToken) resetBusy(); + }; if (!pending || typeof pending.then !== "function") { window.setTimeout(finish, EXPORT_FLASH_MS); @@ -232,7 +247,8 @@ function wireFooterControls() { const backstop = window.setTimeout(finish, EXPORT_BUSY_MAX_MS); pending .catch((err) => { - // A cancelled save dialog resolves, so anything here is a real failure. + // A cancelled dialog resolves false without ever entering the busy + // state, so anything here is a real failure. showError(typeof err === "string" && err ? err : "Export failed.", null, { retry: false }); }) .finally(() => { @@ -241,37 +257,47 @@ function wireFooterControls() { }); } + // Kick off an export: hand the helper a callback that flips the UI into its + // busy state, then wait on the result. + function runExport(start, emptyMessage) { + if (busy || picking) return; + picking = true; + const pending = start(enterBusy); + if (!pending) { + picking = false; + showError(emptyMessage, null, { retry: false }); + return; + } + settleBusy(pending); + } + exportBtn?.addEventListener("click", (e) => { e.stopPropagation(); - if (busy) return; + if (busy || picking) return; panelOpen() ? closePanel() : openPanel(); }); // Export Mix: MP4 produces the video; any other format an audio mix. itemMix?.addEventListener("click", (e) => { e.stopPropagation(); - if (busy) return; - const ok = format === "mp4" ? downloadCurrentVideo() : downloadCurrentMix(format); - if (!ok) { showError("All stems are muted - nothing to export.", null, { retry: false }); return; } - flashBusy(ok); + runExport( + (onStart) => (format === "mp4" ? downloadCurrentVideo(onStart) : downloadCurrentMix(format, onStart)), + "All stems are muted - nothing to export.", + ); }); itemRegion?.addEventListener("click", (e) => { e.stopPropagation(); - if (busy || itemRegion.getAttribute("aria-disabled") === "true") return; - const ok = downloadRegionMix(format); - if (!ok) { showError("All stems are muted - nothing to export.", null, { retry: false }); return; } - flashBusy(ok); + if (itemRegion.getAttribute("aria-disabled") === "true") return; + runExport((onStart) => downloadRegionMix(format, onStart), "All stems are muted - nothing to export."); }); // All Stems = a single backend-built ZIP, named after the song. Audio-only, // so it's disabled (and inert) while MP4 is the selected format. itemStems?.addEventListener("click", (e) => { e.stopPropagation(); - if (busy || itemStems.getAttribute("aria-disabled") === "true") return; - const ok = downloadAllStemsZip(format); - if (!ok) { showError("No stems to export.", null, { retry: false }); return; } - flashBusy(ok); + if (itemStems.getAttribute("aria-disabled") === "true") return; + runExport((onStart) => downloadAllStemsZip(format, onStart), "No stems to export."); }); // Keyboard: ↓ opens/moves into the menu, ↑/↓ cycle rows, Esc closes + restores focus. diff --git a/static/js/player.js b/static/js/player.js index d8b7bb0..9846f74 100644 --- a/static/js/player.js +++ b/static/js/player.js @@ -1558,15 +1558,29 @@ export function updateFooterTrack({ title, thumbnail, key, bpm, stemCount } = {} // Returns a promise that settles when the file is actually on disk, or `true` // when the host gives no completion signal. Callers use the difference to show // a real "Exporting…" state instead of a fixed-length guess. -function _triggerDownload(url, filename) { +// +// `onTransferStart` fires when bytes actually begin moving, which on desktop is +// after the user has chosen a destination. Awaiting one combined command made +// the button read "Exporting…" for however long the save dialog sat open, when +// nothing was being exported yet (#338). Callers enter their busy state here +// rather than on click. +function _triggerDownload(url, filename, onTransferStart) { const fullUrl = url.startsWith("http") ? url : `${location.origin}${url}`; - if (window.__TAURI__?.core?.invoke) { - // save_audio_file streams to a temp file and renames, resolving only once - // the whole body is written -- so this covers the save dialog and the copy. - return window.__TAURI__.core.invoke("save_audio_file", { url: fullUrl, filename }); + const invoke = window.__TAURI__?.core?.invoke; + if (invoke) { + // Two commands: the dialog, then the transfer. A cancelled dialog resolves + // to null and never starts a transfer, so no busy state is entered and + // there is none to unwind. + return invoke("pick_export_destination", { filename }).then((token) => { + if (!token) return false; + onTransferStart?.(); + return invoke("download_to_path", { token, url: fullUrl }); + }); } // A browser is fire-and-forget: the fetch is owned by the - // download manager and reports nothing back to the page. + // download manager and reports nothing back to the page. There is no dialog + // to wait on, so the transfer is under way as soon as the click lands. + onTransferStart?.(); const a = document.createElement("a"); a.href = fullUrl; a.download = filename; @@ -1659,16 +1673,16 @@ function _exportFilename(ext) { // The download functions return true when a download was triggered and false // when there is nothing audible to export (every lane muted), so the caller can // surface a message. -export function downloadCurrentMix(ext = "wav") { +export function downloadCurrentMix(ext = "wav", onTransferStart) { const url = _mixdownUrl(ext, false); if (!url) return false; - return _triggerDownload(url, _exportFilename(ext)); + return _triggerDownload(url, _exportFilename(ext), onTransferStart); } // MP4 export: the preserved source video muxed with the current audio mix. // Only meaningful for mp4-sourced jobs (currentJobHasVideo()); returns false when // there's no video track or every lane is muted. -export function downloadCurrentVideo() { +export function downloadCurrentVideo(onTransferStart) { if (!currentJobId || !_currentHasVideo) return false; const { names, gains } = _effectiveMixGains(); if (!names.length) return false; @@ -1683,34 +1697,14 @@ export function downloadCurrentVideo() { .slice(0, 80) .replace(/^_+|_+$/g, ""); const name = safe ? `${safe}_video.mp4` : "video.mp4"; - return _triggerDownload(`/api/jobs/${currentJobId}/video.mp4?${q}`, name); + return _triggerDownload(`/api/jobs/${currentJobId}/video.mp4?${q}`, name, onTransferStart); } -export function downloadCurrentStems(format = "wav", onProgress) { - const stems = _currentStems.filter((s) => s.name !== "original"); - const total = stems.length; - if (!total) { onProgress?.(0, 0); return; } - // Name each file "_." using the same title - // sanitization as the mix/region exports. - const safe = _currentTitle - .replace(/[^a-zA-Z0-9]+/g, "_") - .replace(/_{2,}/g, "_") - .slice(0, 80) - .replace(/^_+|_+$/g, ""); - stems.forEach((s, i) => { - window.setTimeout(() => { - const url = format === "mp3" ? s.url.replace(/\.wav(\?|$)/, ".mp3$1") : s.url; - const fname = safe ? `${safe}_${s.name}.${format}` : `${s.name}.${format}`; - _triggerDownload(url, fname); - onProgress?.(i + 1, total); - }, i * 150); - }); -} // Returns false when there is nothing to zip, matching downloadCurrentMix and // downloadCurrentVideo, so the caller can skip the "Exporting…" state instead of // showing it for a download that never starts. -export function downloadAllStemsZip(format = "wav") { +export function downloadAllStemsZip(format = "wav", onTransferStart) { if (!currentJobId) return false; // Only the active (selected) stems loaded in the DAW — not all 6. const names = _currentStems.filter((s) => s.name !== "original").map((s) => s.name); @@ -1722,7 +1716,7 @@ export function downloadAllStemsZip(format = "wav") { .replace(/^_+|_+$/g, ""); const name = safe ? `${safe}_stems.zip` : "stems.zip"; const q = new URLSearchParams({ format, stems: names.join(",") }); - return _triggerDownload(`/api/jobs/${currentJobId}/stems/all.zip?${q}`, name); + return _triggerDownload(`/api/jobs/${currentJobId}/stems/all.zip?${q}`, name, onTransferStart); } function _regionFilename(ext) { @@ -1734,9 +1728,9 @@ function _regionFilename(ext) { return `${safe || "region"}_region.${ext}`; } -export function downloadRegionMix(ext = "wav") { +export function downloadRegionMix(ext = "wav", onTransferStart) { if (!loopEnabled || loopStart >= loopEnd) return false; const url = _mixdownUrl(ext, true); if (!url) return false; - return _triggerDownload(url, _regionFilename(ext)); + return _triggerDownload(url, _regionFilename(ext), onTransferStart); } diff --git a/tests/e2e/export-menu.spec.mjs b/tests/e2e/export-menu.spec.mjs index 9056afc..d59a43d 100644 --- a/tests/e2e/export-menu.spec.mjs +++ b/tests/e2e/export-menu.spec.mjs @@ -12,6 +12,11 @@ import { test, expect } from "@playwright/test"; import { openStudio, exportUi } from "./helpers.mjs"; +// Desktop exports go through a save dialog first. Picking a destination is what +// starts the transfer, so most tests have to answer the dialog before there is +// any busy state to assert on (#338). +const choosePath = (page) => page.evaluate(() => window.__e2e.choosePath()); + test.describe("export menu, desktop (Tauri) mode", () => { test("every row is usable again after an export completes", async ({ page }) => { await openStudio(page, { tauri: true }); @@ -19,6 +24,7 @@ test.describe("export menu, desktop (Tauri) mode", () => { await ui.open(); await ui.stems.click(); + await choosePath(page); // Mid-export: the menu is busy and says so. await expect(ui.label).toHaveText(/Exporting/); @@ -40,12 +46,13 @@ test.describe("export menu, desktop (Tauri) mode", () => { for (const pass of [1, 2]) { await ui.open(); await ui.stems.click(); + await choosePath(page); await expect(ui.label).toHaveText(/Exporting/, { timeout: 5000 }); await page.evaluate(() => window.__e2e.finishSave()); await expect(ui.label).toHaveText("Export Mix"); expect( - await page.evaluate(() => window.__e2e.callsFor("save_audio_file").length), - `save_audio_file should have fired on pass ${pass}`, + await page.evaluate(() => window.__e2e.callsFor("download_to_path").length), + `the transfer should have fired on pass ${pass}`, ).toBe(pass); } }); @@ -58,6 +65,7 @@ test.describe("export menu, desktop (Tauri) mode", () => { await ui.open(); await ui.mix.click(); + await choosePath(page); await expect(ui.label).toHaveText(/Exporting/); await page.waitForTimeout(3000); // comfortably past the 1200 ms guess @@ -74,6 +82,7 @@ test.describe("export menu, desktop (Tauri) mode", () => { await ui.open(); await ui.mix.click(); + await choosePath(page); await expect(ui.label).toHaveText(/Exporting/); await page.evaluate(() => window.__e2e.failSave("disk full")); @@ -94,6 +103,7 @@ test.describe("export menu, desktop (Tauri) mode", () => { await ui.open(); await ui.mix.click(); + await choosePath(page); await page.evaluate(() => window.__e2e.failSave("nope")); await expect(ui.error).toBeVisible(); @@ -102,6 +112,88 @@ test.describe("export menu, desktop (Tauri) mode", () => { }); }); +test.describe("the save dialog phase (#338)", () => { + test("the label does not claim to be exporting while the picker is open", async ({ page }) => { + // The whole point of splitting the command. Awaiting one combined + // save_audio_file meant the button read "Exporting..." from the moment it + // was clicked, including however long the user spent choosing a folder, + // when nothing was being exported yet. + await openStudio(page, { tauri: true }); + const ui = exportUi(page); + + await ui.open(); + await ui.mix.click(); + + await expect.poll(() => page.evaluate(() => window.__e2e.pickPending())).toBe(true); + await expect(ui.label).toHaveText("Export Mix"); + await expect(ui.button).not.toHaveClass(/is-busy/); + // Nothing has been transferred, so no transfer command has been issued. + expect(await page.evaluate(() => window.__e2e.callsFor("download_to_path").length)).toBe(0); + + await choosePath(page); + await expect(ui.label).toHaveText(/Exporting/); + expect(await page.evaluate(() => window.__e2e.callsFor("download_to_path").length)).toBe(1); + + await page.evaluate(() => window.__e2e.finishSave()); + await expect(ui.label).toHaveText("Export Mix"); + }); + + test("cancelling the dialog leaves the menu exactly as it was", async ({ page }) => { + // No busy state is ever entered, so there is none to unwind. + await openStudio(page, { tauri: true }); + const ui = exportUi(page); + + await ui.open(); + await ui.mix.click(); + await expect.poll(() => page.evaluate(() => window.__e2e.pickPending())).toBe(true); + + await page.evaluate(() => window.__e2e.cancelPick()); + + await expect(ui.label).toHaveText("Export Mix"); + await expect(ui.button).not.toHaveClass(/is-busy/); + expect(await page.evaluate(() => window.__e2e.callsFor("download_to_path").length)).toBe(0); + await expect(ui.error).toHaveCount(0); + + // The panel was never closed: only entering the busy state does that, and + // a cancelled pick never gets there. So the menu is still open and usable. + await expect(ui.panel).not.toHaveClass(/hidden/); + await ui.mix.click(); + await choosePath(page); + await expect(ui.label).toHaveText(/Exporting/); + }); + + test("a second export cannot be queued while the picker is open", async ({ page }) => { + // The dialog is app-modal on a real desktop, but the guard must not depend + // on that: `busy` is deliberately still false during this phase. + await openStudio(page, { tauri: true }); + const ui = exportUi(page); + + await ui.open(); + await ui.mix.click(); + await expect.poll(() => page.evaluate(() => window.__e2e.pickPending())).toBe(true); + + await ui.mix.click(); + await ui.button.click(); + expect(await page.evaluate(() => window.__e2e.callsFor("pick_export_destination").length)).toBe(1); + expect(await page.evaluate(() => window.__e2e.callsFor("download_to_path").length)).toBe(0); + }); + + test("the transfer is told where to write by token, never by path", async ({ page }) => { + // The destination stays in Rust. A path argument here would be an arbitrary + // write primitive for anything running in the WebView. + await openStudio(page, { tauri: true }); + const ui = exportUi(page); + + await ui.open(); + await ui.mix.click(); + await choosePath(page); + + const args = await page.evaluate(() => window.__e2e.callsFor("download_to_path")[0].args); + expect(Object.keys(args).sort()).toEqual(["token", "url"]); + expect(args.token).toBeTruthy(); + }); +}); + test.describe("export menu, browser mode", () => { test("rows recover after the fire-and-forget download path", async ({ page }) => { // No Tauri bridge: _triggerDownload falls back to a synthetic , @@ -156,6 +248,7 @@ test.describe("busy state", () => { await ui.open(); await ui.mix.click(); + await choosePath(page); await expect(ui.label).toHaveText(/Exporting/); await expect(ui.panel).toHaveClass(/hidden/); diff --git a/tests/e2e/helpers.mjs b/tests/e2e/helpers.mjs index f7620dd..fc03798 100644 --- a/tests/e2e/helpers.mjs +++ b/tests/e2e/helpers.mjs @@ -47,19 +47,37 @@ export async function seedLibrary(page) { /** * Install a fake Tauri bridge so the app takes its desktop code path. * - * `save_audio_file` is left pending until the test resolves or rejects it, - * which is the whole point: the export busy state is a promise state machine, - * and #335 was a stuck one. Tests drive it through window.__e2e. + * The export is two commands, and the test controls each independently: + * + * pick_export_destination the native save dialog + * download_to_path the transfer + * + * Holding the dialog open is what makes #338 testable -- the label must still + * read "Export Mix" while the user is choosing a folder, because nothing is + * being exported yet. Holding the transfer open is what makes #335 testable. + * Tests drive both through window.__e2e. */ export async function stubTauri(page) { await page.addInitScript(() => { const calls = []; let pendingResolve = null; let pendingReject = null; + let pickResolve = null; + + const settle = (fn) => (v) => { + pendingResolve = null; + pendingReject = null; + fn(v); + }; window.__e2e = { calls, - // Settle the export that is currently in flight. + // Choose a destination, as if the user hit Save in the dialog. + choosePath: () => pickResolve && (pickResolve("token-1"), (pickResolve = null)), + // Dismiss the dialog. No transfer follows. + cancelPick: () => pickResolve && (pickResolve(null), (pickResolve = null)), + pickPending: () => Boolean(pickResolve), + // Settle the transfer that is currently in flight. finishSave: (value) => pendingResolve && pendingResolve(value ?? null), failSave: (message) => pendingReject && pendingReject(message ?? "save failed"), savePending: () => Boolean(pendingResolve), @@ -71,10 +89,13 @@ export async function stubTauri(page) { invoke: (cmd, args) => { calls.push({ cmd, args }); switch (cmd) { + case "pick_export_destination": + return new Promise((resolve) => { pickResolve = resolve; }); + case "download_to_path": case "save_audio_file": return new Promise((resolve, reject) => { - pendingResolve = (v) => { pendingResolve = null; pendingReject = null; resolve(v); }; - pendingReject = (e) => { pendingResolve = null; pendingReject = null; reject(e); }; + pendingResolve = settle(resolve); + pendingReject = settle(reject); }); // The library store lives in the Tauri store on desktop. Back it // with localStorage so seedLibrary works in this mode too.