Repository navigation
Support TCP servers in native app discover page - #149
Merged
Merged
Conversation
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughAdds Tauri runtime detection and extends server discovery to support separate Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant HomeScreen
participant PlatformDetector
participant AddServerModal
participant Store
User->>HomeScreen: click server entry
HomeScreen->>PlatformDetector: isTauri()
PlatformDetector-->>HomeScreen: true/false
HomeScreen->>HomeScreen: select protocol (ircs vs wss), parse host/port, decide useWebSocket & lockWebSocket
HomeScreen->>AddServerModal: open(prefill: host, port, useWebSocket, lockWebSocket, title)
AddServerModal->>Store: dispatch save/update (if user confirms)
Store-->>AddServerModal: ack
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
|
Automated deployment preview for the PR in the Cloudflare Pages. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/ui/HomeScreen.tsx (1)
42-55:⚠️ Potential issue | 🟠 Major
new URL()calls are unguarded and an absent URI leaves the modal in a broken state.Two problems in
handleServerClick:
Unhandled
TypeErrorfrom invalid URLs. Bothnew URL(uri).hostname(line 47) and thenew URL(uri)insideuriPort(line 40) throw ifuriis not a well-formed URL. Theuri ?guard only catches falsy values, not malformed strings sourced from the remote JSON. One bad entry in the server list will crash the click handler silently.Empty host with disabled field. When the relevant URI field is absent (e.g., a server only has
wssbut noircs, and the user is running in Tauri),uriisundefined, sohostbecomes"". Combined withdisableServerConnectionInfo: true, the modal opens with a locked, empty host — the server is unconnectable and the user has no way to fix it.🐛 Proposed fix
const handleServerClick = (server: { name: string; description: string; wss?: string; ircs?: string }) => { const uri = isTauri() ? server.ircs : server.wss; const defaultPort = isTauri() ? "6697" : "443"; + if (!uri) { + // This environment (browser vs native) has no supported URI for this server. + toggleAddServerModal(true, { + name: server.name, + host: "", + port: defaultPort, + nickname: "", + ui: { title: server.name }, + }); + return; + } + + let hostname: string; + let port: string; + try { + const parsed = new URL(uri); + hostname = parsed.hostname; + port = parsed.port || defaultPort; + } catch { + console.error(`Invalid server URI: ${uri}`); + return; + } + toggleAddServerModal(true, { name: server.name, - host: uri ? new URL(uri).hostname : "", - port: uri ? uriPort(uri, defaultPort) : defaultPort, + host: hostname, + port, nickname: "", ui: { disableServerConnectionInfo: true, title: server.name, }, }); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ui/HomeScreen.tsx` around lines 42 - 55, The click handler handleServerClick currently calls new URL(uri) (and uriPort which also constructs a URL) without guarding against malformed strings, causing a TypeError and/or producing an empty, locked host when uri is undefined; wrap URL parsing in a try/catch and validate uri before using new URL, falling back to a safe host value (e.g., server.host || server.name || "") and a sane port default when parsing fails, and ensure toggleAddServerModal is called with disableServerConnectionInfo: false if the host is empty so the user can edit connection info; also update uriPort (the function invoked for port extraction) to return the default port on invalid/malformed uri instead of throwing.
🧹 Nitpick comments (1)
src/components/ui/HomeScreen.tsx (1)
39-40: MoveuriPortoutside the component.
uriPortis a pure function with no closure over component state or props. Defining it inside the component body recreates it on every render.♻️ Proposed refactor
+const uriPort = (uri: string, defaultPort: string) => + new URL(uri).port || defaultPort; + const DiscoverGrid = () => { ... - const uriPort = (uri: string, defaultPort: string) => - new URL(uri).port || defaultPort; - const handleServerClick = ...🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ui/HomeScreen.tsx` around lines 39 - 40, The helper function uriPort is defined inside the HomeScreen component causing it to be recreated every render; move the const uriPort = (uri: string, defaultPort: string) => new URL(uri).port || defaultPort; declaration to module/top-level scope (above the HomeScreen component) so it is instantiated once, keep the same signature/TypeScript types, and update any local references to call the top-level uriPort; this keeps behavior identical while avoiding per-render allocations.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/ui/HomeScreen.tsx`:
- Line 42: Change the parameter type of handleServerClick to match the concrete
servers item shape instead of Record<string,string>: use the exact object type
(e.g. { name: string; description: string; wss?: string; ircs?: string }) or a
named alias/interface used for the servers state so optional properties wss and
ircs are allowed; update the function signature for handleServerClick
accordingly to remove the strict-mode mismatch with the servers array type.
---
Outside diff comments:
In `@src/components/ui/HomeScreen.tsx`:
- Around line 42-55: The click handler handleServerClick currently calls new
URL(uri) (and uriPort which also constructs a URL) without guarding against
malformed strings, causing a TypeError and/or producing an empty, locked host
when uri is undefined; wrap URL parsing in a try/catch and validate uri before
using new URL, falling back to a safe host value (e.g., server.host ||
server.name || "") and a sane port default when parsing fails, and ensure
toggleAddServerModal is called with disableServerConnectionInfo: false if the
host is empty so the user can edit connection info; also update uriPort (the
function invoked for port extraction) to return the default port on
invalid/malformed uri instead of throwing.
---
Nitpick comments:
In `@src/components/ui/HomeScreen.tsx`:
- Around line 39-40: The helper function uriPort is defined inside the
HomeScreen component causing it to be recreated every render; move the const
uriPort = (uri: string, defaultPort: string) => new URL(uri).port ||
defaultPort; declaration to module/top-level scope (above the HomeScreen
component) so it is instantiated once, keep the same signature/TypeScript types,
and update any local references to call the top-level uriPort; this keeps
behavior identical while avoiding per-render allocations.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/App.tsxsrc/components/layout/AppLayout.tsxsrc/components/layout/ChatArea.tsxsrc/components/ui/AddServerModal.tsxsrc/components/ui/HomeScreen.tsxsrc/hooks/useKeyboardResize.tssrc/lib/tauri.ts
ValwareIRC
approved these changes
Mar 8, 2026
…o feat/update-discover-page-schema
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/components/ui/HomeScreen.tsx (1)
43-43:⚠️ Potential issue | 🟡 MinorReuse the server item type here instead of
Record<string, string>.The state on Line 12 stores items with optional
wss/ircs, soRecord<string, string>overstates whathandleServerClickactually receives and masks the nullability this branch depends on. A sharedServerEntrytype would keep the state and handler in sync.#!/bin/bash # Verify the current server item shape and the handler signature side by side. # Expected: the handler parameter should reuse the same server-entry type as the state array. rg -n -C2 'useState<|wss\?: string|ircs\?: string|handleServerClick|Record<string, string>' src/components/ui/HomeScreen.tsx🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ui/HomeScreen.tsx` at line 43, Define or reuse a ServerEntry type that matches the state shape (with optional wss?: string and ircs?: string) and update both the state generic and the handleServerClick parameter to use ServerEntry instead of Record<string, string>; specifically, declare/interface ServerEntry { /* ... wss?: string; ircs?: string; other fields... */ } then change useState<...> to use ServerEntry[] and change const handleServerClick = (server: Record<string, string>) => to const handleServerClick = (server: ServerEntry) => so the handler's nullability assumptions match the actual stored items.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/ui/HomeScreen.tsx`:
- Around line 43-52: The handleServerClick handler opens a locked AddServer
modal even when platform-specific URI (server.ircs or server.wss) is missing;
update handleServerClick to first compute uri = isTauri() ? server.ircs :
server.wss and if uri is falsy call toggleAddServerModal with
ui.disableServerConnectionInfo set to false (and host/port left empty or default
editable values) so the user can enter connection details, otherwise continue to
parse new URL(uri).hostname and uriPort as now; reference handleServerClick,
toggleAddServerModal, server.ircs/server.wss, uriPort, and isTauri when making
the change.
---
Duplicate comments:
In `@src/components/ui/HomeScreen.tsx`:
- Line 43: Define or reuse a ServerEntry type that matches the state shape (with
optional wss?: string and ircs?: string) and update both the state generic and
the handleServerClick parameter to use ServerEntry instead of Record<string,
string>; specifically, declare/interface ServerEntry { /* ... wss?: string;
ircs?: string; other fields... */ } then change useState<...> to use
ServerEntry[] and change const handleServerClick = (server: Record<string,
string>) => to const handleServerClick = (server: ServerEntry) => so the
handler's nullability assumptions match the actual stored items.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 030387c1-d5f7-4e9d-8755-5e4fb273d69c
📒 Files selected for processing (1)
src/components/ui/HomeScreen.tsx
Comment on lines
43
to
52
| const handleServerClick = (server: Record<string, string>) => { | ||
| const uri = isTauri() ? server.ircs : server.wss; | ||
| const defaultPort = isTauri() ? "6697" : "443"; | ||
| toggleAddServerModal(true, { | ||
| name: server.name, | ||
| host: server.server || "", // Use empty string if server is undefined | ||
| port: server.port || "443", // Default to 443 if port is undefined | ||
| host: uri ? new URL(uri).hostname : "", | ||
| port: uri ? uriPort(uri, defaultPort) : defaultPort, | ||
| nickname: "", // Generate a default nickname | ||
| ui: { | ||
| disableServerConnectionInfo: true, |
Contributor
There was a problem hiding this comment.
Don't open a locked server config when the current transport URI is missing.
wss/ircs are optional in the server shape, but this branch still opens the modal with host: "" and disableServerConnectionInfo: true when the platform-specific URI is absent. That leaves the user stuck with an invalid, non-editable connection.
Suggested fix
const handleServerClick = (server: Record<string, string>) => {
const uri = isTauri() ? server.ircs : server.wss;
+ if (!uri) {
+ return;
+ }
const defaultPort = isTauri() ? "6697" : "443";
+ const parsed = new URL(uri);
toggleAddServerModal(true, {
name: server.name,
- host: uri ? new URL(uri).hostname : "",
- port: uri ? uriPort(uri, defaultPort) : defaultPort,
+ host: parsed.hostname,
+ port: parsed.port || defaultPort,
nickname: "", // Generate a default nickname
ui: {
disableServerConnectionInfo: true,
title: server.name,
},🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/ui/HomeScreen.tsx` around lines 43 - 52, The handleServerClick
handler opens a locked AddServer modal even when platform-specific URI
(server.ircs or server.wss) is missing; update handleServerClick to first
compute uri = isTauri() ? server.ircs : server.wss and if uri is falsy call
toggleAddServerModal with ui.disableServerConnectionInfo set to false (and
host/port left empty or default editable values) so the user can enter
connection details, otherwise continue to parse new URL(uri).hostname and
uriPort as now; reference handleServerClick, toggleAddServerModal,
server.ircs/server.wss, uriPort, and isTauri when making the change.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Improvements