Repository navigation
cmux NEXT agent pane: a status read on a too-long socket path throws instead of trapping (base crash) - #28630
teamleaderleo wants to merge 1 commit into
Conversation
…instead of trapping The page's _acpmux/status is read natively from the connection's socket path (#27102). NWEndpoint.unix traps (SIGTRAP inside Network.framework) on a path past the 103-byte Unix socket limit or an empty one, so a long path took the whole process down. AgentPaneSendPickedFolderPromptTests' journalReplayWaitsForDeferredTrustAcknowledgement opens a connection on a ~117-byte socket under the temporary folder and sends a status: the swift test process dies with signal 5 (or hangs in the Swift backtracer until the stuck watchdog ends the shard). AcpmuxStatusClient.unixEndpoint refuses such a path as .unreachable; call and watchHarnesses use it, so the page gets "status unavailable". Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?id=aid-1288bbb2b5d2f5c6b66a
|
Owned by an agent session · session |
|
Found 5 test failures on Blacksmith runners: Failures
|
|
Review (codex gpt-6-astra high fast, independent review): SHIP at 47f543f No blocking problems found.
|
|
cmux status (one comment, edited in place; each line is updated by the job that owns it) Review App of this head (app.zip, kept 1 day)
Default changesThis pull request changes 2 defaults at
Signed off on this commit by Regenerate the snapshot with |
|
Closing: #28495 landed the same guard on feat-cmux-next (432fb31), so this one is superseded. On this head, its swift test passed the new suite and One point from the review here still holds for the merged code: |
![Fix with [code]smith](https://pr-comments-assets.blacksmith.sh/codesmith/fix-with-codesmith-light-3x.png)
Fixes a base crash in the cmux NEXT swift tests (and the same trap in the app).
What breaks
#27102 reads the page's
_acpmux/statusnatively:AgentPaneTransport.analyzestarts a detachedAcpmuxStatusClient.call(socketPath:)on the connection's socket path.callbuildsNWConnection(to: .unix(path:)), andNWEndpoint.unixtraps (SIGTRAP inside Network.framework) on a path longer than the Unix socket limit (103 bytes) or an empty one.LocalRouterClient.endpointalready documents and guards this trap; the status client did not.AgentPaneSendPickedFolderPromptTests.journalReplayWaitsForDeferredTrustAcknowledgementopens a connection on<temporary folder>/send-picked-<UUID>/acpmux.sock(about 117 bytes) and sends a status, so the swift test process dies:CmuxNextAgentPaneTests.Agat base 214b514 and at 5a39be4 (that base plus cmux-next swift test: main-thread-heavy suites in their own processes, the broad pass sharded, SIGPIPE caught as in the app #26809's shard changes): both end withswiftpm-testing-helper ... exited with unexpected signal code 5injournalReplayWaitsForDeferredTrustAcknowledgement(recovery:), case 0.SWIFT_BACKTRACEset, the trap instead hangs in_swift_spawnBacktracer→__wait4until the stuck watchdog ends the shard (exit 143). The sample showsclosure #1 in AgentPaneTransport.analyze(AgentPaneTransport+Check.swift:45) →AcpmuxStatusClient.call(AcpmuxStatusClient.swift:223) → Network →_sigtramp→handle_fatal_signal.Fix
AcpmuxStatusClient.unixEndpoint(_:)refuses an empty path or one pastAcpmuxEnvironment.maxSocketPathByteswithFailure.unreachable.callandAcpmuxEnvironment.watchHarnesses(the other.unix(path:)site in the module) use it, so the page gets its normal "status unavailable" error and the process lives on. Production paths fromAcpmuxEnvironmentare already short-aliased; this covers a connection whose socket path is taken as given.Tests
AcpmuxStatusLongSocketPathTests:callon a 104- and 160-byte path and on an empty path throws; a 103-byte path still makes an endpoint; the page's status on a connection with a 160-byte socket path is answered with an error. Without the guard,callon such a path traps the test process: the base replay below, and #28495's red refs, which show the trap's frame inAcpmuxStatusClient.call.No capture: nothing on screen changes; a path that used to crash the app now gets the page's existing "status unavailable" error.
Risk: low. Only paths that trapped before change behavior; they now fail like an absent daemon.
Overlap
#28495 fixes the same trap more widely (a shared
NWEndpointguard in CmuxNextCompat, plus the mobile daemon lane and the Cloud hub opener). An independent review blocked it at 4999e4b: its.socketPathTooLongescapesAcpmuxEnvironment.harness, which maps only.unreachableto the documentedAcpmuxRPCErrorwithisUnreachable. This PR is the narrow fix for the base crash and maps the refusal to.unreachable, so every existing unavailable-daemon path handles it. #28495's other sites stay with it.Agent session: agent session
🤖 Generated with Claude Code