Bump browser ssh versions for ssh rewrite#510
Conversation
WalkthroughThe PR replaces version parsing with semantic version comparison, adds SSH server type detection with conditional JWT handling, updates the default WASM package path to v0.60.0, introduces version testing, and removes the cypress devDependency. These changes modernize version logic and improve SSH configuration. Changes
Sequence Diagram(s)sequenceDiagram
participant React as React Component
participant useSSH
participant client as SSH Client
participant conn as SSH Connection
React->>useSSH: connect(config, accessToken)
useSSH->>client: detectSSHServerType(hostname, port)
client-->>useSSH: requiresJwt: boolean
alt requiresJwt = true
useSSH->>conn: createSSHConnection(config, accessToken)
else requiresJwt = false
useSSH->>conn: createSSHConnection(config)
end
conn-->>useSSH: connection
useSSH-->>React: connection ready
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/utils/version.ts (1)
39-63: Consider documenting pre-release version handling behavior.The
compareVersionsfunction handles basic semantic versions correctly. However, the behavior with pre-release suffixes (e.g., "v0.60.0-beta") may not be immediately obvious:
- Line 48:
split(".").map(Number)converts "0.60.0-beta" to[0, 60, NaN]- Lines 54-60: NaN comparisons always return false, causing the function to fall through and return
true- This treats "0.60.0-beta" as
>= "0.60.0", which appears intentional for feature support checksWhile this works for the current test cases and use cases, consider adding a comment explaining this behavior, or explicitly handle pre-release versions if stricter semver compliance is needed in the future.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
package.json(0 hunks)src/modules/peer/PeerSSHInstructions.tsx(2 hunks)src/modules/peers/PeerVersionCell.tsx(2 hunks)src/modules/remote-access/ssh/useSSH.ts(2 hunks)src/utils/config.ts(1 hunks)src/utils/version.test.ts(1 hunks)src/utils/version.ts(2 hunks)
💤 Files with no reviewable changes (1)
- package.json
🧰 Additional context used
🧬 Code graph analysis (2)
src/utils/version.test.ts (1)
src/utils/version.ts (1)
isNativeSSHSupported(83-86)
src/modules/peers/PeerVersionCell.tsx (1)
src/utils/version.ts (1)
compareVersions(43-63)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build_n_push
- GitHub Check: build_n_push
🔇 Additional comments (7)
src/modules/peer/PeerSSHInstructions.tsx (1)
42-42: LGTM!The simplified description is clearer and more concise, focusing on the core purpose without implementation details.
src/modules/remote-access/ssh/useSSH.ts (2)
43-53: LGTM!The SSH server type detection and conditional JWT authentication are well-implemented. The error handling correctly catches exceptions from both
detectSSHServerTypeandcreateSSHConnection.
74-74: LGTM!Correctly added
accessTokento the dependency array since it's now used within the callback. This ensures the callback is recreated when the token changes, preventing stale closure issues.src/utils/version.test.ts (1)
1-34: Excellent test coverage!The test suite comprehensively covers edge cases including:
- Versions with/without "v" prefix
- Pre-release suffixes (beta, rc)
- Development builds
- Boundary conditions around v0.60.0
The CI-friendly exit code handling ensures build failures on test failures.
src/modules/peers/PeerVersionCell.tsx (1)
10-10: LGTM!The migration to
compareVersionsis implemented correctly:
- Line 34: The guard for missing
latestVersionprevents runtime errors- Line 35: The logic
!compareVersions(version, latestVersion)correctly identifies whenversion < latestVersion(update available)The import reordering has no functional impact.
Also applies to: 17-17, 34-35
src/utils/version.ts (2)
71-76: LGTM!The migration to
compareVersionsmaintains the same logic while using the new semantic version comparison approach.
78-86: LGTM!The native SSH support check now correctly uses the new
compareVersionsfunction with the v0.60.0 threshold, aligning with the PR's objective to bump SSH versions for the SSH rewrite.
Summary by CodeRabbit
New Features
Improvements
Chores