Conversation
Introduce a unified ConnectionProfile model and UI to support direct, proxy-basic and Pangolin connection modes. Added lib/connection.ts with normalization, validation, token parsing and candidate URL generation; new ConnectionProfileForm component and updated LoginPage/MorePage to use it. Refactored ApiClient and API factories to accept a ConnectionProfile (adds proxy basic auth headers, Pangolin token decoration, ws URL building and improved response handling) and updated health/terminal APIs accordingly. Reworked ApiContext to persist/ migrate profiles, attempt multiple connection candidates with better validation/error messages, and expose api/connectionProfile; OfflineConnectionBanner now checks reachability via the API. Enhanced useTerminal to support async WS URL generation and pre-open auth refresh retries. Mobile platform tweaks: Android build enables minification/shrinking and conditional signing, capacitor config uses http/android localhost scheme, and MainActivity no longer forces mixed content. Tests and minor UI/diagnostic fixes included.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a robust connection profile system, enabling support for direct connections, basic auth proxies, and Pangolin authentication. Key changes include the addition of a ConnectionProfileForm component, a significant refactor of ApiContext and ApiClient to manage these connection modes, and updates to the terminal hook for session refreshing. The PR also includes health API improvements to prevent crashes on missing data and updates to the Android build configuration. Feedback identifies a logic error in the advanced settings visibility toggle, a potential encoding issue with Basic Auth credentials, and an unimplemented options parameter in the WebSocket URL generation.
| const [showAdvanced, setShowAdvanced] = useState(value.pangolinTokenParam !== DEFAULT_PANGOLIN_TOKEN_PARAM); | ||
| const shouldShowAdvanced = showAdvanced || value.pangolinTokenParam !== DEFAULT_PANGOLIN_TOKEN_PARAM; |
There was a problem hiding this comment.
The logic for shouldShowAdvanced prevents the advanced section from being hidden if a custom pangolinTokenParam is set. Because it uses an OR condition with the non-default check, shouldShowAdvanced will remain true even when showAdvanced is toggled to false. This also causes the toggle button text to be stuck on 'Hide'.
| const [showAdvanced, setShowAdvanced] = useState(value.pangolinTokenParam !== DEFAULT_PANGOLIN_TOKEN_PARAM); | |
| const shouldShowAdvanced = showAdvanced || value.pangolinTokenParam !== DEFAULT_PANGOLIN_TOKEN_PARAM; | |
| const [showAdvanced, setShowAdvanced] = useState(value.pangolinTokenParam !== DEFAULT_PANGOLIN_TOKEN_PARAM); | |
| const shouldShowAdvanced = showAdvanced; |
| function encodeBasicCredentials(username: string, password: string): string { | ||
| return btoa(username + ':' + password); | ||
| } |
There was a problem hiding this comment.
The btoa function only supports characters in the Latin1 range. If a user provides a proxy username or password containing non-Latin1 characters (e.g., emojis or certain international characters), this will throw a DOMException. Encoding the string to UTF-8 before passing it to btoa ensures compatibility.
| function encodeBasicCredentials(username: string, password: string): string { | |
| return btoa(username + ':' + password); | |
| } | |
| function encodeBasicCredentials(username: string, password: string): string { | |
| return btoa(unescape(encodeURIComponent(username + ':' + password))); | |
| } |
| return; | ||
| } | ||
|
|
||
| async getWebSocketUrl(path: string, params?: RequestOptions['params'], _options?: WebSocketUrlOptions): Promise<string> { |
There was a problem hiding this comment.
The _options parameter (specifically forceRefreshAuth) is currently ignored in getWebSocketUrl. This renders the refresh logic in useTerminal.ts ineffective, as it will not trigger any session update before attempting to build the WebSocket URL. If Pangolin requires session management, this parameter should be used to call ensurePangolinSession(true).
![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: 33f3859cfd
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return; | ||
| } | ||
|
|
||
| async getWebSocketUrl(path: string, params?: RequestOptions['params'], _options?: WebSocketUrlOptions): Promise<string> { |
There was a problem hiding this comment.
ApiClient.getWebSocketUrl is now async, so callers that were not updated now receive a Promise instead of a URL string. createLogsApi.streamUrl still forwards this value and LogsPage passes it directly to new WebSocket(...), which throws (Invalid URL / [object Promise]) and breaks live log streaming for all environments using that path. Either keep getWebSocketUrl synchronous or update every consumer (notably logs streaming) to await it before constructing the socket.
Useful? React with 👍 / 👎.
1 participant
No description provided.