-
Notifications
You must be signed in to change notification settings - Fork 14
Add Qwen OAuth support with email pre-collection #14
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Add Qwen authentication with browser-based OAuth flow - Implement pre-auth email collection dialog for seamless UX - Add automatic email submission after OAuth completion - Add Qwen service section to settings UI with icon - Add auth status tracking for Qwen credentials - Increase settings window height (440px β 490px) - Update README and CHANGELOG for v1.0.6 Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
WalkthroughAdds Qwen OAuth support across docs, auth status model, server auth command/flow (including email capture and auto-fill), and settings UI. Introduces Qwen connection management (connect/reconnect/disconnect), status tracking, and a Qwen email prompt. Updates CHANGELOG and README. Adjusts settings window height. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant SettingsView
participant ServerManager
participant AuthProcess as Auth CLI/Process
participant OAuthBrowser as OAuth Browser
participant AuthStatus as Auth Status
User->>SettingsView: Click "Connect Qwen"
SettingsView->>SettingsView: Show email prompt
User->>SettingsView: Enter email
SettingsView->>ServerManager: qwenLogin(email)
ServerManager->>AuthProcess: Start process with -qwen-login
AuthProcess-->>OAuthBrowser: Open Qwen OAuth URL
OAuthBrowser-->>AuthProcess: OAuth callback (success)
note over ServerManager,AuthProcess #e5f5e0: wait ~10s then auto-send email
ServerManager->>AuthProcess: Write "<email>\n" to stdin
AuthProcess-->>ServerManager: Auth result
ServerManager->>AuthStatus: Update qwenStatus (success/error)
AuthStatus-->>SettingsView: Published status update
SettingsView-->>User: Show Connected / Error
Estimated code review effortπ― 3 (Moderate) | β±οΈ ~25 minutes Poem
Pre-merge checks and finishing touchesβ Failed checks (1 warning)
β Passed checks (4 passed)
β¨ Finishing touches
π§ͺ 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
π§Ή Nitpick comments (3)
src/Sources/ServerManager.swift (1)
262-262: Consider using forced String to Data conversion.SwiftLint suggests using the non-optional
Data(_:)initializer when converting String to Data, as UTF-8 encoding of a Swift String should never fail.Apply this diff:
- if let data = "\(email)\n".data(using: .utf8) { - try? inputPipe.fileHandleForWriting.write(contentsOf: data) - NSLog("[Auth] Sent Qwen email: %@", email) - } + let data = Data("\(email)\n".utf8) + try? inputPipe.fileHandleForWriting.write(contentsOf: data) + NSLog("[Auth] Sent Qwen email: %@", email)src/Sources/SettingsView.swift (2)
310-335: Consider adding email format validation.The email prompt only validates that the field is not empty (line 329). Adding basic format validation would improve user experience by catching typos before initiating the OAuth flow.
You could add a simple validation helper:
private func isValidEmail(_ email: String) -> Bool { let emailPattern = #"^[A-Z0-9a-z._%+-]+@[A-Za-z0-9.-]+\.[A-Za-z]{2,}$"# return email.range(of: emailPattern, options: .regularExpression) != nil }Then update the Continue button:
Button("Continue") { showingQwenEmailPrompt = false startQwenAuth(email: qwenEmail) } - .disabled(qwenEmail.isEmpty) + .disabled(qwenEmail.isEmpty || !isValidEmail(qwenEmail)) .keyboardShortcut(.defaultAction)
202-243: Consider extracting service UI into a reusable component.The Qwen service section (lines 202-243) duplicates the same pattern used for Claude, Codex, and Gemini. While this works, extracting a reusable
ServiceRowcomponent would reduce duplication and make future service additions easier to maintain.Example structure:
struct ServiceRow: View { let iconName: String let serviceName: String let status: AuthStatus let isAuthenticating: Bool let onConnect: () -> Void let onDisconnect: () -> Void var body: some View { // Common UI structure } }
π Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
β Files ignored due to path filters (1)
src/Sources/Resources/icon-qwen.pngis excluded by!**/*.png
π Files selected for processing (5)
CHANGELOG.md(2 hunks)README.md(4 hunks)src/Sources/AuthStatus.swift(5 hunks)src/Sources/ServerManager.swift(3 hunks)src/Sources/SettingsView.swift(4 hunks)
π§° Additional context used
𧬠Code graph analysis (1)
src/Sources/SettingsView.swift (2)
src/Sources/IconCatalog.swift (1)
image(12-37)src/Sources/ServerManager.swift (1)
runAuthCommand(203-325)
πͺ LanguageTool
README.md
[grammar] ~37-~37: There might be a mistake here.
Context: ..., Gemini, and Qwen directly from the app - π Real-Time Status - Live connectio...
(QB_NEW_EN)
CHANGELOG.md
[grammar] ~10-~10: There might be a mistake here.
Context: ...ml). ## [1.0.6] - 2025-10-15 ### Added - Qwen Support - Full integration with Q...
(QB_NEW_EN)
[grammar] ~11-~11: There might be a mistake here.
Context: ...on with Qwen AI via OAuth authentication - Browser-based Qwen OAuth flow with autom...
(QB_NEW_EN)
[grammar] ~12-~12: There might be a mistake here.
Context: ...uth flow with automatic email submission - Pre-authentication email collection dial...
(QB_NEW_EN)
[grammar] ~13-~13: There might be a mistake here.
Context: ... email collection dialog for seamless UX - Automatic credential file creation with ...
(QB_NEW_EN)
[grammar] ~14-~14: There might be a mistake here.
Context: ...edential file creation with type: "qwen" - Connection status display with email and...
(QB_NEW_EN)
[grammar] ~15-~15: There might be a mistake here.
Context: ...splay with email and expiration tracking - Qwen added to end of service providers l...
(QB_NEW_EN)
[grammar] ~18-~18: There might be a mistake here.
Context: ... of service providers list ### Improved - Settings Window - Increased height fro...
(QB_NEW_EN)
[grammar] ~19-~19: There might be a mistake here.
Context: ...Increased height from 440px to 490px to accommodate Qwen service section ## [1.0.5] - 2025...
(QB_NEW_EN)
πͺ SwiftLint (0.57.0)
src/Sources/ServerManager.swift
[Warning] 262-262: Prefer non-optional Data(_:) initializer when converting String to Data
(non_optional_string_data_conversion)
π Additional comments (6)
CHANGELOG.md (1)
8-20: LGTM!The changelog entry accurately documents the Qwen OAuth integration with clear, concise bullet points covering all major features and improvements.
src/Sources/AuthStatus.swift (1)
31-31: LGTM!The Qwen authentication status tracking follows the exact same pattern as existing services (Claude, Codex, Gemini), maintaining excellent consistency throughout the codebase.
Also applies to: 40-40, 84-87, 109-112, 121-121
README.md (1)
14-14: LGTM!Documentation updates consistently reflect Qwen support across all relevant sections (features, usage, requirements, and assets).
Also applies to: 19-19, 37-37, 68-68, 109-110
src/Sources/ServerManager.swift (1)
222-234: LGTM!The qwenLogin command implementation correctly follows the established pattern for authentication commands, properly storing the email for later use.
src/Sources/SettingsView.swift (2)
475-510: LGTM!The Qwen authentication methods (connectQwen, startQwenAuth, disconnectQwen) correctly implement the email collection flow and follow the established patterns for other services.
309-309: LGTM!The window height increase from 440px to 490px appropriately accommodates the new Qwen service section.
- Increase OAuth completion delay from 5s to 10s for more reliable timing - Add detailed comment explaining why 10s was chosen - Document that OAuth typically completes in 5-8s but can vary - Note future improvement: monitor OAuth completion signal directly Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
π§Ή Nitpick comments (1)
src/Sources/ServerManager.swift (1)
257-272: Good improvements from the previous review.The 10-second delay and comprehensive documentation address the previous timing concerns. The approach is reasonable for an initial implementation.
Optional: Address the SwiftLint warning.
Line 266: SwiftLint prefers the non-optional
Data(_:)initializer. While the optional version is safe for UTF-8 strings, the non-optional form is more idiomatic.Apply this diff:
- if let data = "\(email)\n".data(using: .utf8) { - try? inputPipe.fileHandleForWriting.write(contentsOf: data) + let data = Data("\(email)\n".utf8) + try? inputPipe.fileHandleForWriting.write(contentsOf: data) NSLog("[Auth] Sent Qwen email: %@", email) - }Note on error handling: The
try?pattern silently ignores pipe write failures. This is consistent with the existing Gemini flow (line 250), but for a critical auth step, consider logging failures in future iterations to help diagnose issues if users report incomplete authentication.
π Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
π Files selected for processing (1)
src/Sources/ServerManager.swift(3 hunks)
π§° Additional context used
πͺ SwiftLint (0.57.0)
src/Sources/ServerManager.swift
[Warning] 266-266: Prefer non-optional Data(_:) initializer when converting String to Data
(non_optional_string_data_conversion)
π Additional comments (3)
src/Sources/ServerManager.swift (3)
222-223: LGTM!The variable declaration is clean and appropriately scoped for capturing the email parameter from the auth command.
231-233: LGTM!The Qwen login case handling is implemented consistently with the other authentication commands and correctly captures the email for later submission.
395-395: LGTM!The enum case definition is clean and follows Swift conventions for associated values.
π New Feature: Qwen Support
This PR adds full OAuth integration for Qwen AI, allowing users to authenticate and use Qwen with VibeProxy.
β¨ Key Features
π§ Technical Details
Changes Made
AuthStatus.swift
qwenStatustrackingServerManager.swift
qwenLogin(email: String)auth command-qwen-loginCLI flag supportSettingsView.swift
Resources
icon-qwen.png(4.8K)Documentation
π§ͺ Testing
β Tested successfully:
π User Flow
π Related Issues
Closes #10
Summary by CodeRabbit
New Features
Improvements
Documentation