feat: add ProxyCommand support for SSH connections#1117
Conversation
📝 WalkthroughWalkthroughAdds support for SSH ProxyCommand connections. Introduces Possibly related PRs
Suggested reviewers
🚥 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)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the 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 |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/l10n/app_zh.arb`:
- Around line 277-280: The ARB keys added in app_zh.arb (e.g.,
"jumpServerAndProxyCommandCannotBeUsedTogether",
"proxyCommandOnlySupportedOnDesktop") are not reflected in the generated Dart
localization files; run `flutter gen-l10n` to regenerate the localization
outputs so the typed accessors and updated LocalizationsDelegates include these
keys, then add the generated Dart files to the repo (commit the files under the
lib/l10n output directory), and re-run tests/CI to ensure there are no
missing-localization errors; always regenerate (`flutter gen-l10n`) after
modifying any lib/l10n/*.arb.
In `@lib/view/page/server/edit/actions.dart`:
- Around line 69-72: Trim the proxy command text before checking desktop gating
and before saving: replace uses of _proxyCommandCtrl.text.isNotEmpty with
_proxyCommandCtrl.text.trim().isNotEmpty (and use trimmed value when assigning
proxyCommand) so whitespace-only input is treated as empty; apply the same
change for the other occurrence around the save logic at the second check (the
block at ~120) to ensure both gating and persistence use the trimmed string.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 63e6ccde-85e8-4c3c-8130-2e46fa77cb42
⛔ Files ignored due to path filters (15)
lib/generated/l10n/l10n.dartis excluded by!**/generated/**lib/generated/l10n/l10n_de.dartis excluded by!**/generated/**lib/generated/l10n/l10n_en.dartis excluded by!**/generated/**lib/generated/l10n/l10n_es.dartis excluded by!**/generated/**lib/generated/l10n/l10n_fr.dartis excluded by!**/generated/**lib/generated/l10n/l10n_id.dartis excluded by!**/generated/**lib/generated/l10n/l10n_it.dartis excluded by!**/generated/**lib/generated/l10n/l10n_ja.dartis excluded by!**/generated/**lib/generated/l10n/l10n_ko.dartis excluded by!**/generated/**lib/generated/l10n/l10n_nl.dartis excluded by!**/generated/**lib/generated/l10n/l10n_pt.dartis excluded by!**/generated/**lib/generated/l10n/l10n_ru.dartis excluded by!**/generated/**lib/generated/l10n/l10n_tr.dartis excluded by!**/generated/**lib/generated/l10n/l10n_uk.dartis excluded by!**/generated/**lib/generated/l10n/l10n_zh.dartis excluded by!**/generated/**
📒 Files selected for processing (8)
lib/core/utils/proxy_command_socket.dartlib/core/utils/server.dartlib/core/utils/ssh_config.dartlib/data/model/server/server_private_info.dartlib/data/provider/server/all.dartlib/l10n/app_en.arblib/l10n/app_zh.arblib/view/page/server/edit/actions.dart
✅ Files skipped from review due to trivial changes (1)
- lib/l10n/app_en.arb
🚧 Files skipped from review as they are similar to previous changes (2)
- lib/core/utils/ssh_config.dart
- lib/core/utils/proxy_command_socket.dart
There was a problem hiding this comment.
🧹 Nitpick comments (1)
lib/data/model/server/server_private_info.dart (1)
85-87: Avoid hard-coded English exception text in the model layer.This literal should be represented as a typed/code error and localized when rendered, instead of embedding a fixed English message here.
♻️ Proposed refactor
+class SpiValidationException implements Exception { + final SpiValidationError error; + const SpiValidationException(this.error); +} + extension Spix on Spi { void validateOrThrow() { final validationError = validate(); if (validationError == null) return; - switch (validationError) { - case SpiValidationError.jumpServerAndProxyCommandConflict: - throw ArgumentError( - 'Jump server and ProxyCommand cannot be used together.', - ); - } + throw SpiValidationException(validationError); } }As per coding guidelines
lib/**/*.dart: "Use libL10n and l10n for localization strings, prioritizing libL10n from fl_lib package to avoid duplication".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/data/model/server/server_private_info.dart` around lines 85 - 87, Replace the hard-coded English ArgumentError message in server_private_info.dart with a typed error and a localization key: define/throw a specific exception (e.g., JumpServerProxyCommandConflict or ArgumentError with a distinct errorCode field) from the same module instead of the literal string, and move the human-readable message into libL10n/l10n (prefer libL10n from fl_lib) so UI code can localize it when rendering; update any callers/tests to check the typed error or errorCode rather than the exact English text.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@lib/data/model/server/server_private_info.dart`:
- Around line 85-87: Replace the hard-coded English ArgumentError message in
server_private_info.dart with a typed error and a localization key: define/throw
a specific exception (e.g., JumpServerProxyCommandConflict or ArgumentError with
a distinct errorCode field) from the same module instead of the literal string,
and move the human-readable message into libL10n/l10n (prefer libL10n from
fl_lib) so UI code can localize it when rendering; update any callers/tests to
check the typed error or errorCode rather than the exact English text.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 8a4bc23a-2e78-4a84-b145-20b25cbf3616
📒 Files selected for processing (3)
lib/core/utils/proxy_command_socket.dartlib/data/model/server/server_private_info.dartlib/view/page/server/edit/actions.dart
🚧 Files skipped from review as they are similar to previous changes (2)
- lib/view/page/server/edit/actions.dart
- lib/core/utils/proxy_command_socket.dart
No issue now: the hard-coded English ArgumentError message in server_private_info.dart has already been replaced with a typed SpiValidationException, and UI rendering is localized in actions.dart. |
1 participant
Resolve #949.
Resolve #803.
Summary by CodeRabbit
New Features
Bug Fixes / Validation
Localization