From f39d0e2d81beb3c7cc65bd5855c9de9e53adcf69 Mon Sep 17 00:00:00 2001 From: JR Date: Sat, 4 Apr 2026 13:08:02 +0200 Subject: [PATCH 1/2] fix: handle split-window in __tmux-compat when surface resolution fails When `cmux __tmux-compat split-window -h` is called and the client-side surface resolution fails (e.g., env vars missing/stale, transient server state), the command now falls back to resolving just the workspace and lets the server pick the focused surface for the split. This matches the behavior of `cmux new-split`, which works because the server's surface.split handler already has focused-surface fallback logic. Previously, tmuxSplitWindow would bail out immediately if tmuxResolveSurfaceTarget returned an error, even though the server could still perform the split using its internal focused surface. Fixes #2592 --- daemon/remote/cmd/cmuxd-remote/tmux_compat.go | 9 +- .../cmd/cmuxd-remote/tmux_split_ref_test.go | 281 ++++++++++++++++++ 2 files changed, 289 insertions(+), 1 deletion(-) diff --git a/daemon/remote/cmd/cmuxd-remote/tmux_compat.go b/daemon/remote/cmd/cmuxd-remote/tmux_compat.go index 0f06008642ec..db8b3beb59f9 100644 --- a/daemon/remote/cmd/cmuxd-remote/tmux_compat.go +++ b/daemon/remote/cmd/cmuxd-remote/tmux_compat.go @@ -1188,7 +1188,14 @@ func tmuxSplitWindow(rc *rpcContext, args []string) error { targetWs, _, targetSurface, err := tmuxResolveSurfaceTarget(rc, p.value("-t")) if err != nil { - return err + // Surface resolution can fail when env vars are missing or stale. + // Fall back to resolving just the workspace and let the server + // pick the focused surface for the split. + targetWs, err = tmuxResolveWorkspaceTarget(rc, p.value("-t")) + if err != nil { + return err + } + targetSurface = "" } direction := "down" diff --git a/daemon/remote/cmd/cmuxd-remote/tmux_split_ref_test.go b/daemon/remote/cmd/cmuxd-remote/tmux_split_ref_test.go index f3811173de6a..199590c1d20c 100644 --- a/daemon/remote/cmd/cmuxd-remote/tmux_split_ref_test.go +++ b/daemon/remote/cmd/cmuxd-remote/tmux_split_ref_test.go @@ -173,6 +173,287 @@ func TestTmuxSplitWindowCanonicalizesCallerSurfaceRefs(t *testing.T) { } } +// startMockTmuxCompatSocketWithFocusFallback creates a mock that accepts +// surface.split with either the known surface UUID or an empty surface_id, +// mimicking the real server's focused-surface fallback. +func startMockTmuxCompatSocketWithFocusFallback(t *testing.T) string { + t.Helper() + sockPath := makeShortUnixSocketPath(t) + + ln, err := net.Listen("unix", sockPath) + if err != nil { + t.Fatalf("failed to listen: %v", err) + } + t.Cleanup(func() { ln.Close() }) + + go func() { + for { + conn, err := ln.Accept() + if err != nil { + return + } + go func(conn net.Conn) { + defer conn.Close() + reader := bufio.NewReader(conn) + line, err := reader.ReadBytes('\n') + if err != nil { + return + } + + var req map[string]any + if err := json.Unmarshal(line, &req); err != nil { + _, _ = conn.Write([]byte(`{"ok":false,"error":{"code":"parse","message":"bad json"}}` + "\n")) + return + } + + method, _ := req["method"].(string) + resp := map[string]any{ + "id": req["id"], + "ok": true, + } + + switch method { + case "workspace.list": + resp["result"] = map[string]any{ + "workspaces": []map[string]any{{ + "id": "11111111-1111-4111-8111-111111111111", + "ref": "workspace:1", + "index": 1, + "title": "demo", + }}, + } + case "workspace.current": + resp["result"] = map[string]any{ + "workspace_id": "11111111-1111-4111-8111-111111111111", + "workspace_ref": "workspace:1", + } + case "surface.list": + resp["result"] = map[string]any{"surfaces": []map[string]any{{ + "id": "44444444-4444-4444-8444-444444444444", + "ref": "surface:1", + "focused": true, + "pane_id": "33333333-3333-4333-8333-333333333333", + "title": "leader", + }}} + case "surface.current": + resp["result"] = map[string]any{ + "workspace_id": "11111111-1111-4111-8111-111111111111", + "pane_id": "33333333-3333-4333-8333-333333333333", + "surface_id": "44444444-4444-4444-8444-444444444444", + } + case "pane.list": + resp["result"] = map[string]any{"panes": []map[string]any{{ + "id": "33333333-3333-4333-8333-333333333333", + "ref": "pane:1", + "index": 1, + }}} + case "surface.split": + // Accept the known surface UUID or empty (server fallback) + resp["result"] = map[string]any{ + "surface_id": "77777777-7777-4777-8777-777777777777", + "pane_id": "66666666-6666-4666-8666-666666666666", + } + case "workspace.equalize_splits": + resp["result"] = map[string]any{"ok": true} + case "pane.surfaces": + resp["result"] = map[string]any{"surfaces": []map[string]any{{ + "id": "44444444-4444-4444-8444-444444444444", + "selected": true, + }}} + default: + resp["ok"] = false + resp["error"] = map[string]any{ + "code": "unsupported", + "message": method, + } + } + + payload, _ := json.Marshal(resp) + _, _ = conn.Write(append(payload, '\n')) + }(conn) + } + }() + + return sockPath +} + +func TestTmuxSplitWindowWithoutSurfaceEnv(t *testing.T) { + origHome := os.Getenv("HOME") + origWorkspace := os.Getenv("CMUX_WORKSPACE_ID") + origSurface := os.Getenv("CMUX_SURFACE_ID") + origPane := os.Getenv("TMUX_PANE") + os.Setenv("HOME", t.TempDir()) + os.Unsetenv("CMUX_WORKSPACE_ID") + os.Unsetenv("CMUX_SURFACE_ID") + os.Unsetenv("TMUX_PANE") + defer func() { + os.Setenv("HOME", origHome) + if origWorkspace != "" { + os.Setenv("CMUX_WORKSPACE_ID", origWorkspace) + } else { + os.Unsetenv("CMUX_WORKSPACE_ID") + } + if origSurface != "" { + os.Setenv("CMUX_SURFACE_ID", origSurface) + } else { + os.Unsetenv("CMUX_SURFACE_ID") + } + if origPane != "" { + os.Setenv("TMUX_PANE", origPane) + } else { + os.Unsetenv("TMUX_PANE") + } + }() + + sockPath := startMockTmuxCompatSocketWithFocusFallback(t) + rc := &rpcContext{socketPath: sockPath} + + output := captureStdout(t, func() { + if err := dispatchTmuxCommand(rc, "split-window", []string{"-h", "-P", "-F", "#{pane_id}"}); err != nil { + t.Fatalf("split-window without env vars: %v", err) + } + }) + + if got := output; got != "%66666666-6666-4666-8666-666666666666\n" { + t.Fatalf("stdout = %q, want %%66666666-6666-4666-8666-666666666666", got) + } +} + +// startMockTmuxCompatSocketSurfaceResolveFails creates a mock where +// surface.current returns no surface_id (simulating the scenario where +// surface resolution fails on the client side but surface.split still +// works because the server falls back to its focused surface). +func startMockTmuxCompatSocketSurfaceResolveFails(t *testing.T) string { + t.Helper() + sockPath := makeShortUnixSocketPath(t) + + ln, err := net.Listen("unix", sockPath) + if err != nil { + t.Fatalf("failed to listen: %v", err) + } + t.Cleanup(func() { ln.Close() }) + + go func() { + for { + conn, err := ln.Accept() + if err != nil { + return + } + go func(conn net.Conn) { + defer conn.Close() + reader := bufio.NewReader(conn) + line, err := reader.ReadBytes('\n') + if err != nil { + return + } + + var req map[string]any + if err := json.Unmarshal(line, &req); err != nil { + _, _ = conn.Write([]byte(`{"ok":false,"error":{"code":"parse","message":"bad json"}}` + "\n")) + return + } + + method, _ := req["method"].(string) + resp := map[string]any{ + "id": req["id"], + "ok": true, + } + + switch method { + case "workspace.list": + resp["result"] = map[string]any{ + "workspaces": []map[string]any{{ + "id": "11111111-1111-4111-8111-111111111111", + "ref": "workspace:1", + "index": 1, + "title": "demo", + }}, + } + case "workspace.current": + resp["result"] = map[string]any{ + "workspace_id": "11111111-1111-4111-8111-111111111111", + "workspace_ref": "workspace:1", + } + case "surface.current": + // Return no surface_id — simulates transient server state + // where the focused surface is not yet available to the + // current endpoint but surface.split can still use it. + resp["result"] = map[string]any{ + "workspace_id": "11111111-1111-4111-8111-111111111111", + } + case "surface.list": + // Return empty surfaces — surface resolution will fail + resp["result"] = map[string]any{"surfaces": []map[string]any{}} + case "pane.list": + resp["result"] = map[string]any{"panes": []map[string]any{}} + case "surface.split": + // Server accepts split with any/empty surface_id and uses + // its internal focused surface (like the real server does). + resp["result"] = map[string]any{ + "surface_id": "77777777-7777-4777-8777-777777777777", + "pane_id": "66666666-6666-4666-8666-666666666666", + } + case "workspace.equalize_splits": + resp["result"] = map[string]any{"ok": true} + default: + resp["ok"] = false + resp["error"] = map[string]any{ + "code": "unsupported", + "message": method, + } + } + + payload, _ := json.Marshal(resp) + _, _ = conn.Write(append(payload, '\n')) + }(conn) + } + }() + + return sockPath +} + +func TestTmuxSplitWindowFallsBackWhenSurfaceResolveFails(t *testing.T) { + origHome := os.Getenv("HOME") + origWorkspace := os.Getenv("CMUX_WORKSPACE_ID") + origSurface := os.Getenv("CMUX_SURFACE_ID") + origPane := os.Getenv("TMUX_PANE") + os.Setenv("HOME", t.TempDir()) + os.Unsetenv("CMUX_WORKSPACE_ID") + os.Unsetenv("CMUX_SURFACE_ID") + os.Unsetenv("TMUX_PANE") + defer func() { + os.Setenv("HOME", origHome) + if origWorkspace != "" { + os.Setenv("CMUX_WORKSPACE_ID", origWorkspace) + } else { + os.Unsetenv("CMUX_WORKSPACE_ID") + } + if origSurface != "" { + os.Setenv("CMUX_SURFACE_ID", origSurface) + } else { + os.Unsetenv("CMUX_SURFACE_ID") + } + if origPane != "" { + os.Setenv("TMUX_PANE", origPane) + } else { + os.Unsetenv("TMUX_PANE") + } + }() + + sockPath := startMockTmuxCompatSocketSurfaceResolveFails(t) + rc := &rpcContext{socketPath: sockPath} + + output := captureStdout(t, func() { + if err := dispatchTmuxCommand(rc, "split-window", []string{"-h", "-P", "-F", "#{pane_id}"}); err != nil { + t.Fatalf("split-window should fall back when surface resolution fails: %v", err) + } + }) + + if got := output; got != "%66666666-6666-4666-8666-666666666666\n" { + t.Fatalf("stdout = %q, want %%66666666-6666-4666-8666-666666666666", got) + } +} + func TestTmuxSplitWindowIgnoresStaleUUIDColumnSurface(t *testing.T) { origHome := os.Getenv("HOME") origWorkspace := os.Getenv("CMUX_WORKSPACE_ID") From 844ea55fac518996fb543190e8310aeb504d5981 Mon Sep 17 00:00:00 2001 From: JR Date: Sat, 4 Apr 2026 14:23:07 +0200 Subject: [PATCH 2/2] =?UTF-8?q?fix:=20address=20review=20feedback=20?= =?UTF-8?q?=E2=80=94=20test=20coverage=20and=20explicit=20target=20handlin?= =?UTF-8?q?g?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1. TestTmuxSplitWindowWithoutSurfaceEnv now uses the mock that makes surface resolution fail, so the fallback path is actually exercised. 2. Both env-sensitive tests (WithoutSurfaceEnv, FallsBackWhen...) now save/unset/restore CMUX_PANE_ID so that a runner-set value cannot bypass the fallback via tmuxCallerPaneHandle. 3. tmuxSplitWindow only falls back to workspace-only resolution when no explicit -t target was given. An invalid explicit target now returns the error immediately. --- daemon/remote/cmd/cmuxd-remote/tmux_compat.go | 9 ++++++--- .../cmd/cmuxd-remote/tmux_split_ref_test.go | 16 +++++++++++++++- 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/daemon/remote/cmd/cmuxd-remote/tmux_compat.go b/daemon/remote/cmd/cmuxd-remote/tmux_compat.go index db8b3beb59f9..a3177c2bc023 100644 --- a/daemon/remote/cmd/cmuxd-remote/tmux_compat.go +++ b/daemon/remote/cmd/cmuxd-remote/tmux_compat.go @@ -1188,9 +1188,12 @@ func tmuxSplitWindow(rc *rpcContext, args []string) error { targetWs, _, targetSurface, err := tmuxResolveSurfaceTarget(rc, p.value("-t")) if err != nil { - // Surface resolution can fail when env vars are missing or stale. - // Fall back to resolving just the workspace and let the server - // pick the focused surface for the split. + if p.value("-t") != "" { + return err // explicit target was invalid — don't fall back + } + // No explicit target; surface resolution can fail when env vars + // are missing or stale. Fall back to resolving just the workspace + // and let the server pick the focused surface for the split. targetWs, err = tmuxResolveWorkspaceTarget(rc, p.value("-t")) if err != nil { return err diff --git a/daemon/remote/cmd/cmuxd-remote/tmux_split_ref_test.go b/daemon/remote/cmd/cmuxd-remote/tmux_split_ref_test.go index 199590c1d20c..d6bfb9940b89 100644 --- a/daemon/remote/cmd/cmuxd-remote/tmux_split_ref_test.go +++ b/daemon/remote/cmd/cmuxd-remote/tmux_split_ref_test.go @@ -282,10 +282,12 @@ func TestTmuxSplitWindowWithoutSurfaceEnv(t *testing.T) { origWorkspace := os.Getenv("CMUX_WORKSPACE_ID") origSurface := os.Getenv("CMUX_SURFACE_ID") origPane := os.Getenv("TMUX_PANE") + origCmuxPane := os.Getenv("CMUX_PANE_ID") os.Setenv("HOME", t.TempDir()) os.Unsetenv("CMUX_WORKSPACE_ID") os.Unsetenv("CMUX_SURFACE_ID") os.Unsetenv("TMUX_PANE") + os.Unsetenv("CMUX_PANE_ID") defer func() { os.Setenv("HOME", origHome) if origWorkspace != "" { @@ -303,9 +305,14 @@ func TestTmuxSplitWindowWithoutSurfaceEnv(t *testing.T) { } else { os.Unsetenv("TMUX_PANE") } + if origCmuxPane != "" { + os.Setenv("CMUX_PANE_ID", origCmuxPane) + } else { + os.Unsetenv("CMUX_PANE_ID") + } }() - sockPath := startMockTmuxCompatSocketWithFocusFallback(t) + sockPath := startMockTmuxCompatSocketSurfaceResolveFails(t) rc := &rpcContext{socketPath: sockPath} output := captureStdout(t, func() { @@ -417,10 +424,12 @@ func TestTmuxSplitWindowFallsBackWhenSurfaceResolveFails(t *testing.T) { origWorkspace := os.Getenv("CMUX_WORKSPACE_ID") origSurface := os.Getenv("CMUX_SURFACE_ID") origPane := os.Getenv("TMUX_PANE") + origCmuxPane := os.Getenv("CMUX_PANE_ID") os.Setenv("HOME", t.TempDir()) os.Unsetenv("CMUX_WORKSPACE_ID") os.Unsetenv("CMUX_SURFACE_ID") os.Unsetenv("TMUX_PANE") + os.Unsetenv("CMUX_PANE_ID") defer func() { os.Setenv("HOME", origHome) if origWorkspace != "" { @@ -438,6 +447,11 @@ func TestTmuxSplitWindowFallsBackWhenSurfaceResolveFails(t *testing.T) { } else { os.Unsetenv("TMUX_PANE") } + if origCmuxPane != "" { + os.Setenv("CMUX_PANE_ID", origCmuxPane) + } else { + os.Unsetenv("CMUX_PANE_ID") + } }() sockPath := startMockTmuxCompatSocketSurfaceResolveFails(t)