From 06b14df307d5e190d37638774340e3a54af67628 Mon Sep 17 00:00:00 2001 From: NimBold Date: Mon, 22 Jun 2026 11:19:18 +0330 Subject: [PATCH] fix(core): harden download lifecycle and scheduling --- CHANGELOG.md | 2 +- INTERACTION_REVIEW.md | 6 +- YouTube_media_download_handoff.md | 173 ++++++++++ src-tauri/capabilities/default.json | 1 + src-tauri/src/commands.rs | 47 +-- src-tauri/src/download.rs | 7 +- src-tauri/src/download_ownership.rs | 51 ++- src-tauri/src/ipc.rs | 18 +- src-tauri/src/lib.rs | 312 +++++++++++++----- src-tauri/src/queue.rs | 53 +-- src-tauri/src/scheduler.rs | 70 ++-- src-tauri/src/settings.rs | 5 + src-tauri/tests/download_engine.rs | 7 +- src-tauri/tests/queue_manager.rs | 73 ++-- src/App.tsx | 68 +++- src/ErrorBoundary.tsx | 2 +- src/bindings/ActiveView.ts | 2 +- src/bindings/DownloadItem.ts | 2 +- src/bindings/DownloadStatus.ts | 2 +- src/bindings/EnqueueAccepted.ts | 3 + src/bindings/EnqueueItem.ts | 2 +- src/bindings/EnqueueResult.ts | 2 +- src/bindings/SchedulerSettings.ts | 2 +- src/components/AddDownloadsModal.tsx | 152 ++++++--- src/components/DownloadItem.tsx | 35 +- src/components/DownloadTable.tsx | 21 +- .../{DiagnosticsView.tsx => LogsView.tsx} | 77 +++-- src/components/SchedulerView.tsx | 84 ++++- src/components/SettingsView.tsx | 2 +- src/components/Sidebar.tsx | 16 +- src/index.css | 42 ++- src/ipc.ts | 16 +- src/main.tsx | 26 ++ src/store/useDownloadStore.test.ts | 76 ++++- src/store/useDownloadStore.ts | 191 +++++++---- src/store/useSettingsStore.ts | 25 +- src/utils/downloadActions.ts | 3 +- src/utils/downloads.ts | 9 + 38 files changed, 1257 insertions(+), 428 deletions(-) create mode 100644 YouTube_media_download_handoff.md create mode 100644 src/bindings/EnqueueAccepted.ts rename src/components/{DiagnosticsView.tsx => LogsView.tsx} (64%) diff --git a/CHANGELOG.md b/CHANGELOG.md index e9c387e..12fd9c7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -333,7 +333,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [0.4.0] - 2026-06-03 ### Changes -- Reorganized Settings sections so related download preferences sit together and app diagnostics live under App. +- Reorganized Settings sections so related download preferences sit together and app logs live under App. - Hardened the release workflow with explicit macOS 26 SDK checks, newer GitHub Actions, and app signature verification. - Prefer the bundled `aria2c` binary inside release builds. diff --git a/INTERACTION_REVIEW.md b/INTERACTION_REVIEW.md index 02c64cc..f773d31 100644 --- a/INTERACTION_REVIEW.md +++ b/INTERACTION_REVIEW.md @@ -182,7 +182,7 @@ users. Manual QA is needed to confirm current focus behavior. | SB-09 | Delete Queue | queue context menu | `removeQueue` | DB persistence subscription | Reassign items to Main Queue and delete custom queue | wired; no confirmation | unit test | | SB-10 | Open Scheduler | `ToolItem` | `setActiveView` | none | Show Scheduler | wired | code trace | | SB-11 | Open Speed Limiter | `ToolItem` | `setActiveView` | none | Show Speed Limiter | wired | code trace | -| SB-12 | Open Diagnostics | `ToolItem` | `setActiveView` | none | Show Diagnostics | wired | code trace | +| SB-12 | Open Logs | `ToolItem` | `setActiveView` | none | Show Logs | wired | code trace | | SB-13 | Open Settings | footer button | `setActiveView` | none | Show Settings | wired | code trace | Recommendation: make queue membership explicit and total. Every non-completed @@ -351,7 +351,7 @@ to the frontend. |---|---|---|---|---|---| | ST-25 | Prevent system sleep | setter plus download-state sync; `set_prevent_sleep` | Keep system awake during active downloads | wired but duplicated in setter/store synchronization | integration test, Manual QA needed | | ST-26 | Recheck engines | four engine status commands | Validate packaged sidecars | wired | integration test, build check | -| ST-27 | Show/hide engine details | local expanded state | Reveal diagnostics | wired | code trace | +| ST-27 | Show/hide engine details | local expanded state | Reveal engine status | wired | code trace | | ST-28 | Browser Cookies Source | `setMediaCookieSource` | Pass selected browser to media metadata/downloads | wired; Manual QA needed for browser permissions | integration test | | ST-29 | Copy pairing token | clipboard handler | Copy keychain-hydrated token | wired; success toast is shown without awaiting clipboard result | code trace, Manual QA needed | | ST-30 | Regenerate pairing token | `regeneratePairingToken`; keychain + App effect | Rotate token and reconfigure local server | wired but fragile: UI reports success before keychain/server calls confirm | integration test | @@ -401,7 +401,7 @@ native transfers that the command does not implement. --- -## 8. Diagnostics +## 8. Logs | ID | Action | Component / function | IPC / Rust | Expected behavior | Status | Validation | |---|---|---|---|---|---|---| diff --git a/YouTube_media_download_handoff.md b/YouTube_media_download_handoff.md new file mode 100644 index 0000000..58d568b --- /dev/null +++ b/YouTube_media_download_handoff.md @@ -0,0 +1,173 @@ +# YouTube and Media Download Handoff + +## Repository state + +The YouTube and media fixes were committed and pushed in: + +- `a8b7920 fix(media): harden YouTube metadata loading` +- `340ef09 fix(media): correct YouTube size estimates` +- `226c791 fix(media): correct HLS progress tracking` + +Do not discard unrelated worktree changes while working in this area. + +## Non-negotiable architecture + +### Keep the self-contained yt-dlp onedir distribution + +Do not replace the bundled onedir distribution with yt-dlp onefile or a system +yt-dlp installation. + +The onefile executable incurred roughly 17 seconds of extraction and startup +latency. Users cannot be expected to have Python or yt-dlp installed or +available through `PATH`. + +The packaged layout must contain: + +- `yt-dlp-` +- The adjacent `_internal/` directory +- The embedded Python runtime +- The `yt_dlp_ejs` solver files + +`scripts/verify-binaries.js` enforces this layout. Cross-platform builds must +provide an equivalent onedir distribution for each target. Do not implement +cross-platform support by falling back to a user-managed `PATH`. + +### Keep metadata loading deterministic + +The metadata implementation in `src-tauri/src/lib.rs` deliberately: + +- Resolves only the bundled yt-dlp executable. +- Passes bundled Deno and FFmpeg by absolute path. +- Uses a minimal system `PATH`. +- Uses `--skip-download`. +- Requests only `title`, `duration`, `thumbnail`, and `formats`. +- Includes the cookie browser and credentials in the cache key. +- Deduplicates concurrent identical requests. +- Caches successful metadata for 60 seconds. +- Limits the cache to 128 entries. + +Frontend request deduplication also exists in +`src/utils/mediaMetadata.ts`. Do not remove either deduplication layer. React +development behavior can otherwise start duplicate yt-dlp processes. + +Deno is a JavaScript runtime used by yt-dlp extractors. It is not the metadata +engine. + +### Keep media-format interpretation in the backend + +`build_media_format_options` in `src-tauri/src/lib.rs` is the source of truth. +Do not reintroduce a separate format parser in `AddDownloadsModal.tsx`. +Duplicating this logic caused format selection and size estimation to drift. + +Required invariants: + +- Do not add a synthetic `Best` option. +- Exclude storyboards, MHTML, thumbnails, subtitles, and non-media entries. +- Match resolutions exactly. A 1080p stream must never produce a 1440p row. +- Bind each displayed option to concrete yt-dlp stream IDs, such as `301+251`. +- MKV, MP4, and WebM options must describe the streams actually selected. +- Do not add a separate audio stream when the selected video already has audio. + +### Keep size values honest + +Size semantics: + +- `filesize` is exact. +- `filesize_approx` is approximate. +- When yt-dlp provides neither, estimate from bitrate multiplied by duration + and mark the result approximate. +- For split formats, combine the video and audio sizes. +- Approximate sizes in the UI must have a `~` prefix. +- Temporary component-stream totals must not overwrite the download row's + estimate. +- Successful completion must replace the estimate with the actual output file + size read from disk. +- Stores may treat a progress size as authoritative only when + `size_is_final` is true. + +The primary UI consumers are: + +- `src/components/AddDownloadsModal.tsx` +- `src/components/QualityModal.tsx` + +### Do not trust temporary HLS byte totals for progress + +yt-dlp can initially emit data similar to: + +```text +downloaded_bytes=1024 +total_bytes_estimate=1024 +fragment_index=0 +fragment_count=354 +_percent_str=100.0% +``` + +This does not mean the download is complete. It represents an early HLS +fragment with a temporary size estimate. + +`parse_media_progress_line` must therefore prefer: + +```text +fragment_index / fragment_count +``` + +when fragment information is available. + +For separate video and audio streams, `aggregate_media_fraction` advances to +the next track only when the previous track was effectively complete and the +new track restarts near zero. Do not restore the old heuristic that treated any +large percentage drop as a track transition. + +Visible speed is derived from downloaded-byte deltas where possible. Raw +yt-dlp speed is only a fallback. + +## Required regression checks + +Before accepting changes to yt-dlp arguments, binary packaging, metadata, +progress, sizes, format selection, pause/resume, or the media UI, run: + +```bash +cargo test --all-targets +npm run build +npm test -- --run +node scripts/verify-binaries.js +git diff --check +``` + +Run the live format smoke with a currently available multi-quality video: + +```bash +FIRELINK_LIVE_YOUTUBE_URL='https://www.youtube.com/watch?v=' \ +cargo test filters_live_youtube_metadata_from_env --lib -- --ignored --nocapture +``` + +Preserve these Rust regression tests: + +- `builds_compact_media_options_without_storyboards` +- `estimates_missing_video_size_from_bitrate_and_uses_exact_stream_ids` +- `uses_fragment_progress_instead_of_temporary_hls_size_estimates` +- `advances_tracks_only_after_a_completed_track_restarts` +- `derives_main_window_speed_from_downloaded_byte_delta` +- The structured yt-dlp, aria2, and legacy progress parser tests + +## Manual packaged-app checks + +Test the packaged application, not only development mode: + +1. Launch it outside the repository working directory. +2. Select a browser cookie source. +3. Paste a YouTube URL into Add Downloads. +4. Confirm media formats appear within the expected warm-start budget, with a + target below eight seconds. +5. Confirm there is no `Best` row. +6. Confirm each displayed quality matches its actual resolution. +7. Confirm approximate sizes include `~`. +8. Start a split-stream or HLS download. +9. Confirm progress increases gradually instead of immediately reaching 100%. +10. Confirm the final displayed size equals the completed file's size on disk. +11. Pause and resume, confirming that the selected stream IDs and estimated + size remain intact. + +Do not treat a deleted, private, age-restricted, or geographically unavailable +test video as proof of a product regression. Reproduce the problem with another +public multi-quality video first. diff --git a/src-tauri/capabilities/default.json b/src-tauri/capabilities/default.json index 5ebbb48..2cab0ae 100644 --- a/src-tauri/capabilities/default.json +++ b/src-tauri/capabilities/default.json @@ -8,6 +8,7 @@ "core:window:allow-start-dragging", "opener:default", "dialog:default", + "log:default", "notification:default", "notification:allow-is-permission-granted" ] diff --git a/src-tauri/src/commands.rs b/src-tauri/src/commands.rs index ad4e020..3f7ba97 100644 --- a/src-tauri/src/commands.rs +++ b/src-tauri/src/commands.rs @@ -7,7 +7,7 @@ pub async fn reveal_in_file_manager( app_handle: tauri::AppHandle, path: String, ) -> Result<(), String> { - let primary = authorize_download_path(&app_handle, &path, DownloadAsset::Primary)?; + let primary = authorize_download_path(&app_handle, &path)?; let path = existing_download_asset(&primary).ok_or_else(|| { format!( "Downloaded file or partial file is missing: {}", @@ -28,7 +28,7 @@ pub async fn open_downloaded_file( app_handle: tauri::AppHandle, path: String, ) -> Result<(), String> { - let path = authorize_download_path(&app_handle, &path, DownloadAsset::Primary)?; + let path = authorize_download_path(&app_handle, &path)?; if !path.exists() { return Err(format!("Downloaded file is missing: {}", path.display())); } @@ -41,52 +41,11 @@ pub async fn open_downloaded_file( Ok(()) } -#[tauri::command] -pub async fn trash_download_assets( - app_handle: tauri::AppHandle, - path: String, - partial_paths: Vec, -) -> Result<(), String> { - let primary = authorize_download_path(&app_handle, &path, DownloadAsset::Primary)?; - let partials = partial_paths - .iter() - .map(|partial| authorize_download_path(&app_handle, partial, DownloadAsset::Partial)) - .collect::, _>>()?; - - if primary.exists() { - trash::delete(&primary).map_err(|e| format!("Failed to trash primary file: {}", e))?; - } - - for partial in partials { - if partial.exists() { - trash::delete(&partial).map_err(|e| format!("Failed to trash partial file: {}", e))?; - } - } - - Ok(()) -} - -#[derive(Clone, Copy)] -enum DownloadAsset { - Primary, - Partial, -} - fn authorize_download_path( app_handle: &tauri::AppHandle, requested: &str, - asset: DownloadAsset, ) -> Result { - let known_paths = known_download_paths(app_handle)?; - let allowed_paths = match asset { - DownloadAsset::Primary => known_paths, - DownloadAsset::Partial => known_paths - .iter() - .flat_map(|path| [append_suffix(path, ".aria2"), append_suffix(path, ".part")]) - .collect(), - }; - - authorize_exact_path(Path::new(requested), &allowed_paths) + authorize_exact_path(Path::new(requested), &known_download_paths(app_handle)?) } fn known_download_paths(app_handle: &tauri::AppHandle) -> Result, String> { diff --git a/src-tauri/src/download.rs b/src-tauri/src/download.rs index 47bebd6..f4ffa25 100644 --- a/src-tauri/src/download.rs +++ b/src-tauri/src/download.rs @@ -26,7 +26,7 @@ pub enum DownloadCmd { Start(Box), Pause(Uuid), PauseWithAck(Uuid, tokio::sync::oneshot::Sender<()>), - Cancel(Uuid), + CancelWithAck(Uuid, tokio::sync::oneshot::Sender<()>), CaptureUrls(Vec), FrontendReady(bool), } @@ -338,9 +338,12 @@ async fn run_coordinator( let _ = ack.send(()); } } - DownloadCmd::Cancel(id) => { + DownloadCmd::CancelWithAck(id, ack) => { if let Some(download) = active.remove(&id) { let _ = download.control_tx.send(DownloadControl::Cancel).await; + pending_acks.insert(id, ack); + } else { + let _ = ack.send(()); } } DownloadCmd::CaptureUrls(urls) => { diff --git a/src-tauri/src/download_ownership.rs b/src-tauri/src/download_ownership.rs index b3fe15c..2f78051 100644 --- a/src-tauri/src/download_ownership.rs +++ b/src-tauri/src/download_ownership.rs @@ -9,6 +9,30 @@ struct DownloadOwnershipRecord { primary_path: String, } +pub fn canonical_download_filename(filename: &str) -> String { + let leaf = filename.replace('\\', "/"); + let leaf = Path::new(&leaf) + .file_name() + .and_then(|name| name.to_str()) + .unwrap_or("download"); + let sanitized = leaf + .chars() + .map(|character| { + if character.is_control() || matches!(character, '<' | '>' | ':' | '"' | '/' | '\\' | '|' | '?' | '*') { + '-' + } else { + character + } + }) + .collect::(); + let sanitized = sanitized.trim().trim_end_matches(['.', ' ']); + if sanitized.is_empty() || matches!(sanitized, "." | "..") { + "download".to_string() + } else { + sanitized.to_string() + } +} + pub fn expected_primary_path( app_handle: &tauri::AppHandle, destination: &str, @@ -19,10 +43,7 @@ pub fn expected_primary_path( return Err("Path traversal blocked".to_string()); } - let safe_filename = Path::new(&filename.replace('\\', "/")) - .file_name() - .ok_or_else(|| "Download filename is invalid".to_string())? - .to_owned(); + let safe_filename = canonical_download_filename(filename); Ok(resolved_dest.join(safe_filename)) } @@ -67,6 +88,16 @@ pub fn remove(app_handle: &tauri::AppHandle, id: &str) -> Result<(), String> { crate::db::remove_ownership(&connection, id) } +pub fn primary_path_for_id( + app_handle: &tauri::AppHandle, + id: &str, +) -> Result, String> { + Ok(load_records(app_handle)? + .into_iter() + .find(|record| record.id == id) + .map(|record| PathBuf::from(record.primary_path))) +} + pub fn known_primary_paths(app_handle: &tauri::AppHandle) -> Result, String> { let mut paths: Vec = load_records(app_handle)? .into_iter() @@ -168,3 +199,15 @@ fn legacy_download_queue_paths(app_handle: &tauri::AppHandle) -> Result &'static str { match self { Self::Ready => "ready", + Self::Staged => "staged", Self::Downloading => "downloading", Self::Processing => "processing", Self::Paused => "paused", @@ -100,6 +103,8 @@ pub struct DownloadItem { #[ts(optional)] pub queue_id: Option, #[ts(optional)] + pub queue_position: Option, + #[ts(optional)] pub has_been_dispatched: Option, } @@ -110,9 +115,19 @@ pub struct EnqueueResult { pub id: String, pub success: bool, #[ts(optional)] + pub filename: Option, + #[ts(optional)] pub error: Option, } +#[derive(Clone, Debug, Serialize, Deserialize, TS)] +#[serde(rename_all = "camelCase")] +#[ts(export, export_to = "../../src/bindings/")] +pub struct EnqueueAccepted { + pub id: String, + pub filename: String, +} + #[derive(Clone, Debug, Serialize, Deserialize, TS)] #[serde(rename_all = "camelCase")] #[ts(export, export_to = "../../src/bindings/")] @@ -170,7 +185,7 @@ pub enum ActiveView { Scheduler, #[serde(rename = "speedLimiter")] SpeedLimiter, - Diagnostics, + Logs, } #[derive(Clone, Debug, Serialize, Deserialize, TS)] @@ -219,6 +234,7 @@ pub struct SchedulerSettings { pub stop_time: String, pub everyday: bool, pub selected_days: Vec, + pub selected_queue_ids: Vec, pub post_queue_action: PostQueueAction, } diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index 3110d39..872f346 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -665,6 +665,14 @@ fn aggregate_media_fraction( } async fn cleanup_media_processing_artifacts(out_path: &std::path::Path) { + cleanup_media_artifacts(out_path, true).await; +} + +async fn cleanup_media_sidecars(out_path: &std::path::Path) { + cleanup_media_artifacts(out_path, false).await; +} + +async fn cleanup_media_artifacts(out_path: &std::path::Path, remove_primary: bool) { let Some(parent) = out_path.parent() else { return; }; @@ -676,7 +684,9 @@ async fn cleanup_media_processing_artifacts(out_path: &std::path::Path) { .and_then(|name| name.to_str()) .unwrap_or(base_name); - let _ = tokio::fs::remove_file(out_path).await; + if remove_primary { + let _ = tokio::fs::remove_file(out_path).await; + } let Ok(mut entries) = tokio::fs::read_dir(parent).await else { return; @@ -1425,7 +1435,7 @@ async fn test_aria2c(app_handle: tauri::AppHandle, state: tauri::State<'_, AppSt .ok_or_else(|| "aria2 returned an invalid version response".to_string()) } -// ── get_engine_status: Structured engine diagnostics ────────────── +// ── get_engine_status: Structured engine status ────────────── async fn run_sidecar_version( app_handle: &tauri::AppHandle, @@ -1915,11 +1925,7 @@ pub(crate) async fn start_media_download_internal( max_tries: Option, cancel_rx: &mut tokio::sync::watch::Receiver, ) -> Result<(), String> { - let safe_filename = std::path::Path::new(&filename.replace('\\', "/")) - .file_name() - .and_then(|n| n.to_str()) - .unwrap_or("download") - .to_string(); + let safe_filename = crate::download_ownership::canonical_download_filename(&filename); let resolved_dest = resolve_path(&destination, &app_handle); @@ -2413,11 +2419,10 @@ async fn remove_download( app_handle: tauri::AppHandle, state: tauri::State<'_, AppState>, id: String, - filepath: Option, + delete_assets: bool, ) -> Result<(), String> { log::info!("remove_download called for id: {}", id); - let _ = crate::download_ownership::remove(&app_handle, &id); - state.queue_manager.release_registered_id(&id).await; + let primary_path = crate::download_ownership::primary_path_for_id(&app_handle, &id)?; let active_kind = state.queue_manager.active_kind(&id).await; state.queue_manager.remove_from_pending(&id).await; @@ -2447,9 +2452,8 @@ async fn remove_download( } else if let Ok(download_id) = Uuid::parse_str(&id) { state .download_coordinator - .send(download::DownloadCmd::Cancel(download_id)) + .send(download::DownloadCmd::CancelWithAck(download_id, tx)) .await?; - let _ = tx.send(()); } else { let _ = tx.send(()); } @@ -2468,41 +2472,61 @@ async fn remove_download( crate::ipc::DownloadStateEvent::new(id.clone(), crate::ipc::DownloadStatus::Paused), ); - if let Some(path) = filepath { - if !path.is_empty() { - let p = std::path::Path::new(&path); - if is_safe_path(p, &app_handle) { - if p.exists() { - let _ = tokio::fs::remove_file(p).await; - } - let aria2_path = format!("{}.aria2", path); - let p_aria2 = std::path::Path::new(&aria2_path); - if p_aria2.exists() { - let _ = tokio::fs::remove_file(p_aria2).await; - } + if delete_assets { + if let Some(path) = primary_path.as_deref() { + remove_download_assets(path, &app_handle).await?; + } + } else if let Some(path) = primary_path.as_deref() { + remove_partial_download_assets(path, &app_handle).await?; + } - if let Some(parent) = p.parent() { - if let Some(stem) = p.file_stem().and_then(|s| s.to_str()) { - let stem_with_dot = format!("{}.", stem); - if let Ok(mut entries) = tokio::fs::read_dir(parent).await { - while let Ok(Some(entry)) = entries.next_entry().await { - if let Ok(file_name) = entry.file_name().into_string() { - if file_name.starts_with(&stem_with_dot) && - (file_name.ends_with(".part") || file_name.ends_with(".ytdl") || file_name.ends_with(".aria2")) { - let path_to_remove = entry.path(); - if is_safe_path(&path_to_remove, &app_handle) { - let _ = tokio::fs::remove_file(path_to_remove).await; - } - } - } - } - } - } - } - } + crate::download_ownership::remove(&app_handle, &id)?; + state.queue_manager.release_registered_id(&id).await; + Ok(()) +} + +async fn remove_download_assets( + primary: &std::path::Path, + app_handle: &tauri::AppHandle, +) -> Result<(), String> { + if !is_safe_path(primary, app_handle) { + return Err("Download asset path is outside an allowed download location".to_string()); + } + + if primary.exists() { + trash::delete(primary) + .map_err(|error| format!("failed to move downloaded file to Trash: {error}"))?; + } + + for suffix in [".aria2", ".part", ".ytdl"] { + let candidate = std::path::PathBuf::from(format!("{}{}", primary.display(), suffix)); + if candidate.exists() && is_safe_path(&candidate, app_handle) { + tokio::fs::remove_file(&candidate) + .await + .map_err(|error| format!("failed to remove '{}': {error}", candidate.display()))?; } } + cleanup_media_processing_artifacts(primary).await; + Ok(()) +} + +async fn remove_partial_download_assets( + primary: &std::path::Path, + app_handle: &tauri::AppHandle, +) -> Result<(), String> { + if !is_safe_path(primary, app_handle) { + return Err("Download asset path is outside an allowed download location".to_string()); + } + for suffix in [".aria2", ".part", ".ytdl"] { + let candidate = std::path::PathBuf::from(format!("{}{}", primary.display(), suffix)); + if candidate.exists() && is_safe_path(&candidate, app_handle) { + tokio::fs::remove_file(&candidate) + .await + .map_err(|error| format!("failed to remove '{}': {error}", candidate.display()))?; + } + } + cleanup_media_sidecars(primary).await; Ok(()) } @@ -2703,17 +2727,43 @@ fn perform_system_action(action: crate::ipc::PostQueueAction) -> Result<(), Stri } #[tauri::command] -async fn get_pending_order(state: tauri::State<'_, AppState>) -> Result, AppError> { - Ok(state.queue_manager.pending_order().await) +fn ack_schedule_trigger( + app_handle: tauri::AppHandle, + action: String, + key: String, +) -> Result<(), String> { + crate::settings::update_settings_state(&app_handle, |state| match action.as_str() { + "start" => { + state.insert("schedulerLastStartKey".to_string(), serde_json::json!(key)); + } + "stop" => { + state.insert("schedulerLastStopKey".to_string(), serde_json::json!(key)); + } + _ => {} + })?; + match action.as_str() { + "start" | "stop" => Ok(()), + _ => Err("Unknown scheduler trigger action".to_string()), + } +} + +#[tauri::command] +async fn get_pending_order( + state: tauri::State<'_, AppState>, + queue_id: Option, +) -> Result, AppError> { + Ok(state.queue_manager.pending_order(queue_id.as_deref()).await) } #[tauri::command] async fn enqueue_download( app_handle: tauri::AppHandle, state: tauri::State<'_, AppState>, - item: queue::EnqueueItem, -) -> Result { + mut item: queue::EnqueueItem, +) -> Result { let id = item.id.clone(); + item.filename = crate::download_ownership::canonical_download_filename(&item.filename); + let accepted_filename = item.filename.clone(); crate::download_ownership::register_expected( &app_handle, &item.id, @@ -2725,15 +2775,21 @@ async fn enqueue_download( state.queue_manager.release_registered_id(&id).await; return Err(AppError::Internal(e)); } - Ok(id) + Ok(crate::ipc::EnqueueAccepted { + id, + filename: accepted_filename, + }) } #[tauri::command] async fn enqueue_many( app_handle: tauri::AppHandle, state: tauri::State<'_, AppState>, - items: Vec, + mut items: Vec, ) -> Result, AppError> { + for item in &mut items { + item.filename = crate::download_ownership::canonical_download_filename(&item.filename); + } for item in &items { crate::download_ownership::register_expected( &app_handle, @@ -2759,9 +2815,13 @@ async fn enqueue_many( async fn move_in_queue( state: tauri::State<'_, AppState>, id: String, + queue_id: String, direction: crate::ipc::QueueDirection, ) -> Result, AppError> { - Ok(state.queue_manager.move_in_queue(&id, direction).await) + Ok(state + .queue_manager + .move_in_queue(&id, &queue_id, direction) + .await) } #[tauri::command] @@ -3010,26 +3070,120 @@ fn delete_file(app_handle: tauri::AppHandle, path: String) -> Result<(), String> } } -#[tauri::command] -async fn export_logs(app_handle: tauri::AppHandle, dest_path: String) -> Result { +async fn log_files(app_handle: &tauri::AppHandle) -> Result, String> { use tauri::Manager; let log_dir = app_handle.path().app_log_dir().map_err(|e| e.to_string())?; - let log_file = log_dir.join("firelink.log"); - let src = if log_file.exists() { - log_file - } else { - let mut found = None; - if let Ok(mut entries) = tokio::fs::read_dir(&log_dir).await { - while let Ok(Some(entry)) = entries.next_entry().await { - if entry.path().extension().is_some_and(|e| e == "log") { - found = Some(entry.path()); - break; - } + let mut files = Vec::new(); + if let Ok(mut entries) = tokio::fs::read_dir(&log_dir).await { + while let Ok(Some(entry)) = entries.next_entry().await { + let path = entry.path(); + if path.is_file() + && path + .file_name() + .is_some_and(|name| name.to_string_lossy().contains(".log")) + { + files.push(path); } } - found.ok_or_else(|| "No log file found in app log directory".to_string())? - }; - tokio::fs::copy(&src, &dest_path).await.map_err(|e| e.to_string())?; + } + files.sort(); + Ok(files) +} + +fn redact_log_line(line: &str) -> String { + use std::sync::OnceLock; + static SECRET: OnceLock = OnceLock::new(); + static QUERY: OnceLock = OnceLock::new(); + let secret = SECRET.get_or_init(|| { + regex::Regex::new( + r"(?i)(authorization|cookie|password|token|secret)\s*[:=]\s*([^\s,;]+)", + ) + .expect("valid secret redaction regex") + }); + let query = QUERY.get_or_init(|| { + regex::Regex::new(r"(https?://[^\s?]+)\?[^\s]+") + .expect("valid URL query redaction regex") + }); + let redacted = secret.replace_all(line, "$1=[redacted]"); + query.replace_all(&redacted, "$1?[redacted]").into_owned() +} + +#[tauri::command] +async fn read_logs(app_handle: tauri::AppHandle, limit: usize) -> Result, String> { + let mut lines = Vec::new(); + for file in log_files(&app_handle).await? { + let content = tokio::fs::read_to_string(&file) + .await + .map_err(|error| format!("failed to read '{}': {error}", file.display()))?; + lines.extend(content.lines().map(redact_log_line)); + } + let keep = limit.clamp(1, 10_000); + if lines.len() > keep { + lines.drain(..lines.len() - keep); + } + Ok(lines) +} + +#[tauri::command] +async fn export_logs( + app_handle: tauri::AppHandle, + state: tauri::State<'_, AppState>, + dest_path: String, +) -> Result { + let mut output = format!( + "Firelink support logs\nVersion: {}\nOS: {} {}\nArchitecture: {}\nGenerated: {}\n\n", + env!("CARGO_PKG_VERSION"), + std::env::consts::OS, + std::env::consts::FAMILY, + std::env::consts::ARCH, + chrono::Utc::now().to_rfc3339(), + ); + let (aria2, ytdlp, ffmpeg, deno) = tokio::join!( + check_aria2(&app_handle, state.aria2_port, &state.aria2_secret), + check_ytdlp(&app_handle), + check_ffmpeg(&app_handle), + check_deno(&app_handle), + ); + output.push_str("Engine status:\n"); + for engine in [aria2, ytdlp, ffmpeg, deno] { + output.push_str(&format!( + "- {}: {}{}\n", + engine.name, + if engine.ready { "ready" } else { "unavailable" }, + engine + .version + .as_deref() + .map(|version| format!(" ({version})")) + .unwrap_or_default() + )); + if let Some(error) = engine.error { + output.push_str(&format!(" Error: {}\n", redact_log_line(&error))); + } + } + if let Ok(settings) = crate::settings::load_settings(&app_handle) { + output.push_str(&format!( + "\nRuntime settings:\n- Max concurrent downloads: {}\n- Per-server connections: {}\n- Automatic retries: {}\n- Proxy mode: {:?}\n- Scheduler enabled: {}\n\n", + settings.max_concurrent_downloads, + settings.per_server_connections, + settings.max_automatic_retries, + settings.proxy_mode, + settings.scheduler.enabled, + )); + } + for file in log_files(&app_handle).await? { + output.push_str(&format!("===== {} =====\n", file.display())); + let content = tokio::fs::read_to_string(&file) + .await + .map_err(|error| format!("failed to read '{}': {error}", file.display()))?; + for line in content.lines() { + output.push_str(&redact_log_line(line)); + output.push('\n'); + } + output.push('\n'); + } + tokio::fs::write(&dest_path, output) + .await + .map_err(|e| e.to_string())?; Ok(dest_path) } @@ -3154,7 +3308,7 @@ mod tests { aggregate_media_fraction, build_media_format_options, collect_download_uris, is_excluded_yt_dlp_format, json_lower, media_progress_speed, normalize_speed_limit_for_aria2, parse_firelink_urls, parse_media_progress_line, - MediaProgress, MEDIA_PROGRESS_PREFIX, + redact_log_line, MediaProgress, MEDIA_PROGRESS_PREFIX, }; use serde_json::json; use std::time::{Duration, Instant}; @@ -3168,6 +3322,16 @@ mod tests { assert_eq!(normalize_speed_limit_for_aria2("bad"), None); } + #[test] + fn redacts_secrets_and_signed_url_queries_from_support_logs() { + let line = "Authorization: bearer-secret Cookie=session=abc https://example.com/file?token=secret"; + let redacted = redact_log_line(line); + assert!(!redacted.contains("bearer-secret")); + assert!(!redacted.contains("session=abc")); + assert!(!redacted.contains("token=secret")); + assert!(redacted.contains("[redacted]")); + } + #[test] fn collects_primary_url_and_unique_mirrors_in_order() { let uris = collect_download_uris( @@ -3802,18 +3966,11 @@ pub fn run() { .targets([ tauri_plugin_log::Target::new(tauri_plugin_log::TargetKind::Stdout), tauri_plugin_log::Target::new(tauri_plugin_log::TargetKind::LogDir { file_name: None }), - tauri_plugin_log::Target::new(tauri_plugin_log::TargetKind::Webview), ]) .level(if cfg!(debug_assertions) { log::LevelFilter::Debug } else { log::LevelFilter::Info }) .max_file_size(10_000_000) .rotation_strategy(tauri_plugin_log::RotationStrategy::KeepSome(3)) - .format(move |out, message, _record| { - let msg = message.to_string(); - if msg.contains("[download]") && msg.contains('%') { - return; - } - out.finish(format_args!("{}\n", msg)); - }) + .timezone_strategy(tauri_plugin_log::TimezoneStrategy::UseLocal) .build(), ) .plugin(tauri_plugin_dialog::init()) @@ -3833,6 +3990,7 @@ pub fn run() { get_deno_engine_status, test_ytdlp, test_aria2c, test_ffmpeg, test_deno, open_file, show_in_folder, pause_download, resume_download, fetch_metadata, fetch_media_metadata, update_dock_badge, set_prevent_sleep, get_free_space, perform_system_action, + ack_schedule_trigger, request_automation_permission, open_automation_settings, set_keychain_password, get_keychain_password, delete_keychain_password, hydrate_extension_pairing_token, acknowledge_pairing_token_change, @@ -3840,12 +3998,12 @@ pub fn run() { get_extension_server_port, set_extension_frontend_ready, set_concurrent_limit, set_global_speed_limit, remove_download, detach_download_for_reconfigure, enqueue_download, enqueue_many, move_in_queue, remove_from_queue, get_pending_order, - commands::reveal_in_file_manager, commands::open_downloaded_file, commands::trash_download_assets, + commands::reveal_in_file_manager, commands::open_downloaded_file, parity::get_system_proxy, parity::get_file_category, parity::check_for_updates, parity::is_supported_media, parity::get_supported_media_domains, parity::create_category_directories, db_save_settings, db_load_settings, db_get_all_downloads, db_replace_downloads, db_get_all_queues, db_replace_queues, - export_logs + read_logs, export_logs ]) .build(tauri::generate_context!()) .expect("error while building tauri application") diff --git a/src-tauri/src/queue.rs b/src-tauri/src/queue.rs index 46825ff..293574b 100644 --- a/src-tauri/src/queue.rs +++ b/src-tauri/src/queue.rs @@ -37,6 +37,7 @@ pub enum TaskKind { #[derive(Debug, Clone)] pub struct QueuedTask { pub id: String, + pub queue_id: String, pub kind: TaskKind, pub payload: SpawnPayload, } @@ -158,11 +159,12 @@ impl QueueManager { } /// Current pending order, as id list. Returned by move_in_queue. - pub async fn pending_order(&self) -> Vec { + pub async fn pending_order(&self, queue_id: Option<&str>) -> Vec { self.pending .lock() .await .iter() + .filter(|task| queue_id.is_none_or(|queue_id| task.queue_id == queue_id)) .map(|t| t.id.clone()) .collect() } @@ -721,26 +723,38 @@ impl QueueManager { pub async fn move_in_queue( &self, id: &str, + queue_id: &str, direction: QueueDirection, ) -> Vec { let mut pending = self.pending.lock().await; - let pos = pending.iter().position(|t| t.id == id); - if let Some(pos) = pos { + let queue_positions = pending + .iter() + .enumerate() + .filter_map(|(index, task)| (task.queue_id == queue_id).then_some(index)) + .collect::>(); + let queue_pos = queue_positions + .iter() + .position(|index| pending[*index].id == id); + if let Some(queue_pos) = queue_pos { let target = match direction { - QueueDirection::Up => pos.checked_sub(1), + QueueDirection::Up => queue_pos.checked_sub(1), QueueDirection::Down => { - if pos + 1 < pending.len() { - Some(pos + 1) + if queue_pos + 1 < queue_positions.len() { + Some(queue_pos + 1) } else { None } } }; if let Some(target) = target { - pending.swap(pos, target); + pending.swap(queue_positions[queue_pos], queue_positions[target]); } } - pending.iter().map(|t| t.id.clone()).collect() + pending + .iter() + .filter(|task| task.queue_id == queue_id) + .map(|task| task.id.clone()) + .collect() } /// Remove a task from pending if present (used by remove_download). @@ -765,10 +779,12 @@ impl QueueManager { for task in tasks { let id = task.id.clone(); + let filename = task.payload.filename.clone(); if registered.contains(&id) { results.push(crate::ipc::EnqueueResult { id: id.clone(), success: false, + filename: None, error: Some("Duplicate task".to_string()), }); continue; @@ -779,6 +795,7 @@ impl QueueManager { results.push(crate::ipc::EnqueueResult { id, success: true, + filename: Some(filename), error: None, }); } @@ -814,11 +831,7 @@ impl SidecarSpawner for ProductionSpawner { "dir".to_string(), serde_json::json!(resolved_dest.to_string_lossy().to_string()), ); - let safe_filename = std::path::Path::new(&payload.filename.replace('\\', "/")) - .file_name() - .and_then(|n| n.to_str()) - .unwrap_or("download") - .to_string(); + let safe_filename = crate::download_ownership::canonical_download_filename(&payload.filename); options.insert("out".to_string(), serde_json::json!(safe_filename)); let conn = payload.connections.unwrap_or(1); options.insert("split".to_string(), serde_json::json!(conn.to_string())); @@ -880,11 +893,7 @@ impl SidecarSpawner for ProductionSpawner { log::warn!("aria2 addUri failed, falling back to native: {}", e); let download_id = uuid::Uuid::parse_str(id).map_err(|e| e.to_string())?; let mt = payload.max_tries.unwrap_or(1).max(1) as u32; - let safe_filename = std::path::Path::new(&payload.filename.replace('\\', "/")) - .file_name() - .and_then(|n| n.to_str()) - .unwrap_or("download") - .to_string(); + let safe_filename = crate::download_ownership::canonical_download_filename(&payload.filename); state .download_coordinator .send(crate::download::DownloadCmd::Start(Box::new( @@ -969,11 +978,7 @@ impl SidecarSpawner for ProductionSpawner { let download_id = uuid::Uuid::parse_str(id).map_err(|e| e.to_string())?; let mt = payload.max_tries.unwrap_or(1).max(1) as u32; let resolved_dest = crate::resolve_path(&payload.destination, &self.app_handle); - let safe_filename = std::path::Path::new(&payload.filename.replace('\\', "/")) - .file_name() - .and_then(|n| n.to_str()) - .unwrap_or("download") - .to_string(); + let safe_filename = crate::download_ownership::canonical_download_filename(&payload.filename); let output_path = resolved_dest.join(safe_filename); let _ = crate::download_ownership::set_primary_path(&self.app_handle, id, &output_path); state @@ -1002,6 +1007,7 @@ impl SidecarSpawner for ProductionSpawner { #[ts(export, export_to = "../../src/bindings/")] pub struct EnqueueItem { pub id: String, + pub queue_id: String, pub url: String, pub destination: String, pub filename: String, @@ -1032,6 +1038,7 @@ impl EnqueueItem { let id = self.id.clone(); QueuedTask { id, + queue_id: self.queue_id, kind, payload: SpawnPayload { url: self.url, diff --git a/src-tauri/src/scheduler.rs b/src-tauri/src/scheduler.rs index 6fe53a3..013be92 100644 --- a/src-tauri/src/scheduler.rs +++ b/src-tauri/src/scheduler.rs @@ -1,10 +1,19 @@ -use chrono::{Datelike, Local}; +use chrono::{Datelike, Local, Timelike}; +use std::collections::HashMap; use std::time::Duration; use tauri::Emitter; +fn minute_of_day(value: &str) -> Option { + let (hour, minute) = value.split_once(':')?; + let hour = hour.parse::().ok()?; + let minute = minute.parse::().ok()?; + (hour < 24 && minute < 60).then_some(hour * 60 + minute) +} + pub fn spawn_scheduler(app_handle: tauri::AppHandle) { tauri::async_runtime::spawn(async move { let mut interval = tokio::time::interval(Duration::from_secs(1)); + let mut last_emit: HashMap<&'static str, std::time::Instant> = HashMap::new(); loop { interval.tick().await; @@ -15,7 +24,7 @@ pub fn spawn_scheduler(app_handle: tauri::AppHandle) { } let now = Local::now(); - let current_time = now.format("%H:%M").to_string(); + let current_minute = now.hour() * 60 + now.minute(); let current_day = now.weekday().num_days_from_sunday(); let allowed_today = @@ -25,37 +34,60 @@ pub fn spawn_scheduler(app_handle: tauri::AppHandle) { } let date_key = now.format("%Y-%m-%d").to_string(); - let trigger_key = format!("{}-{}", date_key, current_time); + let start_key = format!("{date_key}-start"); + let stop_key = format!("{date_key}-stop"); + let start_minute = minute_of_day(&scheduler.start_time); + let stop_minute = minute_of_day(&scheduler.stop_time); + let before_stop = !scheduler.stop_time_enabled + || stop_minute.is_some_and(|stop| current_minute < stop); - if scheduler.start_time == current_time - && settings.scheduler_last_start_key != trigger_key + if start_minute.is_some_and(|start| current_minute >= start) + && before_stop + && settings.scheduler_last_start_key != start_key + && last_emit + .get("start") + .is_none_or(|instant| instant.elapsed() >= Duration::from_secs(5)) { - let key = trigger_key.clone(); - let _ = crate::settings::update_settings_state(&app_handle, |state| { - state.insert("schedulerLastStartKey".to_string(), serde_json::json!(key)); - state.insert("schedulerRunning".to_string(), serde_json::json!(true)); - }); let _ = app_handle.emit("schedule-trigger", serde_json::json!({ "action": "start", - "key": key + "key": start_key })); + last_emit.insert("start", std::time::Instant::now()); } if scheduler.stop_time_enabled - && scheduler.stop_time == current_time - && settings.scheduler_last_stop_key != trigger_key + && stop_minute.is_some_and(|stop| current_minute >= stop) + && settings.scheduler_last_stop_key != stop_key + && last_emit + .get("stop") + .is_none_or(|instant| instant.elapsed() >= Duration::from_secs(5)) { - let key = trigger_key.clone(); - let _ = crate::settings::update_settings_state(&app_handle, |state| { - state.insert("schedulerLastStopKey".to_string(), serde_json::json!(key)); - state.insert("schedulerRunning".to_string(), serde_json::json!(false)); - }); let _ = app_handle.emit("schedule-trigger", serde_json::json!({ "action": "stop", - "key": key + "key": stop_key })); + last_emit.insert("stop", std::time::Instant::now()); } } } }); } + +#[cfg(test)] +mod tests { + use super::minute_of_day; + + #[test] + fn parses_valid_scheduler_times() { + assert_eq!(minute_of_day("00:00"), Some(0)); + assert_eq!(minute_of_day("23:59"), Some(1439)); + assert_eq!(minute_of_day("06:30"), Some(390)); + } + + #[test] + fn rejects_invalid_scheduler_times() { + assert_eq!(minute_of_day("24:00"), None); + assert_eq!(minute_of_day("12:60"), None); + assert_eq!(minute_of_day("bad"), None); + } +} diff --git a/src-tauri/src/settings.rs b/src-tauri/src/settings.rs index cb41cd2..acf856a 100644 --- a/src-tauri/src/settings.rs +++ b/src-tauri/src/settings.rs @@ -243,6 +243,7 @@ fn default_settings() -> PersistedSettings { stop_time: "08:00".to_string(), everyday: true, selected_days: vec![0, 1, 2, 3, 4, 5, 6], + selected_queue_ids: vec!["00000000-0000-0000-0000-000000000001".to_string()], post_queue_action: PostQueueAction::None, }, scheduler_last_start_key: String::new(), @@ -299,6 +300,10 @@ mod tests { assert!(settings.scheduler.enabled); assert_eq!(settings.scheduler.start_time, "06:30"); assert_eq!(settings.scheduler.selected_days, vec![1, 3, 5]); + assert_eq!( + settings.scheduler.selected_queue_ids, + vec!["00000000-0000-0000-0000-000000000001"] + ); assert_eq!(settings.base_download_folder, "~/Downloads"); } diff --git a/src-tauri/tests/download_engine.rs b/src-tauri/tests/download_engine.rs index 3a30c41..15a6a0f 100644 --- a/src-tauri/tests/download_engine.rs +++ b/src-tauri/tests/download_engine.rs @@ -438,7 +438,12 @@ async fn cancel_removes_partial_file_without_terminal_success_event() { .await .unwrap(); wait_for_progress(&mut events, id, 256 * 1024).await; - coordinator.send(DownloadCmd::Cancel(id)).await.unwrap(); + let (cancelled_tx, cancelled_rx) = tokio::sync::oneshot::channel(); + coordinator + .send(DownloadCmd::CancelWithAck(id, cancelled_tx)) + .await + .unwrap(); + cancelled_rx.await.unwrap(); tokio::time::timeout(TEST_TIMEOUT, async { while output_path.exists() { diff --git a/src-tauri/tests/queue_manager.rs b/src-tauri/tests/queue_manager.rs index 25837cf..ba4cb4a 100644 --- a/src-tauri/tests/queue_manager.rs +++ b/src-tauri/tests/queue_manager.rs @@ -58,6 +58,7 @@ fn make_manager(capacity: usize) -> (QueueManager, Arc fn sample_task(id: &str) -> QueuedTask { QueuedTask { id: id.to_string(), + queue_id: "main".to_string(), kind: TaskKind::Native, payload: SpawnPayload::default(), } @@ -66,9 +67,9 @@ fn sample_task(id: &str) -> QueuedTask { #[tokio::test] async fn push_appends_to_pending_and_emits_queued() { let (mgr, _spawner) = make_manager(2); - mgr.push(sample_task("a")).await; - mgr.push(sample_task("b")).await; - let order = mgr.pending_order().await; + mgr.push(sample_task("a")).await.unwrap(); + mgr.push(sample_task("b")).await.unwrap(); + let order = mgr.pending_order(None).await; assert_eq!(order, vec!["a".to_string(), "b".to_string()]); } @@ -116,8 +117,8 @@ async fn forgetting_aria2_gid_clears_mapping_without_releasing_twice() { #[tokio::test] async fn push_then_pop_front_drains_fifo() { let (mgr, _spawner) = make_manager(2); - mgr.push(sample_task("a")).await; - mgr.push(sample_task("b")).await; + mgr.push(sample_task("a")).await.unwrap(); + mgr.push(sample_task("b")).await.unwrap(); let first = mgr.pop_front().await.expect("some task"); let second = mgr.pop_front().await.expect("some task"); assert_eq!(first.id, "a"); @@ -179,7 +180,7 @@ async fn grow_releases_immediately_and_dispatches_waiting_tasks() { let mgr_arc = Arc::new(mgr); for i in 0..4 { - mgr_arc.push(sample_task(&format!("t{i}"))).await; + mgr_arc.push(sample_task(&format!("t{i}"))).await.unwrap(); } let handle = { let mgr_clone = Arc::clone(&mgr_arc); @@ -206,7 +207,7 @@ async fn shrink_converges_to_target_without_killing_active() { let mgr_arc = Arc::new(mgr); for i in 0..6 { - mgr_arc.push(sample_task(&format!("t{i}"))).await; + mgr_arc.push(sample_task(&format!("t{i}"))).await.unwrap(); } let handle = { let mgr_clone = Arc::clone(&mgr_arc); @@ -239,6 +240,7 @@ async fn shrink_converges_to_target_without_killing_active() { fn aria2_task(id: &str) -> QueuedTask { QueuedTask { id: id.to_string(), + queue_id: "main".to_string(), kind: TaskKind::Aria2, payload: SpawnPayload::default(), } @@ -247,6 +249,7 @@ fn aria2_task(id: &str) -> QueuedTask { fn media_task(id: &str) -> QueuedTask { QueuedTask { id: id.to_string(), + queue_id: "main".to_string(), kind: TaskKind::Media, payload: SpawnPayload::default(), } @@ -305,7 +308,7 @@ fn emitted_statuses(event_rx: &std::sync::mpsc::Receiver) -> Vec async fn media_terminal_error_emits_failed_without_completed() { let (manager, event_rx) = make_media_manager(Err("terminal media failure".to_string())); let manager = Arc::new(manager); - manager.push(media_task("media-failed")).await; + manager.push(media_task("media-failed")).await.unwrap(); let dispatcher = { let manager = Arc::clone(&manager); tokio::spawn(async move { manager.run_dispatcher().await }) @@ -325,7 +328,7 @@ async fn media_cancellation_does_not_emit_completed() { let (manager, event_rx) = make_media_manager(Err(MEDIA_RUN_CANCELLED.to_string())); let manager = Arc::new(manager); - manager.push(media_task("media-cancelled")).await; + manager.push(media_task("media-cancelled")).await.unwrap(); let dispatcher = { let manager = Arc::clone(&manager); tokio::spawn(async move { manager.run_dispatcher().await }) @@ -344,7 +347,7 @@ async fn media_cancellation_does_not_emit_completed() { async fn aria2_permit_survives_rpc_return() { let (mgr, spawner) = make_manager(1); let mgr_arc = Arc::new(mgr); - mgr_arc.push(aria2_task("a")).await; + mgr_arc.push(aria2_task("a")).await.unwrap(); let handle = { let mgr_clone = Arc::clone(&mgr_arc); tokio::spawn(async move { mgr_clone.run_dispatcher().await }) @@ -373,7 +376,7 @@ async fn gid_completion_before_store_buffers_and_reconciles() { let (mgr, _spawner) = make_manager(1); let mgr_arc = Arc::new(mgr); - mgr_arc.push(aria2_task("a")).await; + mgr_arc.push(aria2_task("a")).await.unwrap(); let handle = { let mgr_clone = Arc::clone(&mgr_arc); tokio::spawn(async move { mgr_clone.run_dispatcher().await }) @@ -396,7 +399,7 @@ async fn gid_completion_before_store_buffers_and_reconciles() { assert_eq!(mgr_arc.available_permits(), 1); // Push another aria2 task; its gid will be "gid-2". - mgr_arc.push(aria2_task("b")).await; + mgr_arc.push(aria2_task("b")).await.unwrap(); tokio::time::sleep(Duration::from_millis(100)).await; mgr_arc.release_permit("b").await; tokio::time::sleep(Duration::from_millis(50)).await; @@ -425,21 +428,43 @@ async fn move_up_down_reorders_pending() { let (mgr, _spawner) = make_manager(3); let mgr_arc = Arc::new(mgr); - mgr_arc.push(sample_task("a")).await; - mgr_arc.push(sample_task("b")).await; - mgr_arc.push(sample_task("c")).await; + mgr_arc.push(sample_task("a")).await.unwrap(); + mgr_arc.push(sample_task("b")).await.unwrap(); + mgr_arc.push(sample_task("c")).await.unwrap(); - mgr_arc.move_in_queue("c", QueueDirection::Down).await; - assert_eq!(mgr_arc.pending_order().await, vec!["a", "b", "c"]); + mgr_arc.move_in_queue("c", "main", QueueDirection::Down).await; + assert_eq!(mgr_arc.pending_order(None).await, vec!["a", "b", "c"]); - mgr_arc.move_in_queue("c", QueueDirection::Up).await; - assert_eq!(mgr_arc.pending_order().await, vec!["a", "c", "b"]); + mgr_arc.move_in_queue("c", "main", QueueDirection::Up).await; + assert_eq!(mgr_arc.pending_order(None).await, vec!["a", "c", "b"]); - mgr_arc.move_in_queue("a", QueueDirection::Down).await; - assert_eq!(mgr_arc.pending_order().await, vec!["c", "a", "b"]); + mgr_arc.move_in_queue("a", "main", QueueDirection::Down).await; + assert_eq!(mgr_arc.pending_order(None).await, vec!["c", "a", "b"]); - mgr_arc.move_in_queue("c", QueueDirection::Up).await; - assert_eq!(mgr_arc.pending_order().await, vec!["c", "a", "b"]); + mgr_arc.move_in_queue("c", "main", QueueDirection::Up).await; + assert_eq!(mgr_arc.pending_order(None).await, vec!["c", "a", "b"]); +} + +#[tokio::test] +async fn moving_one_queue_does_not_reorder_another_queue() { + use firelink_lib::ipc::QueueDirection; + + let (mgr, _spawner) = make_manager(3); + let mut a1 = sample_task("a1"); + a1.queue_id = "a".to_string(); + let mut b1 = sample_task("b1"); + b1.queue_id = "b".to_string(); + let mut a2 = sample_task("a2"); + a2.queue_id = "a".to_string(); + let mut b2 = sample_task("b2"); + b2.queue_id = "b".to_string(); + mgr.push(a1).await.unwrap(); + mgr.push(b1).await.unwrap(); + mgr.push(a2).await.unwrap(); + mgr.push(b2).await.unwrap(); + + assert_eq!(mgr.move_in_queue("a2", "a", QueueDirection::Up).await, vec!["a2", "a1"]); + assert_eq!(mgr.pending_order(Some("b")).await, vec!["b1", "b2"]); } #[tokio::test] @@ -455,7 +480,7 @@ async fn notify_fires_on_push_and_release() { tokio::spawn(async move { mgr_clone.run_dispatcher().await }) }; - mgr_arc.push(sample_task("x")).await; + mgr_arc.push(sample_task("x")).await.unwrap(); let dispatched = timeout( Duration::from_millis(150), async { diff --git a/src/App.tsx b/src/App.tsx index a30d503..b47095d 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -14,11 +14,20 @@ import { useSettingsStore } from "./store/useSettingsStore"; import { isPermissionGranted, requestPermission, sendNotification } from '@tauri-apps/plugin-notification'; import SchedulerView from "./components/SchedulerView"; import SpeedLimiterView from "./components/SpeedLimiterView"; -import DiagnosticsView from "./components/DiagnosticsView"; +import LogsView from "./components/LogsView"; import { useToast } from "./contexts/ToastContext"; import { openUrl } from '@tauri-apps/plugin-opener'; let automaticUpdateCheckStarted = false; +const processingScheduleKeys = new Set(); + +const getScheduledQueueIds = () => { + const downloadState = useDownloadStore.getState(); + const availableQueueIds = new Set(downloadState.queues.map(queue => queue.id)); + const selectedQueueIds = useSettingsStore.getState().scheduler.selectedQueueIds + .filter(queueId => availableQueueIds.has(queueId)); + return selectedQueueIds.length > 0 ? selectedQueueIds : [MAIN_QUEUE_ID]; +}; function App() { const [filter, setFilter] = useState('all'); @@ -40,9 +49,12 @@ function App() { const extensionPairingToken = useSettingsStore(state => state.extensionPairingToken); const downloads = useDownloadStore(state => state.downloads); const activeDownloadCount = downloads.filter(download => download.status === 'downloading').length; - const queuedCount = downloads.filter(download => download.status === 'queued').length; + const queuedCount = downloads.filter(download => + download.status === 'queued' || download.status === 'staged' + ).length; const doneCount = downloads.filter(download => download.status === 'completed').length; const schedulerRunning = useSettingsStore(state => state.schedulerRunning); + const schedulerActiveDownloadIds = useSettingsStore(state => state.schedulerActiveDownloadIds); const globalSpeedLimit = useSettingsStore(state => state.globalSpeedLimit); const previousSpeedLimit = useRef(null); const maxConcurrentDownloads = useSettingsStore(state => state.maxConcurrentDownloads); @@ -218,15 +230,28 @@ function App() { useEffect(() => { const unlisten = listen('schedule-trigger', async (event) => { const state = useSettingsStore.getState(); - const payload = event.payload as any; - if (payload.action === 'start') { - state.setSchedulerLastStartKey(payload.key); - const started = await useDownloadStore.getState().startQueue(MAIN_QUEUE_ID); - state.setSchedulerRunning(started > 0); - } else if (payload.action === 'stop') { - state.setSchedulerLastStopKey(payload.key); - await useDownloadStore.getState().pauseQueue(MAIN_QUEUE_ID); - state.setSchedulerRunning(false); + const payload = event.payload; + if (processingScheduleKeys.has(payload.key)) return; + processingScheduleKeys.add(payload.key); + try { + if (payload.action === 'start') { + const startedResults = await Promise.all( + getScheduledQueueIds().map(queueId => useDownloadStore.getState().startQueue(queueId)) + ); + const acceptedIds = startedResults.flat(); + state.setSchedulerActiveDownloadIds(acceptedIds); + state.setSchedulerRunning(acceptedIds.length > 0); + await invoke('ack_schedule_trigger', { action: 'start', key: payload.key }); + } else if (payload.action === 'stop') { + await Promise.all( + getScheduledQueueIds().map(queueId => useDownloadStore.getState().pauseQueue(queueId)) + ); + state.setSchedulerActiveDownloadIds([]); + state.setSchedulerRunning(false); + await invoke('ack_schedule_trigger', { action: 'stop', key: payload.key }); + } + } finally { + processingScheduleKeys.delete(payload.key); } }); @@ -237,19 +262,32 @@ function App() { useEffect(() => { if (!schedulerRunning) return; + if (schedulerActiveDownloadIds.length === 0) return; + const scheduledIds = new Set(schedulerActiveDownloadIds); const hasPendingScheduledWork = downloads.some(download => - isActiveDownloadStatus(download.status) + scheduledIds.has(download.id) && isActiveDownloadStatus(download.status) ); if (hasPendingScheduledWork) return; const settings = useSettingsStore.getState(); + const scheduledItems = schedulerActiveDownloadIds.map(id => + downloads.find(download => download.id === id) + ); + const hasFailures = scheduledItems.some(item => !item || item.status === 'failed'); + settings.setSchedulerActiveDownloadIds([]); settings.setSchedulerRunning(false); - if (settings.scheduler.postQueueAction !== 'none') { + if (hasFailures) { + addToast({ + message: 'Scheduled downloads finished with failures. The post-queue system action was skipped.', + variant: 'warning', + isActionable: true + }); + } else if (settings.scheduler.postQueueAction !== 'none') { invoke('perform_system_action', { action: settings.scheduler.postQueueAction }).catch(error => { console.error('Scheduled post action failed:', error); }); } - }, [downloads, schedulerRunning]); + }, [addToast, downloads, schedulerRunning, schedulerActiveDownloadIds]); useEffect(() => { const initNotifications = async () => { @@ -395,7 +433,7 @@ function App() { {activeView === 'settings' && } {activeView === 'scheduler' && } {activeView === 'speedLimiter' && } - {activeView === 'diagnostics' && } + {activeView === 'logs' && } {/* Status Bar */} diff --git a/src/ErrorBoundary.tsx b/src/ErrorBoundary.tsx index faaa760..8453338 100644 --- a/src/ErrorBoundary.tsx +++ b/src/ErrorBoundary.tsx @@ -33,7 +33,7 @@ export class ErrorBoundary extends Component {

Firelink could not display this window.

- The error was written to Diagnostics. Reload the interface to reconnect to the running download service. + The error was written to Logs. Reload the interface to reconnect to the running download service.

-
+
{filteredDownloads.map((d, index) => ( = ({ filter }) => { onClick={handleItemClick} /> ))} - {Array.from({ length: Math.max(0, 50 - filteredDownloads.length) }).map((_, i) => { - const globalIndex = filteredDownloads.length + i; - return ( -
- ); - })} -
+
@@ -411,7 +402,9 @@ export const DownloadTable: React.FC = ({ filter }) => { {queues.map(q => ( @@ -526,7 +519,9 @@ export const DownloadTable: React.FC = ({ filter }) => { {queues.map(q => ( diff --git a/src/components/DiagnosticsView.tsx b/src/components/LogsView.tsx similarity index 64% rename from src/components/DiagnosticsView.tsx rename to src/components/LogsView.tsx index e2c777f..eaf789e 100644 --- a/src/components/DiagnosticsView.tsx +++ b/src/components/LogsView.tsx @@ -1,5 +1,4 @@ import { useEffect, useRef, useState } from 'react'; -import { attachLogger } from '@tauri-apps/plugin-log'; import { invokeCommand as invoke } from '../ipc'; import { save } from '@tauri-apps/plugin-dialog'; import { FileDown, Trash2, Terminal, Filter } from 'lucide-react'; @@ -11,39 +10,46 @@ interface LogEntry { message: string; } -const getLevelStr = (level: number): LogEntry['level'] => { - switch (level) { - case 1: return 'Trace'; - case 2: return 'Debug'; - case 3: return 'Info'; - case 4: return 'Warn'; - case 5: return 'Error'; - default: return 'Debug'; - } -}; - -export default function DiagnosticsView() { +export default function LogsView() { const { addToast } = useToast(); const [logs, setLogs] = useState([]); const [levelFilter, setLevelFilter] = useState('All'); const scrollRef = useRef(null); + const rawLineCountRef = useRef(0); + const clearedThroughRef = useRef(0); + const lastSnapshotRef = useRef(''); const MAX_LOG_LINES = 2000; useEffect(() => { let active = true; - const unlistenPromise = attachLogger((logRecord) => { - if (!active) return; - const level = getLevelStr(logRecord.level); - const message = logRecord.message; - if (message.includes('[download]') && message.includes('%')) return; - setLogs(prev => { - const next = [...prev, { level, message }]; - return next.length > MAX_LOG_LINES ? next.slice(-MAX_LOG_LINES) : next; - }); - }); + const refresh = async () => { + try { + const lines = await invoke('read_logs', { limit: MAX_LOG_LINES }); + if (!active) return; + if (lines.length < clearedThroughRef.current) { + clearedThroughRef.current = 0; + } + const snapshot = `${lines.length}:${lines[lines.length - 1] || ''}`; + if (snapshot === lastSnapshotRef.current) return; + lastSnapshotRef.current = snapshot; + rawLineCountRef.current = lines.length; + setLogs(lines.slice(clearedThroughRef.current).map(message => { + const level = message.includes('[ERROR]') ? 'Error' + : message.includes('[WARN]') ? 'Warn' + : message.includes('[INFO]') ? 'Info' + : message.includes('[TRACE]') ? 'Trace' + : 'Debug'; + return { level, message }; + })); + } catch { + if (active) setLogs([]); + } + }; + void refresh(); + const interval = window.setInterval(refresh, 2000); return () => { active = false; - void unlistenPromise.then(unlisten => unlisten()).catch(() => undefined); + window.clearInterval(interval); }; }, []); @@ -56,19 +62,22 @@ export default function DiagnosticsView() { const handleExport = async () => { try { const path = await save({ - defaultPath: 'Firelink-Diagnostics.log', + defaultPath: 'Firelink-Support-Logs.log', filters: [{ name: 'Log Files', extensions: ['log'] }], }); if (!path) return; await invoke('export_logs', { destPath: path }); - addToast({ message: 'Diagnostics exported', variant: 'success' }); + addToast({ message: 'Support logs exported', variant: 'success' }); } catch (e) { console.error('Export failed:', e); - addToast({ message: `Could not export diagnostics: ${String(e)}`, variant: 'error', isActionable: true }); + addToast({ message: `Could not export logs: ${String(e)}`, variant: 'error', isActionable: true }); } }; - const handleClear = () => setLogs([]); + const handleClear = () => { + clearedThroughRef.current = rawLineCountRef.current; + setLogs([]); + }; const severityClass = (level: string) => { switch (level) { @@ -80,14 +89,14 @@ export default function DiagnosticsView() { }; return ( -
+
{/* Toolbar */} -
+
- Diagnostics Console + Logs ({logs.length} entries)
@@ -110,7 +119,7 @@ export default function DiagnosticsView() { @@ -126,9 +135,9 @@ export default function DiagnosticsView() {
{/* Console */} -
+
{logs.length === 0 && ( -
Waiting for log entries...
+
No persisted log entries are available yet.
)} {logs.filter(entry => levelFilter === 'All' || entry.level === levelFilter).map((entry, i) => (
diff --git a/src/components/SchedulerView.tsx b/src/components/SchedulerView.tsx index dac9b19..304a4fd 100644 --- a/src/components/SchedulerView.tsx +++ b/src/components/SchedulerView.tsx @@ -26,6 +26,11 @@ const postActions: { value: PostQueueAction; label: string; icon: typeof Moon }[ { value: 'shutdown', label: 'Shut down', icon: Power }, ]; +const minuteOfDay = (value: string) => { + const [hour, minute] = value.split(':').map(Number); + return hour * 60 + minute; +}; + function nextScheduledRun(settings: SchedulerSettings): string { if (!settings.enabled) return 'Scheduler is disabled'; @@ -55,6 +60,7 @@ export default function SchedulerView() { const savedSettings = useSettingsStore(state => state.scheduler); const schedulerRunning = useSettingsStore(state => state.schedulerRunning); const setScheduler = useSettingsStore(state => state.setScheduler); + const queues = useDownloadStore(state => state.queues); const [draft, setDraft] = useState(savedSettings); const { addToast } = useToast(); const [permissionMessage, setPermissionMessage] = useState(''); @@ -81,12 +87,41 @@ export default function SchedulerView() { })); }; + const availableQueueIds = new Set(queues.map(queue => queue.id)); + const selectedQueueIds = draft.selectedQueueIds.filter(queueId => availableQueueIds.has(queueId)); + const effectiveSelectedQueueIds = selectedQueueIds.length > 0 + ? selectedQueueIds + : [MAIN_QUEUE_ID]; + + const toggleQueue = (queueId: string) => { + setDraft(current => { + const isSelected = current.selectedQueueIds.includes(queueId); + const availableSelectionCount = current.selectedQueueIds + .filter(id => availableQueueIds.has(id)) + .length; + if (isSelected && availableSelectionCount === 1) return current; + return { + ...current, + selectedQueueIds: isSelected + ? current.selectedQueueIds.filter(id => id !== queueId) + : [...current.selectedQueueIds, queueId] + }; + }); + }; + const save = () => { + if (!draft.everyday && draft.selectedDays.length === 0) { + addToast({ message: 'Select at least one day for the scheduler', variant: 'error', isActionable: true }); + return; + } + if (draft.stopTimeEnabled && minuteOfDay(draft.stopTime) <= minuteOfDay(draft.startTime)) { + addToast({ message: 'Stop time must be later than start time', variant: 'error', isActionable: true }); + return; + } const normalized = { ...draft, - selectedDays: draft.everyday || draft.selectedDays.length > 0 - ? draft.selectedDays - : savedSettings.selectedDays + selectedDays: draft.selectedDays, + selectedQueueIds: effectiveSelectedQueueIds }; setScheduler(normalized); setDraft(normalized); @@ -94,18 +129,27 @@ export default function SchedulerView() { }; const runNow = async () => { - const count = await useDownloadStore.getState().startQueue(MAIN_QUEUE_ID); + const results = await Promise.all( + effectiveSelectedQueueIds.map(queueId => useDownloadStore.getState().startQueue(queueId)) + ); + const count = results.reduce((total, ids) => total + ids.length, 0); + const acceptedIds = results.flat(); if (count > 0) { useSettingsStore.getState().setSchedulerRunning(true); + useSettingsStore.getState().setSchedulerActiveDownloadIds(acceptedIds); addToast({ message: `Started ${count} download${count === 1 ? '' : 's'}`, variant: 'success' }); } else { - addToast({ message: 'No paused or failed downloads to start', variant: 'info' }); + addToast({ message: 'No downloads in the selected queues can be started', variant: 'info' }); } }; const pauseNow = async () => { - const count = await useDownloadStore.getState().pauseQueue(MAIN_QUEUE_ID); + const counts = await Promise.all( + effectiveSelectedQueueIds.map(queueId => useDownloadStore.getState().pauseQueue(queueId)) + ); + const count = counts.reduce((total, queueCount) => total + queueCount, 0); useSettingsStore.getState().setSchedulerRunning(false); + useSettingsStore.getState().setSchedulerActiveDownloadIds([]); addToast({ message: count > 0 ? `Paused ${count} active download${count === 1 ? '' : 's'}` : 'No active downloads', variant: 'info' }); }; @@ -230,6 +274,9 @@ export default function SchedulerView() { updateDraft('stopTime', event.target.value)} disabled={!draft.enabled || !draft.stopTimeEnabled} className="app-control px-3 py-2 text-text-primary disabled:opacity-50" />
+

+ If Firelink is asleep at the start time, it starts the selected queues when it returns later that day, unless the stop time has already passed. +

@@ -294,7 +296,17 @@ export const Sidebar: React.FC = (props) => { {!queues.find(q => q.id === contextMenu.id)?.isMain && (