-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Replace localhost with 127.0.0.1 for correct Codex handshake #1057
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -36,7 +36,7 @@ public static string BuildCodexServerBlock(string uvPath) | |
| if (useHttpTransport) | ||
| { | ||
| // HTTP mode: Use url field | ||
| string httpUrl = HttpEndpointUtility.GetMcpRpcUrl(); | ||
| string httpUrl = GetCodexHttpUrl(); | ||
| unityMCP["url"] = new TomlString { Value = httpUrl }; | ||
|
|
||
| // Enable Codex's Rust MCP client for HTTP/SSE transport | ||
|
|
@@ -191,7 +191,7 @@ private static TomlTable CreateUnityMcpTable(string uvPath) | |
| if (useHttpTransport) | ||
| { | ||
| // HTTP mode: Use url field | ||
| string httpUrl = HttpEndpointUtility.GetMcpRpcUrl(); | ||
| string httpUrl = GetCodexHttpUrl(); | ||
| unityMCP["url"] = new TomlString { Value = httpUrl }; | ||
| } | ||
| else | ||
|
|
@@ -229,6 +229,17 @@ private static TomlTable CreateUnityMcpTable(string uvPath) | |
| return unityMCP; | ||
| } | ||
|
|
||
| private static string GetCodexHttpUrl() | ||
| { | ||
| // Codex has intermittently failed the MCP handshake when configured with hostname-based | ||
| // loopback URLs such as localhost/::1, even though Unity MCP itself is reachable. | ||
| // Keep normalizing loopback hosts to 127.0.0.1 for Codex until the client routes them | ||
| // reliably again. If Codex fixes that behavior in the future, this is the workaround | ||
| // to revisit or remove before changing shared endpoint generation. | ||
| return HttpEndpointUtility.GetMcpRpcUrl() | ||
| .Replace("http://localhost:", "http://127.0.0.1:"); | ||
| } | ||
|
Comment on lines
+232
to
+241
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "== Locate HttpEndpointUtility and inspect URL construction =="
fd -a "HttpEndpointUtility.cs" | while read -r f; do
echo "--- $f ---"
rg -n -C3 'GetBaseUrl|GetMcpRpcUrl|localhost|https|Uri|port' "$f"
done
echo
echo "== Find localhost URL patterns and transport-related usage across C# files =="
rg -n -C2 --type=cs 'http://localhost|https://localhost|GetMcpRpcUrl\(|UseHttpTransport'Repository: CoplayDev/unity-mcp Length of output: 50375 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "=== Inspect CodexConfigHelper.cs GetCodexHttpUrl method ==="
fd -a "CodexConfigHelper.cs" | while read -r f; do
echo "--- $f (lines 230-250) ---"
sed -n '230,250p' "$f"
done
echo
echo "=== Inspect HttpEndpointUtility GetLocalBaseUrl and GetRemoteBaseUrl ==="
fd -a "HttpEndpointUtility.cs" | while read -r f; do
echo "--- GetLocalBaseUrl method ---"
sed -n '33,90p' "$f"
echo
echo "--- GetRemoteBaseUrl method ---"
sed -n '103,130p' "$f" 2>/dev/null || echo "(not in expected range)"
done
echo
echo "=== Check for HTTPS usage with localhost ==="
rg -n 'https.*localhost|localhost.*https' --type=cs -i
echo
echo "=== Check what AppendPathSegment does ==="
rg -n -A10 'AppendPathSegment' --type=cs | head -30Repository: CoplayDev/unity-mcp Length of output: 4312 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "=== Find DefaultLocalBaseUrl and DefaultRemoteBaseUrl constants ==="
rg -n 'DefaultLocalBaseUrl|DefaultRemoteBaseUrl|LocalPrefKey|RemotePrefKey' --type=cs | head -20
echo
echo "=== Find NormalizeBaseUrl implementation ==="
rg -n -A30 'private static string NormalizeBaseUrl' --type=cs
echo
echo "=== Check what schemes are assigned in the normalization process ==="
rg -n 'http:|https:' --type=cs -C2 | grep -A2 -B2 'scheme\|http://' | head -40Repository: CoplayDev/unity-mcp Length of output: 10211 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "=== Search for GetCodexHttpUrl usage ==="
rg -n 'GetCodexHttpUrl' --type=cs
echo
echo "=== Context around CodexConfigHelper GetCodexHttpUrl and its callers ==="
fd -a "CodexConfigHelper.cs" | while read -r f; do
echo "--- Full context (lines 220-250) ---"
sed -n '220,250p' "$f"
done
echo
echo "=== Check if localhost ever appears in remote scope settings ==="
rg -n 'HttpRemoteBaseUrl|RemoteBaseUrl.*localhost' --type=cs
echo
echo "=== Find any tests for CodexConfigHelper ==="
fd -a "*CodexConfigHelper*Test*" --type fRepository: CoplayDev/unity-mcp Length of output: 4263 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "=== Context of GetCodexHttpUrl calls at lines 39 and 194 ==="
fd -a "CodexConfigHelper.cs" | while read -r f; do
echo "--- Lines 30-50 (first call at 39) ---"
sed -n '30,50p' "$f"
echo
echo "--- Lines 185-205 (second call at 194) ---"
sed -n '185,205p' "$f"
done
echo
echo "=== Check if GetCodexHttpUrl result is only used for local scope ==="
fd -a "CodexConfigHelper.cs" | while read -r f; do
echo "--- Full method context around GetCodexHttpUrl calls ---"
sed -n '25,75p' "$f"
echo
sed -n '175,220p' "$f"
done
echo
echo "=== Look for tests of CodexConfigHelper with HTTP URLs ==="
fd -a "CodexConfigHelperTests.cs" | head -1 | xargs -I {} sed -n '460,540p' {}Repository: CoplayDev/unity-mcp Length of output: 10227 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "=== Check if IsRemoteScope can be true when useHttpTransport=true ==="
rg -n -B5 -A5 'GetCodexHttpUrl\|GetMcpRpcUrl\|IsRemoteScope' MCPForUnity/Editor/Helpers/CodexConfigHelper.cs | head -30
echo
echo "=== Verify remote scope can use localhost ==="
rg -n 'HttpRemoteBaseUrl|RemoteBaseUrl|IsRemoteScope' MCPForUnity/Editor/Helpers/HttpEndpointUtility.cs | head -20
echo
echo "=== Check actual URL construction for remote scope ==="
fd -a "HttpEndpointUtility.cs" | while read -r f; do
echo "--- NormalizeBaseUrl with remoteScope examples ---"
sed -n '312,340p' "$f"
doneRepository: CoplayDev/unity-mcp Length of output: 132 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "=== Check if IsRemoteScope can be true and what URLs it produces ==="
fd -a "HttpEndpointUtility.cs" | while read -r f; do
echo "--- GetBaseUrl and IsRemoteScope logic ---"
sed -n '29,32p' "$f"
echo
echo "--- NormalizeBaseUrl remoteScope=true branch (lines 312-340) ---"
sed -n '312,340p' "$f"
done
echo
echo "=== Check if GetRemoteBaseUrl can return a localhost URL ==="
rg -n 'GetRemoteBaseUrl|DefaultRemoteBaseUrl|RemotePrefKey' MCPForUnity/Editor/Helpers/HttpEndpointUtility.cs | head -15
echo
echo "=== Verify: Can a user configure RemoteBaseUrl with localhost? ==="
rg -n -C3 'SaveRemoteBaseUrl|SetHttpRemoteBaseUrl' --type=cs | head -40
echo
echo "=== Check validation of remote URLs ==="
fd -a "HttpEndpointUtility.cs" | while read -r f; do
echo "--- ValidateRemoteBaseUrl method ---"
sed -n '255,290p' "$f"
doneRepository: CoplayDev/unity-mcp Length of output: 7806 Rewrite localhost URL handling to use URI-based host replacement instead of string replacement. The current implementation only matches Suggested fix private static string GetCodexHttpUrl()
{
- return HttpEndpointUtility.GetMcpRpcUrl()
- .Replace("http://localhost:", "http://127.0.0.1:");
+ string rawUrl = HttpEndpointUtility.GetMcpRpcUrl();
+ if (!Uri.TryCreate(rawUrl, UriKind.Absolute, out Uri uri))
+ {
+ return rawUrl;
+ }
+
+ if (!string.Equals(uri.Host, "localhost", StringComparison.OrdinalIgnoreCase))
+ {
+ return rawUrl;
+ }
+
+ var builder = new UriBuilder(uri)
+ {
+ Host = "127.0.0.1"
+ };
+
+ return builder.Uri.ToString();
}🤖 Prompt for AI Agents |
||
|
|
||
| /// <summary> | ||
| /// Ensures the features table contains the rmcp_client flag for HTTP/SSE transport. | ||
| /// </summary> | ||
|
|
||
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.
suggestion (bug_risk): Make the localhost-to-127.0.0.1 conversion more robust (https, non-prefix matches, future changes).
This relies on
HttpEndpointUtility.GetMcpRpcUrl()always returninghttp://localhost:<port>. If it ever returnshttps://localhost:...orlocalhostappears elsewhere in the URL, this string replace may fail or corrupt the URL. Instead, parse the URL withUri, check thatHost == "localhost", and rebuild it withHost = "127.0.0.1"while preserving the original scheme, port, path, and query to avoid these assumptions.Suggested implementation:
If
Systemis not already imported at the top ofCodexConfigHelper.cs, addusing System;so thatUri,UriBuilder, andStringComparisonresolve correctly.