Skip to content

Fix tmux compat store decoding, layout cleanup, and cross-workspace fallback #2207

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
lawrencecchen merged 5 commits into from
Mar 27, 2026
Merged

Fix tmux compat store decoding, layout cleanup, and cross-workspace fallback #2207

Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
493da5c
Fix tmux compat store decoding, layout cleanup, and cross-workspace f…
lawrencecchen Mar 26, 2026
e73ee78
Add auto-equalize after teammate splits
lawrencecchen Mar 26, 2026
2034079
Refactor: DRY up claude-teams and omo shared launcher code
lawrencecchen Mar 27, 2026
1e11f5b
Address PR review comments: store load, stale state, pane targets
lawrencecchen Mar 27, 2026
2a5e6e1
Fix select-layout -t fallback: don't apply to wrong workspace
lawrencecchen Mar 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 35 additions & 2 deletions CLI/cmux.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10032,10 +10032,14 @@ struct CMUXCLI {
// Claude's agent teams targets arbitrary panes (from list-panes),
// not necessarily the leader pane from TMUX_PANE. Override the
// target to anchor all teammate splits to the leader surface.
// Only apply caller anchoring when the caller's workspace resolves
// successfully. Falling back to target.workspaceId would pair
// the caller's surface with a different workspace, creating an
// invalid cross-workspace split.
let store = loadTmuxCompatStore()

@greptile-apps greptile-apps Bot Mar 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Unconditional store load before optional-binding guard

loadTmuxCompatStore() (a file-system read) is executed unconditionally on every split-window call, even when tmuxCallerSurfaceHandle(), tmuxCallerWorkspaceHandle(), or resolveWorkspaceId fail and the store is never consulted. The store load is only needed when the if let block below actually executes.

Consider moving the let store = loadTmuxCompatStore() line inside the if let body (before the mainVerticalLayouts lookup) to avoid the unnecessary I/O on every non-agent-team split:

Suggested change
let store = loadTmuxCompatStore()
if let callerSurface = tmuxCallerSurfaceHandle(),
let callerWorkspace = tmuxCallerWorkspaceHandle(),
let wsId = try? resolveWorkspaceId(callerWorkspace, client: client) {
let store = loadTmuxCompatStore()

(This is a pre-existing pattern, not a regression from this PR, but the fix here is a good moment to address it.)

if let callerSurface = tmuxCallerSurfaceHandle(),
let callerWorkspace = tmuxCallerWorkspaceHandle() {
let wsId = (try? resolveWorkspaceId(callerWorkspace, client: client)) ?? target.workspaceId
let callerWorkspace = tmuxCallerWorkspaceHandle(),
let wsId = try? resolveWorkspaceId(callerWorkspace, client: client) {
if let mvState = store.mainVerticalLayouts[wsId],
let lastColumn = mvState.lastColumnSurfaceId {
// Main-vertical active: stack in right column.
Expand Down Expand Up @@ -10079,6 +10083,14 @@ struct CMUXCLI {
try saveTmuxCompatStore(updatedStore)
}

// Equalize vertical splits so teammate panes are evenly distributed.
// Use orientation: "vertical" to only equalize the agent column,
// preserving the leader/column horizontal divider position.
_ = try? client.sendV2(method: "workspace.equalize_splits", params: [
"workspace_id": target.workspaceId,
"orientation": "vertical"
])
Comment on lines +10098 to +10104

@coderabbitai coderabbitai Bot Mar 27, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Add deferred equalization to avoid post-split layout races.

Line 10089 should include deferred: true so equalization runs after the next layout pass; otherwise the new split can be equalized against a stale tree.

💡 Proposed fix 
             _ = try? client.sendV2(method: "workspace.equalize_splits", params: [
                 "workspace_id": target.workspaceId,
-                "orientation": "vertical"
+                "orientation": "vertical",
+                "deferred": true
             ])
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@CLI/cmux.swift` around lines 10086 - 10092, The call to client.sendV2 for
"workspace.equalize_splits" should pass deferred: true so equalization runs
after the next layout pass; update the params dictionary provided to sendV2 in
the block that calls workspace.equalize_splits (where client.sendV2 is invoked
with "workspace_id": target.workspaceId and "orientation": "vertical") to
include "deferred": true so the equalization uses the updated layout tree.


if let text = tmuxShellCommandText(commandTokens: parsed.positional, cwd: parsed.value("-c")) {
_ = try client.sendV2(method: "surface.send_text", params: [
"workspace_id": target.workspaceId,
Expand Down Expand Up @@ -10369,6 +10381,14 @@ struct CMUXCLI {
)
try saveTmuxCompatStore(store)
}
} else if !layoutName.isEmpty {
// Non-main-vertical layout selected: clear stale state so
// future splits don't incorrectly redirect to the old column.
let workspaceId = try tmuxResolveWorkspaceTarget(parsed.value("-t"), client: client)
var store = loadTmuxCompatStore()
if store.mainVerticalLayouts.removeValue(forKey: workspaceId) != nil {
try saveTmuxCompatStore(store)
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
}
Comment on lines +10412 to 10421

@greptile-apps greptile-apps Bot Mar 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 lastSplitSurface not cleared alongside mainVerticalLayouts

The comment says "clear stale state," but only mainVerticalLayouts[workspaceId] is removed. lastSplitSurface[workspaceId] is left with the surface ID from before the layout switch.

This matters on the path: split-window → select-layout main-vertical → select-layout tiled → select-layout main-vertical again. On the second main-vertical call, mainVerticalLayouts[workspaceId] is nil (correctly cleared here), so the code at line 9854 falls back to store.lastSplitSurface[workspaceId] to seed lastColumnSurfaceId. That stale value refers to a pane whose position may be arbitrary after the tiled re-layout.

Consider also removing lastSplitSurface[workspaceId] in the same block, and only writing the store when either removal found an entry, to keep both fields in sync.


case "set-option", "set", "set-window-option", "setw", "source-file", "refresh-client", "attach-session", "detach-client":
Expand Down Expand Up @@ -10400,6 +10420,19 @@ struct CMUXCLI {
/// Used to seed lastColumnSurfaceId when select-layout main-vertical
/// is called after the first split.
var lastSplitSurface: [String: String] = [:]

/// Custom decoder so older store files missing newer keys
/// (mainVerticalLayouts, lastSplitSurface) decode gracefully
/// instead of throwing and resetting the entire store.
init(from decoder: Decoder) throws {
let container = try decoder.container(keyedBy: CodingKeys.self)
buffers = try container.decodeIfPresent([String: String].self, forKey: .buffers) ?? [:]
hooks = try container.decodeIfPresent([String: String].self, forKey: .hooks) ?? [:]
mainVerticalLayouts = try container.decodeIfPresent([String: MainVerticalState].self, forKey: .mainVerticalLayouts) ?? [:]
lastSplitSurface = try container.decodeIfPresent([String: String].self, forKey: .lastSplitSurface) ?? [:]
}

init() {}
}

private func tmuxCompatStoreURL() -> URL {
Expand Down
Loading