Conversation
📝 WalkthroughWalkthroughAdds browser cookie extraction and persistence, Cloudflare challenge detection and retry, server filename adoption during HTTP downloads, Tauri commands and frontend UI for importing cookies and capturing User-Agent, plus tests, locales, and workspace/config updates. ChangesBrowser Cookie Import and Cloudflare Recovery
Sequence Diagram(s)sequenceDiagram
participant Frontend
participant TauriCmds
participant RisukoCookies
participant CookieStore
participant TaskManager
participant HTTP_Engine
participant RemoteServer
Frontend->>TauriCmds: import_browser_cookies(browser,url,persist?)
TauriCmds->>RisukoCookies: cookies_for_url(browser,url)
RisukoCookies-->>TauriCmds: HostCookies (cookies + user_agent)
TauriCmds->>CookieStore: upsert(host, cookies) [optional persist]
Frontend->>TaskManager: retry_with_cookies(gid, cookie, userAgent)
TaskManager->>HTTP_Engine: run_http_download_multi(..., adopted_filename_slot)
HTTP_Engine->>RemoteServer: probe_range_support / GET
RemoteServer-->>HTTP_Engine: Content-Disposition or Cloudflare challenge
HTTP_Engine-->>TaskManager: publish adopted filename or CLOUDFLARE_MARKER error
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
4 issues found
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
src-tauri/risuko-engine/src/engine/manager.rs (1)
1241-1249:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRemove the stored entry that actually matched the URL.
find_for_url()can return a parent-domain entry likeexample.comfor a challenged request ondl.example.com, but this path removes only the exact challenged host. That leaves the stale parent entry in place, so the next retry can auto-apply the same broken cookies again.🛠️ Proposed fix
if code == super::error_code::ErrorCode::CLOUDFLARE_CHALLENGE { if let Some(host) = parse_cf_host(&e) { - if let Err(err) = cookie_store.remove(&host) { + let matched_host = cookie_store + .find_for_url(&format!("https://{host}/")) + .map(|entry| entry.host) + .unwrap_or(host); + if let Err(err) = cookie_store.remove(&matched_host) { log::warn!( - "[task:{gid_clone}] cookie store remove({host}) failed: {err}" + "[task:{gid_clone}] cookie store remove({matched_host}) failed: {err}" ); } } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src-tauri/risuko-engine/src/engine/manager.rs` around lines 1241 - 1249, The code only removes the exact challenged host returned by parse_cf_host, but cookie_store.find_for_url may have matched a parent-domain cookie (e.g., example.com for dl.example.com), leaving stale cookies behind; update the handler (the block using parse_cf_host and cookie_store.remove) to call cookie_store.find_for_url for the challenged URL/host, iterate the returned CookieEntries and remove each matching entry (using cookie_store.remove or the appropriate remove-by-cookie-id API) so any parent-domain or path cookies that matched the request are deleted rather than only the exact host entry.src-tauri/risuko-engine/src/engine/http.rs (1)
905-915:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon't couple range probing to filename adoption.
When
filename_was_url_derivedis false,probe_for_namestaysNone, so thesplit > 1path always falls back to single-connection mode. Any task with a user-chosenoutvalue loses parallel HTTP downloads entirely.🛠️ Proposed fix
- let probe_for_name: Option<ProbeResult> = if is_http && filename_was_url_derived { + let probe_for_name: Option<ProbeResult> = if is_http && (filename_was_url_derived || split > 1) { match probe_range_support(&range_client, uri, &headers).await { Ok(p) => Some(p), Err(e) => { tracing::warn!("Range probe failed, falling back to single: {e}"); NoneAlso applies to: 939-999
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src-tauri/risuko-engine/src/engine/http.rs` around lines 905 - 915, The logic currently skips calling probe_range_support when filename_was_url_derived is false, which prevents parallel/split downloads for user-specified outputs; change the code so probe_range_support(&range_client, uri, &headers).await is invoked whenever is_http is true (independent of filename_was_url_derived) and store its Result in probe_for_name (or a separate probe_result variable), then use filename_was_url_derived only when deciding whether to adopt the probed filename but not when deciding whether to enable split (>1) download logic; update references to probe_for_name, probe_range_support, filename_was_url_derived, and the split-handling branch (the code around the split > 1 logic) accordingly so range capability is detected for all HTTP downloads.pnpm-workspace.yaml (1)
3-13:⚠️ Potential issue | 🟠 Major | ⚡ Quick winpnpm-workspace.yaml: migrate build allowlist from
onlyBuiltDependenciestoallowBuildsWith
packageManager: pnpm@11.3.0, pnpm 11 removedonlyBuiltDependencies; packages not listed inallowBuildswon’t be approved for build scripts—so@swc/core,core-js, andunrs-resolverare currently not approved inpnpm-workspace.yaml(lines 3-13). (pnpm.io)🛠️ Proposed fix
allowBuilds: '`@parcel/watcher`': true + '`@swc/core`': true + core-js: true esbuild: true + unrs-resolver: true vue-demi: true -onlyBuiltDependencies: - - '`@parcel/watcher`' - - '`@swc/core`' - - core-js - - esbuild - - unrs-resolver - - vue-demi🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pnpm-workspace.yaml` around lines 3 - 13, The workspace currently lists build-approved packages under onlyBuiltDependencies but pnpm@11 uses allowBuilds; update pnpm-workspace.yaml by adding the missing packages '`@swc/core`', core-js, and unrs-resolver to the allowBuilds map (alongside '`@parcel/watcher`', esbuild, and vue-demi) and remove or stop relying on onlyBuiltDependencies so the build allowlist (allowBuilds) contains all entries previously in onlyBuiltDependencies.src-tauri/src/commands/engine_cmds.rs (1)
1393-1407:⚠️ Potential issue | 🟠 Major | ⚡ Quick winTest assertions don't match the new
placeholder_download_namebehavior.The test expects
"download"for extensionless URLs, but the updatedinfer_out_from_uri_innernow returns"download-{8-char-hex}"viaplaceholder_download_name. These assertions will fail.🔧 Proposed fix
#[test] fn infer_out_no_extension_falls_back_to_download() { - // Opaque URLs with no extension hint get a generic placeholder - // so the task carries a stable display name; Content-Disposition - // takes over once the engine sees the first response - assert_eq!( - infer_out_from_uri_inner("http://example.com/path/noext"), - "download" - ); - assert_eq!( - infer_out_from_uri_inner( - "https://www.spigotmc.org/resources/storagepeek.134712/download?version=638562" - ), - "download" - ); + // Opaque URLs with no extension hint get a hash-suffixed placeholder + // so distinct extensionless URLs don't collide on the same .part file + let result1 = infer_out_from_uri_inner("http://example.com/path/noext"); + assert!(result1.starts_with("download-"), "expected download-{{hex}}, got {result1}"); + assert_eq!(result1.len(), "download-".len() + 8); // 4 bytes = 8 hex chars + + let result2 = infer_out_from_uri_inner( + "https://www.spigotmc.org/resources/storagepeek.134712/download?version=638562" + ); + assert!(result2.starts_with("download-"), "expected download-{{hex}}, got {result2}"); + // Different URLs should yield different hashes + assert_ne!(result1, result2); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src-tauri/src/commands/engine_cmds.rs` around lines 1393 - 1407, The test infer_out_no_extension_falls_back_to_download asserts a literal "download" but infer_out_from_uri_inner now returns placeholder_download_name like "download-{8-char-hex}"; update the assertions in infer_out_no_extension_falls_back_to_download to accept the new format by checking that the returned string starts with "download-" and that the suffix matches the 8-hex-character pattern (or otherwise call/compare against placeholder_download_name behavior) so the test validates the new placeholder_download_name output from infer_out_from_uri_inner.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@pnpm-workspace.yaml`:
- Around line 3-13: The workspace currently lists build-approved packages under
onlyBuiltDependencies but pnpm@11 uses allowBuilds; update pnpm-workspace.yaml
by adding the missing packages '`@swc/core`', core-js, and unrs-resolver to the
allowBuilds map (alongside '`@parcel/watcher`', esbuild, and vue-demi) and remove
or stop relying on onlyBuiltDependencies so the build allowlist (allowBuilds)
contains all entries previously in onlyBuiltDependencies.
In `@src-tauri/risuko-engine/src/engine/http.rs`:
- Around line 905-915: The logic currently skips calling probe_range_support
when filename_was_url_derived is false, which prevents parallel/split downloads
for user-specified outputs; change the code so
probe_range_support(&range_client, uri, &headers).await is invoked whenever
is_http is true (independent of filename_was_url_derived) and store its Result
in probe_for_name (or a separate probe_result variable), then use
filename_was_url_derived only when deciding whether to adopt the probed filename
but not when deciding whether to enable split (>1) download logic; update
references to probe_for_name, probe_range_support, filename_was_url_derived, and
the split-handling branch (the code around the split > 1 logic) accordingly so
range capability is detected for all HTTP downloads.
In `@src-tauri/risuko-engine/src/engine/manager.rs`:
- Around line 1241-1249: The code only removes the exact challenged host
returned by parse_cf_host, but cookie_store.find_for_url may have matched a
parent-domain cookie (e.g., example.com for dl.example.com), leaving stale
cookies behind; update the handler (the block using parse_cf_host and
cookie_store.remove) to call cookie_store.find_for_url for the challenged
URL/host, iterate the returned CookieEntries and remove each matching entry
(using cookie_store.remove or the appropriate remove-by-cookie-id API) so any
parent-domain or path cookies that matched the request are deleted rather than
only the exact host entry.
In `@src-tauri/src/commands/engine_cmds.rs`:
- Around line 1393-1407: The test infer_out_no_extension_falls_back_to_download
asserts a literal "download" but infer_out_from_uri_inner now returns
placeholder_download_name like "download-{8-char-hex}"; update the assertions in
infer_out_no_extension_falls_back_to_download to accept the new format by
checking that the returned string starts with "download-" and that the suffix
matches the 8-hex-character pattern (or otherwise call/compare against
placeholder_download_name behavior) so the test validates the new
placeholder_download_name output from infer_out_from_uri_inner.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4b55f14d-ccdc-44c6-b1fd-226c51c29d0b
📒 Files selected for processing (15)
package.jsonpnpm-workspace.yamlsrc-tauri/risuko-cookies/src/lib.rssrc-tauri/risuko-engine/src/engine/cookie_store.rssrc-tauri/risuko-engine/src/engine/http.rssrc-tauri/risuko-engine/src/engine/manager.rssrc-tauri/risuko-engine/src/engine/mod.rssrc-tauri/src/commands/cookie_cmds.rssrc-tauri/src/commands/engine_cmds.rssrc/renderer/api/Api.tssrc/renderer/components/Native/EngineClient.vuesrc/renderer/components/Task/AddTask.vuesrc/renderer/components/Task/BrowserCookiePicker.vuesrc/renderer/components/Task/CloudflareDialog.vuesrc/renderer/store/app.ts
There was a problem hiding this comment.
1 issue found across 15 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src-tauri/risuko-engine/src/engine/cookie_store.rs (1)
265-271:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftDon't serialize every stored cookie into every request.
cookies_to_headerignoressecure,path, andexpires, so imported browser cookies that should be withheld can be replayed on unrelated requests, including plainhttp://downloads. That can leak session cookies and produce invalid request state. Make this helper request-aware and filter cookies before building the header.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src-tauri/risuko-engine/src/engine/cookie_store.rs` around lines 265 - 271, The current cookies_to_header(cookies: &[StoredCookie]) serializes every stored cookie without regard to secure, path, expires (and domain), causing leakage; change it into a request-aware filter: update the signature (e.g., cookies_to_header(cookies: &[StoredCookie], request_url: &Url)) and before formatting only include cookies that are not expired (check expires), whose secure flag matches the request scheme (if cookie.secure then request_url.scheme() == "https"), whose path matches the request path (cookie.path is a prefix or exact match of request_url.path()), and whose domain matches the request host per cookie domain-matching rules; then join those filtered cookies into the header. Ensure you reference and use StoredCookie fields (secure, path, expires, domain) and adjust call sites to pass the request URL.src-tauri/risuko-engine/src/engine/http.rs (1)
905-915:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRestore the probe guard for non-adoptable single-stream downloads.
This now sends a preflight GET for every HTTP download, even when
split == 1and the output name is already user-chosen. That extra request is pure overhead and can burn one-shot/signed URLs before the real transfer starts. Only probe when multi-chunk needs range metadata or whenfilename_was_url_derivedcan actually adoptContent-Disposition.Suggested fix
- let probe_for_name: Option<ProbeResult> = if is_http { + let should_probe = is_http && (split > 1 || filename_was_url_derived); + let probe_for_name: Option<ProbeResult> = if should_probe { match probe_range_support(&range_client, uri, &headers).await { Ok(p) => Some(p), Err(e) => { tracing::warn!("Range probe failed, falling back to single: {e}"); None🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src-tauri/risuko-engine/src/engine/http.rs` around lines 905 - 915, The current code always calls probe_range_support for any HTTP download; change the probe guard so probe_range_support(&range_client, uri, &headers).await is only invoked when is_http is true AND either split > 1 (multi-chunk download needs range metadata) OR filename_was_url_derived is true (output name can be adopted from Content-Disposition); otherwise set probe_for_name = None to avoid the extra preflight on non-adoptable single-stream downloads. Use the existing symbols probe_for_name, is_http, probe_range_support, split, and filename_was_url_derived and keep the same error handling (tracing::warn on Err) when you do call the probe.
♻️ Duplicate comments (1)
src-tauri/risuko-engine/src/engine/http.rs (1)
2302-2308:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winKeep the placeholder when the adopted final name already exists.
Allowing adoption when
dir_path/<candidate>already exists turns the collision-freedownload-<hash>fallback back into a clobber path oncefinalize_downloadruns. The new test at Lines 2669-2685 locks in that data-loss case instead of preserving the unique placeholder name.Suggested fix
fn adopt_suggested_filename( suggested: &str, current_filename: &str, current_part_path: &Path, dir_path: &Path, ) -> Option<(String, std::path::PathBuf)> { let candidate = sanitize_filename(suggested); if candidate.is_empty() || candidate == current_filename { return None; } // Refuse to rename a download that already has bytes on disk if current_part_path.exists() && fs::metadata(current_part_path) .map(|m| m.len() > 0) .unwrap_or(false) { return None; } + let final_path = dir_path.join(candidate.strip_suffix(PART_SUFFIX).unwrap_or(&candidate)); + if final_path.exists() { + return None; + } let new_part = if candidate.ends_with(PART_SUFFIX) { dir_path.join(&candidate) } else { dir_path.join(format!("{candidate}{PART_SUFFIX}")) };Also applies to: 2328-2344
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src-tauri/risuko-engine/src/engine/http.rs` around lines 2302 - 2308, The adoption code currently renames the placeholder to the candidate final name even if that final path already exists; change the logic in the adoption path (the code that calls/implements finalize_download and the adoption branch) to first check whether dir_path/<candidate> already exists and, if it does, do not perform the rename/adoption — return or error so the placeholder (download-<hash>.part) is kept intact; ensure finalize_download continues to overwrite only when it is explicitly called on a legitimate finalized file, and update the branch that handles adoption collisions to preserve the unique placeholder name instead of clobbering the existing file.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src-tauri/risuko-engine/src/engine/manager.rs`:
- Around line 1245-1255: The code currently removes cookies by looking up
task.uris.first(), which ignores the Cloudflare challenge marker; instead, call
parse_cf_host(&e) (the error/marker variable) and, if it returns Some(host), use
that host to find/remove the cookie entry (i.e., pass the host to
cookie_store.find_for_url/remove); only if parse_cf_host(&e) returns None fall
back to task.uris.first().map(|u| u.as_str()).unwrap_or(""). Ensure you use the
parsed host for both find_for_url and cookie_store.remove so the challenged host
is evicted, not necessarily task.uris[0].
---
Outside diff comments:
In `@src-tauri/risuko-engine/src/engine/cookie_store.rs`:
- Around line 265-271: The current cookies_to_header(cookies: &[StoredCookie])
serializes every stored cookie without regard to secure, path, expires (and
domain), causing leakage; change it into a request-aware filter: update the
signature (e.g., cookies_to_header(cookies: &[StoredCookie], request_url: &Url))
and before formatting only include cookies that are not expired (check expires),
whose secure flag matches the request scheme (if cookie.secure then
request_url.scheme() == "https"), whose path matches the request path
(cookie.path is a prefix or exact match of request_url.path()), and whose domain
matches the request host per cookie domain-matching rules; then join those
filtered cookies into the header. Ensure you reference and use StoredCookie
fields (secure, path, expires, domain) and adjust call sites to pass the request
URL.
In `@src-tauri/risuko-engine/src/engine/http.rs`:
- Around line 905-915: The current code always calls probe_range_support for any
HTTP download; change the probe guard so probe_range_support(&range_client, uri,
&headers).await is only invoked when is_http is true AND either split > 1
(multi-chunk download needs range metadata) OR filename_was_url_derived is true
(output name can be adopted from Content-Disposition); otherwise set
probe_for_name = None to avoid the extra preflight on non-adoptable
single-stream downloads. Use the existing symbols probe_for_name, is_http,
probe_range_support, split, and filename_was_url_derived and keep the same error
handling (tracing::warn on Err) when you do call the probe.
---
Duplicate comments:
In `@src-tauri/risuko-engine/src/engine/http.rs`:
- Around line 2302-2308: The adoption code currently renames the placeholder to
the candidate final name even if that final path already exists; change the
logic in the adoption path (the code that calls/implements finalize_download and
the adoption branch) to first check whether dir_path/<candidate> already exists
and, if it does, do not perform the rename/adoption — return or error so the
placeholder (download-<hash>.part) is kept intact; ensure finalize_download
continues to overwrite only when it is explicitly called on a legitimate
finalized file, and update the branch that handles adoption collisions to
preserve the unique placeholder name instead of clobbering the existing file.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: a770a111-55af-4886-91ac-2929906544af
📒 Files selected for processing (6)
pnpm-workspace.yamlsrc-tauri/risuko-engine/src/engine/cookie_store.rssrc-tauri/risuko-engine/src/engine/http.rssrc-tauri/risuko-engine/src/engine/manager.rssrc-tauri/src/commands/engine_cmds.rssrc/renderer/components/Task/CloudflareDialog.vue
💤 Files with no reviewable changes (1)
- src/renderer/components/Task/CloudflareDialog.vue
There was a problem hiding this comment.
3 issues found across 6 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src-tauri/risuko-engine/src/engine/cookie_store.rs`:
- Around line 233-244: In write_to_disk, don't unconditionally remove path on
any rename error; instead capture the first rename error (the variable 'e'
returned from std::fs::rename(&tmp, path)), check e.kind() against
std::io::ErrorKind::AlreadyExists (or equivalent overwrite/collision kinds), and
only call std::fs::remove_file(path) and retry std::fs::rename(&tmp, path) when
it's AlreadyExists; for any other e.kind() return the original formatted rename
error (keeping tmp untouched) so the existing cookie store isn't deleted on
transient/permission errors. Ensure you reference the same tmp and path
variables and use std::io::ErrorKind in the check.
In `@src-tauri/risuko-engine/src/engine/http.rs`:
- Line 2370: The function is_placeholder_download_name currently hardcodes a
maximum hash length with rest.len() <= 32 which breaks the "download-<hash>"
contract for longer hex digests; update the predicate to remove the hardcoded
length cap and instead validate only that rest is non-empty and all characters
are ASCII hex digits (e.g., replace the condition `!rest.is_empty() &&
rest.len() <= 32 && rest.chars().all(|c| c.is_ascii_hexdigit())` with
`!rest.is_empty() && rest.chars().all(|c| c.is_ascii_hexdigit())`) so the
function accepts any-length hex digests while still ensuring the suffix is a hex
string.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 02a03611-d3bd-4f5f-a94d-f68b1ac01f22
📒 Files selected for processing (8)
src-tauri/risuko-cookies/src/lib.rssrc-tauri/risuko-engine/src/engine/cookie_store.rssrc-tauri/risuko-engine/src/engine/http.rssrc-tauri/risuko-engine/src/engine/manager.rssrc-tauri/risuko-engine/src/engine/mod.rssrc-tauri/src/commands/cookie_cmds.rssrc-tauri/src/commands/engine_cmds.rssrc/renderer/components/Task/BrowserCookiePicker.vue
Summary by cubic
Adds browser cookie import and a guided Cloudflare retry flow to unblock protected downloads. Detects Cloudflare challenges (315) and reuses imported cookies + User-Agent per domain, with mid-download filename adoption from Content-Disposition.
New Features
Dependencies
risuko-cookiescrate (wrapsrookie) for browser cookie extraction and integrate with the engine.pnpm@11.3.0and switch workspace config toallowBuildsfor@parcel/watcher,@swc/core,core-js,esbuild,unrs-resolver, andvue-demi.tokiofeatureio-utilfor new command helpers.Written for commit aaa3f2d. Summary will update on new commits. Review in cubic
Summary by CodeRabbit
New Features
Bug Fixes