fix: close post-merge Major/Minor reviews, #453 drag, #521 portal cascade, #518 Delta Sync step timeout - #525
Merged
Merged
Conversation
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
|
Warning Review limit reached
Next review available in: 16 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (73)
📝 WalkthroughWalkthroughThe PR updates CI timeouts, portal chooser tests, native scanning and speedtest behavior, CLI catalog paging, server reordering, duplicate/trash UI handling, provider markup, and localization data. ChangesCI and portal validation
Native scanning and CLI flows
UI behavior and server ordering
Provider markup and localization
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
Member
Author
|
Re-opening to re-trigger Actions (no workflow runs attached to the head SHA). |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
🧹 Nitpick comments (3)
tests/portal-chooser/selftest-precondition.sh (1)
35-38: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDrop the unused
summarycapture. Shellcheck flags it (SC2034); nothing reads it, and the three counters below already cover the assertions.♻️ Remove dead assignment
-summary=$(grep -E '^passed:|^ FAIL |^ skip |^ ok ' "$LOG" || true) fail_n=$(grep -c '^ FAIL ' "$LOG" || true)🤖 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 `@tests/portal-chooser/selftest-precondition.sh` around lines 35 - 38, Remove the unused summary assignment from the test script, leaving the fail_n, skip_n, and ok_n counter assignments unchanged.Source: Linters/SAST tools
tests/portal-chooser/fake-portal/src/lib.rs (1)
106-114: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRedundant predicate and dead assertion.
*l == "3"is subsumed bystarts_with("3"), and theassert!afterwards is always true for whateverfindreturned, so its message is unreachable. The.expectis doing the real work.♻️ Collapse to a single check
- let first_return_line = version_body - .lines() - .map(str::trim) - .find(|l| l.starts_with("3") || *l == "3") - .expect("NetworkMonitor version body should return 3"); - assert!( - first_return_line.starts_with('3'), - "NetworkMonitor must advertise version 3, got {first_return_line:?}" - ); + let first_value_line = version_body + .lines() + .map(str::trim) + .find(|l| !l.is_empty() && !l.starts_with("//")) + .unwrap_or_default(); + assert!( + first_value_line.starts_with('3'), + "NetworkMonitor must advertise version 3, got {first_value_line:?}" + );🤖 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 `@tests/portal-chooser/fake-portal/src/lib.rs` around lines 106 - 114, In the version-body check, simplify the `find` predicate to only test `starts_with("3")` and remove the subsequent `assert!`, relying on the existing `expect` in the `first_return_line` assignment as the single validation.src/utils/reorderByIndex.test.ts (1)
39-68: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for
reorderVisibleInFull's defensive branches.The empty-
visibleearly return and theslots.length !== nextVisible.lengthstale-mismatch fallback (visible containing an id no longer infull) aren't exercised by any test here, even though they're the safety net for the "stale UI" scenario the source comments call out.✅ Suggested additional test cases
+ it('returns a copy when visible is empty', () => { + const next = reorderVisibleInFull(full, [], 0, 1); + expect(next).not.toBe(full); + expect(ids(next)).toEqual(ids(full)); + }); + + it('defensively no-ops when visible is not a subset of full (stale UI)', () => { + const staleVisible = [row('b'), row('d'), row('zzz')]; + const next = reorderVisibleInFull(full, staleVisible, 0, 2); + expect(ids(next)).toEqual(ids(full)); + });🤖 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/utils/reorderByIndex.test.ts` around lines 39 - 68, Add tests in the reorderVisibleInFull suite covering both defensive branches: verify an empty visible list returns the original full list, and verify a visible item whose id is absent from full triggers the stale-mismatch fallback with the expected unchanged result. Use the existing row, ids, and full/visible fixtures, and assert the fallback behavior explicitly.
🤖 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/src/speedtest.rs`:
- Around line 407-410: Update the cancellation branch in the speed test flow
after provider.connect() succeeds to call the provider’s disconnect operation
before returning the cancellation error. Preserve the existing "Test cancelled
while creating scratch directory" error and ensure cleanup occurs before every
return from that branch.
- Around line 801-820: Update is_public_web_document_root to treat each
protected document root as a path prefix on component boundaries, so the root
itself and all descendants such as uploads or bench are rejected. Preserve
case-insensitive matching and avoid matching unrelated names such as /www-data;
centralize the protected-root checks as needed within this function.
In `@src/i18n/locales/bg.json`:
- Line 3403: Update the deleteConfirm translation in the Bulgarian locale so
count 1 uses grammatically singular wording while other counts remain correct,
using the project’s existing pluralization convention or neutral wording that
works for every count.
In `@src/i18n/locales/bn.json`:
- Line 3404: Update the Bengali value for fuzzyCutoffPlaceholder from স্বয়ং to
স্বয়ংক্রিয়, matching the existing common.auto translation.
In `@src/i18n/locales/cs.json`:
- Line 3403: Update the Czech locale’s deleteConfirm translation to use the
corrected plural phrase “duplicitních souborů” while preserving the rest of the
confirmation message and its {count} placeholder.
In `@src/i18n/locales/et.json`:
- Line 3404: Update the fuzzyCutoffPlaceholder value in the Estonian locale to
use the existing “Automaatne” translation (or the locale’s “vaikimisi” wording)
instead of “auto.”
In `@src/i18n/locales/eu.json`:
- Line 3404: Update the fuzzyCutoffPlaceholder value in the locale data to use
the existing Basque translation “Automatikoa” instead of the user-visible
English “auto”, matching the common.auto translation.
In `@src/i18n/locales/fi.json`:
- Line 3403: Update the deleteConfirm translation value to replace the
misspelled Finnish word “kaksoiskappeletta” with “kaksoiskappaletta”, preserving
the rest of the message and {count} placeholder.
In `@src/i18n/locales/fr.json`:
- Line 3403: Update the deleteConfirm translation in fr.json to use the
correctly accented French wording “Êtes-vous sûr de vouloir supprimer…” while
preserving the existing placeholder and message structure.
In `@src/i18n/locales/hr.json`:
- Around line 3403-3404: Update the fuzzyCutoffPlaceholder value in the Croatian
locale to use the existing Croatian translation “Automatski” instead of “auto”,
matching the locale’s established wording for this concept.
In `@src/i18n/locales/hu.json`:
- Around line 3403-3404: Update the Hungarian locale entry
fuzzyCutoffPlaceholder from the English value "auto" to the established
Hungarian wording "automatikus", leaving the surrounding translations unchanged.
In `@src/i18n/locales/is.json`:
- Around line 3403-3404: Update the fuzzyCutoffPlaceholder translation value in
the Icelandic locale from the abbreviated “sjálf” to the standard “sjálfgefið”
term, leaving the surrounding translations unchanged.
In `@src/i18n/locales/sk.json`:
- Line 3404: Update the fuzzyCutoffPlaceholder value in the Slovak locale to a
Slovak translation, reusing the existing localized wording for common.auto where
appropriate instead of leaving it as “auto”.
In `@src/i18n/locales/sl.json`:
- Line 3404: Update the fuzzyCutoffPlaceholder translation in the Slovenian
locale from the misleading “avto” value to “samodejno” or the project’s
established Slovenian equivalent for “automatic.”
In `@src/i18n/locales/sr.json`:
- Around line 3403-3404: Update the Serbian locale’s deleteConfirm translation
to use the locale’s supported pluralization syntax for count, providing
grammatically correct singular and plural forms while preserving the existing
confirmation message meaning.
In `@src/i18n/locales/sw.json`:
- Line 3404: Update the fuzzyCutoffPlaceholder value in the Swahili locale to
use the existing localized term “Kiotomatiki” instead of the English “auto”,
keeping the duplicate finder input hint consistent.
In `@tests/portal-chooser/portal-chooser-test.sh`:
- Around line 221-225: Guard the comparison in the noportal validation using
wins_ref so it is skipped when the cancel reference case produced no output,
rather than reporting a fallback mismatch. Preserve the existing equality check
and success/failure messages when wins_ref is nonzero, updating the surrounding
logic in the windows-after comparison block.
---
Nitpick comments:
In `@src/utils/reorderByIndex.test.ts`:
- Around line 39-68: Add tests in the reorderVisibleInFull suite covering both
defensive branches: verify an empty visible list returns the original full list,
and verify a visible item whose id is absent from full triggers the
stale-mismatch fallback with the expected unchanged result. Use the existing
row, ids, and full/visible fixtures, and assert the fallback behavior
explicitly.
In `@tests/portal-chooser/fake-portal/src/lib.rs`:
- Around line 106-114: In the version-body check, simplify the `find` predicate
to only test `starts_with("3")` and remove the subsequent `assert!`, relying on
the existing `expect` in the `first_return_line` assignment as the single
validation.
In `@tests/portal-chooser/selftest-precondition.sh`:
- Around line 35-38: Remove the unused summary assignment from the test script,
leaving the fail_n, skip_n, and ok_n counter assignments unchanged.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 63e58f6a-917e-4137-9a3f-8a3890cd184c
📒 Files selected for processing (74)
.github/workflows/delta-sync-integration.yml.github/workflows/portal-chooser.ymlREADME.mdsrc-tauri/src/bin/aeroftp_cli.rssrc-tauri/src/dedupe/mod.rssrc-tauri/src/lib.rssrc-tauri/src/speedtest.rssrc/components/DuplicateFinderDialog.test.tssrc/components/DuplicateFinderDialog.tsxsrc/components/FileLuTrashManager.tsxsrc/components/IntroHub/MyServersPanel.tsxsrc/components/IntroHub/MyServersTable.tsxsrc/components/SavedServers.tsxsrc/components/Trash/fileLuDeletedLabel.test.tssrc/components/Trash/fileLuDeletedLabel.tssrc/components/providerCatalog.tssrc/i18n/drimeQuota.test.tssrc/i18n/locales/bg.jsonsrc/i18n/locales/bn.jsonsrc/i18n/locales/ca.jsonsrc/i18n/locales/cs.jsonsrc/i18n/locales/cy.jsonsrc/i18n/locales/da.jsonsrc/i18n/locales/de.jsonsrc/i18n/locales/el.jsonsrc/i18n/locales/en.jsonsrc/i18n/locales/es.jsonsrc/i18n/locales/et.jsonsrc/i18n/locales/eu.jsonsrc/i18n/locales/fi.jsonsrc/i18n/locales/fr.jsonsrc/i18n/locales/gl.jsonsrc/i18n/locales/hi.jsonsrc/i18n/locales/hr.jsonsrc/i18n/locales/hu.jsonsrc/i18n/locales/hy.jsonsrc/i18n/locales/id.jsonsrc/i18n/locales/is.jsonsrc/i18n/locales/it.jsonsrc/i18n/locales/ja.jsonsrc/i18n/locales/ka.jsonsrc/i18n/locales/km.jsonsrc/i18n/locales/ko.jsonsrc/i18n/locales/lt.jsonsrc/i18n/locales/lv.jsonsrc/i18n/locales/mk.jsonsrc/i18n/locales/ms.jsonsrc/i18n/locales/nl.jsonsrc/i18n/locales/no.jsonsrc/i18n/locales/pl.jsonsrc/i18n/locales/pt.jsonsrc/i18n/locales/ro.jsonsrc/i18n/locales/ru.jsonsrc/i18n/locales/sk.jsonsrc/i18n/locales/sl.jsonsrc/i18n/locales/sr.jsonsrc/i18n/locales/sv.jsonsrc/i18n/locales/sw.jsonsrc/i18n/locales/th.jsonsrc/i18n/locales/tl.jsonsrc/i18n/locales/tr.jsonsrc/i18n/locales/uk.jsonsrc/i18n/locales/vi.jsonsrc/i18n/locales/zh.jsonsrc/utils/nativeE2e.test.tssrc/utils/reorderByIndex.test.tssrc/utils/reorderByIndex.tstests/portal-chooser/README.mdtests/portal-chooser/assert-private-a11y.pytests/portal-chooser/fake-portal/src/bin/portal-probe.rstests/portal-chooser/fake-portal/src/lib.rstests/portal-chooser/portal-chooser-test.shtests/portal-chooser/selftest-portal.shtests/portal-chooser/selftest-precondition.sh
Comment on lines
+407
to
+410
| Ok(()) => {} | ||
| Err(msg) if msg == "Test cancelled while creating scratch directory" => { | ||
| return Err(msg); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Disconnect the already-connected provider before returning cancellation.
This branch runs after provider.connect() succeeds, but returns without disconnect(). Repeated cancellation while creating the scratch directory can leak active provider sessions.
Proposed fix
Err(msg) if msg == "Test cancelled while creating scratch directory" => {
+ let _ = provider.disconnect().await;
return Err(msg);
}📝 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
| Ok(()) => {} | |
| Err(msg) if msg == "Test cancelled while creating scratch directory" => { | |
| return Err(msg); | |
| } | |
| Ok(()) => {} | |
| Err(msg) if msg == "Test cancelled while creating scratch directory" => { | |
| let _ = provider.disconnect().await; | |
| return Err(msg); | |
| } |
🤖 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/speedtest.rs` around lines 407 - 410, Update the cancellation
branch in the speed test flow after provider.connect() succeeds to call the
provider’s disconnect operation before returning the cancellation error.
Preserve the existing "Test cancelled while creating scratch directory" error
and ensure cleanup occurs before every return from that branch.
Comment on lines
+801
to
+820
| fn is_public_web_document_root(remote_dir: &str) -> bool { | ||
| let trimmed = remote_dir.trim().trim_end_matches('/'); | ||
| let p = if trimmed.is_empty() { "/" } else { trimmed }; | ||
| let lower = p.to_ascii_lowercase(); | ||
| matches!( | ||
| lower.as_str(), | ||
| "/var/www" | ||
| | "/var/www/html" | ||
| | "/var/www/htdocs" | ||
| | "/usr/share/nginx/html" | ||
| | "/usr/share/nginx" | ||
| | "/srv/www" | ||
| | "/srv/http" | ||
| | "/opt/homebrew/var/www" | ||
| ) || lower.ends_with("/public_html") | ||
| || lower.ends_with("/htdocs") | ||
| || lower.ends_with("/wwwroot") | ||
| // Trailing "/www" but not shorter false friends like "/www-data". | ||
| || lower.ends_with("/www") | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Reject descendants of public document roots too.
/var/www/html/uploads and /home/alice/public_html/bench pass today, yet the scratch payload remains web-accessible. Match protected roots on path-component boundaries and reject every descendant, not only the root itself.
Proposed fix
- matches!(
- lower.as_str(),
- "/var/www"
- | "/var/www/html"
- | "/var/www/htdocs"
- | "/usr/share/nginx/html"
- | "/usr/share/nginx"
- | "/srv/www"
- | "/srv/http"
- | "/opt/homebrew/var/www"
- ) || lower.ends_with("/public_html")
- || lower.ends_with("/htdocs")
- || lower.ends_with("/wwwroot")
- // Trailing "/www" but not shorter false friends like "/www-data".
- || lower.ends_with("/www")
+ const PUBLIC_ROOTS: &[&str] = &[
+ "/var/www", "/var/www/html", "/var/www/htdocs",
+ "/usr/share/nginx/html", "/usr/share/nginx",
+ "/srv/www", "/srv/http", "/opt/homebrew/var/www",
+ ];
+ PUBLIC_ROOTS.iter().any(|root| lower == *root || lower.starts_with(&format!("{root}/")))
+ || ["/public_html", "/htdocs", "/wwwroot", "/www"].iter().any(|suffix| {
+ lower.ends_with(suffix)
+ || lower.contains(&format!("{suffix}/"))
+ })📝 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
| fn is_public_web_document_root(remote_dir: &str) -> bool { | |
| let trimmed = remote_dir.trim().trim_end_matches('/'); | |
| let p = if trimmed.is_empty() { "/" } else { trimmed }; | |
| let lower = p.to_ascii_lowercase(); | |
| matches!( | |
| lower.as_str(), | |
| "/var/www" | |
| | "/var/www/html" | |
| | "/var/www/htdocs" | |
| | "/usr/share/nginx/html" | |
| | "/usr/share/nginx" | |
| | "/srv/www" | |
| | "/srv/http" | |
| | "/opt/homebrew/var/www" | |
| ) || lower.ends_with("/public_html") | |
| || lower.ends_with("/htdocs") | |
| || lower.ends_with("/wwwroot") | |
| // Trailing "/www" but not shorter false friends like "/www-data". | |
| || lower.ends_with("/www") | |
| } | |
| fn is_public_web_document_root(remote_dir: &str) -> bool { | |
| let trimmed = remote_dir.trim().trim_end_matches('/'); | |
| let p = if trimmed.is_empty() { "/" } else { trimmed }; | |
| let lower = p.to_ascii_lowercase(); | |
| const PUBLIC_ROOTS: &[&str] = &[ | |
| "/var/www", | |
| "/var/www/html", | |
| "/var/www/htdocs", | |
| "/usr/share/nginx/html", | |
| "/usr/share/nginx", | |
| "/srv/www", | |
| "/srv/http", | |
| "/opt/homebrew/var/www", | |
| ]; | |
| PUBLIC_ROOTS | |
| .iter() | |
| .any(|root| lower == *root || lower.starts_with(&format!("{root}/"))) | |
| || ["/public_html", "/htdocs", "/wwwroot", "/www"].iter().any(|suffix| { | |
| lower.ends_with(suffix) || lower.contains(&format!("{suffix}/")) | |
| }) | |
| } |
🤖 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/speedtest.rs` around lines 801 - 820, Update
is_public_web_document_root to treat each protected document root as a path
prefix on component boundaries, so the root itself and all descendants such as
uploads or bench are rejected. Preserve case-insensitive matching and avoid
matching unrelated names such as /www-data; centralize the protected-root checks
as needed within this function.
| "delete": "ИЗТРИВАНЕ", | ||
| "skip": "ПРОПУСКАНЕ", | ||
| "deleteConfirm": "Сигурни ли сте, че искате да изтриете {count} дублирани файла?" | ||
| "deleteConfirm": "Сигурни ли сте, че искате да изтриете {count} дублирани файла?", |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Handle the singular count correctly.
When {count} is 1, this renders as 1 дублирани файла, which is grammatically incorrect. Use pluralization support or neutral wording that remains correct for every count.
🤖 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/i18n/locales/bg.json` at line 3403, Update the deleteConfirm translation
in the Bulgarian locale so count 1 uses grammatically singular wording while
other counts remain correct, using the project’s existing pluralization
convention or neutral wording that works for every count.
| "skip": "এড়িয়ে যান", | ||
| "deleteConfirm": "আপনি কি নিশ্চিত যে আপনি {count} টি ডুপ্লিকেট ফাইল মুছতে চান?" | ||
| "deleteConfirm": "আপনি কি নিশ্চিত যে আপনি {count} টি ডুপ্লিকেট ফাইল মুছতে চান?", | ||
| "fuzzyCutoffPlaceholder": "স্বয়ং" |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use the Bengali translation for “automatic”.
স্বয়ং may be read as “self/oneself”; use স্বয়ংক্রিয় to match the existing common.auto translation and clearly communicate the default automatic value.
- "fuzzyCutoffPlaceholder": "স্বয়ং"
+ "fuzzyCutoffPlaceholder": "স্বয়ংক্রিয়"📝 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
| "fuzzyCutoffPlaceholder": "স্বয়ং" | |
| "fuzzyCutoffPlaceholder": "স্বয়ংক্রিয়" |
🤖 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/i18n/locales/bn.json` at line 3404, Update the Bengali value for
fuzzyCutoffPlaceholder from স্বয়ং to স্বয়ংক্রিয়, matching the existing
common.auto translation.
| "delete": "SMAZAT", | ||
| "skip": "PRESKOCIT", | ||
| "deleteConfirm": "Opravdu chcete smazat {count} duplicitnich souboru?" | ||
| "deleteConfirm": "Opravdu chcete smazat {count} duplicitnich souboru?", |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fix the Czech plural form in the deletion confirmation.
duplicitnich souboru is missing diacritics and uses the wrong plural ending; use duplicitních souborů.
Proposed fix
- "deleteConfirm": "Opravdu chcete smazat {count} duplicitnich souboru?",
+ "deleteConfirm": "Opravdu chcete smazat {count} duplicitních souborů?",📝 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
| "deleteConfirm": "Opravdu chcete smazat {count} duplicitnich souboru?", | |
| "deleteConfirm": "Opravdu chcete smazat {count} duplicitních souborů?", |
🤖 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/i18n/locales/cs.json` at line 3403, Update the Czech locale’s
deleteConfirm translation to use the corrected plural phrase “duplicitních
souborů” while preserving the rest of the confirmation message and its {count}
placeholder.
| "skip": "PRESKOCIT", | ||
| "deleteConfirm": "Naozaj chcete zmazat {count} duplicitnych suborov?" | ||
| "deleteConfirm": "Naozaj chcete zmazat {count} duplicitnych suborov?", | ||
| "fuzzyCutoffPlaceholder": "auto" |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Localize the fuzzy-cutoff placeholder.
Line [3404] is displayed in the Slovak UI but remains English. Use a Slovak equivalent such as automaticky or predvolené; this file already translates common.auto as Automatická.
Proposed fix
- "fuzzyCutoffPlaceholder": "auto"
+ "fuzzyCutoffPlaceholder": "automaticky"📝 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
| "fuzzyCutoffPlaceholder": "auto" | |
| "fuzzyCutoffPlaceholder": "automaticky" |
🤖 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/i18n/locales/sk.json` at line 3404, Update the fuzzyCutoffPlaceholder
value in the Slovak locale to a Slovak translation, reusing the existing
localized wording for common.auto where appropriate instead of leaving it as
“auto”.
| "skip": "PRESKOČI", | ||
| "deleteConfirm": "Ali ste prepričani, da želite izbrisati {count} podvojenih datotek?" | ||
| "deleteConfirm": "Ali ste prepričani, da želite izbrisati {count} podvojenih datotek?", | ||
| "fuzzyCutoffPlaceholder": "avto" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Use a Slovenian translation for “automatic.”
"avto" means “car” in Slovenian, so the fuzzy-cutoff input will show misleading placeholder text. Use "samodejno" (or the project’s established equivalent) instead.
Proposed fix
- "fuzzyCutoffPlaceholder": "avto"
+ "fuzzyCutoffPlaceholder": "samodejno"📝 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
| "fuzzyCutoffPlaceholder": "avto" | |
| "fuzzyCutoffPlaceholder": "samodejno" |
🤖 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/i18n/locales/sl.json` at line 3404, Update the fuzzyCutoffPlaceholder
translation in the Slovenian locale from the misleading “avto” value to
“samodejno” or the project’s established Slovenian equivalent for “automatic.”
Comment on lines
+3403
to
+3404
| "deleteConfirm": "Da li ste sigurni da želite da obrišete {count} duplikata fajlova?", | ||
| "fuzzyCutoffPlaceholder": "ауто" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Handle Serbian plural forms for the confirmation count.
For count = 1, this renders as “1 дупликата фајлова”, which is grammatically incorrect. Use the locale’s supported pluralization mechanism so singular and plural counts receive the correct Serbian forms.
🤖 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/i18n/locales/sr.json` around lines 3403 - 3404, Update the Serbian
locale’s deleteConfirm translation to use the locale’s supported pluralization
syntax for count, providing grammatically correct singular and plural forms
while preserving the existing confirmation message meaning.
| "skip": "RUKA", | ||
| "deleteConfirm": "Una uhakika unataka kufuta faili nakala {count}?" | ||
| "deleteConfirm": "Una uhakika unataka kufuta faili nakala {count}?", | ||
| "fuzzyCutoffPlaceholder": "auto" |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Localize the fuzzy-cutoff placeholder.
Line 3404 remains English ("auto"), while this locale already translates the same concept as "Kiotomatiki". Use the localized term so the duplicate finder’s input hint is consistent with Swahili.
Proposed fix
- "fuzzyCutoffPlaceholder": "auto"
+ "fuzzyCutoffPlaceholder": "Kiotomatiki"📝 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
| "fuzzyCutoffPlaceholder": "auto" | |
| "fuzzyCutoffPlaceholder": "Kiotomatiki" |
🤖 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/i18n/locales/sw.json` at line 3404, Update the fuzzyCutoffPlaceholder
value in the Swahili locale to use the existing localized term “Kiotomatiki”
instead of the English “auto”, keeping the duplicate finder input hint
consistent.
Comment on lines
+221
to
+225
| wins=$(count_lines "$WORK/noportal/windows-after.txt") | ||
| wins_ref=$(count_lines "$WORK/cancel/windows-after.txt") | ||
| [ "$wins" = "$wins_ref" ] && | ||
| ok "no extra window appears without a portal (measured: no native fallback)" || | ||
| bad "window count changed ($wins vs $wins_ref): the fallback behaviour is not what was pinned" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Cross-case reference can still produce a chooser-shaped FAIL. wins_ref comes from case 1's output. If cancel failed its precondition but noportal succeeded, count_lines yields 0 and this asserts "the fallback behaviour is not what was pinned" — exactly the misleading claim #521 removes.
🐛 Skip when the reference case has no output
wins=$(count_lines "$WORK/noportal/windows-after.txt")
wins_ref=$(count_lines "$WORK/cancel/windows-after.txt")
- [ "$wins" = "$wins_ref" ] &&
+ if [ ! -f "$WORK/cancel/windows-after.txt" ]; then
+ skip "no extra window appears without a portal (case 1 reference missing)"
+ else
+ [ "$wins" = "$wins_ref" ] &&
ok "no extra window appears without a portal (measured: no native fallback)" ||
bad "window count changed ($wins vs $wins_ref): the fallback behaviour is not what was pinned"
+ fi📝 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
| wins=$(count_lines "$WORK/noportal/windows-after.txt") | |
| wins_ref=$(count_lines "$WORK/cancel/windows-after.txt") | |
| [ "$wins" = "$wins_ref" ] && | |
| ok "no extra window appears without a portal (measured: no native fallback)" || | |
| bad "window count changed ($wins vs $wins_ref): the fallback behaviour is not what was pinned" | |
| wins=$(count_lines "$WORK/noportal/windows-after.txt") | |
| wins_ref=$(count_lines "$WORK/cancel/windows-after.txt") | |
| if [ ! -f "$WORK/cancel/windows-after.txt" ]; then | |
| skip "no extra window appears without a portal (case 1 reference missing)" | |
| else | |
| [ "$wins" = "$wins_ref" ] && | |
| ok "no extra window appears without a portal (measured: no native fallback)" || | |
| bad "window count changed ($wins vs $wins_ref): the fallback behaviour is not what was pinned" | |
| fi |
🧰 Tools
🪛 Shellcheck (0.11.0)
[info] 223-223: Note that A && B || C is not if-then-else. C may run when A is true.
(SC2015)
🤖 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 `@tests/portal-chooser/portal-chooser-test.sh` around lines 221 - 225, Guard
the comparison in the noportal validation using wins_ref so it is skipped when
the cancel reference case produced no output, rather than reporting a fallback
mismatch. Preserve the existing equality check and success/failure messages when
wins_ref is nonzero, updating the surrounding logic in the windows-after
comparison block.
CodeRabbit Majors on #519, #505 and #504 that stayed unresolved after merge. Each change is pinned by a test that fails when the defect is reintroduced. 1. nativeE2e catalogue matcher (#519). The single-line `[^\n]*?` pattern only saw PROTOCOLS_FALLBACK; the live PROTOCOLS list puts type and badge on different lines. Entry-bounded extraction by successive type fields, plus a synthetic multiline pin that shows the old pattern matching nothing. 2. assert-private-a11y empty bus (#505). Already fail-closed (exit 7); extracted classify_names and a --selftest pin so "empty means pass" cannot return without a failing assertion. 3. fake-portal NetworkMonitor v3 (#505). GetStatus/CanReach already implemented; probe + selftest now exercise CanReach, and a source pin refuses a stand-in that advertises v3 without that surface. 4. speedtest scratch mkdir (#504). provider.mkdir was outside run_cancelable, so a stalled provider hung the test before upload. Cancellation now aborts with an explicit message; ordinary mkdir errors stay soft. Structure pin + hanging-future pin. Gates: vitest nativeE2e 9/9, python --selftest, fake-portal selftest 14/14, cargo test speedtest::tests 14/14, cargo fmt, clippy -D warnings --lib, tsc --noEmit. Co-Authored-By: Grok L2 <noreply@x.ai>
The residual Major on #504: a payload under /var/www/html/.aeroftp-speedtest is still servable. Reject known document roots before any network work; keep account root (/) allowed as the SFTP/S3 default, still nested under .aeroftp-speedtest/. Pin: public_web_document_roots_are_rejected (and the confined-path test no longer treats /var/www/html as an allowed dir). Co-Authored-By: Grok L2 <noreply@x.ai>
Align Drime free tier to 20 GB (official), surface open/reveal failures in DuplicateFinderDialog instead of silent catch, fix FileLu trash empty date label so TrashTable shows '-', and add alt text on the provider logo grid (Drime and peers) via the generated catalog. Pins: drimeQuota, fileLuDeletedLabel, DuplicateFinderDialog open-handlers.
Non-identical signature pass used to break on budget exhaustion, so a 3 GB file at the head of the list starved every smaller candidate. Skip oversize/over-budget files and keep walking. After a local recursive scan, emit an unconditional sync_scan_progress tick so small trees still report dirs_found and bytes_found. Pins: oversized_head_file_does_not_starve_remaining_candidates, small_tree_final_local_progress_payload_reports_dirs_and_bytes.
Add duplicates.fuzzyCutoffPlaceholder (synced to all 46 locales) for the hard-coded 'auto' in DuplicateFinderDialog. Clarify cy/sr sortWasted and vi/mk fuzzyCutoff labels per review on merged #519.
(a) ConnectMetadata::from_config never sees server_info; rename the IPv6 test to what it actually pins and drop the tautological home: assert. (b) interactive New(N) now pages catalog picks (20/page, Enter=next) so a bare n no longer dumps 65 rows; offset was already implemented and tested but the caller always passed 0. Pins: connect_metadata_preserves_ipv6_host_and_nonstandard_port, catalog_pick_page_caps_empty_query_and_covers_all_rows.
…453) Drag state used full-vault indices. With a sidebar protocol chip the visible rows had gaps in that index space, so table drop rewrote slots the user was not looking at and the gesture felt dead. Drag/drop now keys on the visible (filtered) list and writes the new order back into the vault via reorderVisibleInFull, preserving non-visible profiles. Also centralise inherit-index move (no classic to-1) in reorderByIndex for SavedServers and My Servers. Pins: reorderByIndex.test.ts (9).
) When a portal scenario fails its readiness precondition, the gate used to keep asserting on chooser evidence that a failed session could not produce. Report one session failure, skip the dependent chooser claims, and optionally retry the scenario once (logged). Pin the shape with selftest-precondition.sh (no app build) and wire it into the portal workflow selftest job.
axpnet
force-pushed
the
fix/major-reviews-post-merge
branch
from
July 30, 2026 09:53
0d3d440 to
40a5a88
Compare
axpnet
added a commit
that referenced
this pull request
Jul 30, 2026
- disconnect the provider when the run is cancelled while creating the scratch directory: connect() already succeeded at that point and every other post-connect error path disconnects (pin: cancel_after_connect_disconnects_provider) - reject descendants of public web document roots, not just exact roots: prefix match on a path-component boundary plus per-segment match for public_html/htdocs/wwwroot/www; false friends like /var/www-data stay allowed (extended pin: public_web_document_roots_are_rejected)
axpnet
added a commit
that referenced
this pull request
Jul 30, 2026
When the case 1 (cancel) session fails its precondition, its windows-after.txt is never written and count_lines quietly reads it as 0, turning a missing baseline into a chooser-shaped FAIL on the noportal case. Missing baseline is a skip, same contract as the #521 cascade-stop (CodeRabbit Minor from #525).
axpnet
added a commit
that referenced
this pull request
Jul 30, 2026
fix(speedtest,portal): CodeRabbit Majors from #525 + wins_ref skip
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.
Post-merge cleanup that was sitting as nine local commits on
fix/major-reviews-post-merge. Review findings left open on already-merged PRs, plus a few product/CI fixes that landed in the same worktree.What this is
Nine commits on top of current
main:8b2eb8922e7b6c3c2fremote_dir190b130ed2fbe19a41375dea8b526e0644b370a4b630d800c57c52selftest-preconditionpin0d3d44014timeout-minutes: 12onInstall system dependencies, job cap 25→35 on both Ubuntu jobsParallel PRs (please do not merge blindly on top of each other)
Other tabs already opened dedicated PRs for some of the same issues:
delta-sync-integration.ymlat merge time.#521commit also adds workflow step +selftest-precondition.sh+ README. Prefer one coherent portal-gate shape.Not in this PR
Test plan
delta-sync-integration.ymlselftest-precondition.sh6/6,selftest-portal.sh14/14 (at Portal file chooser gate: a cold start over 10s fails as eleven chooser assertions #521 commit)reorderByIndex9/9 (at [BUG] Manually sorting profiles in the GUI lands them in the wrong index number #453 commit)Summary by CodeRabbit