Repository navigation
Export CMUX_SOCKET alongside CMUX_SOCKET_PATH in terminal env - #1991
Conversation
The app only exported CMUX_SOCKET_PATH when setting up the terminal environment, but some scripts and hooks (e.g. claude-hook) expect CMUX_SOCKET. The CLI launcher code already exports both (cmux.swift lines 9288-9289), but the app-side terminal setup was missing the alias. This caused claude-hook stop to fail with 'TabManager not available' when CMUX_SOCKET was empty. Fixes manaflow-ai#1905 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
@anthhub is attempting to deploy a commit to the Manaflow Team on Vercel. A member of the Team first needs to authorize it. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthrough
Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 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)
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 |
Greptile SummaryThis PR fixes a missing environment variable in the terminal session setup by exporting
Confidence Score: 5/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant App as GhosttyTerminalView
participant Env as Terminal Environment
participant Hook as claude-hook stop
participant CLI as cmux CLI
App->>Env: setManagedEnvironmentValue("CMUX_SOCKET_PATH", socketPath())
Note over App,Env: existed before this PR
App->>Env: setManagedEnvironmentValue("CMUX_SOCKET", socketPath())
Note over App,Env: added by this PR
Hook->>Env: read CMUX_SOCKET
Env-->>Hook: socket path (was empty before fix)
Hook->>CLI: connect to socket
CLI-->>Hook: TabManager available ✓
Reviews (1): Last reviewed commit: "Export CMUX_SOCKET alongside CMUX_SOCKET..." | Re-trigger Greptile |
| setManagedEnvironmentValue("CMUX_SOCKET_PATH", SocketControlSettings.socketPath()) | ||
| // Backward-compatible alias expected by older scripts and third-party integrations. | ||
| setManagedEnvironmentValue("CMUX_SOCKET", SocketControlSettings.socketPath()) |
There was a problem hiding this comment.
Double call to
socketPath() — consider caching the result
SocketControlSettings.socketPath() is called twice in consecutive lines. While it's deterministic and lightweight today, caching the result in a local let keeps the two assignments provably identical and avoids any future drift if the function ever becomes non-trivial.
| setManagedEnvironmentValue("CMUX_SOCKET_PATH", SocketControlSettings.socketPath()) | |
| // Backward-compatible alias expected by older scripts and third-party integrations. | |
| setManagedEnvironmentValue("CMUX_SOCKET", SocketControlSettings.socketPath()) | |
| let socketPath = SocketControlSettings.socketPath() | |
| setManagedEnvironmentValue("CMUX_SOCKET_PATH", socketPath) | |
| // Backward-compatible alias expected by older scripts and third-party integrations. | |
| setManagedEnvironmentValue("CMUX_SOCKET", socketPath) |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Thank you for the contribution! |
…ow-ai#1991) * Export CMUX_SOCKET alongside CMUX_SOCKET_PATH in terminal environment The app only exported CMUX_SOCKET_PATH when setting up the terminal environment, but some scripts and hooks (e.g. claude-hook) expect CMUX_SOCKET. The CLI launcher code already exports both (cmux.swift lines 9288-9289), but the app-side terminal setup was missing the alias. This caused claude-hook stop to fail with 'TabManager not available' when CMUX_SOCKET was empty. Fixes manaflow-ai#1905 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Cache socketPath to avoid redundant call Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
CMUX_SOCKET_PATHwhen setting up terminal environment variables, but some hooks (e.g.claude-hook stop) look forCMUX_SOCKETcmux.swift:9288-9289), but the app-side terminal setup inGhosttyTerminalView.swiftwas missing theCMUX_SOCKETaliasCMUX_SOCKET_PATHandCMUX_SOCKETare set in the terminal environment, consistent with the CLI launcherTest plan
./scripts/reload.sh --tag fix-socket-envecho $CMUX_SOCKET— should show the socket path (was empty before)echo $CMUX_SOCKET_PATH— should still show the same socket pathcmux claude-hook stopshould no longer report 'TabManager not available'Fixes #1905
🤖 Generated with Claude Code
Summary by cubic
Exports
CMUX_SOCKETalongsideCMUX_SOCKET_PATHin app-launched terminals and caches the socket path to avoid redundant lookups; this matches the CLI and restores compatibility with scripts expectingCMUX_SOCKET(e.g.,cmux claude-hook stop). Fixes #1905.CMUX_SOCKET_PATHand its aliasCMUX_SOCKETfrom a cachedSocketControlSettings.socketPath()inGhosttyTerminalView.swift.Written for commit cdb7ea4. Summary will update on new commits.
Summary by CodeRabbit