Repository navigation
Fix intermittent CLI socket-not-found errors for tagged/debug runs - #832
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughImplements socket autodiscovery for the CLI with a new resolver that locates tagged sockets, validates ownership, and retries connections on transient errors. Adds socket discovery capabilities including last-used path reading and tagged socket discovery in Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI Entry Point
participant Resolver as Socket Path Resolver
participant Discovery as Socket Discovery
participant Client as SocketClient
participant Socket as Tagged Socket
CLI->>Resolver: Resolve socket path (CMUX_TAG, env, etc.)
Resolver->>Discovery: Discover tagged sockets in /tmp
Discovery->>Discovery: Filter by CMUX_TAG
Discovery-->>Resolver: Candidate socket paths
Resolver->>Socket: Validate socket ownership & type
Socket-->>Resolver: Validation result
Resolver-->>CLI: Resolved socket path + source
CLI->>Client: Connect to resolved socket
Client->>Socket: Attempt connection
alt Connection fails (transient error)
Socket-->>Client: Error (EAGAIN, ENOENT, etc.)
Client->>Client: Within retry window?
Client->>Socket: Retry connection
Socket-->>Client: Success
else Connection succeeds
Socket-->>Client: Connected
end
Client-->>CLI: Connection established
CLI->>Socket: Send ping command
Socket-->>CLI: Receive PONG response
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly Related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
Contributor
Greptile SummaryAdds CLI socket path autodiscovery to resolve intermittent socket-not-found errors when using tagged/debug builds. The implementation includes:
The autodiscovery only activates when socket path is implicitly defaulted (not explicitly set via Minor cleanup needed: unreachable code at Confidence Score: 4/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
Start[CLI invoked] --> CheckSource{Socket path<br/>source?}
CheckSource -->|explicitFlag or<br/>non-default env| UseRequested[Use requested path]
CheckSource -->|implicitDefault| BuildCandidates[Build candidate list]
BuildCandidates --> CheckTag{CMUX_TAG<br/>set?}
CheckTag -->|Yes| AddTagged[Add /tmp/cmux-debug-tag.sock<br/>/tmp/cmux-tag.sock]
CheckTag -->|No| AddDefaults[Add /tmp/cmux.sock]
AddTagged --> AddDefaults
AddDefaults --> AddFallbacks[Add /tmp/cmux-debug.sock<br/>/tmp/cmux-staging.sock]
AddFallbacks --> ScanFS[Scan /tmp for cmux*.sock<br/>sort by mtime]
ScanFS --> TryConnect{Try connect<br/>to candidates}
TryConnect -->|Connected| Return[Use connected socket]
TryConnect -->|No connection| CheckFiles{Socket files<br/>exist?}
CheckFiles -->|Yes| UseFirst[Use first existing socket]
CheckFiles -->|No| UseRequested
UseFirst --> ConnectRetry
UseRequested --> ConnectRetry
Return --> ConnectRetry
ConnectRetry[Connect with retry] --> VerifyOwner{Owner is<br/>current user?}
VerifyOwner -->|No| ThrowError[Throw security error]
VerifyOwner -->|Yes| AttemptConnect[Attempt connect]
AttemptConnect -->|Success| Done[Connected]
AttemptConnect -->|ENOENT/ECONNREFUSED<br/>within 2s window| Sleep[Sleep 100ms]
Sleep --> ConnectRetry
AttemptConnect -->|Other error or<br/>timeout| ThrowError
Last reviewed commit: 54cd0c3 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| throw error | ||
| } | ||
|
|
||
| throw lastError ?? CLIError(message: "Failed to connect to socket at \(path)") |
Contributor
There was a problem hiding this comment.
unreachable code - the while true loop above only exits via return (line 668) or throw (lines 638, 641, 644, 649, 681), so this line can never execute
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 2 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="tests/test_cli_socket_autodiscovery.py">
<violation number="1" location="tests/test_cli_socket_autodiscovery.py:123">
P2: The test can pass without ever receiving the ping on the tagged test socket because the server thread is only joined for 2 seconds.</violation>
</file>
<file name="CLI/cmux.swift">
<violation number="1" location="CLI/cmux.swift:539">
P2: Socket autodiscovery can pick sockets not owned by the current user, which then fails immediately in `SocketClient.connect()` and can mask a valid socket candidate.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| print(f"FAIL: invoking cmux ping failed: {exc}") | ||
| return 1 | ||
| finally: | ||
| server.join(timeout=2.0) |
There was a problem hiding this comment.
P2: The test can pass without ever receiving the ping on the tagged test socket because the server thread is only joined for 2 seconds.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/test_cli_socket_autodiscovery.py, line 123:
<comment>The test can pass without ever receiving the ping on the tagged test socket because the server thread is only joined for 2 seconds.</comment>
<file context>
@@ -0,0 +1,150 @@
+ print(f"FAIL: invoking cmux ping failed: {exc}")
+ return 1
+ finally:
+ server.join(timeout=2.0)
+ try:
+ os.remove(socket_path)
</file context>
Comment on lines
+539
to
+542
| private static func isSocketFile(_ path: String) -> Bool { | ||
| var st = stat() | ||
| return lstat(path, &st) == 0 && (st.st_mode & mode_t(S_IFMT)) == mode_t(S_IFSOCK) | ||
| } |
There was a problem hiding this comment.
P2: Socket autodiscovery can pick sockets not owned by the current user, which then fails immediately in SocketClient.connect() and can mask a valid socket candidate.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At CLI/cmux.swift, line 539:
<comment>Socket autodiscovery can pick sockets not owned by the current user, which then fails immediately in `SocketClient.connect()` and can mask a valid socket candidate.</comment>
<file context>
@@ -451,9 +451,159 @@ private enum SocketPasswordResolver {
+ return discovered.prefix(limit).map(\.path)
+ }
+
+ private static func isSocketFile(_ path: String) -> Bool {
+ var st = stat()
+ return lstat(path, &st) == 0 && (st.st_mode & mode_t(S_IFMT)) == mode_t(S_IFSOCK)
</file context>
Suggested change
| private static func isSocketFile(_ path: String) -> Bool { | |
| var st = stat() | |
| return lstat(path, &st) == 0 && (st.st_mode & mode_t(S_IFMT)) == mode_t(S_IFSOCK) | |
| } | |
| private static func isSocketFile(_ path: String) -> Bool { | |
| var st = stat() | |
| return lstat(path, &st) == 0 && | |
| (st.st_mode & mode_t(S_IFMT)) == mode_t(S_IFSOCK) && | |
| st.st_uid == getuid() | |
| } |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
tests/test_cli_socket_autodiscovery.py (1)
15-17: Fail fast whenCMUX_CLI_BIN/CMUX_CLIis set but invalid.If an explicit path is provided but not executable, silently falling back can make this regression run against an unintended binary.
🔧 Proposed fix
def resolve_cmux_cli() -> str: explicit = os.environ.get("CMUX_CLI_BIN") or os.environ.get("CMUX_CLI") - if explicit and os.path.exists(explicit) and os.access(explicit, os.X_OK): - return explicit + if explicit: + if os.path.exists(explicit) and os.access(explicit, os.X_OK): + return explicit + raise RuntimeError(f"CMUX_CLI_BIN/CMUX_CLI is not executable: {explicit}")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_cli_socket_autodiscovery.py` around lines 15 - 17, The code currently falls back silently when an explicit CLI path (from env vars CMUX_CLI_BIN or CMUX_CLI) is provided but missing or not executable; update the logic around the explicit variable to fail fast: after reading explicit, if explicit is truthy and (not os.path.exists(explicit) or not os.access(explicit, os.X_OK)), raise a clear error (e.g., raise RuntimeError or SystemExit with a message referencing the env var and path) instead of returning None or continuing, so callers see the misconfigured CMUX_CLI_BIN/CMUX_CLI immediately.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CLI/cmux.swift`:
- Around line 478-485: When CMUX_TAG is set, the selection logic for candidates
must prefer the tagged socket instead of returning the first connectable path;
change the loops that currently iterate over candidates and call canConnect(to:)
/ isSocketFile(path) so they first filter candidates to those matching the
CMUX_TAG (the tag-matching predicate used elsewhere in this file) and only
consider connectable or existing socket files within that filtered list, falling
back to the global candidate loops only if no tagged candidates are found;
update both the connectable-check loop (using canConnect(to:)) and the
existing-socket loop (using isSocketFile(path)) to implement this behavior.
- Around line 553-557: Before calling strncpy on addr.sun_path, validate the
path length against the capacity of sockaddr_un.sun_path: compute maxLength =
MemoryLayout.size(ofValue: addr.sun_path) (or use MemoryLayout.size(ofValue:
addr.sun_path) - 1 if you reserve a NUL), check path.utf8.count < maxLength (or
<= maxLength-1) and if it exceeds, return/throw an error (or propagate a
failure) instead of truncating; apply this check in the canConnect code path
that uses path.withCString/addr.sun_path/strncpy and in the socket connection
method that does the same, so neither call silently truncates into a different
socket path.
In `@tests/test_cli_socket_autodiscovery.py`:
- Around line 123-131: The test can report a false PASS because
server.join(timeout=2.0) may return before the server thread's accept (6s)
completes; change the teardown to wait for the server thread to fully finish
before removing socket_path and checking server.error — e.g., call server.join()
without a short timeout or use a timeout >6s (e.g., 8s) and/or assert not
server.is_alive() before os.remove(socket_path); ensure the error check uses
server.error after this guaranteed join so the test can't pass while the server
is still running.
---
Nitpick comments:
In `@tests/test_cli_socket_autodiscovery.py`:
- Around line 15-17: The code currently falls back silently when an explicit CLI
path (from env vars CMUX_CLI_BIN or CMUX_CLI) is provided but missing or not
executable; update the logic around the explicit variable to fail fast: after
reading explicit, if explicit is truthy and (not os.path.exists(explicit) or not
os.access(explicit, os.X_OK)), raise a clear error (e.g., raise RuntimeError or
SystemExit with a message referencing the env var and path) instead of returning
None or continuing, so callers see the misconfigured CMUX_CLI_BIN/CMUX_CLI
immediately.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5d0316a5-2cda-43c2-b363-cc3fbfbf8cf7
📒 Files selected for processing (2)
CLI/cmux.swifttests/test_cli_socket_autodiscovery.py
Comment on lines
+478
to
+485
| for path in candidates where canConnect(to: path) { | ||
| return path | ||
| } | ||
|
|
||
| // If the listener is still starting, prefer existing socket files. | ||
| for path in candidates where isSocketFile(path) { | ||
| return path | ||
| } |
There was a problem hiding this comment.
CMUX_TAG can still route commands to the wrong instance during startup race.
With CMUX_TAG present (Lines 493-497), Line 478 still returns the first connectable socket across all candidates. If the tagged socket is not yet accepting connections but /tmp/cmux.sock is, the CLI can connect to the wrong instance.
🔧 Proposed fix
static func resolve(
requestedPath: String,
source: CLISocketPathSource,
environment: [String: String] = ProcessInfo.processInfo.environment
) -> String {
guard source == .implicitDefault else {
return requestedPath
}
- let candidates = dedupe(candidatePaths(requestedPath: requestedPath, environment: environment))
+ let tagged = dedupe(taggedCandidatePaths(environment: environment))
+ let candidates = dedupe(candidatePaths(requestedPath: requestedPath, environment: environment))
+ // When CMUX_TAG is set, prefer tagged sockets first (including startup race fallback).
+ if !tagged.isEmpty {
+ for path in tagged where canConnect(to: path) { return path }
+ for path in tagged where isSocketFile(path) { return path }
+ }
+
// Prefer sockets that are currently accepting connections.
for path in candidates where canConnect(to: path) {
return path
}
@@
}
+
+ private static func taggedCandidatePaths(environment: [String: String]) -> [String] {
+ guard let tag = normalized(environment["CMUX_TAG"]) else { return [] }
+ let slug = sanitizeTagSlug(tag)
+ return [
+ "/tmp/cmux-debug-\(slug).sock",
+ "/tmp/cmux-\(slug).sock"
+ ]
+ }Also applies to: 493-497
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@CLI/cmux.swift` around lines 478 - 485, When CMUX_TAG is set, the selection
logic for candidates must prefer the tagged socket instead of returning the
first connectable path; change the loops that currently iterate over candidates
and call canConnect(to:) / isSocketFile(path) so they first filter candidates to
those matching the CMUX_TAG (the tag-matching predicate used elsewhere in this
file) and only consider connectable or existing socket files within that
filtered list, falling back to the global candidate loops only if no tagged
candidates are found; update both the connectable-check loop (using
canConnect(to:)) and the existing-socket loop (using isSocketFile(path)) to
implement this behavior.
Comment on lines
+553
to
+557
| path.withCString { ptr in | ||
| withUnsafeMutablePointer(to: &addr.sun_path) { pathPtr in | ||
| let buf = UnsafeMutableRawPointer(pathPtr).assumingMemoryBound(to: CChar.self) | ||
| strncpy(buf, ptr, maxLength - 1) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
head -c 50000 CLI/cmux.swift | tail -c +20000 | wc -lRepository: manaflow-ai/cmux
Length of output: 63
🏁 Script executed:
wc -l CLI/cmux.swiftRepository: manaflow-ai/cmux
Length of output: 79
🏁 Script executed:
sed -n '540,570p' CLI/cmux.swiftRepository: manaflow-ai/cmux
Length of output: 1317
🏁 Script executed:
sed -n '640,670p' CLI/cmux.swiftRepository: manaflow-ai/cmux
Length of output: 1389
🏁 Script executed:
rg -n -C3 'sockaddr_un|sun_path' CLI/cmux.swiftRepository: manaflow-ai/cmux
Length of output: 1820
🏁 Script executed:
rg -n 'lengthOfBytes|ENAMETOOLONG|Socket path is too long' CLI/cmux.swiftRepository: manaflow-ai/cmux
Length of output: 42
Validate sun_path length before copying socket paths.
Path bytes are copied with strncpy at lines 553 and 655 without validating against sockaddr_un.sun_path capacity. Overlong paths are silently truncated, causing attempts to connect to a different (truncated) socket.
Add a length check before each strncpy call:
Proposed fixes
Line 553 (canConnect method):
let maxLength = MemoryLayout.size(ofValue: addr.sun_path)
+ guard path.lengthOfBytes(using: .utf8) < maxLength else { return false }
path.withCString { ptr inLine 655 (socket connection method):
let maxLength = MemoryLayout.size(ofValue: addr.sun_path)
+ guard path.lengthOfBytes(using: .utf8) < maxLength else {
+ throw CLIError(message: "Socket path is too long: \(path)")
+ }
path.withCString { ptr in📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| path.withCString { ptr in | |
| withUnsafeMutablePointer(to: &addr.sun_path) { pathPtr in | |
| let buf = UnsafeMutableRawPointer(pathPtr).assumingMemoryBound(to: CChar.self) | |
| strncpy(buf, ptr, maxLength - 1) | |
| } | |
| let maxLength = MemoryLayout.size(ofValue: addr.sun_path) | |
| guard path.lengthOfBytes(using: .utf8) < maxLength else { return false } | |
| path.withCString { ptr in | |
| withUnsafeMutablePointer(to: &addr.sun_path) { pathPtr in | |
| let buf = UnsafeMutableRawPointer(pathPtr).assumingMemoryBound(to: CChar.self) | |
| strncpy(buf, ptr, maxLength - 1) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@CLI/cmux.swift` around lines 553 - 557, Before calling strncpy on
addr.sun_path, validate the path length against the capacity of
sockaddr_un.sun_path: compute maxLength = MemoryLayout.size(ofValue:
addr.sun_path) (or use MemoryLayout.size(ofValue: addr.sun_path) - 1 if you
reserve a NUL), check path.utf8.count < maxLength (or <= maxLength-1) and if it
exceeds, return/throw an error (or propagate a failure) instead of truncating;
apply this check in the canConnect code path that uses
path.withCString/addr.sun_path/strncpy and in the socket connection method that
does the same, so neither call silently truncates into a different socket path.
Comment on lines
+123
to
+131
| server.join(timeout=2.0) | ||
| try: | ||
| os.remove(socket_path) | ||
| except OSError: | ||
| pass | ||
|
|
||
| if server.error is not None: | ||
| print(f"FAIL: socket server error: {server.error}") | ||
| return 1 |
There was a problem hiding this comment.
Prevent false-positive PASS when the server thread may still be running.
Line 123 waits only 2 seconds, but the server accept timeout is 6 seconds (Line 57). If cmux ping succeeds against a different socket, server.error can still be nil at Line 129 and this test can pass incorrectly.
🔧 Proposed fix
class PingServer:
@@
def join(self, timeout: float) -> None:
self._thread.join(timeout=timeout)
+
+ def is_alive(self) -> bool:
+ return self._thread.is_alive()
@@
finally:
- server.join(timeout=2.0)
+ server.join(timeout=7.0)
try:
os.remove(socket_path)
except OSError:
pass
+
+ if server.is_alive():
+ print("FAIL: socket server did not finish; ping may not have reached test socket")
+ return 1🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/test_cli_socket_autodiscovery.py` around lines 123 - 131, The test can
report a false PASS because server.join(timeout=2.0) may return before the
server thread's accept (6s) completes; change the teardown to wait for the
server thread to fully finish before removing socket_path and checking
server.error — e.g., call server.join() without a short timeout or use a timeout
>6s (e.g., 8s) and/or assert not server.is_alive() before
os.remove(socket_path); ensure the error check uses server.error after this
guaranteed join so the test can't pass while the server is still running.
This was referenced Mar 5, 2026
This was referenced Mar 12, 2026
bn-l
pushed a commit
to bn-l/cmux
that referenced
this pull request
Apr 3, 2026
This was referenced Apr 29, 2026
This branch was successfully deployed
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
1 participant
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
/tmp/cmux.sock) so tagged/debug sockets are foundCMUX_SOCKET_PATH=/tmp/cmux.sockas implicit default to avoid false hard-fail when caller env is staleCMUX_TAGTesting
xcodebuild -project GhosttyTabs.xcodeproj -scheme cmux -configuration Debug -destination 'platform=macOS' build./scripts/reload.sh --tag issue-811-socketCMUX_CLI_BIN=/tmp/cmux-issue-811-socket/Build/Products/Debug/cmux python3 tests/test_cli_socket_autodiscovery.pyCMUX_CLI_BIN=/tmp/cmux-issue-811-socket/Build/Products/Debug/cmux python3 tests/test_claude_hook_missing_socket_error.pypython3 tests/test_cli_socket_sentry_scope.pyCloses #811
Summary by cubic
Fix intermittent CLI “socket not found” errors for tagged/debug runs by auto-discovering the correct socket and retrying connects during startup. Addresses #811.
Written for commit 54cd0c3. Summary will update on new commits.
Summary by CodeRabbit