Skip to content

fix: derive WebSocket URL from API_BASE_URL in conversation screen#130

Merged
elifouts merged 4 commits intoadd-commentsfrom
copilot/sub-pr-128
Mar 2, 2026
Merged

fix: derive WebSocket URL from API_BASE_URL in conversation screen#130
elifouts merged 4 commits intoadd-commentsfrom
copilot/sub-pr-128

Conversation

Copy link
Contributor

Copilot AI commented Mar 1, 2026

  • Update conversation/[username].tsx to use API_BASE_URL instead of hard-coded host/protocol for WebSocket URL
    • Added API_BASE_URL import from @/services/api
    • Added getWebSocketBaseUrl helper (matching the pattern in terminal.tsx)
    • Replaced hard-coded __DEV__ ? "10.0.2.2:8080" : "devbits.ddns.net" logic with getWebSocketBaseUrl(API_BASE_URL)
  • Resolve conflict with base branch by incorporating reconnection logic added in add-comments
    • Added isActive flag, reconnectAttempts, and reconnectTimeoutId state
    • Wrapped connection setup in a connect() function with exponential backoff retry (max 5 attempts)
    • Added proper cleanup in the effect return that cancels pending reconnects and closes the socket

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Co-authored-by: grillinr <169214325+grillinr@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix logic to use API_BASE_URL based on review feedback fix: derive WebSocket URL from API_BASE_URL in conversation screen Mar 1, 2026
@elifouts
Copy link
Collaborator

elifouts commented Mar 2, 2026

@copilot resolve conflicts please.

…_URL approach

Co-authored-by: elifouts <116454864+elifouts@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Mar 2, 2026

@copilot resolve conflicts please.

Resolved in 3482dd0. The base branch (add-comments) had added reconnection logic (exponential backoff, isActive guard, cleanup) to the same WebSocket effect. I've merged both changes: the reconnection logic is now incorporated while keeping the API_BASE_URL-based URL derivation.

Copy link
Collaborator

@elifouts elifouts left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks ok merge to other pr

@elifouts elifouts marked this pull request as ready for review March 2, 2026 16:07
Copilot AI review requested due to automatic review settings March 2, 2026 16:07
@elifouts elifouts merged commit 0fe6af8 into add-comments Mar 2, 2026
1 check passed
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates the conversation screen’s WebSocket connection to derive its base URL from the same API_BASE_URL used for HTTP API calls, avoiding hard-coded dev/prod host/protocol selection.

Changes:

  • Import API_BASE_URL into the conversation screen.
  • Add a getWebSocketBaseUrl helper to convert http(s) base URLs to ws(s).
  • Build the conversation WebSocket URL using getWebSocketBaseUrl(API_BASE_URL) instead of hard-coded __DEV__ host/protocol logic.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +33 to +39
if (baseUrl.startsWith("https://")) {
return `wss://${baseUrl.slice("https://".length)}`;
}
if (baseUrl.startsWith("http://")) {
return `ws://${baseUrl.slice("http://".length)}`;
}
return baseUrl;
Copy link

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getWebSocketBaseUrl is now duplicated here and in frontend/app/terminal.tsx. To avoid drift (e.g., future protocol/path handling changes), consider extracting this helper into a shared module (e.g., @/services/api or a @/utils/websocket helper) and importing it in both places.

Suggested change
if (baseUrl.startsWith("https://")) {
return `wss://${baseUrl.slice("https://".length)}`;
}
if (baseUrl.startsWith("http://")) {
return `ws://${baseUrl.slice("http://".length)}`;
}
return baseUrl;
try {
const url = new URL(baseUrl);
if (url.protocol === "https:") {
url.protocol = "wss:";
} else if (url.protocol === "http:") {
url.protocol = "ws:";
}
return url.toString();
} catch {
// Fallback for non-standard or relative URLs: map http/https prefixes to ws/wss.
if (baseUrl.startsWith("https://")) {
return `wss://${baseUrl.slice("https://".length)}`;
}
if (baseUrl.startsWith("http://")) {
return `ws://${baseUrl.slice("http://".length)}`;
}
return baseUrl;
}

Copilot uses AI. Check for mistakes.
grillinr added a commit that referenced this pull request Mar 7, 2026
* wip: add messaging

* feat: add messages tab

* Apply suggestions from code review

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* fix: derive WebSocket URL from API_BASE_URL in conversation screen (#130)

* Initial plan

* fix: use API_BASE_URL for WebSocket URL in conversation screen

Co-authored-by: grillinr <169214325+grillinr@users.noreply.github.com>

* fix: merge reconnection logic from base branch while keeping API_BASE_URL approach

Co-authored-by: elifouts <116454864+elifouts@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: grillinr <169214325+grillinr@users.noreply.github.com>
Co-authored-by: elifouts <116454864+elifouts@users.noreply.github.com>
Co-authored-by: Eli Fouts <eligfouts@gmail.com>

* feat: enhance MessageBubble and MessageComposer components

- Added authorLabel prop to MessageBubble for displaying message authors.
- Improved styling in MessageBubble for better visual consistency.
- Updated MessageComposer to include layout animations for dynamic height adjustments.
- Refactored input handling in MessageComposer to optimize performance.
- Removed unused terminal animation code from MyHeader component.
- Added terminal icon mapping in IconSymbol for better icon management.

* fix: safe router navigation encoding and /users/search test coverage (#133)

* fix: exclude sentinel user from search results and correct stale test expectations (#136)

* Initial plan

* fix: exclude deleted_user sentinel from search results and update test expectations

Co-authored-by: grillinr <169214325+grillinr@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: grillinr <169214325+grillinr@users.noreply.github.com>

* Apply suggestions from code review

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

---------

Co-authored-by: Nathan Grilliot <grillinr.mail.uc.edu>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
Co-authored-by: grillinr <169214325+grillinr@users.noreply.github.com>
Co-authored-by: elifouts <116454864+elifouts@users.noreply.github.com>
Co-authored-by: Eli Fouts <eligfouts@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants