Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds SHA‑256 release sidecars and verification; bumps workspace/packages to 0.4.0; hardens build/publish tooling; refactors engine/network, CLI/Tauri wiring, renderer stores/components, shared types/utils, and updates locale translations. ChangesRelease and packaging
CLI & installer
Engine and network
BitTorrent, UTP, and related core
CLI/Tauri, renderer, stores, and shared
Estimated code review effort Possibly related PRs
✨ Finishing Touches📝 Generate docstrings
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
7 issues found across 138 files
Note: This PR contains a large number of files. cubic only reviews up to 100 files per PR, so some files may not have been reviewed. cubic prioritizes the most important files to review.
On a pro plan you can use ultrareview for larger PRs.
Re-trigger cubic
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
src-tauri/risuko-engine/src/engine/ftp/ftp_download.rs (1)
87-98:⚠️ Potential issue | 🟠 Major | ⚡ Quick winOnly short-circuit when the server confirmed the remote length.
This now treats the caller-populated
totalatomic as proof that the.partis complete. WhenSIZEis unsupported, a stalefile_sizecan make an incomplete.partget renamed as the finished download with no socket read at all.Suggested fix
- if existing_size > 0 && effective_size > 0 && existing_size == effective_size { + if remote_size > 0 && existing_size > 0 && existing_size == remote_size { $completed.store(existing_size, Ordering::Relaxed); tracing::info!("FTP .part already complete ({existing_size} bytes), skipping transfer"); let _ = $ftp.quit().await; } else {🤖 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/ftp/ftp_download.rs` around lines 87 - 98, The short-circuit currently treats caller-provided $file_size as proof the remote file is complete, which can rename a stale .part when the FTP SIZE command is unsupported; change the condition in the block using effective_size so it only short-circuits when remote_size (the SIZE reply) is > 0 and existing_size == remote_size, and store remote_size into $completed before calling $ftp.quit().await (i.e. require the server-confirmed remote_size rather than the fallback $file_size when deciding the .part is complete).src-tauri/risuko-engine/src/engine/upload/s3.rs (1)
296-299:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRe-check cancellation before
CompleteMultipartUpload.Once the part futures finish, this always commits the multipart upload. A cancel that lands in that gap still publishes the object and returns success, which breaks cancellation semantics for a remote write.
Suggested fix
// -- Complete -- + if ctl.cancel.is_cancelled() { + return Err("cancelled".into()); + } self.complete_multipart(url, upload_id, &parts).await?; ctl.report(total, total); Ok(url.to_string())🤖 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/upload/s3.rs` around lines 296 - 299, After all part futures have completed but before calling self.complete_multipart(...), re-check the cancellation controller (ctl) and if cancellation has been requested, do NOT call complete_multipart; instead invoke the multipart abort routine (e.g. self.abort_multipart or self.abort_multipart_upload with the same upload_id) and return an error/result that indicates the upload was cancelled. Locate the post-join block that currently calls self.complete_multipart(url, upload_id, &parts).await and insert the cancellation check there (using ctl's cancellation/query method) to abort and short-circuit instead of committing the upload.src-tauri/risuko-engine/src/engine/upload/webdav.rs (1)
213-215: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winUpdate stale comment.
The comment states "when no streaming progress is wired through hyper" but streaming progress is now wired via
file_stream_body_with_progress(line 182). The finalctl.reportcall on line 215 still serves as a fix-up for edge cases, but the comment no longer accurately describes the current behavior.📝 Proposed comment update
- // Best-effort progress fix-up — when no streaming progress is wired - // through hyper, at least mark the job complete on success + // Final progress fix-up to ensure the job is marked complete ctl.report(file.size, file.size);🤖 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/upload/webdav.rs` around lines 213 - 215, The existing comment above ctl.report incorrectly states progress isn't streamed via hyper; update it to reflect that streaming progress is now provided by file_stream_body_with_progress (used earlier), and make the comment explain that the final ctl.report(file.size, file.size) is a best-effort fallback to mark the job complete for rare edge cases (e.g., when progress updates were lost or not emitted) rather than the primary progress path; locate this near file_stream_body_with_progress and ctl.report to replace the stale wording.src-tauri/risuko-engine/src/engine/m3u8/segment.rs (1)
67-68:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winClose the progress file before deleting it.
cleanup()removesprogress_pathwhileappend_fileis still open. That works on Unix, but Windows keeps the file locked, so successful M3U8 downloads leak.m3u8.progressand often the temp directory too. Makecleanuptake&mut selfandtake()the handle first.Suggested fix
- pub fn cleanup(&self) { - let _ = std::fs::remove_file(&self.progress_path); + pub fn cleanup(&mut self) { + self.append_file.take(); + let _ = std::fs::remove_file(&self.progress_path); }🤖 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/m3u8/segment.rs` around lines 67 - 68, The cleanup method currently removes progress_path while the file handle (append_file) may still be open causing Windows file-lock leaks; change the signature of cleanup to take &mut self, call take() on the file handle field (e.g., self.append_file.take() or otherwise extract/replace it) to drop/close the File before attempting to remove_file(&self.progress_path), then call std::fs::remove_file; ensure any Option<File> field is updated accordingly and handle errors as before.src-tauri/risuko-engine/src/engine/m3u8/download.rs (1)
412-456:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove the reserved
.mp4when remux never starts.
reserve_unique_output_path()creates the destination file before the ffmpeg preflight. Ifffmpeg -versionfails, or the later ffmpeg process fails to spawn, this returns early and leaves a zero-byte.mp4beside the real.tsfallback. Probe first, or deletemp4_pathon every early-return path.Suggested fix
- let mp4_path = reserve_unique_output_path(parent, &mp4_name).await?; - // Check ffmpeg availability let ffmpeg_check = tokio::process::Command::new("ffmpeg") .arg("-version") .output() .await; if ffmpeg_check.is_err() { return Err("ffmpeg not found on system PATH".to_string()); } + + let mp4_path = reserve_unique_output_path(parent, &mp4_name).await?; let output = tokio::process::Command::new("ffmpeg") .arg("-i") .arg(ts_path) .arg("-c") @@ .arg("-y") .arg(&mp4_path) .output() .await - .map_err(|e| format!("ffmpeg execution failed: {e}"))?; + .map_err(|e| { + let _ = std::fs::remove_file(&mp4_path); + format!("ffmpeg execution failed: {e}") + })?;🤖 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/m3u8/download.rs` around lines 412 - 456, The reserved .mp4 created by reserve_unique_output_path is left behind on early returns; either run the ffmpeg preflight check (ffmpeg_check via tokio::process::Command::new("ffmpeg").arg("-version")) before calling reserve_unique_output_path, or ensure any early-return paths delete the reserved mp4 (use tokio::fs::remove_file(&mp4_path).await) when ffmpeg_check.is_err() or when the ffmpeg spawn/map_err early-fails. Update the logic around reserve_unique_output_path, ffmpeg_check, and the ffmpeg Command invocation so the temporary mp4 is cleaned up on all error paths.src-tauri/risuko-engine/src/engine/rss/mod.rs (1)
170-173:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftFailed feeds now bypass their own interval.
Line 269 computes due-ness from
last_fetched_at, but Lines 172-173 only advance that field on success. After one failed fetch, a feed with a long interval stays permanently due and will be retried every global wake cycle until it is disabled.Please track the last poll attempt separately from the last successful fetch, or advance the scheduling timestamp on failures too.
Also applies to: 200-212, 247-280
🤖 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/rss/mod.rs` around lines 170 - 173, The current logic only updates feed.last_fetched_at on success (in the match for result), causing failed feeds to remain "due" forever; add a separate timestamp field (e.g., feed.last_polled_at or feed.last_attempted_at) to the Feed struct and use that field when computing due-ness instead of last_fetched_at, then set that new timestamp to now_secs() on every fetch attempt (both in the Ok(parsed) branch and the Err branch) while keeping last_fetched_at updated only on success; also ensure error_count is incremented on failure and persisted alongside the new timestamp wherever feeds are saved/serialized.src-tauri/risuko-engine/src/engine/ed2k/download.rs (1)
467-480:⚠️ Potential issue | 🟡 MinorUse the peer’s
partsbitmap when selecting requested chunk ranges
PeerEvent::FileStatus { parts }is received, butcollect_needed_rangesalways callscm.next_needed_chunk_excluding(&[], &picked). SinceChunkManager::next_needed_chunk_excludingonly filters bypeer_partswhen the slice is non-empty, the requested(start, end)ranges can include chunks the peer doesn’t have (wasted requests / slower progress). Storepartsinrun_peer_downloadand pass it intocollect_needed_ranges(or otherwise callnext_needed_chunk(&parts)/next_needed_chunk_excluding(&parts, ...)) so range selection matches the peer’s availability.🤖 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/ed2k/download.rs` around lines 467 - 480, collect_needed_ranges currently ignores the peer's availability bitmap and calls ChunkManager::next_needed_chunk_excluding(&[], &picked), causing requested ranges to include chunks the peer doesn't have; modify run_peer_download to capture PeerEvent::FileStatus { parts } and pass that parts slice into collect_needed_ranges (or change calls inside collect_needed_ranges to use next_needed_chunk(&parts) / next_needed_chunk_excluding(&parts, &picked)) so range selection uses the peer's parts bitmap when choosing (start,end) ranges; update function signatures as needed to thread the parts slice from run_peer_download into collect_needed_ranges and ensure picked logic remains unchanged.
🤖 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 @.github/workflows/release.yml:
- Line 293: Replace the mutable tag reference softprops/action-gh-release@v2
with a pinned commit SHA to eliminate supply-chain risk: find occurrences of
softprops/action-gh-release@v2 in the workflow and update them to
softprops/action-gh-release@<commit-sha> using the exact commit hash for the v2
release (obtainable from the action-gh-release releases/tags page); ensure both
occurrences are updated and commit the change.
In `@packages/risuko-app/package.json`:
- Line 5: The package.json currently lists "main": "index.js" but
packages/risuko-app/index.js is only a comment stub and the real executable is
declared in "bin": { "risuko-app": "bin.js" }; either remove or update the
"main" field to reflect there is no programmatic export (delete "main" or set it
to a real entry), or implement and export the module API inside
packages/risuko-app/index.js (add module.exports / export functions) so "main"
points to a valid programmatic entry; ensure consistency between package.json
("main" and "bin") and the actual files (index.js, bin.js).
In `@src-tauri/risuko-bt/src/dht.rs`:
- Around line 86-98: Add a regression test that verifies the Drop-based cleanup
of KRPC transactions by creating a pending lookup, forcing/aborting it via the
lookup budget (or cancelling the future returned by query_get_peers), and then
asserting that the PendingMap (accessed via the same Arc<Mutex<PendingMap>> used
by PendingGuard) no longer contains the txn after the task is aborted; use the
same mechanisms exercised by PendingGuard (the pending field and txn u16) to
locate the pending entry, spawn the lookup so it becomes pending, cancel it,
await drop completion, and finally lock pending and assert it is empty to lock
in the behavior implemented by impl Drop for PendingGuard and the
query_get_peers path.
In `@src-tauri/risuko-bt/src/utp/socket.rs`:
- Around line 160-165: The Drop impl for UtpSocket currently only aborts
router_handle but doesn't clear the connection registry, leaving cloned
per-connection senders alive; update the UtpSocket::drop implementation to also
clear the registry the same way shutdown() does (e.g., take() or clear the
registry Arc/inner collection) so that all connection senders are dropped and
cfg.incoming.recv() can observe closure; locate the Drop impl for UtpSocket and
the shutdown() method to mirror the registry clearing logic when dropping the
socket.
In `@src-tauri/risuko-cli/src/rpc_client.rs`:
- Around line 21-31: The connect_host mapping and IPv6 detection are wrong:
change the match on host to map "::" and "[::]" to the IPv6 loopback "::1" (keep
""/"0.0.0.0" mapped to "127.0.0.1"), then determine whether to bracket the host
for url by robustly detecting IPv6 via std::net::IpAddr::from_str on the host
with surrounding brackets trimmed (use connect_host.trim_matches(|c|
c=='['||c==']') or parse host first), and build url using the cleaned host and
port so IPv6 addresses are bracketed correctly; update the logic around the
connect_host, host, and url variables accordingly.
In `@src-tauri/risuko-engine/src/engine/ftp/ftp_download.rs`:
- Around line 267-283: The FTPS client config is built directly with
rustls::ClientConfig::builder() which can panic unless a process-level
CryptoProvider is installed first; before calling
rustls::ClientConfig::builder() in functions constructing FTPS configs (e.g., in
ftp_download.rs and the analogous upload/ftp.rs), ensure the default crypto
provider is installed by invoking the same bootstrap used elsewhere (call the
helper that performs rustls::crypto::ring::default_provider().install_default()
— or reuse the existing build_tls()/bootstrap routine from
src-tauri/risuko-http/src/client.rs) so the provider is installed exactly once
before constructing the ClientConfig (retain the current verify_cert conditional
and AcceptAnyCert usage, only add the install step beforehand).
In `@src-tauri/risuko-engine/src/engine/ftp/sftp_download.rs`:
- Around line 291-366: The download loop may treat an early/short EOF as
success; after the main loop but before finalizing (before the
sftp.close(handle) completion and before returning success), check that
bytes_downloaded == file_size (or bytes_downloaded >= file_size if you prefer
tolerant) and if it is less, close the SFTP handle and return an error (e.g.,
"Incomplete download" or similar) so truncated .part files are not treated as
successful; update the code paths around the while !reads.is_empty() loop and
the final sftp.close(handle) call (referencing variables/functions: saw_eof,
bytes_downloaded, file_size, reads, next_sftp_read_len, read_sftp_range, and
sftp.close(handle)) to perform this guard and ensure any outstanding reads are
cleaned up before returning the error.
In `@src-tauri/risuko-engine/src/engine/m3u8/download.rs`:
- Around line 301-314: The temp_dir_name_for() currently uses a fresh
nonce/counter each call which prevents resumable downloads in
segment::download_segments(); change temp_dir_name_for to produce a
deterministic base key derived from the task identity (e.g. sanitized filename
and optionally the process id) instead of a time-based nonce, then only append a
collision suffix (using TEMP_DIR_COUNTER or similar) when that deterministic
directory already exists to handle truly concurrent jobs; ensure you keep
sanitize_filename(filename), TEMP_DIR_COUNTER, and the returned format logic but
first check for an existing directory with the base name and only add the
counter if needed so retries/restarts reuse the same temp dir.
In `@src-tauri/risuko-engine/src/engine/upload/ftp.rs`:
- Around line 196-201: Currently the code unconditionally calls $ftp.rm($remote)
before attempting $ftp.rename(part_remote, $remote), which deletes the good file
if the rename fails; change the logic to attempt $ftp.rename(part_remote,
$remote) first and only if that call fails with a “target exists”/collision
error call $ftp.rm($remote) and retry the rename; for any other rename error
return it (preserving the existing map_err formatting), referencing the rename
call on part_remote -> $remote and the rm call on $remote when implementing the
fallback.
In `@src-tauri/risuko-engine/src/engine/upload/manager.rs`:
- Around line 704-734: In prune_terminal_jobs, avoid filtering jobs twice by
collecting terminal jobs once into a Vec of (created_at, id) (e.g., reuse the
existing local terminal variable) instead of running .filter() two times;
compute terminal_count from terminal.len(), check if terminal.len() <=
max_terminal and only proceed to sort and remove the excess entries, then remove
IDs from jobs by iterating terminal.into_iter().take(to_remove). This keeps the
same behavior while eliminating the redundant first .filter() pass over jobs.
In `@src/renderer/components/Rss/ItemList.vue`:
- Line 467: The current flow sanitizes raw HTML with sanitizeHtml before
injectBaseTag, which causes sanitizeHtml to drop relative href/src values;
change the order or adjust sanitization so relative URLs are preserved and
resolved: either call injectBaseTag(raw, baseUrl, isDark) first and then run
sanitizeHtml on the resulting HTML (update the place where viewerContent is set
around injectBaseTag/sanitizeHtml), or modify sanitizeHtml to accept a baseUrl
and resolve/allow relative href/src values against baseUrl during sanitization;
target the symbols viewerContent, raw, baseUrl, isDark, injectBaseTag and
sanitizeHtml when making the change.
In `@src/renderer/store/task.ts`:
- Around line 65-71: Replace the while-loop LRU eviction in the update that
manages speedHistoryCache with a single conditional: when speedHistoryCache.size
> SPEED_HISTORY_GID_LIMIT, obtain the oldest key via
speedHistoryCache.keys().next().value and call speedHistoryCache.delete(oldest);
remove the unnecessary undefined check and loop since only one gid is inserted
per call and the map must be non-empty when size exceeds the limit.
- Around line 209-216: The totalProgressPercent method currently uses
calcProgress(this.totalLength, this.totalCompletedLength, 1) and strips a
trailing ".0" via string replace, causing inconsistent outputs (e.g., "50" vs
"33.3"); update totalProgressPercent (in src/renderer/store/task.ts) to a
consistent format — either return calcProgress(..., 1).toFixed(1) to always show
one decimal (e.g., "50.0", "33.3") or call calcProgress(..., 0) and format as an
integer (e.g., "50", "33") — pick one and replace the current replace(/\.0$/)
logic accordingly in the totalProgressPercent method.
In `@src/shared/utils/tray.ts`:
- Around line 10-13: The computation of index i in the tray size logic uses
Math.trunc(Math.floor(Math.log(b) / Math.log(1024))) which is redundant; replace
the double wrapper with a single Math.floor call (i.e. use
Math.floor(Math.log(b) / Math.log(1024))) when computing i in the same
expression that also bounds it with sizes.length - 1 to preserve behavior in the
function that defines i.
---
Outside diff comments:
In `@src-tauri/risuko-engine/src/engine/ed2k/download.rs`:
- Around line 467-480: collect_needed_ranges currently ignores the peer's
availability bitmap and calls ChunkManager::next_needed_chunk_excluding(&[],
&picked), causing requested ranges to include chunks the peer doesn't have;
modify run_peer_download to capture PeerEvent::FileStatus { parts } and pass
that parts slice into collect_needed_ranges (or change calls inside
collect_needed_ranges to use next_needed_chunk(&parts) /
next_needed_chunk_excluding(&parts, &picked)) so range selection uses the peer's
parts bitmap when choosing (start,end) ranges; update function signatures as
needed to thread the parts slice from run_peer_download into
collect_needed_ranges and ensure picked logic remains unchanged.
In `@src-tauri/risuko-engine/src/engine/ftp/ftp_download.rs`:
- Around line 87-98: The short-circuit currently treats caller-provided
$file_size as proof the remote file is complete, which can rename a stale .part
when the FTP SIZE command is unsupported; change the condition in the block
using effective_size so it only short-circuits when remote_size (the SIZE reply)
is > 0 and existing_size == remote_size, and store remote_size into $completed
before calling $ftp.quit().await (i.e. require the server-confirmed remote_size
rather than the fallback $file_size when deciding the .part is complete).
In `@src-tauri/risuko-engine/src/engine/m3u8/download.rs`:
- Around line 412-456: The reserved .mp4 created by reserve_unique_output_path
is left behind on early returns; either run the ffmpeg preflight check
(ffmpeg_check via tokio::process::Command::new("ffmpeg").arg("-version")) before
calling reserve_unique_output_path, or ensure any early-return paths delete the
reserved mp4 (use tokio::fs::remove_file(&mp4_path).await) when
ffmpeg_check.is_err() or when the ffmpeg spawn/map_err early-fails. Update the
logic around reserve_unique_output_path, ffmpeg_check, and the ffmpeg Command
invocation so the temporary mp4 is cleaned up on all error paths.
In `@src-tauri/risuko-engine/src/engine/m3u8/segment.rs`:
- Around line 67-68: The cleanup method currently removes progress_path while
the file handle (append_file) may still be open causing Windows file-lock leaks;
change the signature of cleanup to take &mut self, call take() on the file
handle field (e.g., self.append_file.take() or otherwise extract/replace it) to
drop/close the File before attempting to remove_file(&self.progress_path), then
call std::fs::remove_file; ensure any Option<File> field is updated accordingly
and handle errors as before.
In `@src-tauri/risuko-engine/src/engine/rss/mod.rs`:
- Around line 170-173: The current logic only updates feed.last_fetched_at on
success (in the match for result), causing failed feeds to remain "due" forever;
add a separate timestamp field (e.g., feed.last_polled_at or
feed.last_attempted_at) to the Feed struct and use that field when computing
due-ness instead of last_fetched_at, then set that new timestamp to now_secs()
on every fetch attempt (both in the Ok(parsed) branch and the Err branch) while
keeping last_fetched_at updated only on success; also ensure error_count is
incremented on failure and persisted alongside the new timestamp wherever feeds
are saved/serialized.
In `@src-tauri/risuko-engine/src/engine/upload/s3.rs`:
- Around line 296-299: After all part futures have completed but before calling
self.complete_multipart(...), re-check the cancellation controller (ctl) and if
cancellation has been requested, do NOT call complete_multipart; instead invoke
the multipart abort routine (e.g. self.abort_multipart or
self.abort_multipart_upload with the same upload_id) and return an error/result
that indicates the upload was cancelled. Locate the post-join block that
currently calls self.complete_multipart(url, upload_id, &parts).await and insert
the cancellation check there (using ctl's cancellation/query method) to abort
and short-circuit instead of committing the upload.
In `@src-tauri/risuko-engine/src/engine/upload/webdav.rs`:
- Around line 213-215: The existing comment above ctl.report incorrectly states
progress isn't streamed via hyper; update it to reflect that streaming progress
is now provided by file_stream_body_with_progress (used earlier), and make the
comment explain that the final ctl.report(file.size, file.size) is a best-effort
fallback to mark the job complete for rare edge cases (e.g., when progress
updates were lost or not emitted) rather than the primary progress path; locate
this near file_stream_body_with_progress and ctl.report to replace the stale
wording.
🪄 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: 4be5e651-d8cb-424e-9e5d-bb53abbca33a
⛔ Files ignored due to path filters (4)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlsrc-tauri/Cargo.lockis excluded by!**/*.locksrc-tauri/gen/android/app/src/main/java/app/risuko/mobile/MainActivity.ktis excluded by!**/gen/**src-tauri/gen/android/app/src/main/res/xml/file_paths.xmlis excluded by!**/gen/**
📒 Files selected for processing (134)
.github/workflows/release.ymlpackages/risuko-app/bin.jspackages/risuko-app/package.jsonpackages/risuko-cli/npm/darwin-arm64/package.jsonpackages/risuko-cli/npm/darwin-x64/package.jsonpackages/risuko-cli/npm/linux-arm64-gnu/package.jsonpackages/risuko-cli/npm/linux-x64-gnu/package.jsonpackages/risuko-cli/npm/win32-arm64-msvc/package.jsonpackages/risuko-cli/npm/win32-x64-msvc/package.jsonpackages/risuko-cli/package.jsonpackages/risuko-js/npm/darwin-arm64/package.jsonpackages/risuko-js/npm/darwin-x64/package.jsonpackages/risuko-js/npm/linux-arm64-gnu/package.jsonpackages/risuko-js/npm/linux-x64-gnu/package.jsonpackages/risuko-js/npm/win32-arm64-msvc/package.jsonpackages/risuko-js/npm/win32-x64-msvc/package.jsonpackages/risuko-js/package.jsonscripts/bootstrap-npm-local.mjsscripts/build-web.mjsscripts/build.mjsscripts/dev.mjsscripts/ensure-package-artifacts.mjssrc-tauri/Cargo.tomlsrc-tauri/risuko-bt/src/bencode.rssrc-tauri/risuko-bt/src/core/merkle.rssrc-tauri/risuko-bt/src/dht.rssrc-tauri/risuko-bt/src/lsd.rssrc-tauri/risuko-bt/src/magnet.rssrc-tauri/risuko-bt/src/peer.rssrc-tauri/risuko-bt/src/peer/connection.rssrc-tauri/risuko-bt/src/peer/state.rssrc-tauri/risuko-bt/src/piece/chunk_tracker.rssrc-tauri/risuko-bt/src/piece/piece_tracker.rssrc-tauri/risuko-bt/src/session.rssrc-tauri/risuko-bt/src/storage.rssrc-tauri/risuko-bt/src/torrent.rssrc-tauri/risuko-bt/src/torrent/stats.rssrc-tauri/risuko-bt/src/upnp.rssrc-tauri/risuko-bt/src/utp/socket.rssrc-tauri/risuko-bt/src/utp/stream.rssrc-tauri/risuko-bt/src/wire/extended.rssrc-tauri/risuko-bt/src/wire/handshake.rssrc-tauri/risuko-bt/src/wire/mse.rssrc-tauri/risuko-cli/src/commands.rssrc-tauri/risuko-cli/src/progress.rssrc-tauri/risuko-cli/src/rpc_client.rssrc-tauri/risuko-engine/Cargo.tomlsrc-tauri/risuko-engine/src/engine/adc/mod.rssrc-tauri/risuko-engine/src/engine/cookie_store.rssrc-tauri/risuko-engine/src/engine/ed2k/chunks.rssrc-tauri/risuko-engine/src/engine/ed2k/download.rssrc-tauri/risuko-engine/src/engine/ed2k/peer.rssrc-tauri/risuko-engine/src/engine/error_code.rssrc-tauri/risuko-engine/src/engine/ftp/ftp_download.rssrc-tauri/risuko-engine/src/engine/ftp/sftp_download.rssrc-tauri/risuko-engine/src/engine/http.rssrc-tauri/risuko-engine/src/engine/m3u8/download.rssrc-tauri/risuko-engine/src/engine/m3u8/segment.rssrc-tauri/risuko-engine/src/engine/manager.rssrc-tauri/risuko-engine/src/engine/media.rssrc-tauri/risuko-engine/src/engine/p2p_tests.rssrc-tauri/risuko-engine/src/engine/rpc.rssrc-tauri/risuko-engine/src/engine/rss/mod.rssrc-tauri/risuko-engine/src/engine/rss/rule_engine.rssrc-tauri/risuko-engine/src/engine/task.rssrc-tauri/risuko-engine/src/engine/torrent.rssrc-tauri/risuko-engine/src/engine/upload/ftp.rssrc-tauri/risuko-engine/src/engine/upload/manager.rssrc-tauri/risuko-engine/src/engine/upload/s3.rssrc-tauri/risuko-engine/src/engine/upload/sftp.rssrc-tauri/risuko-engine/src/engine/upload/webdav.rssrc-tauri/risuko-http/src/client.rssrc-tauri/risuko-http/src/connector.rssrc-tauri/risuko-http/src/response.rssrc-tauri/src/cli/commands.rssrc-tauri/src/cli/rpc_client.rssrc-tauri/src/commands/app_cmds.rssrc-tauri/src/commands/engine_cmds.rssrc-tauri/src/lib.rssrc-tauri/src/state.rssrc-tauri/tauri.conf.jsonsrc/renderer/api/Api.tssrc/renderer/api/index.tssrc/renderer/components/Native/DynamicTray.vuesrc/renderer/components/Native/EngineClient.vuesrc/renderer/components/Preference/Advanced.vuesrc/renderer/components/Preference/Basic.vuesrc/renderer/components/Preference/CloudSinks.vuesrc/renderer/components/Preference/HistoryDirectory.vuesrc/renderer/components/Rss/Index.vuesrc/renderer/components/Rss/ItemList.vuesrc/renderer/components/Task/AddTask.vuesrc/renderer/components/Task/Index.vuesrc/renderer/components/Task/TaskActions.vuesrc/renderer/components/Task/TaskItemActions.vuesrc/renderer/components/Task/TaskStatus.vuesrc/renderer/components/TaskDetail/Index.vuesrc/renderer/components/TaskDetail/TaskActivity.vuesrc/renderer/pages/index/App.vuesrc/renderer/store/rss.tssrc/renderer/store/task.tssrc/renderer/store/uploadSink.tssrc/renderer/styles/global.csssrc/renderer/workers/tray.worker.tssrc/shared/locales/ar/preferences.tssrc/shared/locales/bg/preferences.tssrc/shared/locales/ca/preferences.tssrc/shared/locales/de/preferences.tssrc/shared/locales/el/preferences.tssrc/shared/locales/es/preferences.tssrc/shared/locales/fa/preferences.tssrc/shared/locales/fr/preferences.tssrc/shared/locales/hu/preferences.tssrc/shared/locales/id/preferences.tssrc/shared/locales/index.tssrc/shared/locales/it/preferences.tssrc/shared/locales/ja/preferences.tssrc/shared/locales/ko/preferences.tssrc/shared/locales/nb/preferences.tssrc/shared/locales/nl/preferences.tssrc/shared/locales/pl/preferences.tssrc/shared/locales/pt-BR/preferences.tssrc/shared/locales/ro/preferences.tssrc/shared/locales/ru/preferences.tssrc/shared/locales/th/preferences.tssrc/shared/locales/tr/preferences.tssrc/shared/locales/uk/preferences.tssrc/shared/locales/vi/preferences.tssrc/shared/locales/zh-CN/preferences.tssrc/shared/locales/zh-TW/preferences.tssrc/shared/types/config.tssrc/shared/types/rss.tssrc/shared/utils/index.tssrc/shared/utils/tray.ts
💤 Files with no reviewable changes (13)
- src-tauri/risuko-bt/src/peer.rs
- src/renderer/api/index.ts
- src-tauri/risuko-bt/src/peer/state.rs
- src-tauri/src/commands/app_cmds.rs
- src/renderer/workers/tray.worker.ts
- src/renderer/components/Preference/CloudSinks.vue
- src-tauri/risuko-bt/src/core/merkle.rs
- src-tauri/src/lib.rs
- src-tauri/src/state.rs
- src-tauri/risuko-bt/src/magnet.rs
- src-tauri/risuko-bt/src/piece/chunk_tracker.rs
- src/renderer/components/Task/TaskStatus.vue
- src/renderer/components/TaskDetail/TaskActivity.vue
|
|
||
| - name: Upload standardized app bundle to GitHub Release | ||
| if: startsWith(github.ref, 'refs/tags/') | ||
| uses: softprops/action-gh-release@v2 |
There was a problem hiding this comment.
Pin GitHub actions to commit SHAs for supply-chain security.
The static analysis tool correctly identifies that softprops/action-gh-release@v2 should be pinned to a specific commit hash rather than a mutable tag reference. Mutable tags can be force-pushed, potentially injecting malicious code into your release pipeline.
🔒 Suggested fix
Replace the tag reference with the commit SHA for v2. For example:
- uses: softprops/action-gh-release@v2
+ uses: softprops/action-gh-release@c062e08bd532815e2082a85e87e3ef29c3e6d191 # v2.0.8Check the action-gh-release releases page for the current v2 commit hash.
Also applies to: 350-350
🧰 Tools
🪛 zizmor (1.25.2)
[error] 293-293: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[info] 293-293: action functionality is already included by the runner (superfluous-actions): use gh release in a script step
(superfluous-actions)
🤖 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 @.github/workflows/release.yml at line 293, Replace the mutable tag reference
softprops/action-gh-release@v2 with a pinned commit SHA to eliminate
supply-chain risk: find occurrences of softprops/action-gh-release@v2 in the
workflow and update them to softprops/action-gh-release@<commit-sha> using the
exact commit hash for the v2 release (obtainable from the action-gh-release
releases/tags page); ensure both occurrences are updated and commit the change.
Source: Linters/SAST tools
Comment on lines
+86
to
+98
| /// Removes its transaction id from `pending` on drop | ||
| /// | ||
| /// Keeps aborted lookup tasks from leaving orphaned pending entries | ||
| struct PendingGuard { | ||
| pending: Arc<Mutex<PendingMap>>, | ||
| txn: u16, | ||
| } | ||
|
|
||
| impl Drop for PendingGuard { | ||
| fn drop(&mut self) { | ||
| self.pending.lock().remove(&self.txn); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Add a regression test for aborted lookup cleanup.
This drop-based cleanup is now the only thing preventing leaked KRPC transactions when query_get_peers is canceled by the lookup budget. A focused test that aborts a pending query and asserts the pending map is emptied would lock this behavior down.
Also applies to: 395-443
🤖 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-bt/src/dht.rs` around lines 86 - 98, Add a regression test
that verifies the Drop-based cleanup of KRPC transactions by creating a pending
lookup, forcing/aborting it via the lookup budget (or cancelling the future
returned by query_get_peers), and then asserting that the PendingMap (accessed
via the same Arc<Mutex<PendingMap>> used by PendingGuard) no longer contains the
txn after the task is aborted; use the same mechanisms exercised by PendingGuard
(the pending field and txn u16) to locate the pending entry, spawn the lookup so
it becomes pending, cancel it, await drop completion, and finally lock pending
and assert it is empty to lock in the behavior implemented by impl Drop for
PendingGuard and the query_get_peers path.
Comment on lines
+21
to
+31
| // Map wildcard binds to loopback; 0.0.0.0 and :: are listeners, not dial targets | ||
| let connect_host = match host.trim() { | ||
| "" | "0.0.0.0" | "::" | "[::]" => "127.0.0.1", | ||
| h => h, | ||
| }; | ||
| // Bracket bare IPv6 literals so the URL authority parses correctly | ||
| let url = if connect_host.contains(':') && !connect_host.starts_with('[') { | ||
| format!("http://[{}]:{}/jsonrpc", connect_host, port) | ||
| } else { | ||
| format!("http://{}:{}/jsonrpc", connect_host, port) | ||
| }; |
There was a problem hiding this comment.
IPv6 wildcard incorrectly maps to IPv4 loopback.
The mapping of IPv6 wildcard "::" and "[::]" to "127.0.0.1" (IPv4 loopback) breaks protocol consistency. If the engine binds to IPv6-only (:: on an IPv6-only network or with net.ipv6.bindv6only=1), the CLI will attempt to connect via IPv4 and fail. Map IPv6 wildcards to "::1" (IPv6 loopback) instead.
Additionally, the IPv6 detection on line 27 (contains(':') && !starts_with('[')) is simple but fragile—it won't handle malformed input like "[::1" (missing closing bracket). Consider parsing the host with std::net::IpAddr::from_str to reliably detect IPv6 vs IPv4, or at minimum validate bracket pairing.
🔧 Suggested fix
- // Map wildcard binds to loopback; 0.0.0.0 and :: are listeners, not dial targets
- let connect_host = match host.trim() {
- "" | "0.0.0.0" | "::" | "[::]" => "127.0.0.1",
- h => h,
- };
+ // Map wildcard binds to respective loopback addresses
+ let connect_host = match host.trim() {
+ "" | "0.0.0.0" => "127.0.0.1",
+ "::" | "[::]" => "::1",
+ h => h,
+ };
// Bracket bare IPv6 literals so the URL authority parses correctly
- let url = if connect_host.contains(':') && !connect_host.starts_with('[') {
+ let url = if connect_host.contains(':')
+ && !connect_host.starts_with('[')
+ && !connect_host.starts_with("127.")
+ && connect_host != "::1" {
format!("http://[{}]:{}/jsonrpc", connect_host, port)
} else {
format!("http://{}:{}/jsonrpc", connect_host, port)Alternatively, use std::net::IpAddr::from_str for robust IP detection:
let connect_host = match host.trim() {
"" | "0.0.0.0" => "127.0.0.1",
"::" | "[::]" => "::1",
h => h,
};
let url = if let Ok(std::net::IpAddr::V6(_)) = connect_host.trim_matches(|c| c == '[' || c == ']').parse() {
format!("http://[{}]:{}/jsonrpc", connect_host.trim_matches(|c| c == '[' || c == ']'), port)
} else {
format!("http://{}:{}/jsonrpc", connect_host, port)
};🤖 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-cli/src/rpc_client.rs` around lines 21 - 31, The
connect_host mapping and IPv6 detection are wrong: change the match on host to
map "::" and "[::]" to the IPv6 loopback "::1" (keep ""/"0.0.0.0" mapped to
"127.0.0.1"), then determine whether to bracket the host for url by robustly
detecting IPv6 via std::net::IpAddr::from_str on the host with surrounding
brackets trimmed (use connect_host.trim_matches(|c| c=='['||c==']') or parse
host first), and build url using the cleaned host and port so IPv6 addresses are
bracketed correctly; update the logic around the connect_host, host, and url
variables accordingly.
Comment on lines
+704
to
+734
| /// Evict oldest terminal jobs so at most `max_terminal` remain | ||
| fn prune_terminal_jobs(jobs: &mut HashMap<String, UploadJob>, max_terminal: usize) { | ||
| let terminal_count = jobs | ||
| .values() | ||
| .filter(|j| { | ||
| matches!( | ||
| j.status, | ||
| JobStatus::Complete | JobStatus::Failed | JobStatus::Cancelled | ||
| ) | ||
| }) | ||
| .count(); | ||
| if terminal_count <= max_terminal { | ||
| return; | ||
| } | ||
| // Collect terminal job ids oldest-first, then drop the excess | ||
| let mut terminal: Vec<(u64, String)> = jobs | ||
| .values() | ||
| .filter(|j| { | ||
| matches!( | ||
| j.status, | ||
| JobStatus::Complete | JobStatus::Failed | JobStatus::Cancelled | ||
| ) | ||
| }) | ||
| .map(|j| (j.created_at, j.id.clone())) | ||
| .collect(); | ||
| terminal.sort_by_key(|(created_at, _)| *created_at); | ||
| let to_remove = terminal_count - max_terminal; | ||
| for (_, id) in terminal.into_iter().take(to_remove) { | ||
| jobs.remove(&id); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Reduce redundant terminal job filtering.
The function filters terminal jobs twice: once to count (lines 706-714) and again to collect (lines 719-727). Collect once, then check the length.
♻️ Proposed refactor
fn prune_terminal_jobs(jobs: &mut HashMap<String, UploadJob>, max_terminal: usize) {
- let terminal_count = jobs
+ // Collect terminal job ids oldest-first
+ let mut terminal: Vec<(u64, String)> = jobs
.values()
.filter(|j| {
matches!(
j.status,
JobStatus::Complete | JobStatus::Failed | JobStatus::Cancelled
)
})
- .count();
+ .map(|j| (j.created_at, j.id.clone()))
+ .collect();
+ let terminal_count = terminal.len();
if terminal_count <= max_terminal {
return;
}
- // Collect terminal job ids oldest-first, then drop the excess
- let mut terminal: Vec<(u64, String)> = jobs
- .values()
- .filter(|j| {
- matches!(
- j.status,
- JobStatus::Complete | JobStatus::Failed | JobStatus::Cancelled
- )
- })
- .map(|j| (j.created_at, j.id.clone()))
- .collect();
terminal.sort_by_key(|(created_at, _)| *created_at);
let to_remove = terminal_count - max_terminal;
for (_, id) in terminal.into_iter().take(to_remove) {
jobs.remove(&id);
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| /// Evict oldest terminal jobs so at most `max_terminal` remain | |
| fn prune_terminal_jobs(jobs: &mut HashMap<String, UploadJob>, max_terminal: usize) { | |
| let terminal_count = jobs | |
| .values() | |
| .filter(|j| { | |
| matches!( | |
| j.status, | |
| JobStatus::Complete | JobStatus::Failed | JobStatus::Cancelled | |
| ) | |
| }) | |
| .count(); | |
| if terminal_count <= max_terminal { | |
| return; | |
| } | |
| // Collect terminal job ids oldest-first, then drop the excess | |
| let mut terminal: Vec<(u64, String)> = jobs | |
| .values() | |
| .filter(|j| { | |
| matches!( | |
| j.status, | |
| JobStatus::Complete | JobStatus::Failed | JobStatus::Cancelled | |
| ) | |
| }) | |
| .map(|j| (j.created_at, j.id.clone())) | |
| .collect(); | |
| terminal.sort_by_key(|(created_at, _)| *created_at); | |
| let to_remove = terminal_count - max_terminal; | |
| for (_, id) in terminal.into_iter().take(to_remove) { | |
| jobs.remove(&id); | |
| } | |
| } | |
| /// Evict oldest terminal jobs so at most `max_terminal` remain | |
| fn prune_terminal_jobs(jobs: &mut HashMap<String, UploadJob>, max_terminal: usize) { | |
| // Collect terminal job ids oldest-first | |
| let mut terminal: Vec<(u64, String)> = jobs | |
| .values() | |
| .filter(|j| { | |
| matches!( | |
| j.status, | |
| JobStatus::Complete | JobStatus::Failed | JobStatus::Cancelled | |
| ) | |
| }) | |
| .map(|j| (j.created_at, j.id.clone())) | |
| .collect(); | |
| let terminal_count = terminal.len(); | |
| if terminal_count <= max_terminal { | |
| return; | |
| } | |
| terminal.sort_by_key(|(created_at, _)| *created_at); | |
| let to_remove = terminal_count - max_terminal; | |
| for (_, id) in terminal.into_iter().take(to_remove) { | |
| jobs.remove(&id); | |
| } | |
| } |
🤖 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/upload/manager.rs` around lines 704 - 734,
In prune_terminal_jobs, avoid filtering jobs twice by collecting terminal jobs
once into a Vec of (created_at, id) (e.g., reuse the existing local terminal
variable) instead of running .filter() two times; compute terminal_count from
terminal.len(), check if terminal.len() <= max_terminal and only proceed to sort
and remove the excess entries, then remove IDs from jobs by iterating
terminal.into_iter().take(to_remove). This keeps the same behavior while
eliminating the redundant first .filter() pass over jobs.
| const baseUrl = item.link || item.enclosure_url || ""; | ||
| const isDark = document.documentElement.classList.contains("dark"); | ||
| this.viewerContent = injectBaseTag(raw, baseUrl, isDark); | ||
| this.viewerContent = injectBaseTag(sanitizeHtml(raw), baseUrl, isDark); |
There was a problem hiding this comment.
Preserve relative URLs before sanitizing.
Line 467 now sanitizes the raw HTML before injectBaseTag(), but sanitizeHtml() only keeps href/src values that are already absolute http(s) URLs. That means relative links and images are stripped before the <base> tag can resolve them, so many downloaded HTML pages will now lose navigation and assets in the embedded viewer. Resolve href/src against baseUrl during sanitization, or allow relative URLs through until after base resolution.
🤖 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/renderer/components/Rss/ItemList.vue` at line 467, The current flow
sanitizes raw HTML with sanitizeHtml before injectBaseTag, which causes
sanitizeHtml to drop relative href/src values; change the order or adjust
sanitization so relative URLs are preserved and resolved: either call
injectBaseTag(raw, baseUrl, isDark) first and then run sanitizeHtml on the
resulting HTML (update the place where viewerContent is set around
injectBaseTag/sanitizeHtml), or modify sanitizeHtml to accept a baseUrl and
resolve/allow relative href/src values against baseUrl during sanitization;
target the symbols viewerContent, raw, baseUrl, isDark, injectBaseTag and
sanitizeHtml when making the change.
Comment on lines
+65
to
+71
| while (speedHistoryCache.size > SPEED_HISTORY_GID_LIMIT) { | ||
| const oldest = speedHistoryCache.keys().next().value; | ||
| if (oldest === undefined) { | ||
| break; | ||
| } | ||
| speedHistoryCache.delete(oldest); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Simplify LRU eviction to a single conditional delete.
The while loop can execute at most once because exactly one gid is inserted per call. Replace the loop with a simple conditional:
if (speedHistoryCache.size > SPEED_HISTORY_GID_LIMIT) {
const oldest = speedHistoryCache.keys().next().value;
speedHistoryCache.delete(oldest);
}The undefined check is unnecessary—when size > SPEED_HISTORY_GID_LIMIT, the map is non-empty and keys().next().value is guaranteed to exist.
🤖 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/renderer/store/task.ts` around lines 65 - 71, Replace the while-loop LRU
eviction in the update that manages speedHistoryCache with a single conditional:
when speedHistoryCache.size > SPEED_HISTORY_GID_LIMIT, obtain the oldest key via
speedHistoryCache.keys().next().value and call speedHistoryCache.delete(oldest);
remove the unnecessary undefined check and loop since only one gid is inserted
per call and the map must be non-empty when size exceeds the limit.
Comment on lines
+209
to
+216
| totalProgressPercent() { | ||
| const result = calcProgress( | ||
| this.totalLength, | ||
| this.totalCompletedLength, | ||
| 1, | ||
| ); | ||
| return `${result}`.replace(/\.0$/, ""); | ||
| }, |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | 💤 Low value
Clarify the percent formatting intent.
The regex /\.0$/ strips only trailing ".0" but preserves other decimals like ".5" or ".3". This means totalProgressPercent sometimes returns "50" (no decimal) and sometimes "33.3" (with decimal), creating inconsistent display.
If the goal is "show one decimal place except when it's .0," the current logic achieves that—but the UX inconsistency should be intentional. Consider standardizing:
- Always show one decimal:
calcProgress(..., 1).toFixed(1) - Never show decimals:
Math.floor(calcProgress(..., 0))
🤖 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/renderer/store/task.ts` around lines 209 - 216, The totalProgressPercent
method currently uses calcProgress(this.totalLength, this.totalCompletedLength,
1) and strips a trailing ".0" via string replace, causing inconsistent outputs
(e.g., "50" vs "33.3"); update totalProgressPercent (in
src/renderer/store/task.ts) to a consistent format — either return
calcProgress(..., 1).toFixed(1) to always show one decimal (e.g., "50.0",
"33.3") or call calcProgress(..., 0) and format as an integer (e.g., "50", "33")
— pick one and replace the current replace(/\.0$/) logic accordingly in the
totalProgressPercent method.
Comment on lines
+10
to
+13
| const i = Math.min( | ||
| Math.trunc(Math.floor(Math.log(b) / Math.log(1024))), | ||
| sizes.length - 1, | ||
| ); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | 💤 Low value
Remove redundant Math.trunc wrapper.
Math.floor already returns an integer, so wrapping it with Math.trunc is redundant:
const i = Math.min(
- Math.trunc(Math.floor(Math.log(b) / Math.log(1024))),
+ Math.floor(Math.log(b) / Math.log(1024)),
sizes.length - 1,
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const i = Math.min( | |
| Math.trunc(Math.floor(Math.log(b) / Math.log(1024))), | |
| sizes.length - 1, | |
| ); | |
| const i = Math.min( | |
| Math.floor(Math.log(b) / Math.log(1024)), | |
| sizes.length - 1, | |
| ); |
🤖 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/shared/utils/tray.ts` around lines 10 - 13, The computation of index i in
the tray size logic uses Math.trunc(Math.floor(Math.log(b) / Math.log(1024)))
which is redundant; replace the double wrapper with a single Math.floor call
(i.e. use Math.floor(Math.log(b) / Math.log(1024))) when computing i in the same
expression that also bounds it with sizes.length - 1 to preserve behavior in the
function that defines i.
There was a problem hiding this comment.
1 issue found across 9 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: 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 `@packages/risuko-app/bin.js`:
- Around line 242-251: The verifySha256 function currently treats a
DownloadHttpError with statusCode === 404 as a non-fatal skip; change this so
missing checksum sidecars fail closed: in verifySha256 (and any call sites that
currently proceed when it returns false) do not return false on a 404—rethrow or
throw a new error instead so installation halts; specifically update the catch
around downloadText (and any logic after verifySha256 that continues
installation) to propagate the error when err instanceof DownloadHttpError &&
err.statusCode === 404 rather than treating it as success.
In `@scripts/write-sha256-sidecar.mjs`:
- Around line 14-16: The current code uses readFileSync(assetPath) which fully
buffers the file into memory before hashing; replace that with a streaming
approach: create a readable stream from the file (e.g.,
fs.createReadStream(assetPath)), pipe its chunks into the crypto Hash returned
by createHash("sha256") (or manually update the Hash on 'data' events), wait for
the stream to finish, then call digest("hex") to produce the hex string (ensure
errors on the stream are handled and the final hex is assigned to the existing
digest variable). Use the existing symbols createHash, assetPath and digest so
only the file-reading part is changed to a stream-based implementation to avoid
full-file buffering.
🪄 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: 97bb728a-f8e2-4e29-b9ef-2569cfd877bc
📒 Files selected for processing (9)
.github/workflows/release.ymlpackages/risuko-app/bin.jspackages/risuko-app/index.jspackages/risuko-app/package.jsonscripts/build-web.mjsscripts/ensure-package-artifacts.mjsscripts/write-sha256-sidecar.mjssrc-tauri/risuko-bt/src/dht.rssrc-tauri/risuko-bt/src/utp/socket.rs
💤 Files with no reviewable changes (1)
- packages/risuko-app/index.js
There was a problem hiding this comment.
1 issue found across 2 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.
1 issue found across 1 file (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.
♻️ Duplicate comments (1)
packages/risuko-app/bin.js (1)
267-280:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMake checksum bypass for legacy releases explicitly opt-in.
The current flow silently allows unsigned installs for any version
< 0.4.0when the sidecar 404s. That weakens the integrity guarantee and effectively reopens the previous fail-open path. Keep fail-closed by default, and require an explicit CLI flag to allow legacy no-checksum installs.Suggested minimal change
let version = PKG_VERSION; let noCache = false; +let allowLegacyNoChecksum = false; const appArgs = []; @@ } else if (arg === "--no-cache") { noCache = true; + } else if (arg === "--allow-legacy-no-checksum") { + allowLegacyNoChecksum = true; @@ -async function verifySha256(assetPath, checksumUrl, assetName) { +async function verifySha256( + assetPath, + checksumUrl, + assetName, + releaseVersion, + allowLegacyNoChecksum, +) { @@ - if (isLegacyChecksumRelease(version)) { + if (allowLegacyNoChecksum && isLegacyChecksumRelease(releaseVersion)) { return false; } throw new Error( `SHA-256 sidecar not found for ${assetName}: ${checksumUrl}`, ); @@ - const checksumVerified = await verifySha256( + const checksumVerified = await verifySha256( tmpPath, checksumUrl, entry.asset, + version, + allowLegacyNoChecksum, );Also applies to: 354-365
🤖 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 `@packages/risuko-app/bin.js` around lines 267 - 280, The code currently returns false (allowing unsigned installs) whenever downloadText(checksumUrl) 404s and isLegacyChecksumRelease(version) is true; change this to require an explicit opt-in flag instead: introduce and check a boolean CLI/config option (e.g., allowLegacyNoChecksum or allowLegacyUnsignedInstalls) before returning false in the downloadText error handler around checksumUrl/assetName/version; if the flag is not set, throw the SHA-256 sidecar not found error as before. Make the same change in the other identical handler (the second downloadText/checksumUrl block) so legacy bypass is only allowed when the explicit flag is provided.
🤖 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.
Duplicate comments:
In `@packages/risuko-app/bin.js`:
- Around line 267-280: The code currently returns false (allowing unsigned
installs) whenever downloadText(checksumUrl) 404s and
isLegacyChecksumRelease(version) is true; change this to require an explicit
opt-in flag instead: introduce and check a boolean CLI/config option (e.g.,
allowLegacyNoChecksum or allowLegacyUnsignedInstalls) before returning false in
the downloadText error handler around checksumUrl/assetName/version; if the flag
is not set, throw the SHA-256 sidecar not found error as before. Make the same
change in the other identical handler (the second downloadText/checksumUrl
block) so legacy bypass is only allowed when the explicit flag is provided.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9bb88114-3f6f-474b-9e5c-4d9c7c5ac5aa
📒 Files selected for processing (1)
packages/risuko-app/bin.js
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 (1)
packages/risuko-app/bin.js (1)
249-272:⚠️ Potential issue | 🟠 Major | ⚡ Quick winThe legacy-checksum gate does not match the documented prerelease behavior.
The new logic only allows the 404 bypass when the whole version sorts below
0.4.0. That means0.5.0-beta/1.0.0-rc.1still fail closed on Line 289 even with--allow-legacy-no-checksum, while0.4.0+build.1is misclassified as legacy becausecompareReleaseVersions()treats any trailing text as lower precedence. If the intended rule is “pre-0.4.0 or prereleases,” parse prerelease/build metadata separately instead of ordering raw suffix strings.Suggested fix
function compareReleaseVersions(left, right) { - const leftMatch = /^v?(\d+)\.(\d+)\.(\d+)(.*)/.exec(left); - const rightMatch = /^v?(\d+)\.(\d+)\.(\d+)(.*)/.exec(right); + const leftMatch = + /^v?(\d+)\.(\d+)\.(\d+)(?:-([0-9A-Za-z.-]+))?(?:\+[0-9A-Za-z.-]+)?$/.exec(left); + const rightMatch = + /^v?(\d+)\.(\d+)\.(\d+)(?:-([0-9A-Za-z.-]+))?(?:\+[0-9A-Za-z.-]+)?$/.exec(right); if (!leftMatch || !rightMatch) { return null; } @@ - const leftSuffix = leftMatch[4]; - const rightSuffix = rightMatch[4]; + const leftSuffix = leftMatch[4] || ""; + const rightSuffix = rightMatch[4] || ""; if (leftSuffix === rightSuffix) { return 0; } @@ } function isLegacyChecksumRelease(releaseVersion) { + if (/^v?\d+\.\d+\.\d+-[0-9A-Za-z.-]+(?:\+[0-9A-Za-z.-]+)?$/.test(releaseVersion)) { + return true; + } const order = compareReleaseVersions( releaseVersion, SHA256_SIDECAR_REQUIRED_VERSION, ); return order !== null && order < 0; }Also applies to: 275-280, 288-290
🤖 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 `@packages/risuko-app/bin.js` around lines 249 - 272, compareReleaseVersions currently treats any trailing text as a raw suffix which misorders prereleases (e.g., "0.5.0-beta") and build metadata; instead parse the trailing part into semver prerelease and build metadata and implement semver precedence: after numeric major/minor/patch equality, treat absence of a prerelease as higher precedence than presence (i.e., release > prerelease), ignore build metadata for ordering, and when both have prereleases compare dot-separated identifiers (numeric identifiers numerically, non-numeric lexicographically, numeric < non-numeric); update compareReleaseVersions to extract prerelease/build from leftMatch[4]/rightMatch[4] and apply this logic so prereleases sort below their corresponding release and builds don’t affect ordering.
🤖 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 `@packages/risuko-app/bin.js`:
- Around line 250-251: The script currently accepts version strings with a
leading "v" (seen in the regex for leftMatch/rightMatch) but later composes
download URLs and manual hints using `v${version}`, which causes double "v"
(e.g., "vv0.4.0"); normalize the version once during argument parsing by
stripping a single optional leading "v" from the parsed version variable (the
same variable used when building the download path and manual-download hint) so
all downstream code (URL composition and hints) uses a canonical version like
"0.4.0"; update the parser that sets `version` (and any places where
`left`/`right` are derived) to remove the leading "v" if present, leaving the
rest of the code unchanged.
---
Outside diff comments:
In `@packages/risuko-app/bin.js`:
- Around line 249-272: compareReleaseVersions currently treats any trailing text
as a raw suffix which misorders prereleases (e.g., "0.5.0-beta") and build
metadata; instead parse the trailing part into semver prerelease and build
metadata and implement semver precedence: after numeric major/minor/patch
equality, treat absence of a prerelease as higher precedence than presence
(i.e., release > prerelease), ignore build metadata for ordering, and when both
have prereleases compare dot-separated identifiers (numeric identifiers
numerically, non-numeric lexicographically, numeric < non-numeric); update
compareReleaseVersions to extract prerelease/build from
leftMatch[4]/rightMatch[4] and apply this logic so prereleases sort below their
corresponding release and builds don’t affect ordering.
🪄 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: d227b175-f40a-45ff-8557-e983cf2c5ced
📒 Files selected for processing (1)
packages/risuko-app/bin.js
There was a problem hiding this comment.
1 issue found across 1 file (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.
6 issues found across 4 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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/risuko-app/bin.js (1)
330-345:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
--allow-legacy-no-checksumdoes not actually cover prereleases above0.4.0.
isLegacyChecksumRelease()only returnstruewhen the version sorts lower than0.4.0. That means0.4.1-rc.1,1.0.0-beta.1, etc. still fail closed on a missing sidecar even though the CLI help explicitly says prereleases are allowed. Treat prerelease suffixes as eligible independently of the floor comparison.Suggested fix
+function isPrereleaseVersion(releaseVersion) { + return /^v?\d+\.\d+\.\d+-/.test(releaseVersion); +} + function isLegacyChecksumRelease(releaseVersion) { + if (isPrereleaseVersion(releaseVersion)) { + return true; + } const order = compareReleaseVersions( releaseVersion, SHA256_SIDECAR_REQUIRED_VERSION, ); return order !== null && order < 0; }🤖 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 `@packages/risuko-app/bin.js` around lines 330 - 345, The allow-legacy-no-checksum logic currently only treats versions strictly less than SHA256_SIDECAR_REQUIRED_VERSION as legacy; to cover prereleases like 0.4.1-rc.1 or 1.0.0-beta.1 you should update isLegacyChecksumRelease(releaseVersion) to also return true for any prerelease suffixes regardless of the numeric ordering (e.g., detect semver prerelease via a hyphen or semver.prerelease check) in addition to the existing compareReleaseVersions check; this will ensure verifySha256(assetPath, checksumUrl, assetName) (which calls isLegacyChecksumRelease) correctly allows missing sidecars when --allow-legacy-no-checksum is used for prereleases.
🤖 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-bt/src/torrent.rs`:
- Around line 1793-1797: Move the holepunch_attempted.insert(target) so it only
runs after the rendezvous message is successfully queued; currently
holepunch_attempted is set before relay.cmd_tx.try_send(...) which can fail and
cause future attempts to be skipped. Change the order around the try_send call
on relay.cmd_tx (the PeerCommand::Send with Message::Extended built by
build_holepunch and ext_id relay_hp) so you insert into holepunch_attempted only
when try_send returns Ok; if try_send returns Err, do not insert (and optionally
log or handle the error) so retries can occur on subsequent disconnects.
In `@src-tauri/risuko-bt/src/wire/extended.rs`:
- Around line 356-360: The parser currently treats an ERROR holepunch (when
msg_type == holepunch_type::ERROR) as valid even if the 4-byte error code is
missing; update the ut_holepunch parsing logic (the branch where msg_type,
port_off, eo and err_code are used) to reject truncated ERROR payloads by
returning None or an Err when payload.len() < eo + 4 instead of defaulting
err_code to 0, ensuring the function exits early on short payloads.
In `@src-tauri/src/commands/file_cmds.rs`:
- Around line 1200-1209: The content-based delete is too broad: change the
deletion predicate so we only delete when either
generated_torrent_hex_stem(file_name) equals hash OR the content matches AND the
file proves it’s an app-generated sidecar; update
matches_generated_torrent_sidecar_by_content(&path, hash) to also check a
launcher-specific provenance marker (e.g., a custom comment/metadata field or a
sidecar-only flag embedded in the torrent) and return false if that marker is
absent, and ensure the caller uses that stricter function (or rename it to
matches_generated_torrent_sidecar_by_content_and_provenance) so only proven
sidecars are removed.
---
Outside diff comments:
In `@packages/risuko-app/bin.js`:
- Around line 330-345: The allow-legacy-no-checksum logic currently only treats
versions strictly less than SHA256_SIDECAR_REQUIRED_VERSION as legacy; to cover
prereleases like 0.4.1-rc.1 or 1.0.0-beta.1 you should update
isLegacyChecksumRelease(releaseVersion) to also return true for any prerelease
suffixes regardless of the numeric ordering (e.g., detect semver prerelease via
a hyphen or semver.prerelease check) in addition to the existing
compareReleaseVersions check; this will ensure verifySha256(assetPath,
checksumUrl, assetName) (which calls isLegacyChecksumRelease) correctly allows
missing sidecars when --allow-legacy-no-checksum is used for prereleases.
🪄 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: 1669f46e-3b93-4456-8f78-9313f4252da5
📒 Files selected for processing (4)
packages/risuko-app/bin.jssrc-tauri/risuko-bt/src/torrent.rssrc-tauri/risuko-bt/src/wire/extended.rssrc-tauri/src/commands/file_cmds.rs
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
0.4.0
Summary by cubic
Risuko 0.4.0 refactors the engine and UI for better stability and speed, and hardens releases/installs with SHA‑256 sidecars plus cross‑platform build tooling. Transfers, RSS, and uploads are more resilient with smarter scheduling, safer temp/progress handling, and stricter staging.
Refactors
ut_holepunch.fontFamily/fontSizeoptions; simplified tray icon logic; RSS reader sanitizes HTML and tightens iframe sandboxing..sha256sidecars (CI writes and uploads);@risuko/appverifies checksums; platform packages (@risuko/cli-*,@risuko/js-*) run a prepack artifact check; scripts callpnpm.cmd/npx.cmdon Windows; all packages bumped to 0.4.0.tellStatusskips largefilesunless requested.Bug Fixes
FileProviderrestricts URIs to app files/cache.Written for commit 9a7303c. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes & Improvements
UI/UX Enhancements