Repository navigation
Launch workspace initial commands via the user's login shell - #8801
Merged
Merged
Conversation
📝 WalkthroughWalkthroughWorkspace creation now wraps non-empty initial commands in supported login-shell invocations, preserves command content through quoting, includes the helper in both build targets, and updates tests for shell selection, shim paths, whitespace, newlines, and concurrency. ChangesWorkspace initial command wrapping
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant WorkspaceCreation
participant WorkspaceCreateExecutionPreparation
participant WorkspaceInitialCommandLoginShell
participant LoginShell
WorkspaceCreation->>WorkspaceCreateExecutionPreparation: prepare initial command
WorkspaceCreateExecutionPreparation->>WorkspaceInitialCommandLoginShell: wrap non-empty command
WorkspaceInitialCommandLoginShell->>LoginShell: resolve shell and build -lc payload
LoginShell-->>WorkspaceInitialCommandLoginShell: return wrapped command
WorkspaceInitialCommandLoginShell-->>WorkspaceCreateExecutionPreparation: provide wrapped command
WorkspaceCreateExecutionPreparation-->>WorkspaceCreation: store initial command
Suggested reviewers: Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (2 errors, 2 warnings)
✅ Passed checks (21 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Contributor
Greptile SummaryThis PR fixes a bug where workspace
Confidence Score: 5/5Safe to merge — the change is a pure string transform applied at a single call site, with no side effects on non-command workspace creates and good test coverage for all shell paths and edge cases. The fix is narrow and well-contained: one call site in v2PrepareWorkspaceCreateExecution now wraps any non-blank initial_command through the user's login shell before handing it to Ghostty. The quoting logic is correct, the fallback chain is sensible, and the fish/POSIX split handles the two meaningful shell families. All affected test assertions were updated and new snapshot tests pin every shell branch. No files require special attention. Important Files Changed
Reviews (2): Last reviewed commit: "Launch workspace initial commands via us..." | Re-trigger Greptile |
| import Darwin | ||
| import Foundation | ||
|
|
||
| enum WorkspaceInitialCommandLoginShell { |
Contributor
There was a problem hiding this comment.
Caseless enum as static-function namespace
WorkspaceInitialCommandLoginShell is a caseless enum whose entire API is static functions — the exact shape flagged by the cmux-no-ambient-global-state rule. Because there are no cases, the type can never be constructed or injected, making it a static-only namespace bag. The preferred shape per the rule is a constructable, injectable owning type (a struct with an init, or the wrap logic moved as extension methods onto a type that already owns the wrapping concern). This is a new top-level type introduced by this PR.
Rule Used: Flag new ambient global state in production Swift:... (source)
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!
Ghostty launches string commands through a profile-free Bash, leaving task agents with the app-inherited PATH. Resolve the user login shell, run the original command through -lc, and re-prepend the per-surface shim directory so cmux wrapper hooks stay active. Session restore and dock startup already use login-shell wrapping for the same profile-loading behavior.
azooz2003-bit
force-pushed
the
feat-task-login-shell-path
branch
from
July 24, 2026 03:09
0b132cf to
d506ed0
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Sources/WorkspaceInitialCommandLoginShell.swift`:
- Around line 4-67: Replace the static-only WorkspaceInitialCommandLoginShell
enum with a constructable wrapper type that stores the resolved shell
dependency, allowing tests to inject it directly. Move default login-shell
resolution to the workspace-creation ownership boundary, and make the wrapper’s
command-wrapping behavior instance-based while preserving shell selection and
payload generation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 1e6bd38c-91e7-48d1-9354-12208375aedf
📒 Files selected for processing (4)
Sources/TerminalController+WorkspaceCreateExecutionPreparation.swiftSources/WorkspaceInitialCommandLoginShell.swiftcmux.xcodeproj/project.pbxprojcmuxTests/WorkspaceCreateWorkingDirectoryTests.swift
Comment on lines
+4
to
+67
| enum WorkspaceInitialCommandLoginShell { | ||
| /// Resolves the user's login shell and wraps an externally supplied workspace command. | ||
| /// | ||
| /// Ghostty otherwise launches string commands through Bash with `--noprofile --norc`, | ||
| /// so the user's profile cannot contribute tools such as Homebrew-installed agents. | ||
| /// Ghostty prepends `exec -l`, so the returned command starts with the quoted shell path. | ||
| static func wrap(_ command: String) -> String { | ||
| let databaseShell: String? | ||
| if let record = getpwuid(getuid()), | ||
| let shell = record.pointee.pw_shell { | ||
| let copiedShell = String(cString: shell) | ||
| databaseShell = copiedShell.isEmpty ? nil : copiedShell | ||
| } else { | ||
| databaseShell = nil | ||
| } | ||
|
|
||
| let userShell = databaseShell | ||
| ?? ProcessInfo.processInfo.environment["SHELL"] | ||
| ?? "/bin/zsh" | ||
| return wrap(command, userShell: userShell) | ||
| } | ||
|
|
||
| /// Wraps a command in a supported login shell while preserving the command verbatim. | ||
| /// | ||
| /// Login profiles can prepend other tool directories (Homebrew's `shellenv` puts | ||
| /// `/opt/homebrew/bin` first) ahead of the per-surface shim directory that cmux | ||
| /// seeds into the spawned PATH, which would route `claude`/`codex` around cmux's | ||
| /// wrapper hooks. The payload therefore re-prepends the shim directory | ||
| /// unconditionally after profiles run; a duplicate PATH entry is harmless and | ||
| /// matches what interactive shell integration already produces. | ||
| static func wrap(_ command: String, userShell: String?) -> String { | ||
| var shellPath: String | ||
| if let userShell, userShell.hasPrefix("/") { | ||
| shellPath = userShell | ||
| } else { | ||
| shellPath = "/bin/zsh" | ||
| } | ||
| let payload: String | ||
|
|
||
| switch (shellPath as NSString).lastPathComponent { | ||
| case "fish": | ||
| payload = """ | ||
| if test -n "$CMUX_CLAUDE_WRAPPER_SHIM_ROOT"; and test -d "$CMUX_CLAUDE_WRAPPER_SHIM_ROOT"; set -gx PATH "$CMUX_CLAUDE_WRAPPER_SHIM_ROOT" $PATH; end | ||
| \(command) | ||
| """ | ||
| case "zsh", "bash", "sh", "ksh", "dash": | ||
| payload = """ | ||
| if [ -n "${CMUX_CLAUDE_WRAPPER_SHIM_ROOT:-}" ] && [ -d "${CMUX_CLAUDE_WRAPPER_SHIM_ROOT}" ]; then PATH="${CMUX_CLAUDE_WRAPPER_SHIM_ROOT}${PATH:+:$PATH}"; export PATH; fi | ||
| \(command) | ||
| """ | ||
| default: | ||
| shellPath = "/bin/zsh" | ||
| payload = """ | ||
| if [ -n "${CMUX_CLAUDE_WRAPPER_SHIM_ROOT:-}" ] && [ -d "${CMUX_CLAUDE_WRAPPER_SHIM_ROOT}" ]; then PATH="${CMUX_CLAUDE_WRAPPER_SHIM_ROOT}${PATH:+:$PATH}"; export PATH; fi | ||
| \(command) | ||
| """ | ||
| } | ||
|
|
||
| return "\(shellSingleQuoted(shellPath)) -lc \(shellSingleQuoted(payload))" | ||
| } | ||
|
|
||
| private static func shellSingleQuoted(_ value: String) -> String { | ||
| "'" + value.replacingOccurrences(of: "'", with: "'\"'\"'") + "'" | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Replace the static-only namespace with an injectable wrapper.
This adds a namespace-only enum with global behavior. Make the wrapper constructable, inject the resolved shell for tests, and keep resolution at the workspace-creation ownership boundary.
As per coding guidelines, “Production Swift code must not add … static-only namespaces … Put state and behavior on a constructable, injectable owning type.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Sources/WorkspaceInitialCommandLoginShell.swift` around lines 4 - 67, Replace
the static-only WorkspaceInitialCommandLoginShell enum with a constructable
wrapper type that stores the resolved shell dependency, allowing tests to inject
it directly. Move default login-shell resolution to the workspace-creation
ownership boundary, and make the wrapper’s command-wrapping behavior
instance-based while preserving shell selection and payload generation.
Source: Coding guidelines
This was referenced Aug 4, 2026
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.
Fixes the iOS "New Task" failure where every template command died with
Error: claude not found in PATH.Bug. Workspace-create
initial_command(iOS task composer, CLIcmux workspace create --command) was passed verbatim to Ghostty, which launches string commands as/usr/bin/login -flp <user> /bin/bash --noprofile --norc -c "exec -l <cmd>". No shell profile runs, so PATH is the app-inherited launchd default plus the per-surfacecmux-cli-shimsdir. The claude shim is found and runs, but itsfind_real_claudescans that same bare PATH, misses/opt/homebrew/bin/claude, prints the error, and exits 127.Fix.
v2PrepareWorkspaceCreateExecutionnow wraps a non-blankinitial_commandvia the newWorkspaceInitialCommandLoginShellhelper into'<usershell>' -lc '<payload>'. The payload re-prepends$CMUX_CLAUDE_WRAPPER_SHIM_ROOTto PATH (login profiles prepend/opt/homebrew/bin, which would otherwise shadow the cmux claude/codex wrapper hooks) and then runs the original command verbatim. fish gets fish syntax; unknown shells fall back to/bin/zshwith the POSIX payload. This matches the login-shell wrapping that session restore (SessionRestoredTerminalCommandStore) and the right-dock (DockSplitStore+TerminalStartup) already do; first launch was the only path executing user commands with a bare environment.Commit 1 adds the regression test only (CI red), commit 2 adds the fix (CI green). Pure string transform: no temp files, respawn-safe, nothing user-facing to localize.
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by cubic
Run workspace initial commands through the user’s login shell so PATH and profiles load correctly. Fixes the iOS “New Task” failure where commands like claude were not found.
initial_commandvia the user’s shell with-lc, preserving the original command, whitespace, and newlines.$CMUX_CLAUDE_WRAPPER_SHIM_ROOTto PATH so cmux Claude/Codex wrapper hooks stay active./bin/zsh.Written for commit d506ed0. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes