Add cmux claude-teams launcher - #1179
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a new Changes
Sequence DiagramsequenceDiagram
participant User
participant CMUX as CMUX CLI
participant Shim as Shim Dir
participant Claude as Claude Process
participant TmuxCompat as Tmux-Compat
participant APIv2 as CMUX v2 API
User->>CMUX: claude-teams [claude-args...]
CMUX->>CMUX: set env vars (CLAUDE_CODE_EXPERIMENTAL_AGENT_TEAMS, CMUX_CLAUDE_TEAMS_CMUX_BIN)
CMUX->>Shim: create/prepend shim dir to PATH
CMUX->>Claude: spawn Claude (with shimmed PATH, --teammate-mode auto)
Claude->>Shim: invoke "tmux" shim (tmux-like command)
Shim->>TmuxCompat: forward to __tmux-compat handler
TmuxCompat->>APIv2: translate -> workspace/pane/surface v2 API calls
APIv2-->>TmuxCompat: return results
TmuxCompat-->>Claude: send tmux-compatible responses
Claude-->>User: agent-teams interactions continue
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
Greptile SummaryThis PR introduces Key changes:
Issues found:
Confidence Score: 3/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant cmux as cmux (CLI)
participant shim as /tmp/cmux-claude-teams-UUID/tmux
participant claude as claude (subprocess)
participant cmuxCompat as cmux __tmux-compat
User->>cmux: cmux claude-teams [args...]
cmux->>cmux: createClaudeTeamsShimDirectory()
cmux->>shim: write bash shim script (exec cmux __tmux-compat "$@")
cmux->>cmux: set env: CLAUDE_CODE_EXPERIMENTAL_AGENT_TEAMS=1\nCMUX_CLAUDE_TEAMS_CMUX_BIN=<path>\nPATH=shimDir:bundledBin:...
cmux->>claude: exec /usr/bin/env claude [args...] (modified env)
loop Agent Teams tmux commands
claude->>shim: tmux new-window / split-window / send-keys / capture-pane / ...
shim->>cmuxCompat: cmux __tmux-compat <command> [args...]
cmuxCompat->>cmuxCompat: runClaudeTeamsTmuxCompat(command, args)
cmuxCompat-->>claude: stdout (format output / pane content)
end
claude-->>cmux: process exits
cmux->>shim: FileManager.removeItem(shimDirectory)
cmux->>User: Darwin.exit(terminationStatus)
Last reviewed commit: e46bcdf |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if parsed.hasFlag("-p") || !text.isEmpty { | ||
| print(text) |
There was a problem hiding this comment.
capture-pane prints to stdout unconditionally without -p
In real tmux, capture-pane without -p stores the captured content into a paste buffer and produces no stdout output. The caller then retrieves it via show-buffer. The shim's condition parsed.hasFlag("-p") || !text.isEmpty breaks this contract by always printing non-empty content, even without -p.
This has two consequences:
- If Claude Code calls
capture-panewithout-p, the raw pane content will be written to the shim's stdout (Claude Code's tty/pipe), potentially corrupting its output stream. - The buffer-retrieval path (
show-buffer,save-buffer) is not handled at all — both commands fall through to thedefaultcase and throwCLIError(message: "Unsupported tmux compatibility command: ..."), causing a hard error if that workflow is used.
The condition should be changed to only print when -p is explicitly set:
| if parsed.hasFlag("-p") || !text.isEmpty { | |
| print(text) | |
| if parsed.hasFlag("-p") { | |
| print(text) | |
| } |
Additionally, consider adding show-buffer and save-buffer to the no-op or silently-ignored list (set-option, attach-session, etc.) so paste-buffer workflows degrade gracefully rather than crashing.
| if let paneSelector { | ||
| paneHandle = paneSelector.hasPrefix("%") ? String(paneSelector.dropFirst()) : paneSelector |
There was a problem hiding this comment.
Dead code — paneSelector can never start with % here
tmuxPaneSelector already strips the leading % before returning (via String(trimmed.dropFirst())), so by the time control reaches this branch, paneSelector will never begin with %. The ternary condition is always false and paneHandle always equals paneSelector directly.
| if let paneSelector { | |
| paneHandle = paneSelector.hasPrefix("%") ? String(paneSelector.dropFirst()) : paneSelector | |
| paneHandle = paneSelector |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e46bcdfb7e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let workspaceId = try tmuxResolveWorkspaceTarget(tmuxWindowSelector(from: raw), client: client) | ||
| let paneSelector = tmuxPaneSelector(from: raw) | ||
| let paneHandle: String? |
There was a problem hiding this comment.
tmuxResolvePaneTarget always derives a workspace first and then passes that workspace into normalizePaneHandle, so %<pane-id> targets are constrained to the current workspace when no window selector is present. This makes pane IDs emitted by the shim itself (for example from split-window -P on another workspace) unusable in follow-up commands like send-keys -t %... or select-pane -t %... once focus is elsewhere, because lookup fails with Pane target not found even though the pane exists.
Useful? React with 👍 / 👎.
![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="CLI/cmux.swift">
<violation number="1" location="CLI/cmux.swift:1680">
P1: The new `__tmux-compat` path is blocked by the global `-h/--help` precheck, so tmux commands like `split-window -h` exit as unknown instead of executing.</violation>
<violation number="2" location="CLI/cmux.swift:6613">
P1: Only print `capture-pane` output when `-p` is explicitly set. Printing whenever captured text is non-empty breaks tmux behavior and can inject pane contents into the caller's stdout stream.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
tests/test_cli_claude_teams_env.py (1)
63-71: Consider making tmux discovery failure more explicit.If the tmux shim isn't in PATH,
command -v tmuxwill fail and exit the script due toset -e. The test will report "exited non-zero" but won't indicate that tmux discovery specifically failed. Consider adding a fallback or explicit error:♻️ Optional: Add explicit handling for missing tmux
make_executable( real_bin / "claude", """#!/usr/bin/env bash set -euo pipefail printf '%s\\n' "${CLAUDE_CODE_EXPERIMENTAL_AGENT_TEAMS-__UNSET__}" > "$FAKE_AGENT_TEAMS_LOG" -command -v tmux > "$FAKE_TMUX_PATH_LOG" +command -v tmux > "$FAKE_TMUX_PATH_LOG" || printf '' > "$FAKE_TMUX_PATH_LOG" printf '%s\\n' "${CMUX_CLAUDE_TEAMS_CMUX_BIN-__UNSET__}" > "$FAKE_CMUX_BIN_LOG" """, )This allows the test to proceed and fail with the more informative "fake claude did not observe a tmux binary in PATH" message at line 101.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_cli_claude_teams_env.py` around lines 63 - 71, The bash shim created by make_executable currently runs `command -v tmux` under `set -e` so the script will exit with a nonzero status if tmux is not found, hiding the real cause; modify the embedded script (the multi-line string passed to make_executable that writes to "$FAKE_TMUX_PATH_LOG") so tmux discovery never causes the script to exit—e.g., run `command -v tmux > "$FAKE_TMUX_PATH_LOG" || printf '__TMUX_MISSING__' > "$FAKE_TMUX_PATH_LOG"` or otherwise append a fallback/true to the command—so the test can read "$FAKE_TMUX_PATH_LOG" and produce the clearer "fake claude did not observe a tmux binary in PATH" message; update the make_executable script block that references CLAUDE_CODE_EXPERIMENTAL_AGENT_TEAMS, FAKE_AGENT_TEAMS_LOG, FAKE_TMUX_PATH_LOG, and CMUX_CLAUDE_TEAMS_CMUX_BIN accordingly.
🤖 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 6474-6496: The code accepts a -t target flag via
parseTmuxArguments but never uses it; add an explicit check for
parsed.value("-t") before creating the workspace and fail fast (throw a CLIError
with a clear message like "-t/target routing is not supported") so we don't
silently ignore the user's requested target; update the block around
parseTmuxArguments/parsed and the workspace.create/workspace.select sequence
(referencing parseTmuxArguments, parsed.value("-t"), client.sendV2,
workspace.create, workspace.select, and CLIError) to perform this validation and
return the error when -t is present.
- Around line 4261-4279: The hard-coded help text returned in the switch case
"claude-teams" should be localized: replace the raw multiline string with a
String(localized: "cli.claude-teams.usage", defaultValue: "<full English usage
text>") (or similar key) and add the same English text under that key to
Resources/Localizable.xcstrings; apply the same localization change for the
other occurrence referenced (around the other mentioned location). Locate the
return in the switch case for "claude-teams" in CLI/cmux.swift and update the
code to use the String(localized:..., defaultValue:...) API and ensure the new
keys are added to Localizable.xcstrings.
- Around line 6494-6496: The create path must not auto-change app focus: remove
the implicit call to client.sendV2(method: "workspace.select", params:
["workspace_id": workspaceId]) in the create flow (the branch that checks
parsed.hasFlag("-d")), so workspace creation remains passive; instead require an
explicit select action (e.g., a separate select-window/select-pane/--select
flag/command) to call workspace.select. Apply the same change to the similar
block referenced around the other create path (the code near lines 6533-6538) so
no create path issues an automatic workspace.select.
- Around line 935-937: The claude-teams path drops CLI-only socket/password
overrides because runClaudeTeams rebuilds its child environment from
ProcessInfo.processInfo.environment; modify the call/site so the resolved socket
settings are threaded into claude-teams: fetch the resolved socket and password
used by cmux and inject them into the environment passed to runClaudeTeams (or
change runClaudeTeams signature to accept socket/password parameters), ensuring
the child rebuild uses those values rather than the unmodified
ProcessInfo.processInfo.environment so the tmux shim and cmux __tmux-compat will
reconnect with the correct socket and credentials.
- Around line 6131-6134: The helpers (e.g., tmuxResolveWorkspaceTarget)
currently fall back to the live UI selection instead of using caller-provided
environment IDs; change their defaulting so that when normalizedTmuxTarget(raw)
returns nil they first consult the process environment CMUX_WORKSPACE_ID and
CMUX_SURFACE_ID (and any CMUX_PANE_ID equivalent) and only then fall back to
resolveWorkspaceId(nil, client:) or the workspace's focused pane/surface. Update
tmuxResolveWorkspaceTarget and the similar functions in the other blocks (the
pane/surface resolvers at the ranges noted) to seed defaults from
CMUX_SURFACE_ID/CMUX_WORKSPACE_ID before using workspace.current/focused pane
logic so untargeted commands use the caller context.
- Around line 6155-6167: The name-based workspace fallback is unreachable
because normalizeWorkspaceHandle(token, client: client, allowCurrent: true)
throws for non-UUID/name tokens; change the call so thrown errors don't
short-circuit the function (e.g., use a non-throwing attempt like try? or wrap
in a do/catch and ignore the error) so that if normalizeWorkspaceHandle fails
you still fall through to the title lookup using tmuxWorkspaceItems(client:).
Keep the existing behavior of returning the resolved id when
normalizeWorkspaceHandle succeeds and only throw CLIError(message: "Workspace
target not found: \(token)") if neither the resolver nor the title match
(symbols: normalizeWorkspaceHandle, tmuxWorkspaceItems, CLIError, token,
client).
---
Nitpick comments:
In `@tests/test_cli_claude_teams_env.py`:
- Around line 63-71: The bash shim created by make_executable currently runs
`command -v tmux` under `set -e` so the script will exit with a nonzero status
if tmux is not found, hiding the real cause; modify the embedded script (the
multi-line string passed to make_executable that writes to
"$FAKE_TMUX_PATH_LOG") so tmux discovery never causes the script to exit—e.g.,
run `command -v tmux > "$FAKE_TMUX_PATH_LOG" || printf '__TMUX_MISSING__' >
"$FAKE_TMUX_PATH_LOG"` or otherwise append a fallback/true to the command—so the
test can read "$FAKE_TMUX_PATH_LOG" and produce the clearer "fake claude did not
observe a tmux binary in PATH" message; update the make_executable script block
that references CLAUDE_CODE_EXPERIMENTAL_AGENT_TEAMS, FAKE_AGENT_TEAMS_LOG,
FAKE_TMUX_PATH_LOG, and CMUX_CLAUDE_TEAMS_CMUX_BIN accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 947d7eaa-ab19-4e25-af91-dc1ec9c2e344
📒 Files selected for processing (2)
CLI/cmux.swifttests/test_cli_claude_teams_env.py
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8f14b03ecf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| setenv("CLAUDE_CODE_EXPERIMENTAL_AGENT_TEAMS", "1", 1) | ||
| setenv("CMUX_CLAUDE_TEAMS_CMUX_BIN", executablePath, 1) | ||
| setenv("PATH", updatedPath, 1) |
There was a problem hiding this comment.
runClaudeTeams only exports CLAUDE_CODE_EXPERIMENTAL_AGENT_TEAMS, CMUX_CLAUDE_TEAMS_CMUX_BIN, and PATH before execvp, so tmux shim calls from the launched claude process lose CLI-scoped connection context (for example --socket and --password provided to cmux claude-teams). In multi-instance or password-protected setups this makes __tmux-compat target the wrong socket or fail authentication, so team pane/window commands stop working in the intended cmux session.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d0edbc76dc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let resolvedPaneId = paneId ?? (currentPayload["pane_id"] as? String) | ||
| let resolvedSurfaceId = surfaceId ?? (currentPayload["surface_id"] as? String) |
There was a problem hiding this comment.
tmuxFormatContext falls back to surface.current whenever surfaceId is nil, even if a specific paneId was requested. In the new list-panes path this means non-selected panes inherit the currently focused surface metadata, so format fields like #{pane_title}/#{surface_id} can be wrong for every pane except the active one. This breaks pane-aware scripting that relies on list-panes -F ... returning per-pane values.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (6)
CLI/cmux.swift (6)
935-937:⚠️ Potential issue | 🟠 MajorPreserve the resolved socket settings in
claude-teams.
runClaudeTeamsrebuilds the child environment fromProcessInfo.processInfo.environment, so CLI-only--socket/--passwordoverrides never reach the tmux shim.cmux --socket ... claude-teamscan reconnect to the wrong server, and password-protected sockets will fail unless those values already exist in env/settings.Also applies to: 6430-6451
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CLI/cmux.swift` around lines 935 - 937, The claude-teams path loses CLI-only socket/password overrides because runClaudeTeams reconstructs the child environment from ProcessInfo.processInfo.environment; modify runClaudeTeams (or its caller where command == "claude-teams" and commandArgs is available) to merge the resolved CLI overrides (--socket and --password from commandArgs) into the environment passed to the tmux/shim child process so the child sees the overridden values; specifically ensure any resolved socket/password values override ProcessInfo.processInfo.environment entries before building the child env in runClaudeTeams (also apply the same merge logic in the other affected block around lines ~6430-6451).
6510-6520:⚠️ Potential issue | 🟠 MajorDon't silently ignore
new-window -t.This parser accepts
-t, but the create path never uses it.tmux new-window -t ...will always create in the currently selected cmux window instead of the requested target. If target routing is not supported yet, fail fast instead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CLI/cmux.swift` around lines 6510 - 6520, The parser accepts the -t flag via parseTmuxArguments (valueFlags includes "-t") but the new-window code path ignores it and always creates in the current cmux window; update the new-window handling to either route the creation to the specified target or fail fast: read parsed.value("-t") where you build params for client.sendV2(method: "workspace.create", params: params) and if a target is present, either translate it into the correct routing param (e.g., workspace or window identifier) and include it in params before calling client.sendV2, or if routing is unsupported, throw an error/return a failure explaining that -t is not supported so the flag is not silently ignored.
6155-6167:⚠️ Potential issue | 🟠 MajorKeep the workspace-title fallback reachable.
normalizeWorkspaceHandlethrows on plain names, so targets like-t agentsnever reach the title lookup below. Use a non-throwing attempt here, or only call the normalizer for UUID/ref/index inputs before falling through to the title match.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CLI/cmux.swift` around lines 6155 - 6167, normalizeWorkspaceHandle currently throws for plain workspace names, preventing the title fallback from ever running; change the call in the snippet to a non-throwing attempt so the flow can continue to the tmuxWorkspaceItems title lookup. Replace the throwing call to normalizeWorkspaceHandle(token, client: client, allowCurrent: true) with a safe optional try (e.g. try? normalizeWorkspaceHandle(...)) or wrap it in do/catch and ignore the error, then proceed to use tmuxWorkspaceItems(client:) and the title comparison before throwing CLIError.
4261-4279:⚠️ Potential issue | 🟡 MinorLocalize the new
claude-teamshelp text.These new help/usage strings are still hard-coded English in both
subcommandUsageand the top-level usage output.As per coding guidelines, "All user-facing strings must be localized using
String(localized: "key.name", defaultValue: "English text")and added toResources/Localizable.xcstrings".Also applies to: 8023-8023
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CLI/cmux.swift` around lines 4261 - 4279, The hard-coded help text returned in the switch case "claude-teams" (the multiline string in CLI/cmux.swift) must be localized: replace the raw triple-quoted string with a String(localized: "claude-teams.usage", defaultValue: "...") call (use the existing key naming style) and add the full English text to Resources/Localizable.xcstrings under that key; also find the other hard-coded occurrence referenced (around the second occurrence noted) and perform the same replacement using the same localization key or a clearly related one (e.g., "claude-teams.usage.short" if needed) so all user-facing strings for the claude-teams subcommand use String(localized:..., defaultValue:...).
6131-6134:⚠️ Potential issue | 🟠 MajorDefault tmux targets from the caller context, not the live selection.
When
-tis omitted, these helpers fall back toworkspace.current/ focused pane / focused surface. After the user changes tabs or panes, later untargeted tmux calls from Claude can start operating on the wrong workspace/surface. Seed the fallback fromCMUX_WORKSPACE_IDandCMUX_SURFACE_IDfirst.Also applies to: 6170-6188, 6211-6227
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CLI/cmux.swift` around lines 6131 - 6134, In tmuxResolveWorkspaceTarget(_ raw: String?, client: SocketClient) (and the similar helpers tmuxResolveSurfaceTarget and tmuxResolvePaneTarget), change the fallback logic so that when normalizedTmuxTarget(raw) returns nil you first attempt to seed the target from the environment variables CMUX_WORKSPACE_ID (for workspace), CMUX_SURFACE_ID (for surface) and CMUX_PANE_ID (if applicable) before falling back to resolveWorkspaceId(nil, client:), the focused pane, or focused surface; specifically, read ProcessInfo.processInfo.environment["CMUX_WORKSPACE_ID"/"CMUX_SURFACE_ID"/"CMUX_PANE_ID"], use that value as the token (run through normalizedTmuxTarget or the same normalization used for raw) and only if that env value is absent fall back to the existing live-selection resolution code.
6530-6532:⚠️ Potential issue | 🟠 MajorKeep tmux create paths focus-neutral.
new-windowandsplit-windowimplicitly select/focus the newly created workspace/surface unless-dis passed. That lets a background Claude Teams bootstrap yank the user's current selection. Creation should stay passive; explicitselect-window/select-panecommands can handle focus changes.As per coding guidelines, "Socket/CLI commands must not steal macOS app focus; only explicit focus-intent commands may mutate in-app focus/selection".
Also applies to: 6569-6574
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CLI/cmux.swift` around lines 6530 - 6532, The code is automatically changing in-app focus by calling client.sendV2(method: "workspace.select", params: ["workspace_id": workspaceId]) when parsed.hasFlag("-d") is false; remove that implicit selection from the tmux create paths so creation stays passive. Concretely, delete or disable the client.sendV2 call in the create-window/split-window code paths (the branch using parsed.hasFlag("-d")), and instead only perform workspace.select from a dedicated explicit focus command (e.g., a separate select-window/select-pane CLI path) so that only explicit focus-intent code invokes client.sendV2; apply the same change to the other occurrence referenced near the second create path.
🧹 Nitpick comments (2)
tests/test_cli_claude_teams_env.py (1)
16-33: Consider extractingresolve_cmux_cli()to a shared test utility module.This helper function is duplicated verbatim in
test_cli_claude_teams_skips_wrapper_claude.py. Extracting it to a shared module (e.g.,tests/conftest.pyortests/helpers.py) would reduce maintenance burden and ensure consistent CLI resolution across tests.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_cli_claude_teams_env.py` around lines 16 - 33, The helper function resolve_cmux_cli() is duplicated across tests; extract it into a shared test utility (e.g., tests/conftest.py or tests/helpers.py) and import it from both tests to avoid duplication. Move the entire resolve_cmux_cli implementation into the shared module, export it (or add as a fixture if using pytest), update tests test_cli_claude_teams_env.py and test_cli_claude_teams_skips_wrapper_claude.py to import/use resolve_cmux_cli from the shared module instead of defining it locally, and run tests to ensure no import/name conflicts.tests/test_cli_claude_teams_skips_wrapper_claude.py (1)
59-67: Clarify the wrapper detection mechanism in the test.The wrapper script includes a comment
# cmux claude wrapper - injects hooks and session tracking, which appears to be the marker thatcmux claude-teamsuses to detect and skip wrapper scripts. Consider adding a brief comment in the test explaining this detection mechanism, as it would help future maintainers understand why this specific content is used.📝 Suggested clarification
+ # The wrapper includes the "cmux claude wrapper" marker comment that + # cmux claude-teams uses to detect and skip our own wrapper scripts. make_executable( wrapper_bin / "claude", """#!/usr/bin/env bash # cmux claude wrapper - injects hooks and session tracking🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_cli_claude_teams_skips_wrapper_claude.py` around lines 59 - 67, The test currently writes a wrapper script via make_executable(wrapper_bin / "claude", ...) that includes the comment "# cmux claude wrapper - injects hooks and session tracking" which is used by the cmux claude-teams detection logic to recognize and skip wrapper scripts; add a short explanatory comment in the test (near the make_executable call or above the script contents) stating that this exact marker/comment is intentionally included because the wrapper-detection in the codebase looks for that string to identify cmux wrappers and therefore the test uses it to simulate a wrapper that should be skipped.
🤖 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 6434-6443: The current logic for claudeExecutablePath prefers PATH
over the bundled binary; change it so the bundledClaudePath (computed from
resolvedExecutableURL()) is checked first and used if
FileManager.default.isExecutableFile(atPath:) returns true, and only if that
check fails call resolveClaudeExecutable(searchPath:) to find a PATH-installed
claude; update the initialization of claudeExecutablePath to perform that order
(check bundledClaudePath, then fallback to resolveClaudeExecutable) so the
app-prepackaged claude is used when present.
In `@tests/test_cli_claude_teams_existing_shim.py`:
- Around line 17-34: The resolve_cmux_cli function should avoid searching
/tmp/cmux-* or PATH and instead prefer explicit env vars and the recorded last
CLI path; modify resolve_cmux_cli to first check CMUX_CLI_BIN/CMUX_CLI as it
does, then check for the file referenced by /tmp/cmux-last-cli-path (read that
path, verify os.path.exists and os.access X_OK) before falling back to raising
RuntimeError, and remove or disable the glob("/tmp/cmux-*") and
shutil.which("cmux") branches so tests are deterministic and only succeed when
the explicit env or the recorded last-cli path is valid.
---
Duplicate comments:
In `@CLI/cmux.swift`:
- Around line 935-937: The claude-teams path loses CLI-only socket/password
overrides because runClaudeTeams reconstructs the child environment from
ProcessInfo.processInfo.environment; modify runClaudeTeams (or its caller where
command == "claude-teams" and commandArgs is available) to merge the resolved
CLI overrides (--socket and --password from commandArgs) into the environment
passed to the tmux/shim child process so the child sees the overridden values;
specifically ensure any resolved socket/password values override
ProcessInfo.processInfo.environment entries before building the child env in
runClaudeTeams (also apply the same merge logic in the other affected block
around lines ~6430-6451).
- Around line 6510-6520: The parser accepts the -t flag via parseTmuxArguments
(valueFlags includes "-t") but the new-window code path ignores it and always
creates in the current cmux window; update the new-window handling to either
route the creation to the specified target or fail fast: read parsed.value("-t")
where you build params for client.sendV2(method: "workspace.create", params:
params) and if a target is present, either translate it into the correct routing
param (e.g., workspace or window identifier) and include it in params before
calling client.sendV2, or if routing is unsupported, throw an error/return a
failure explaining that -t is not supported so the flag is not silently ignored.
- Around line 6155-6167: normalizeWorkspaceHandle currently throws for plain
workspace names, preventing the title fallback from ever running; change the
call in the snippet to a non-throwing attempt so the flow can continue to the
tmuxWorkspaceItems title lookup. Replace the throwing call to
normalizeWorkspaceHandle(token, client: client, allowCurrent: true) with a safe
optional try (e.g. try? normalizeWorkspaceHandle(...)) or wrap it in do/catch
and ignore the error, then proceed to use tmuxWorkspaceItems(client:) and the
title comparison before throwing CLIError.
- Around line 4261-4279: The hard-coded help text returned in the switch case
"claude-teams" (the multiline string in CLI/cmux.swift) must be localized:
replace the raw triple-quoted string with a String(localized:
"claude-teams.usage", defaultValue: "...") call (use the existing key naming
style) and add the full English text to Resources/Localizable.xcstrings under
that key; also find the other hard-coded occurrence referenced (around the
second occurrence noted) and perform the same replacement using the same
localization key or a clearly related one (e.g., "claude-teams.usage.short" if
needed) so all user-facing strings for the claude-teams subcommand use
String(localized:..., defaultValue:...).
- Around line 6131-6134: In tmuxResolveWorkspaceTarget(_ raw: String?, client:
SocketClient) (and the similar helpers tmuxResolveSurfaceTarget and
tmuxResolvePaneTarget), change the fallback logic so that when
normalizedTmuxTarget(raw) returns nil you first attempt to seed the target from
the environment variables CMUX_WORKSPACE_ID (for workspace), CMUX_SURFACE_ID
(for surface) and CMUX_PANE_ID (if applicable) before falling back to
resolveWorkspaceId(nil, client:), the focused pane, or focused surface;
specifically, read
ProcessInfo.processInfo.environment["CMUX_WORKSPACE_ID"/"CMUX_SURFACE_ID"/"CMUX_PANE_ID"],
use that value as the token (run through normalizedTmuxTarget or the same
normalization used for raw) and only if that env value is absent fall back to
the existing live-selection resolution code.
- Around line 6530-6532: The code is automatically changing in-app focus by
calling client.sendV2(method: "workspace.select", params: ["workspace_id":
workspaceId]) when parsed.hasFlag("-d") is false; remove that implicit selection
from the tmux create paths so creation stays passive. Concretely, delete or
disable the client.sendV2 call in the create-window/split-window code paths (the
branch using parsed.hasFlag("-d")), and instead only perform workspace.select
from a dedicated explicit focus command (e.g., a separate
select-window/select-pane CLI path) so that only explicit focus-intent code
invokes client.sendV2; apply the same change to the other occurrence referenced
near the second create path.
---
Nitpick comments:
In `@tests/test_cli_claude_teams_env.py`:
- Around line 16-33: The helper function resolve_cmux_cli() is duplicated across
tests; extract it into a shared test utility (e.g., tests/conftest.py or
tests/helpers.py) and import it from both tests to avoid duplication. Move the
entire resolve_cmux_cli implementation into the shared module, export it (or add
as a fixture if using pytest), update tests test_cli_claude_teams_env.py and
test_cli_claude_teams_skips_wrapper_claude.py to import/use resolve_cmux_cli
from the shared module instead of defining it locally, and run tests to ensure
no import/name conflicts.
In `@tests/test_cli_claude_teams_skips_wrapper_claude.py`:
- Around line 59-67: The test currently writes a wrapper script via
make_executable(wrapper_bin / "claude", ...) that includes the comment "# cmux
claude wrapper - injects hooks and session tracking" which is used by the cmux
claude-teams detection logic to recognize and skip wrapper scripts; add a short
explanatory comment in the test (near the make_executable call or above the
script contents) stating that this exact marker/comment is intentionally
included because the wrapper-detection in the codebase looks for that string to
identify cmux wrappers and therefore the test uses it to simulate a wrapper that
should be skipped.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 42b92010-b9b1-4e7c-805c-1d1c2269a7c5
📒 Files selected for processing (5)
CLI/cmux.swiftscripts/reload.shtests/test_cli_claude_teams_env.pytests/test_cli_claude_teams_existing_shim.pytests/test_cli_claude_teams_skips_wrapper_claude.py
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 32b8693a7c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
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_claude_teams_env.py">
<violation number="1" location="tests/test_cli_claude_teams_env.py:92">
P2: The new TMUX/TERM assertions rely on inherited host env values, so this regression test can become non-deterministic and miss launcher regressions. Seed conflicting baseline values in `env` before invoking `cmux` so the checks verify launcher behavior, not ambient shell state.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (4)
CLI/cmux.swift (4)
935-937:⚠️ Potential issue | 🟠 MajorPreserve
--socket/--passwordoverrides inclaude-teams.This path still launches
claude-teamswithout the resolved socket/password, socmux --socket ... claude-teamsand--passwordfall back to ambient env/defaults once the tmux shim starts issuing__tmux-compatcommands.Also applies to: 6468-6486
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CLI/cmux.swift` around lines 935 - 937, The claude-teams launch path ignores resolved --socket/--password overrides; modify the handling so runClaudeTeams receives the resolved values instead of falling back to ambient defaults—specifically, when command == "claude-teams" ensure commandArgs (or the env passed into runClaudeTeams) includes the normalized/resolved socket and password flags or sets the corresponding environment variables before calling runClaudeTeams(commandArgs:); update any alternate entry points (including the similar block around runClaudeTeams at lines ~6468-6486) to pass through the same resolved overrides so the tmux shim gets the explicit --socket/--password values.
6157-6167:⚠️ Potential issue | 🟠 MajorWorkspace-title lookup is still unreachable.
normalizeWorkspaceHandle(token, ...)throws for non-UUID/ref/index tokens, so named targets never reach the title match below. Use a non-throwing probe (try?or guarded type check) before falling back totmuxWorkspaceItems.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CLI/cmux.swift` around lines 6157 - 6167, The title-lookup branch is unreachable because normalizeWorkspaceHandle(token, client:allowCurrent:) throws for non-UUID/ref/index tokens; change the call to a non-throwing probe (e.g., use try? or wrap in do/catch and ignore errors) so that if normalization fails you continue to the named-title fallback using tmuxWorkspaceItems; specifically adjust the call to normalizeWorkspaceHandle in this block (currently using try) to use try? (or a guarded do/catch that returns nil) so the needle/title matching logic (needle, items, match, id) can execute when normalization does not succeed.
6546-6568:⚠️ Potential issue | 🟠 MajorKeep create paths passive until target routing is real.
new-windowstill accepts-twithout using it, and both create paths immediately callworkspace.select/surface.focus. That makes the result depend on whichever window/workspace is live when the command runs and lets background Claude bootstrap yank the user's current selection. Either honor caller/target routing without changing selection, or reject unsupported-tfor now. As per coding guidelines,Socket/CLI commands must not steal macOS app focus; only explicit focus-intent commands may mutate in-app focus/selection.Also applies to: 6583-6610
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CLI/cmux.swift` around lines 6546 - 6568, The create-path code calls workspace.create and then unconditionally calls workspace.select (and elsewhere surface.focus), which steals focus and ignores -t; update the logic in the block that uses parseTmuxArguments / parsed (references: parseTmuxArguments, parsed.value("-t"), parsed.hasFlag("-d"), client.sendV2 with "workspace.create"/"workspace.select"/"workspace.rename") so that if a target (-t) is provided you either (A) honor the routing by not changing selection/focus here (do not call "workspace.select" or "surface.focus"), or (B) reject/return an error when -t is passed until routing is implemented; also ensure that the no-focus behavior applies to the analogous create path later (the second block around parseTmuxArguments at 6583-6610). Make the change so only explicit focus-intent commands perform selection/focus.
6133-6136:⚠️ Potential issue | 🟠 MajorDefault untargeted tmux commands from the caller context.
When
-tis omitted, these helpers still fall back toworkspace.current/ focused pane / focused surface. If the user changes tabs or panes after launching Claude, later untargeted commands start mutating the wrong workspace/surface. Seed defaults fromCMUX_WORKSPACE_ID/CMUX_SURFACE_IDfirst.Also applies to: 6172-6190, 6213-6229
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CLI/cmux.swift` around lines 6133 - 6136, The current tmux target resolution in tmuxResolveWorkspaceTarget (and similar helpers around the other ranges) falls back to the live focused workspace/pane when -t is omitted; instead, first check environment defaults CMUX_WORKSPACE_ID and CMUX_SURFACE_ID and use them as the seed/default target before falling back to resolveWorkspaceId(nil, client:) or focused pane logic. Modify tmuxResolveWorkspaceTarget to: call normalizedTmuxTarget(raw) as before, but if nil then read CMUX_WORKSPACE_ID/CMUX_SURFACE_ID from the environment and, if present, use those values to construct the target (or pass the workspace id into resolveWorkspaceId) before using the live-focused fallback; apply the same change pattern to the other helper functions in the blocks around lines 6172-6190 and 6213-6229 (i.e., functions that currently call normalizedTmuxTarget and resolveWorkspaceId) so untargeted tmux commands prefer the seeded env defaults.
🧹 Nitpick comments (2)
tests/test_cli_claude_teams_env.py (2)
94-100: Bound the launcher invocation with a timeout.If
cmux claude-teamsregresses into a hang, this test will wedge the job indefinitely. Give the subprocess a timeout and surface the timeout as the sameFAIL:diagnostic.Proposed change
- proc = subprocess.run( - [cli_path, "claude-teams", "--version"], - capture_output=True, - text=True, - check=False, - env=env, - ) + try: + proc = subprocess.run( + [cli_path, "claude-teams", "--version"], + capture_output=True, + text=True, + check=False, + env=env, + timeout=30, + ) + except subprocess.TimeoutExpired as exc: + print("FAIL: `cmux claude-teams --version` timed out") + print(f"stdout={(exc.stdout or '').strip()}") + print(f"stderr={(exc.stderr or '').strip()}") + return 1🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_cli_claude_teams_env.py` around lines 94 - 100, The subprocess.run call invoking [cli_path, "claude-teams", "--version"] in tests/test_cli_claude_teams_env.py must be bounded with a timeout to avoid hanging; add a timeout argument (e.g., timeout=some_seconds) to the subprocess.run call and wrap the invocation in a try/except catching subprocess.TimeoutExpired, then synthesize the same failure output currently used for other failures (the FAIL: diagnostic) and assign it to the same variable (proc or equivalent) so the rest of the test logic treats a timeout the same as a normal failure; reference the existing symbols proc, cli_path, and env when making this change.
21-33: Prefer the recorded CLI path before scanning build artifacts.Sorting DerivedData and
/tmp/cmux-*by mtime can pick a different checkout’scmuxwhen multiple local builds exist. Checking the last reloaded CLI path first would make this regression much more deterministic for local runs.Proposed change
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 + + recorded_path = Path("/tmp/cmux-last-cli-path") + if recorded_path.exists(): + recorded = recorded_path.read_text(encoding="utf-8").strip() + if recorded and os.path.exists(recorded) and os.access(recorded, os.X_OK): + return recorded candidates: list[str] = []🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_cli_claude_teams_env.py` around lines 21 - 33, The test helper currently scans build artifact locations (candidates list) before checking for an existing CLI path, which can pick the wrong local build; update the lookup order so the recorded/installed CLI is preferred: first check environment variable "CMUX_CLI_BIN" (or the recorded path if used elsewhere), then check shutil.which("cmux") via in_path, and only if those are not found proceed to build-artifact scanning (the existing candidates logic that uses glob, os.path.exists, os.access, sort by os.path.getmtime, etc.); adjust the function that contains candidates, in_path, and the final RuntimeError to follow this new precedence.
🤖 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 6391-6407: The resolveClaudeExecutable helper currently assigns
fallback before checking isCmuxClaudeWrapper, causing wrappers to be treated as
fallbacks and preventing the bundled fallback from being used; change the logic
in resolveClaudeExecutable so you only set fallback when isCmuxClaudeWrapper(at:
candidate) returns true (i.e., move fallback = candidate into the wrapper branch
after the wrapper check) and return non-wrapper candidates immediately as now;
apply the same corrective change to the analogous block referenced (the other
resolver at the same file region, e.g., the block around lines 6472-6481) so
wrappers are only recorded as fallbacks when no real executable exists.
- Around line 6172-6190: tmuxFormatContext emits pane handles as "%<pane-id>"
but the launcher injects a synthetic "%1" so tmuxResolvePaneTarget (which strips
the leading "%" and calls normalizePaneHandle) will always resolve to pane 1;
fix by making the launcher emit the real pane handle from tmuxFormatContext
(preserve the "%<pane-id>" string) instead of hard-coding "%1", and ensure
tmuxResolvePaneTarget/normalizePaneHandle continue to accept and strip the "%"
prefix; update any other places noted (blocks around tmuxResolvePaneTarget, the
launcher code that sets TMUX_PANE, and uses of tmuxFormatContext in the ranges
you mentioned) to propagate the real pane id rather than a fixed value.
In `@tests/test_cli_claude_teams_env.py`:
- Around line 83-92: The test currently copies the caller's environment into env
and thus inherits HOME; update the setup that populates env (the env dict where
PATH and FAKE_* keys are set) to set env["HOME"] to a temporary directory (the
test sandbox) so the launcher cannot read/write ~/.cmuxterm/claude-teams-bin
outside the temp area; locate the env creation code (the env = os.environ.copy()
block and subsequent env["..."] assignments) and add setting HOME to the test
tempdir, and remove or keep reuse-specific assertions about real-home paths out
of this test (leave those checks in the existing-shim reuse test referenced
around the other assertions).
---
Duplicate comments:
In `@CLI/cmux.swift`:
- Around line 935-937: The claude-teams launch path ignores resolved
--socket/--password overrides; modify the handling so runClaudeTeams receives
the resolved values instead of falling back to ambient defaults—specifically,
when command == "claude-teams" ensure commandArgs (or the env passed into
runClaudeTeams) includes the normalized/resolved socket and password flags or
sets the corresponding environment variables before calling
runClaudeTeams(commandArgs:); update any alternate entry points (including the
similar block around runClaudeTeams at lines ~6468-6486) to pass through the
same resolved overrides so the tmux shim gets the explicit --socket/--password
values.
- Around line 6157-6167: The title-lookup branch is unreachable because
normalizeWorkspaceHandle(token, client:allowCurrent:) throws for
non-UUID/ref/index tokens; change the call to a non-throwing probe (e.g., use
try? or wrap in do/catch and ignore errors) so that if normalization fails you
continue to the named-title fallback using tmuxWorkspaceItems; specifically
adjust the call to normalizeWorkspaceHandle in this block (currently using try)
to use try? (or a guarded do/catch that returns nil) so the needle/title
matching logic (needle, items, match, id) can execute when normalization does
not succeed.
- Around line 6546-6568: The create-path code calls workspace.create and then
unconditionally calls workspace.select (and elsewhere surface.focus), which
steals focus and ignores -t; update the logic in the block that uses
parseTmuxArguments / parsed (references: parseTmuxArguments, parsed.value("-t"),
parsed.hasFlag("-d"), client.sendV2 with
"workspace.create"/"workspace.select"/"workspace.rename") so that if a target
(-t) is provided you either (A) honor the routing by not changing
selection/focus here (do not call "workspace.select" or "surface.focus"), or (B)
reject/return an error when -t is passed until routing is implemented; also
ensure that the no-focus behavior applies to the analogous create path later
(the second block around parseTmuxArguments at 6583-6610). Make the change so
only explicit focus-intent commands perform selection/focus.
- Around line 6133-6136: The current tmux target resolution in
tmuxResolveWorkspaceTarget (and similar helpers around the other ranges) falls
back to the live focused workspace/pane when -t is omitted; instead, first check
environment defaults CMUX_WORKSPACE_ID and CMUX_SURFACE_ID and use them as the
seed/default target before falling back to resolveWorkspaceId(nil, client:) or
focused pane logic. Modify tmuxResolveWorkspaceTarget to: call
normalizedTmuxTarget(raw) as before, but if nil then read
CMUX_WORKSPACE_ID/CMUX_SURFACE_ID from the environment and, if present, use
those values to construct the target (or pass the workspace id into
resolveWorkspaceId) before using the live-focused fallback; apply the same
change pattern to the other helper functions in the blocks around lines
6172-6190 and 6213-6229 (i.e., functions that currently call
normalizedTmuxTarget and resolveWorkspaceId) so untargeted tmux commands prefer
the seeded env defaults.
---
Nitpick comments:
In `@tests/test_cli_claude_teams_env.py`:
- Around line 94-100: The subprocess.run call invoking [cli_path,
"claude-teams", "--version"] in tests/test_cli_claude_teams_env.py must be
bounded with a timeout to avoid hanging; add a timeout argument (e.g.,
timeout=some_seconds) to the subprocess.run call and wrap the invocation in a
try/except catching subprocess.TimeoutExpired, then synthesize the same failure
output currently used for other failures (the FAIL: diagnostic) and assign it to
the same variable (proc or equivalent) so the rest of the test logic treats a
timeout the same as a normal failure; reference the existing symbols proc,
cli_path, and env when making this change.
- Around line 21-33: The test helper currently scans build artifact locations
(candidates list) before checking for an existing CLI path, which can pick the
wrong local build; update the lookup order so the recorded/installed CLI is
preferred: first check environment variable "CMUX_CLI_BIN" (or the recorded path
if used elsewhere), then check shutil.which("cmux") via in_path, and only if
those are not found proceed to build-artifact scanning (the existing candidates
logic that uses glob, os.path.exists, os.access, sort by os.path.getmtime,
etc.); adjust the function that contains candidates, in_path, and the final
RuntimeError to follow this new precedence.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 62bd8bc7-bf01-4ded-81c6-97383fd1118a
📒 Files selected for processing (2)
CLI/cmux.swifttests/test_cli_claude_teams_env.py
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8b6120f4fb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| guard let paneId = try normalizePaneHandle( | ||
| paneHandle, | ||
| client: client, | ||
| workspaceHandle: workspaceId, | ||
| allowFocused: true |
There was a problem hiding this comment.
When -t specifies only a window/workspace (for example select-pane -t @2 or resize-pane -t 2), this path calls normalizePaneHandle with paneHandle == nil and allowFocused: true. In that nil branch, normalizePaneHandle returns the globally focused pane from system.identify rather than a pane in workspaceId, so the resulting (workspace_id, pane_id) pair can point to different workspaces and pane.focus/pane.resize will fail or hit the wrong pane. The fallback pane should be resolved from the target workspace (for example via pane.list scoped to workspaceId).
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0375b52ee8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let created = try client.sendV2(method: "surface.split", params: [ | ||
| "workspace_id": target.workspaceId, | ||
| "surface_id": target.surfaceId, | ||
| "direction": direction | ||
| ]) |
There was a problem hiding this comment.
The split-window shim parses -l as a value flag, but the parsed size is never used when issuing the split request, so tmux split-window -l <size> currently produces the same layout as a plain split. This breaks tmux-compatible sizing behavior for Claude teammate flows that rely on explicit pane sizes, and can leave teammates with incorrect pane geometry even though the command appears to succeed.
Useful? React with 👍 / 👎.
* Add claude-teams CLI command * Add claude-teams launcher regression test * Exec claude-teams launcher in place * Add existing-shim claude-teams regression test * Reuse claude-teams shim and refresh dev CLI * Add wrapper-selection claude-teams regression test * Launch real claude binary for claude-teams * Add claude-teams auto-mode launcher regression test * Default claude-teams to fake tmux auto mode * Build tagged reloads under DerivedData * Add claude-teams tmux sequence regression test * Fix claude-teams tmux teammate compatibility * Add claude-teams split focus regression test * Keep claude-teams leader pane focused * Tighten claude-teams review fixes * Pass claude-teams help through to Claude * Use sentinel TERM_PROGRAM in claude-teams test
1 participant
Summary
cmux claude-teamsas a launcher forCLAUDE_CODE_EXPERIMENTAL_AGENT_TEAMS=1 claudetmuxshim that maps supported tmux window and pane commands into cmux workspace and split operationsTesting
swiftc -parse-as-library -typecheck CLI/cmux.swiftpython3 -m py_compile tests/test_cli_claude_teams_env.py./scripts/setup.sh./scripts/reload.sh --tag task-claude-teams-commandclaudebinary and verifiedCLAUDE_CODE_EXPERIMENTAL_AGENT_TEAMS=1, the private tmux shim path, andCMUX_CLAUDE_TEAMS_CMUX_BINIssues
cmux claude-teamscommand that startsCLAUDE_CODE_EXPERIMENTAL_AGENT_TEAMS=1 claudeand intercepts tmux commands so they open cmux workspaces and splits in the same sessionSummary by cubic
Adds a
cmux claude-teamslauncher to run Claude Code with agent teams and tmux compatibility. It defaults to teammate auto mode, injects a tmux-like env so Claude uses cmux splits in the same session, forwards--helptoclaude, and keeps the leader pane focused during splits.New Features
cmux claude-teamssetsCLAUDE_CODE_EXPERIMENTAL_AGENT_TEAMS=1, defaults--teammate-mode auto(unless provided), injects a tmux-like env (TMUX,TMUX_PANE,TERM=screen-256colororCMUX_CLAUDE_TEAMS_TERM, unsetsTERM_PROGRAM), prepends a private tmux shim to PATH (reuses~/.cmuxterm/claude-teams-bin), setsCMUX_CLAUDE_TEAMS_CMUX_BIN, resolves and execs the realclaude(skips cmux wrapper scripts; falls back to a bundled binary), and exportsCMUX_SOCKET_PATH/CMUX_SOCKETandCMUX_SOCKET_PASSWORDwhen provided. Usage/help added and localized.__tmux-compatsupports common window/pane commands (new-session/window, split-window, select/kill, send-keys with literals/special keys, capture-pane with scrollback, list windows/panes, rename-window, resize-pane, display-message; passthrough forwait-forand related). Correctly handlessplit-window -l <size>%, rendersdisplay-messageformats from cmux context, and preserves leader focus during splits. Regression tests cover env injection, stable shim reuse, wrapper skipping, the tmux teammate sequence, split focus, and help passthrough.Refactors
scripts/reload.shbuilds tagged reloads under~/Library/Developer/Xcode/DerivedData/cmux-<tag>, adds a/tmp/cmux-<tag>symlink for compatibility, updates cleanup reminders to cover both paths, and records the last CLI path for tests.Written for commit 0375b52. Summary will update on new commits.
Summary by CodeRabbit
New Features
Tests
Chores