fix(usecase): reject invalid pane split directions
- commit
- e88f43d825c9c3fae01755e6a2147ed006302fc8
- author
- brice · · 2026-06-20 09:49 UTC
- committer
- brice · · 2026-06-20 10:00 UTC
- parent
- 2be8535
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
2 files · +99 -0
M internal/application/usecase/manage_panes.go +8 -0
| @@ -112,6 +112,14 @@ func (uc *ManagePanesUseCase) Split(ctx context.Context, input SplitPaneInput) ( | |||
| 112 | 112 | return nil, fmt.Errorf("target pane is required") | |
| 113 | 113 | } | |
| 114 | 114 | ||
| 115 | + | // Validate split direction before any mutations | |
| 116 | + | switch input.Direction { | |
| 117 | + | case SplitLeft, SplitRight, SplitUp, SplitDown: | |
| 118 | + | // valid | |
| 119 | + | default: | |
| 120 | + | return nil, fmt.Errorf("invalid split direction: %s", input.Direction) | |
| 121 | + | } | |
| 122 | + | ||
| 115 | 123 | // If target is inside a stack, split around the entire stack | |
| 116 | 124 | // (i.e., create the new pane as a sibling to the stack, not inside it) | |
| 117 | 125 | // This applies to all directions - we don't want to nest split containers inside stacks. | |
| @@ -112,6 +112,14 @@ func (uc *ManagePanesUseCase) Split(ctx context.Context, input SplitPaneInput) ( | |||||
| 112 | return nil, fmt.Errorf("target pane is required") | 112 | return nil, fmt.Errorf("target pane is required") | ||
| 113 | } | 113 | } | ||
| 114 | 114 | ||||
| 115 | + | // Validate split direction before any mutations | |||
| 116 | + | switch input.Direction { | |||
| 117 | + | case SplitLeft, SplitRight, SplitUp, SplitDown: | |||
| 118 | + | // valid | |||
| 119 | + | default: | |||
| 120 | + | return nil, fmt.Errorf("invalid split direction: %s", input.Direction) | |||
| 121 | + | } | |||
| 122 | + | ||||
| 115 | // If target is inside a stack, split around the entire stack | 123 | // If target is inside a stack, split around the entire stack | ||
| 116 | // (i.e., create the new pane as a sibling to the stack, not inside it) | 124 | // (i.e., create the new pane as a sibling to the stack, not inside it) | ||
| 117 | // This applies to all directions - we don't want to nest split containers inside stacks. | 125 | // This applies to all directions - we don't want to nest split containers inside stacks. | ||
A internal/application/usecase/manage_panes_split_test.go +91 -0
| @@ -0,0 +1,91 @@ | |||
| 1 | + | package usecase | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "context" | |
| 5 | + | "strings" | |
| 6 | + | "testing" | |
| 7 | + | ||
| 8 | + | "github.com/bnema/dumber/internal/domain/entity" | |
| 9 | + | ) | |
| 10 | + | ||
| 11 | + | func TestManagePanesUseCase_Split_ValidDirections(t *testing.T) { | |
| 12 | + | uc := NewManagePanesUseCase(func() string { return "id" }) | |
| 13 | + | ctx := context.Background() | |
| 14 | + | ||
| 15 | + | dirs := []SplitDirection{SplitLeft, SplitRight, SplitUp, SplitDown} | |
| 16 | + | for _, dir := range dirs { | |
| 17 | + | t.Run(string(dir), func(t *testing.T) { | |
| 18 | + | target := leaf("target") | |
| 19 | + | ws := &entity.Workspace{Root: target, ActivePaneID: "target"} | |
| 20 | + | target.Parent = nil // root | |
| 21 | + | ||
| 22 | + | out, err := uc.Split(ctx, SplitPaneInput{ | |
| 23 | + | Workspace: ws, | |
| 24 | + | TargetPane: target, | |
| 25 | + | Direction: dir, | |
| 26 | + | }) | |
| 27 | + | if err != nil { | |
| 28 | + | t.Fatalf("unexpected error for direction %s: %v", dir, err) | |
| 29 | + | } | |
| 30 | + | if out == nil { | |
| 31 | + | t.Fatalf("output is nil for direction %s", dir) | |
| 32 | + | } | |
| 33 | + | if out.NewPaneNode == nil { | |
| 34 | + | t.Fatalf("new pane node is nil for direction %s", dir) | |
| 35 | + | } | |
| 36 | + | if out.ParentNode == nil { | |
| 37 | + | t.Fatalf("parent node is nil for direction %s", dir) | |
| 38 | + | } | |
| 39 | + | if !out.ParentNode.IsSplit() { | |
| 40 | + | t.Fatalf("parent node should be a split for direction %s, got SplitDir=%v", dir, out.ParentNode.SplitDir) | |
| 41 | + | } | |
| 42 | + | if ws.Root != out.ParentNode { | |
| 43 | + | t.Fatalf("workspace root should be the parent node for direction %s", dir) | |
| 44 | + | } | |
| 45 | + | // Verify the tree has 2 leaf panes | |
| 46 | + | if ws.PaneCount() != 2 { | |
| 47 | + | t.Fatalf("workspace should have 2 panes for direction %s, got %d", dir, ws.PaneCount()) | |
| 48 | + | } | |
| 49 | + | }) | |
| 50 | + | } | |
| 51 | + | } | |
| 52 | + | ||
| 53 | + | func TestManagePanesUseCase_Split_InvalidDirection_ReturnsError(t *testing.T) { | |
| 54 | + | uc := NewManagePanesUseCase(func() string { return "id" }) | |
| 55 | + | ctx := context.Background() | |
| 56 | + | ||
| 57 | + | target := leaf("target") | |
| 58 | + | ws := &entity.Workspace{Root: target, ActivePaneID: "target"} | |
| 59 | + | ||
| 60 | + | // Save initial state | |
| 61 | + | initialRoot := ws.Root | |
| 62 | + | initialActive := ws.ActivePaneID | |
| 63 | + | ||
| 64 | + | _, err := uc.Split(ctx, SplitPaneInput{ | |
| 65 | + | Workspace: ws, | |
| 66 | + | TargetPane: target, | |
| 67 | + | Direction: SplitDirection("diagonal"), | |
| 68 | + | }) | |
| 69 | + | if err == nil { | |
| 70 | + | t.Fatalf("expected error for invalid direction, got nil") | |
| 71 | + | } | |
| 72 | + | if !strings.Contains(err.Error(), "invalid split direction: diagonal") { | |
| 73 | + | t.Fatalf("unexpected error message: %v", err) | |
| 74 | + | } | |
| 75 | + | ||
| 76 | + | // Verify workspace state is unchanged | |
| 77 | + | if ws.Root != initialRoot { | |
| 78 | + | t.Fatalf("workspace root was modified") | |
| 79 | + | } | |
| 80 | + | if ws.ActivePaneID != initialActive { | |
| 81 | + | t.Fatalf("workspace active pane was modified") | |
| 82 | + | } | |
| 83 | + | // Verify target pane parent is still nil (was root) | |
| 84 | + | if target.Parent != nil { | |
| 85 | + | t.Fatalf("target pane parent was modified") | |
| 86 | + | } | |
| 87 | + | // Verify pane count is still 1 | |
| 88 | + | if ws.PaneCount() != 1 { | |
| 89 | + | t.Fatalf("workspace pane count changed: got %d, want 1", ws.PaneCount()) | |
| 90 | + | } | |
| 91 | + | } | |
| @@ -0,0 +1,91 @@ | |||||
| 1 | + | package usecase | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "context" | |||
| 5 | + | "strings" | |||
| 6 | + | "testing" | |||
| 7 | + | ||||
| 8 | + | "github.com/bnema/dumber/internal/domain/entity" | |||
| 9 | + | ) | |||
| 10 | + | ||||
| 11 | + | func TestManagePanesUseCase_Split_ValidDirections(t *testing.T) { | |||
| 12 | + | uc := NewManagePanesUseCase(func() string { return "id" }) | |||
| 13 | + | ctx := context.Background() | |||
| 14 | + | ||||
| 15 | + | dirs := []SplitDirection{SplitLeft, SplitRight, SplitUp, SplitDown} | |||
| 16 | + | for _, dir := range dirs { | |||
| 17 | + | t.Run(string(dir), func(t *testing.T) { | |||
| 18 | + | target := leaf("target") | |||
| 19 | + | ws := &entity.Workspace{Root: target, ActivePaneID: "target"} | |||
| 20 | + | target.Parent = nil // root | |||
| 21 | + | ||||
| 22 | + | out, err := uc.Split(ctx, SplitPaneInput{ | |||
| 23 | + | Workspace: ws, | |||
| 24 | + | TargetPane: target, | |||
| 25 | + | Direction: dir, | |||
| 26 | + | }) | |||
| 27 | + | if err != nil { | |||
| 28 | + | t.Fatalf("unexpected error for direction %s: %v", dir, err) | |||
| 29 | + | } | |||
| 30 | + | if out == nil { | |||
| 31 | + | t.Fatalf("output is nil for direction %s", dir) | |||
| 32 | + | } | |||
| 33 | + | if out.NewPaneNode == nil { | |||
| 34 | + | t.Fatalf("new pane node is nil for direction %s", dir) | |||
| 35 | + | } | |||
| 36 | + | if out.ParentNode == nil { | |||
| 37 | + | t.Fatalf("parent node is nil for direction %s", dir) | |||
| 38 | + | } | |||
| 39 | + | if !out.ParentNode.IsSplit() { | |||
| 40 | + | t.Fatalf("parent node should be a split for direction %s, got SplitDir=%v", dir, out.ParentNode.SplitDir) | |||
| 41 | + | } | |||
| 42 | + | if ws.Root != out.ParentNode { | |||
| 43 | + | t.Fatalf("workspace root should be the parent node for direction %s", dir) | |||
| 44 | + | } | |||
| 45 | + | // Verify the tree has 2 leaf panes | |||
| 46 | + | if ws.PaneCount() != 2 { | |||
| 47 | + | t.Fatalf("workspace should have 2 panes for direction %s, got %d", dir, ws.PaneCount()) | |||
| 48 | + | } | |||
| 49 | + | }) | |||
| 50 | + | } | |||
| 51 | + | } | |||
| 52 | + | ||||
| 53 | + | func TestManagePanesUseCase_Split_InvalidDirection_ReturnsError(t *testing.T) { | |||
| 54 | + | uc := NewManagePanesUseCase(func() string { return "id" }) | |||
| 55 | + | ctx := context.Background() | |||
| 56 | + | ||||
| 57 | + | target := leaf("target") | |||
| 58 | + | ws := &entity.Workspace{Root: target, ActivePaneID: "target"} | |||
| 59 | + | ||||
| 60 | + | // Save initial state | |||
| 61 | + | initialRoot := ws.Root | |||
| 62 | + | initialActive := ws.ActivePaneID | |||
| 63 | + | ||||
| 64 | + | _, err := uc.Split(ctx, SplitPaneInput{ | |||
| 65 | + | Workspace: ws, | |||
| 66 | + | TargetPane: target, | |||
| 67 | + | Direction: SplitDirection("diagonal"), | |||
| 68 | + | }) | |||
| 69 | + | if err == nil { | |||
| 70 | + | t.Fatalf("expected error for invalid direction, got nil") | |||
| 71 | + | } | |||
| 72 | + | if !strings.Contains(err.Error(), "invalid split direction: diagonal") { | |||
| 73 | + | t.Fatalf("unexpected error message: %v", err) | |||
| 74 | + | } | |||
| 75 | + | ||||
| 76 | + | // Verify workspace state is unchanged | |||
| 77 | + | if ws.Root != initialRoot { | |||
| 78 | + | t.Fatalf("workspace root was modified") | |||
| 79 | + | } | |||
| 80 | + | if ws.ActivePaneID != initialActive { | |||
| 81 | + | t.Fatalf("workspace active pane was modified") | |||
| 82 | + | } | |||
| 83 | + | // Verify target pane parent is still nil (was root) | |||
| 84 | + | if target.Parent != nil { | |||
| 85 | + | t.Fatalf("target pane parent was modified") | |||
| 86 | + | } | |||
| 87 | + | // Verify pane count is still 1 | |||
| 88 | + | if ws.PaneCount() != 1 { | |||
| 89 | + | t.Fatalf("workspace pane count changed: got %d, want 1", ws.PaneCount()) | |||
| 90 | + | } | |||
| 91 | + | } | |||