fix(storage): Fixed an issue with the SFTP path history#1077
fix(storage): Fixed an issue with the SFTP path history#1077GT-610 merged 2 commits intolollipopkit:mainfrom
Conversation
Added path validity checks when processing the SFTP path history Standardized the format of stored paths and removed extra slashes
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe PR normalizes SFTP paths (collapsing repeated Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5784e0bd06
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
lib/view/page/storage/sftp.dart
Outdated
| final sftp = await _client.sftp(); | ||
| await sftp.listdir(history); |
There was a problem hiding this comment.
When sftpOpenLastPath is enabled and a history entry exists, this opens a fresh SftpClient just to run listdir(history) and then drops it. In dartssh2, SSHClient.sftp() opens a new SFTP session, and _listDir() later uses _status.client/_client.sftp() for the real browse session, so the instance created here is neither reused nor closed. Re-entering the SFTP page a few times on the same SSH connection can therefore accumulate orphaned channels until the server starts rejecting new SFTP work.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
lib/view/page/storage/sftp.dart (1)
647-651: Use a shared path-normalization helper here for consistency.This loop works, but sharing a single normalization helper with the restore path avoids duplicated logic and keeps canonicalization behavior consistent.
Refactor snippet
- var normalizedPath = listPath; - while (normalizedPath.contains('//')) { - normalizedPath = normalizedPath.replaceAll('//', '/'); - } + final normalizedPath = _normalizeSftpPath(listPath); Stores.history.sftpLastPath.put(widget.args.spi.id, normalizedPath);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/view/page/storage/sftp.dart` around lines 647 - 651, The loop that collapses duplicate slashes for normalizedPath (starting from listPath) should be replaced with the shared path-normalization helper used elsewhere (the same helper used for the restore path) to avoid duplicated logic and keep canonicalization consistent; locate the code around normalizedPath/listPath and replace the manual while-replaceAll logic with a call to the common helper (e.g., normalizePath or canonicalizePath) before calling Stores.history.sftpLastPath.put(widget.args.spi.id, normalizedPath).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/view/page/storage/sftp.dart`:
- Around line 102-105: The stored raw path from
Stores.history.sftpLastPath.fetch(...) must be normalized before validation and
assignment: retrieve the fetched value, normalize it to collapse duplicate
slashes and remove trailing slash (except root) (e.g., use the app's
path-normalization helper or implement simple normalization), then call await
_client.sftp() and await sftp.listdir(normalizedPath) to validate; only on
successful validation assign initPath = normalizedPath instead of the raw
fetched value.
---
Nitpick comments:
In `@lib/view/page/storage/sftp.dart`:
- Around line 647-651: The loop that collapses duplicate slashes for
normalizedPath (starting from listPath) should be replaced with the shared
path-normalization helper used elsewhere (the same helper used for the restore
path) to avoid duplicated logic and keep canonicalization consistent; locate the
code around normalizedPath/listPath and replace the manual while-replaceAll
logic with a call to the common helper (e.g., normalizePath or canonicalizePath)
before calling Stores.history.sftpLastPath.put(widget.args.spi.id,
normalizedPath).
🪄 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: CHILL
Plan: Pro
Run ID: 426da587-fa6e-49a8-bca9-d72a6d62cb27
📒 Files selected for processing (1)
lib/view/page/storage/sftp.dart
…management Extract path normalization logic into a separate method: _normalizeSftpPath Ensure that the SFTP client is properly closed after use
1 participant
Resolve #1030.
Added path validity checks when processing the SFTP path history
Standardized the format of stored paths and removed extra slashes
Summary by CodeRabbit