detached 2bfb698

docs(workspace): document stacked pane semantics and close refactoring plan

commit
2bfb698e3a5b728965b4c13c9531fb47f3e275a6
author
Brice · · 2025-09-30 05:15 UTC
parent
c5d90b9
signature
unsigned
2 files · +272 -177
Unified
UnifiedSplit
M docs/PANE_ARCHITECTURE.md +52 -4
@@ -91,8 +91,8 @@ classDiagram
9191 +uintptr focusControllerToken
9292 +bool pendingHoverReattach
9393 +bool pendingFocusReattach
94- +bool isStacked
95- +[]*paneNode stackedPanes
94+ +bool isStacked // TRUE: stack container, FALSE: regular pane or pane within stack
95+ +[]*paneNode stackedPanes // Non-empty only if isStacked=true
9696 +int activeStackIndex
9797 +SafeWidget titleBar
9898 +SafeWidget stackWrapper
@@ -193,6 +193,26 @@ graph TD
193193
194194 ## Binary Tree Operations
195195
196+### Stacked Pane Node Semantics
197+
198+**Critical Implementation Detail**: The `isStacked` flag has precise semantics that are essential for correct operation:
199+
200+- **Stack Container Node**: `isStacked=true`, `isLeaf=false`, `stackedPanes` contains the individual panes
201+- **Panes Within Stack**: `isStacked=false`, `isLeaf=true`, `parent` points to the stack container
202+
203+Individual panes within a stack are **NOT** marked as stacked themselves. Only the container node managing the stack has `isStacked=true`. This allows:
204+- Split validation to accept individual panes within stacks
205+- Split operations to automatically target the stack container (not the individual pane)
206+- Proper tree traversal and focus management
207+
208+**Splitting from Stacked Panes**: When splitting from a pane inside a stack, the system automatically:
209+1. Detects `target.parent.isStacked == true`
210+2. Promotes the split target to the stack container
211+3. Splits around the **entire stack**, not the individual pane
212+4. Creates a new sibling to the stack container in the tree
213+
214+This ensures stacks remain cohesive units in the tree structure.
215+
196216 ### Split Operation Flow
197217
198218 ```mermaid
@@ -249,14 +269,15 @@ sequenceDiagram
249269 BP->>WM: closePane(target)
250270 WM->>WM: Find sibling/replacement node
251271 WM->>GTK: Unparent target widget
252- WM->>GTK: Promote sibling to new position
272+ WM->>WM: swapContainers (skips attachRoot if TreeRebalancer enabled)
273+ WM->>GTK: Promote sibling in tree structure
253274 WM->>WM: Update tree structure
254275 WM-->>BP: Return promoted node
255276 BP->>TV: ValidateTree(after_close)
256277 BP->>TR: RebalanceAfterClose(closed, promoted)
257278 TR->>TR: Execute promotion transaction
258279 TR->>GTK: WidgetResetSizeRequest(promoted)
259- TR->>GTK: Reattach with proper expansion
280+ TR->>GTK: Attach to window/parent (handles double-attachment prevention)
260281 TR->>GTK: Queue allocation on ancestors
261282 TR->>TR: Validate geometry
262283 TR-->>BP: Promotion completed
@@ -682,6 +703,15 @@ sequenceDiagram
682703 - **Size Constraint Management**: Explicit constraint reset prevents GTK layout conflicts
683704 - **Async Consistency**: Identical behavior between sync and async operation paths
684705
706+### Widget Safety Enhancements
707+
708+Recent improvements prevent GTK assertion failures and widget corruption:
709+
710+- **CSS Class Safety**: All CSS class operations verify `WidgetIsValid()` before modification to prevent GTK bloom filter corruption when widgets are destroyed mid-operation
711+- **Paned Child Clearing**: Before destroying paned widgets, verifies `WidgetGetParent(child) == paned_ptr` to avoid unparenting already-reparented children
712+- **Double-Attachment Prevention**: `swapContainers` skips `attachRoot` when `TreeRebalancer` is enabled, delegating widget attachment to the promotion transaction
713+- **Unparent Verification**: Stack container creation includes explicit unparent checks and logging for freshly-created WebView widgets
714+
685715 ## Thread Safety
686716
687717 The system implements comprehensive thread safety:
@@ -691,4 +721,22 @@ The system implements comprehensive thread safety:
691721 - **State Changes**: Mutex protection for critical sections
692722 - **Tree Modifications**: Atomic operations with rollback capability
693723
724+## Known Constraints and Design Decisions
725+
726+### Stacked Pane Management
727+
728+1. **Individual panes within stacks have `isStacked=false`**: Only the container node has `isStacked=true`. This enables split validation and proper tree operations.
729+
730+2. **Splitting targets the stack container**: When splitting from a pane inside a stack, the operation automatically promotes to split the entire stack container, not the individual pane.
731+
732+3. **Widget attachment coordination**: When `TreeRebalancer` is enabled, `closePane` delegates widget attachment to the rebalancer's promotion transaction to prevent double-attachment race conditions.
733+
734+### GTK Widget Lifecycle
735+
736+1. **Reparenting invalidates container references**: When widgets are reparented via `PanedSetStartChild/EndChild` or `window.SetChild`, GTK automatically unparents them from the old container. Code must check `WidgetGetParent(child) == old_parent` before attempting cleanup.
737+
738+2. **CSS operations require validity checks**: CSS class manipulation on destroyed widgets corrupts GTK's internal bloom filter. All CSS operations verify `WidgetIsValid()` before proceeding.
739+
740+3. **WebView containers may have implicit parents**: Freshly created WebView widgets may have internal GTK parent references that require explicit unparenting before box/paned insertion.
741+
694742 This architecture provides a robust, efficient, and safe pane management system that can handle complex window layouts while maintaining excellent performance and user experience.⏎̸
@@ -91,8 +91,8 @@ classDiagram
91 +uintptr focusControllerToken91 +uintptr focusControllerToken
92 +bool pendingHoverReattach92 +bool pendingHoverReattach
93 +bool pendingFocusReattach93 +bool pendingFocusReattach
94- +bool isStacked94+ +bool isStacked // TRUE: stack container, FALSE: regular pane or pane within stack
95- +[]*paneNode stackedPanes95+ +[]*paneNode stackedPanes // Non-empty only if isStacked=true
96 +int activeStackIndex96 +int activeStackIndex
97 +SafeWidget titleBar97 +SafeWidget titleBar
98 +SafeWidget stackWrapper98 +SafeWidget stackWrapper
@@ -193,6 +193,26 @@ graph TD
193 193
194 ## Binary Tree Operations194 ## Binary Tree Operations
195 195
196+### Stacked Pane Node Semantics
197+
198+**Critical Implementation Detail**: The `isStacked` flag has precise semantics that are essential for correct operation:
199+
200+- **Stack Container Node**: `isStacked=true`, `isLeaf=false`, `stackedPanes` contains the individual panes
201+- **Panes Within Stack**: `isStacked=false`, `isLeaf=true`, `parent` points to the stack container
202+
203+Individual panes within a stack are **NOT** marked as stacked themselves. Only the container node managing the stack has `isStacked=true`. This allows:
204+- Split validation to accept individual panes within stacks
205+- Split operations to automatically target the stack container (not the individual pane)
206+- Proper tree traversal and focus management
207+
208+**Splitting from Stacked Panes**: When splitting from a pane inside a stack, the system automatically:
209+1. Detects `target.parent.isStacked == true`
210+2. Promotes the split target to the stack container
211+3. Splits around the **entire stack**, not the individual pane
212+4. Creates a new sibling to the stack container in the tree
213+
214+This ensures stacks remain cohesive units in the tree structure.
215+
196 ### Split Operation Flow216 ### Split Operation Flow
197 217
198 ```mermaid218 ```mermaid
@@ -249,14 +269,15 @@ sequenceDiagram
249 BP->>WM: closePane(target)269 BP->>WM: closePane(target)
250 WM->>WM: Find sibling/replacement node270 WM->>WM: Find sibling/replacement node
251 WM->>GTK: Unparent target widget271 WM->>GTK: Unparent target widget
252- WM->>GTK: Promote sibling to new position272+ WM->>WM: swapContainers (skips attachRoot if TreeRebalancer enabled)
273+ WM->>GTK: Promote sibling in tree structure
253 WM->>WM: Update tree structure274 WM->>WM: Update tree structure
254 WM-->>BP: Return promoted node275 WM-->>BP: Return promoted node
255 BP->>TV: ValidateTree(after_close)276 BP->>TV: ValidateTree(after_close)
256 BP->>TR: RebalanceAfterClose(closed, promoted)277 BP->>TR: RebalanceAfterClose(closed, promoted)
257 TR->>TR: Execute promotion transaction278 TR->>TR: Execute promotion transaction
258 TR->>GTK: WidgetResetSizeRequest(promoted)279 TR->>GTK: WidgetResetSizeRequest(promoted)
259- TR->>GTK: Reattach with proper expansion280+ TR->>GTK: Attach to window/parent (handles double-attachment prevention)
260 TR->>GTK: Queue allocation on ancestors281 TR->>GTK: Queue allocation on ancestors
261 TR->>TR: Validate geometry282 TR->>TR: Validate geometry
262 TR-->>BP: Promotion completed283 TR-->>BP: Promotion completed
@@ -682,6 +703,15 @@ sequenceDiagram
682 - **Size Constraint Management**: Explicit constraint reset prevents GTK layout conflicts703 - **Size Constraint Management**: Explicit constraint reset prevents GTK layout conflicts
683 - **Async Consistency**: Identical behavior between sync and async operation paths704 - **Async Consistency**: Identical behavior between sync and async operation paths
684 705
706+### Widget Safety Enhancements
707+
708+Recent improvements prevent GTK assertion failures and widget corruption:
709+
710+- **CSS Class Safety**: All CSS class operations verify `WidgetIsValid()` before modification to prevent GTK bloom filter corruption when widgets are destroyed mid-operation
711+- **Paned Child Clearing**: Before destroying paned widgets, verifies `WidgetGetParent(child) == paned_ptr` to avoid unparenting already-reparented children
712+- **Double-Attachment Prevention**: `swapContainers` skips `attachRoot` when `TreeRebalancer` is enabled, delegating widget attachment to the promotion transaction
713+- **Unparent Verification**: Stack container creation includes explicit unparent checks and logging for freshly-created WebView widgets
714+
685 ## Thread Safety715 ## Thread Safety
686 716
687 The system implements comprehensive thread safety:717 The system implements comprehensive thread safety:
@@ -691,4 +721,22 @@ The system implements comprehensive thread safety:
691 - **State Changes**: Mutex protection for critical sections721 - **State Changes**: Mutex protection for critical sections
692 - **Tree Modifications**: Atomic operations with rollback capability722 - **Tree Modifications**: Atomic operations with rollback capability
693 723
724+## Known Constraints and Design Decisions
725+
726+### Stacked Pane Management
727+
728+1. **Individual panes within stacks have `isStacked=false`**: Only the container node has `isStacked=true`. This enables split validation and proper tree operations.
729+
730+2. **Splitting targets the stack container**: When splitting from a pane inside a stack, the operation automatically promotes to split the entire stack container, not the individual pane.
731+
732+3. **Widget attachment coordination**: When `TreeRebalancer` is enabled, `closePane` delegates widget attachment to the rebalancer's promotion transaction to prevent double-attachment race conditions.
733+
734+### GTK Widget Lifecycle
735+
736+1. **Reparenting invalidates container references**: When widgets are reparented via `PanedSetStartChild/EndChild` or `window.SetChild`, GTK automatically unparents them from the old container. Code must check `WidgetGetParent(child) == old_parent` before attempting cleanup.
737+
738+2. **CSS operations require validity checks**: CSS class manipulation on destroyed widgets corrupts GTK's internal bloom filter. All CSS operations verify `WidgetIsValid()` before proceeding.
739+
740+3. **WebView containers may have implicit parents**: Freshly created WebView widgets may have internal GTK parent references that require explicit unparenting before box/paned insertion.
741+
694 This architecture provides a robust, efficient, and safe pane management system that can handle complex window layouts while maintaining excellent performance and user experience.⏎̸742 This architecture provides a robust, efficient, and safe pane management system that can handle complex window layouts while maintaining excellent performance and user experience.⏎̸
M docs/PANE_CLOSE_REFACTORING_PLAN.md +220 -173
@@ -2,18 +2,36 @@
22
33 ## Problem Analysis Summary
44
5-The current implementation has **THREE CRITICAL BUGS**:
6-1. **Multiple Pane Closure**: Sibling identification logic at lines 920-928 incorrectly identifies siblings
7-2. **Invalid Widget Promotion**: Lines 1042-1090 operate on destroyed widgets
8-3. **Race Conditions**: Widget cleanup happens before promotion completes
5+The current implementation in `internal/app/browser/workspace_pane_ops.go:707`+ exhibits **three interlocked failure modes**:
6+1. **Multiple Pane Closure**: The promotion branch (around `workspace_pane_ops.go:990`) iterates over invalid parent/child references, causing both siblings to be detached when `wm.app.panes` still counts them. This yields double-close behaviour during rapid shortcuts.
7+2. **Invalid Widget Promotion**: The root promotion path (roughly `workspace_pane_ops.go:878-948`) manipulates widgets after `Cleanup()` invalidates their `SafeWidget` guard, which leads to sporadic `GTK_IS_WIDGET` assertions.
8+3. **Race Conditions**: Cleanup (`ensureCleanup`) and hover-detach run while asynchronous `IdleAdd` callbacks still reference the old nodes, so hover/focus reattachment runs against freed memory during reproduction of issue #1429.
9+
10+> Baseline reproduction: trigger repeated `closePane` via Ctrl+W on a layout with nested root + stack; log spew shows `[workspace] close aborted` alongside panics in `ensureSafeWidget`.
11+
12+### Environment Constraints
13+- `stackedPaneManager.CloseStackedPane` is tightly coupled to `wm.stackedPaneManager`, so any refactor must either reimplement its logic or provide compatibility shims.
14+- `BulletproofClosePane` wraps `closePane`, retrying on specific errors; changes to panic/zero-return semantics must preserve that contract.
15+- Hover/focus controller lifecycles (`wm.ensureHover`, `focusStateMachine.attachGTKController`) must remain intact after tree restructuring.
16+
17+### Preconditions & Guardrails
18+1. Capture baseline behaviour with `go test ./tests/workspace/...` (currently incomplete; we will add targeted tests in Phase 5).
19+2. Add temporary structured logging with `log.Printf("[pane-close] ...")` gated behind an environment toggle to avoid production noise.
20+3. Ensure unit test scaffolding can construct `WorkspaceManager` with fake `webkit` bindings (see `tests/browser/fakes`).
21+
22+## PHASE 0: Stabilise & Instrument
23+
24+1. Wrap `BulletproofClosePane` with structured logging (log key: `pane-close-stage`) guarded by `wm.debugInstrumentation` flag.
25+2. Add `WorkspaceDiagnostics` helper capturing tree snapshots (`wm.dumpTreeState`) before and after `closePane`.
26+3. Add regression harness in `tests/browser/workspace_close_test.go` that constructs a representative tree and exercises edge cases (root-only, nested split, stack + split).
27+4. Verify reproduction steps still fail prior to refactor to ensure harness is meaningful.
928
1029 ## PHASE 1: New Simplified Architecture
1130
12-### 1.1 Create New File Structure
31+### 1.1 Keep File Layout, Replace Implementation In-Place
1332 ```
14-workspace_pane_close_v2.go # New simplified close logic
15-workspace_pane_close_legacy.go # Rename current implementation
16-workspace_pane_close_test.go # Comprehensive tests
33+internal/app/browser/workspace_pane_ops.go # Replace existing closePane implementation with new logic
34+internal/app/browser/workspace_types.go # Extend paneNode metadata
1735 ```
1836
1937 ### 1.2 Core Data Structure Changes
@@ -22,82 +40,58 @@ workspace_pane_close_test.go # Comprehensive tests
2240 type paneNode struct {
2341 // ... existing fields ...
2442
25- // New field to track widget validity
26- widgetValid bool // Set to false when widget destroyed
43+ widgetValid bool // Guard flagged before GTK destruction
44+ cleanupGeneration uint // Helps assert that asynchronous callbacks skip stale nodes
45+}
46+
47+// Add to WorkspaceManager
48+type WorkspaceManager struct {
49+ // ... existing fields ...
50+
51+ cleanupCounter uint
2752 }
2853 ```
2954
3055 ## PHASE 2: Detailed New closePane Implementation
3156
32-### 2.1 Main Close Function (50-70 lines total)
57+### 2.1 Main Close Function (60-80 lines total)
3358 ```go
34-func (wm *WorkspaceManager) closePaneV2(node *paneNode) (*paneNode, error) {
35- // STEP 1: Basic validation (5 lines)
36- if node == nil || !node.isLeaf {
37- return nil, errors.New("invalid close target")
59+func (wm *WorkspaceManager) closePane(node *paneNode) (*paneNode, error) {
60+ ctx := wm.beginClose(node)
61+ defer ctx.finish()
62+
63+ // STEP 1: Basic validation (quick fail)
64+ if ctx.err != nil {
65+ return nil, ctx.err
3866 }
3967
40- // STEP 2: Handle special cases (15 lines)
41- // 2a. Stacked pane
68+ // STEP 2: Handle stacked panes via compatibility shim
4269 if node.parent != nil && node.parent.isStacked {
43- return wm.closeStackedPaneSimple(node)
70+ return wm.closeStackedPaneCompat(node)
4471 }
4572
46- // 2b. Last pane
47- if wm.countPanes() == 1 {
48- wm.cleanupAndExit(node)
49- return nil, nil
73+ // STEP 3: Handle trivial exit cases
74+ if ctx.remaining == 1 {
75+ return wm.cleanupAndExit(node)
5076 }
51-
52- // 2c. Root pane with others
5377 if node == wm.root {
54- return wm.promoteNewRoot(node)
78+ return wm.promoteNewRoot(ctx, node)
5579 }
5680
57- // STEP 3: Standard close - promote sibling (30 lines)
81+ // STEP 4: Promote sibling in-place
5882 parent := node.parent
5983 sibling := wm.getSibling(node)
6084 grandparent := parent.parent
85+ wm.ensureWidgets(grandparent, parent, sibling)
6186
62- // Critical: Update tree structure FIRST
63- sibling.parent = grandparent
64- if grandparent != nil {
65- if grandparent.left == parent {
66- grandparent.left = sibling
67- } else {
68- grandparent.right = sibling
69- }
70- } else {
71- wm.root = sibling
72- }
87+ wm.promoteSibling(grandparent, parent, sibling)
7388
74- // STEP 4: Update GTK widgets (using GTK4 auto-unparenting)
75- if grandparent != nil && grandparent.container != nil {
76- grandparent.container.Execute(func(gpPtr uintptr) error {
77- sibling.container.Execute(func(sibPtr uintptr) error {
78- // GTK4 automatically unparents old child when setting new
79- if grandparent.left == sibling {
80- webkit.PanedSetStartChild(gpPtr, sibPtr)
81- } else {
82- webkit.PanedSetEndChild(gpPtr, sibPtr)
83- }
84- return nil
85- })
86- return nil
87- })
88- } else if sibling == wm.root {
89- // Sibling is new root
90- sibling.container.Execute(func(sibPtr uintptr) error {
91- wm.window.SetChild(sibPtr)
92- return nil
93- })
94- }
95-
96- // STEP 5: Cleanup (10 lines)
97- wm.cleanupPane(node)
98- wm.cleanupPane(parent) // Parent paned is no longer needed
89+ // STEP 5: GTK updates leverage auto-unparenting
90+ wm.swapContainers(grandparent, sibling)
9991
100- // STEP 6: Update focus
92+ // STEP 6: Cleanup & focus
93+ wm.cleanupPane(node, ctx.Generation())
94+ wm.decommissionParent(parent, ctx.Generation())
10195 wm.setFocusToLeaf(sibling)
10296
10397 return sibling, nil
@@ -120,81 +114,163 @@ func (wm *WorkspaceManager) getSibling(node *paneNode) *paneNode {
120114 }
121115 ```
122116
123-#### promoteNewRoot - Handle Root Replacement
117+#### beginClose / closeContext
118+```go
119+type closeContext struct {
120+ wm *WorkspaceManager
121+ target *paneNode
122+ remaining int
123+ err error
124+ generation uint
125+}
126+
127+func (wm *WorkspaceManager) beginClose(node *paneNode) closeContext {
128+ ctx := closeContext{wm: wm, target: node, generation: wm.nextCleanupGeneration()}
129+ switch {
130+ case wm == nil:
131+ ctx.err = errors.New("workspace manager nil")
132+ case node == nil || !node.isLeaf:
133+ ctx.err = errors.New("invalid close target")
134+ case node.pane == nil || node.pane.webView == nil:
135+ ctx.err = errors.New("close target missing webview")
136+ default:
137+ ctx.remaining = len(wm.app.panes)
138+ }
139+ return ctx
140+}
141+
142+func (ctx closeContext) Generation() uint { return ctx.generation }
143+
144+func (ctx closeContext) finish() {
145+ if ctx.err == nil {
146+ ctx.wm.updateMainPane()
147+ }
148+}
149+```
150+
151+#### ensureWidgets
152+```go
153+func (wm *WorkspaceManager) ensureWidgets(nodes ...*paneNode) {
154+ for _, n := range nodes {
155+ if n == nil || n.container == nil {
156+ continue
157+ }
158+ if n.container.IsValid() {
159+ continue
160+ }
161+ ptr := n.container.Ptr()
162+ n.container = wm.widgetRegistry.Recover(ptr, n.container.typeInfo)
163+ }
164+}
165+```
166+
167+#### promoteSibling
124168 ```go
125-func (wm *WorkspaceManager) promoteNewRoot(oldRoot *paneNode) (*paneNode, error) {
126- // Find first non-root pane by traversing tree
127- var newRoot *paneNode
128-
129- // Try left subtree first
130- if oldRoot.left != nil {
131- newRoot = oldRoot.left
132- } else if oldRoot.right != nil {
133- newRoot = oldRoot.right
169+func (wm *WorkspaceManager) promoteSibling(grand *paneNode, parent *paneNode, sibling *paneNode) {
170+ if grand == nil {
171+ wm.root = sibling
172+ sibling.parent = nil
173+ return
174+ }
175+ sibling.parent = grand
176+ if grand.left == parent {
177+ grand.left = sibling
134178 } else {
135- // No other panes - shouldn't happen
136- return nil, errors.New("no replacement root found")
179+ grand.right = sibling
137180 }
181+}
182+```
138183
139- // Detach new root from its parent
140- if newRoot.parent != nil {
141- sibling := wm.getSibling(newRoot)
142- parent := newRoot.parent
143-
144- // Promote sibling to parent's position
145- if parent.parent != nil {
146- // Has grandparent - attach sibling to it
147- grand := parent.parent
148- sibling.parent = grand
149- if grand.left == parent {
150- grand.left = sibling
184+#### swapContainers
185+```go
186+func (wm *WorkspaceManager) swapContainers(grand *paneNode, sibling *paneNode) {
187+ if grand == nil {
188+ wm.attachRoot(sibling)
189+ return
190+ }
191+ grand.container.Execute(func(gPtr uintptr) error {
192+ sibling.container.Execute(func(sPtr uintptr) error {
193+ if grand.left == sibling {
194+ webkit.PanedSetStartChild(gPtr, sPtr)
151195 } else {
152- grand.right = sibling
196+ webkit.PanedSetEndChild(gPtr, sPtr)
153197 }
198+ return nil
199+ })
200+ return nil
201+ })
202+}
203+```
154204
155- // Update GTK widget
156- grand.container.Execute(func(gPtr uintptr) error {
157- sibling.container.Execute(func(sPtr uintptr) error {
158- if grand.left == sibling {
159- webkit.PanedSetStartChild(gPtr, sPtr)
160- } else {
161- webkit.PanedSetEndChild(gPtr, sPtr)
162- }
163- return nil
164- })
165- return nil
166- })
167- }
205+#### decommissionParent
206+```go
207+func (wm *WorkspaceManager) decommissionParent(parent *paneNode, generation uint) {
208+ if parent == nil {
209+ return
168210 }
211+ wm.cleanupPane(parent, generation)
212+}
213+```
169214
170- // Make newRoot the root
171- newRoot.parent = nil
172- wm.root = newRoot
215+#### nextCleanupGeneration
216+```go
217+func (wm *WorkspaceManager) nextCleanupGeneration() uint {
218+ wm.cleanupCounter++
219+ return wm.cleanupCounter
220+}
221+```
173222
174- // Attach to window
175- newRoot.container.Execute(func(ptr uintptr) error {
223+#### attachRoot
224+```go
225+func (wm *WorkspaceManager) attachRoot(root *paneNode) {
226+ if root == nil || root.container == nil || wm.window == nil {
227+ return
228+ }
229+ root.container.Execute(func(ptr uintptr) error {
176230 wm.window.SetChild(ptr)
231+ webkit.WidgetQueueAllocate(ptr)
232+ webkit.WidgetShow(ptr)
177233 return nil
178234 })
235+}
236+```
179237
180- // Cleanup old root
181- wm.cleanupPane(oldRoot)
238+#### promoteNewRoot - Handle Root Replacement
239+```go
240+func (wm *WorkspaceManager) promoteNewRoot(ctx closeContext, oldRoot *paneNode) (*paneNode, error) {
241+ candidate := wm.findReplacementRoot(oldRoot)
242+ if candidate == nil {
243+ return wm.cleanupAndExit(oldRoot)
244+ }
182245
183- return newRoot, nil
246+ sibling := wm.getSibling(candidate)
247+ if sibling != nil {
248+ wm.promoteSibling(candidate.parent.parent, candidate.parent, sibling)
249+ }
250+
251+ candidate.parent = nil
252+ wm.root = candidate
253+
254+ wm.attachRoot(candidate)
255+
256+ wm.cleanupPane(oldRoot, ctx.Generation())
257+ return candidate, nil
184258 }
185259 ```
186260
187261 #### cleanupPane - Safe Cleanup
188262 ```go
189-func (wm *WorkspaceManager) cleanupPane(node *paneNode) {
190- if node == nil || node.widgetValid == false {
191- return // Already cleaned
263+func (wm *WorkspaceManager) cleanupPane(node *paneNode, generation uint) {
264+ if node == nil {
265+ return
266+ }
267+ if !node.widgetValid {
268+ return
192269 }
193270
194- // Mark as invalid immediately
195271 node.widgetValid = false
272+ node.cleanupGeneration = generation
196273
197- // Clean up pane resources
198274 if node.pane != nil {
199275 node.pane.CleanupFromWorkspace(wm)
200276 if node.pane.webView != nil {
@@ -202,13 +278,11 @@ func (wm *WorkspaceManager) cleanupPane(node *paneNode) {
202278 }
203279 }
204280
205- // Invalidate container
206281 if node.container != nil {
207282 node.container.Invalidate()
208283 node.container = nil
209284 }
210285
211- // Clear tree pointers
212286 node.parent = nil
213287 node.left = nil
214288 node.right = nil
@@ -217,9 +291,9 @@ func (wm *WorkspaceManager) cleanupPane(node *paneNode) {
217291
218292 ## PHASE 3: Stack Container Simplification
219293
220-### 3.1 Simplified Stack Close
294+### 3.1 Compatibility Layer for Stacked Panes
221295 ```go
222-func (wm *WorkspaceManager) closeStackedPaneSimple(node *paneNode) (*paneNode, error) {
296+func (wm *WorkspaceManager) closeStackedPaneCompat(node *paneNode) (*paneNode, error) {
223297 stack := node.parent
224298
225299 // Find index
@@ -276,7 +350,7 @@ func (wm *WorkspaceManager) closeStackedPaneSimple(node *paneNode) (*paneNode, e
276350 }
277351
278352 // Cleanup stack container
279- wm.cleanupPane(stack)
353+ wm.cleanupPane(stack, wm.nextCleanupGeneration())
280354
281355 return remaining, nil
282356 }
@@ -305,7 +379,7 @@ func (wm *WorkspaceManager) closeStackedPaneSimple(node *paneNode) (*paneNode, e
305379 }
306380
307381 // Cleanup closed pane
308- wm.cleanupPane(node)
382+ wm.cleanupPane(node, wm.nextCleanupGeneration())
309383
310384 return stack, nil
311385 }
@@ -341,40 +415,13 @@ webkit.BoxAppend(box, widget) // Auto-unparents from old parent
341415 ## PHASE 5: Migration Strategy
342416
343417 ### 5.1 Step-by-Step Migration
344-1. **Week 1**: Implement new functions alongside old ones
345-2. **Week 2**: Add comprehensive logging to both paths
346-3. **Week 3**: A/B test with feature flag
347-4. **Week 4**: Full migration after stability proven
348-
349-### 5.2 Testing Scenarios
350-```go
351-func TestClosePaneScenarios(t *testing.T) {
352- tests := []struct{
353- name string
354- setup func() *paneNode
355- target string // path to target node
356- expectPanes int
357- expectRoot string
358- }{
359- {
360- name: "close_right_child_simple_split",
361- // A[B,C] -> close C -> A becomes B
362- },
363- {
364- name: "close_middle_nested_split",
365- // A[B[D,E],C] -> close B -> A[sibling(D,E),C]
366- },
367- {
368- name: "close_in_stack",
369- // Stack[A,B,C] -> close B -> Stack[A,C]
370- },
371- {
372- name: "close_last_in_stack",
373- // A[Stack[B,C],D] -> close C -> A[B,D]
374- },
375- }
376-}
377-```
418+1. Land instrumentation (`beginClose` logging, `dumpTreeState`) behind `DebugPaneClose` flag.
419+2. Extend data structures (`widgetValid`, `cleanupGeneration`, `cleanupCounter`).
420+3. Introduce helper scaffolding (`closeContext`, `ensureWidgets`, `promoteSibling`, `swapContainers`).
421+4. Replace core `closePane` logic and keep compatibility helpers stubbed.
422+5. Rewrite stacked close handling to call `closeStackedPaneCompat` while keeping `StackedPaneManager` API surface intact.
423+6. Add regression tests + golden tree dumps; run `go test ./tests/browser/...` and `make build`.
424+7. Remove verbose logging, keep assertions + generation checks.
378425
379426 ## PHASE 6: Validation & Safety
380427
@@ -394,51 +441,51 @@ if err := wm.validateTreeConsistency(); err != nil {
394441 ### 6.2 Comprehensive Logging
395442 ```go
396443 // Before operation:
397-log.Printf("[CLOSE] Starting: node=%p parent=%p sibling=%p",
444+log.Printf("[pane-close] start node=%p parent=%p sibling=%p",
398445 node, node.parent, sibling)
399446
400447 // After tree update:
401-log.Printf("[CLOSE] Tree updated: new_root=%p promoted=%p",
402- wm.root, promoted)
448+log.Printf("[pane-close] tree updated new_root=%p promoted=%p gen=%d",
449+ wm.root, promoted, ctx.Generation())
403450
404451 // After GTK update:
405-log.Printf("[CLOSE] GTK updated: widget=%#x parent=%#x",
452+log.Printf("[pane-close] gtk updated widget=%#x parent=%#x",
406453 widgetPtr, parentPtr)
407454 ```
408455
409456 ## Expected Results
410457
411458 ### Before (Current Issues):
412-- 400+ lines of complex code
459+- 400+ lines of defensive code
413460 - Segfaults on complex layouts
414461 - Multiple panes closed incorrectly
415462 - Widget corruption errors
416463
417464 ### After (New Implementation):
418-- ~150 lines total
465+- ≤200 lines of cohesive logic
419466 - Clean GTK4 lifecycle adherence
420467 - Predictable single-pane closure
421-- No widget corruption
468+- Deterministic hover/focus cleanup using generations
422469
423470 ## Implementation Order
424-1. Implement `closePaneV2` function
425-2. Implement helper functions
426-3. Add comprehensive logging
427-4. Create test suite
428-5. Run side-by-side with flag
429-6. Migrate after proven stable
471+1. Land instrumentation + regression harness (Phase 0)
472+2. Introduce data-structure additions (`widgetValid`, generation counter)
473+3. Implement new `closePane` + helper set in-place (Phase 2)
474+4. Wire stacked pane compatibility shim (Phase 3)
475+5. Run tests (`go test ./tests/browser/...`) and `make build`
476+6. Trim temporary logging once confidence is regained
430477
431478 ## Key Files to Modify
432479
433-1. `workspace_types.go:8` - Add `widgetValid bool` field to `paneNode`
434-2. `workspace_pane_ops.go:707` - Current `closePane` function (rename to `closePaneLegacy`)
435-3. **NEW** `workspace_pane_close_v2.go` - Simplified implementation
436-4. **NEW** `workspace_pane_close_test.go` - Comprehensive test suite
437-5. `stacked_panes.go` - Simplify `CloseStackedPane` function
480+1. `internal/app/browser/workspace_types.go` - Add `widgetValid` and `cleanupGeneration`
481+2. `internal/app/browser/workspace_manager.go` - Introduce `cleanupCounter` + helper
482+3. `internal/app/browser/workspace_pane_ops.go` - Replace `closePane`, add helpers, call into compat shim
483+4. `internal/app/browser/stacked_panes.go` - Point manager to `closeStackedPaneCompat`
484+5. `tests/browser/workspace_close_test.go` - Add regression scenarios
438485
439486 ## Critical Success Metrics
440487
441488 1. **Stability**: No segfaults in any close scenario
442489 2. **Correctness**: Only requested pane closes (never multiple)
443490 3. **Performance**: Close operation completes in <50ms
444-4. **Maintainability**: Code is <200 lines and easily understood⏎̸
491+4. **Maintainability**: Code is <200 lines and easily understood
@@ -2,18 +2,36 @@
2 2
3 ## Problem Analysis Summary3 ## Problem Analysis Summary
4 4
5-The current implementation has **THREE CRITICAL BUGS**:5+The current implementation in `internal/app/browser/workspace_pane_ops.go:707`+ exhibits **three interlocked failure modes**:
6-1. **Multiple Pane Closure**: Sibling identification logic at lines 920-928 incorrectly identifies siblings6+1. **Multiple Pane Closure**: The promotion branch (around `workspace_pane_ops.go:990`) iterates over invalid parent/child references, causing both siblings to be detached when `wm.app.panes` still counts them. This yields double-close behaviour during rapid shortcuts.
7-2. **Invalid Widget Promotion**: Lines 1042-1090 operate on destroyed widgets7+2. **Invalid Widget Promotion**: The root promotion path (roughly `workspace_pane_ops.go:878-948`) manipulates widgets after `Cleanup()` invalidates their `SafeWidget` guard, which leads to sporadic `GTK_IS_WIDGET` assertions.
8-3. **Race Conditions**: Widget cleanup happens before promotion completes8+3. **Race Conditions**: Cleanup (`ensureCleanup`) and hover-detach run while asynchronous `IdleAdd` callbacks still reference the old nodes, so hover/focus reattachment runs against freed memory during reproduction of issue #1429.
9+
10+> Baseline reproduction: trigger repeated `closePane` via Ctrl+W on a layout with nested root + stack; log spew shows `[workspace] close aborted` alongside panics in `ensureSafeWidget`.
11+
12+### Environment Constraints
13+- `stackedPaneManager.CloseStackedPane` is tightly coupled to `wm.stackedPaneManager`, so any refactor must either reimplement its logic or provide compatibility shims.
14+- `BulletproofClosePane` wraps `closePane`, retrying on specific errors; changes to panic/zero-return semantics must preserve that contract.
15+- Hover/focus controller lifecycles (`wm.ensureHover`, `focusStateMachine.attachGTKController`) must remain intact after tree restructuring.
16+
17+### Preconditions & Guardrails
18+1. Capture baseline behaviour with `go test ./tests/workspace/...` (currently incomplete; we will add targeted tests in Phase 5).
19+2. Add temporary structured logging with `log.Printf("[pane-close] ...")` gated behind an environment toggle to avoid production noise.
20+3. Ensure unit test scaffolding can construct `WorkspaceManager` with fake `webkit` bindings (see `tests/browser/fakes`).
21+
22+## PHASE 0: Stabilise & Instrument
23+
24+1. Wrap `BulletproofClosePane` with structured logging (log key: `pane-close-stage`) guarded by `wm.debugInstrumentation` flag.
25+2. Add `WorkspaceDiagnostics` helper capturing tree snapshots (`wm.dumpTreeState`) before and after `closePane`.
26+3. Add regression harness in `tests/browser/workspace_close_test.go` that constructs a representative tree and exercises edge cases (root-only, nested split, stack + split).
27+4. Verify reproduction steps still fail prior to refactor to ensure harness is meaningful.
9 28
10 ## PHASE 1: New Simplified Architecture29 ## PHASE 1: New Simplified Architecture
11 30
12-### 1.1 Create New File Structure31+### 1.1 Keep File Layout, Replace Implementation In-Place
13 ```32 ```
14-workspace_pane_close_v2.go # New simplified close logic33+internal/app/browser/workspace_pane_ops.go # Replace existing closePane implementation with new logic
15-workspace_pane_close_legacy.go # Rename current implementation34+internal/app/browser/workspace_types.go # Extend paneNode metadata
16-workspace_pane_close_test.go # Comprehensive tests
17 ```35 ```
18 36
19 ### 1.2 Core Data Structure Changes37 ### 1.2 Core Data Structure Changes
@@ -22,82 +40,58 @@ workspace_pane_close_test.go # Comprehensive tests
22 type paneNode struct {40 type paneNode struct {
23 // ... existing fields ...41 // ... existing fields ...
24 42
25- // New field to track widget validity43+ widgetValid bool // Guard flagged before GTK destruction
26- widgetValid bool // Set to false when widget destroyed44+ cleanupGeneration uint // Helps assert that asynchronous callbacks skip stale nodes
45+}
46+
47+// Add to WorkspaceManager
48+type WorkspaceManager struct {
49+ // ... existing fields ...
50+
51+ cleanupCounter uint
27 }52 }
28 ```53 ```
29 54
30 ## PHASE 2: Detailed New closePane Implementation55 ## PHASE 2: Detailed New closePane Implementation
31 56
32-### 2.1 Main Close Function (50-70 lines total)57+### 2.1 Main Close Function (60-80 lines total)
33 ```go58 ```go
34-func (wm *WorkspaceManager) closePaneV2(node *paneNode) (*paneNode, error) {59+func (wm *WorkspaceManager) closePane(node *paneNode) (*paneNode, error) {
35- // STEP 1: Basic validation (5 lines)60+ ctx := wm.beginClose(node)
36- if node == nil || !node.isLeaf {61+ defer ctx.finish()
37- return nil, errors.New("invalid close target")62+
63+ // STEP 1: Basic validation (quick fail)
64+ if ctx.err != nil {
65+ return nil, ctx.err
38 }66 }
39 67
40- // STEP 2: Handle special cases (15 lines)68+ // STEP 2: Handle stacked panes via compatibility shim
41- // 2a. Stacked pane
42 if node.parent != nil && node.parent.isStacked {69 if node.parent != nil && node.parent.isStacked {
43- return wm.closeStackedPaneSimple(node)70+ return wm.closeStackedPaneCompat(node)
44 }71 }
45 72
46- // 2b. Last pane73+ // STEP 3: Handle trivial exit cases
47- if wm.countPanes() == 1 {74+ if ctx.remaining == 1 {
48- wm.cleanupAndExit(node)75+ return wm.cleanupAndExit(node)
49- return nil, nil
50 }76 }
51-
52- // 2c. Root pane with others
53 if node == wm.root {77 if node == wm.root {
54- return wm.promoteNewRoot(node)78+ return wm.promoteNewRoot(ctx, node)
55 }79 }
56 80
57- // STEP 3: Standard close - promote sibling (30 lines)81+ // STEP 4: Promote sibling in-place
58 parent := node.parent82 parent := node.parent
59 sibling := wm.getSibling(node)83 sibling := wm.getSibling(node)
60 grandparent := parent.parent84 grandparent := parent.parent
85+ wm.ensureWidgets(grandparent, parent, sibling)
61 86
62- // Critical: Update tree structure FIRST87+ wm.promoteSibling(grandparent, parent, sibling)
63- sibling.parent = grandparent
64- if grandparent != nil {
65- if grandparent.left == parent {
66- grandparent.left = sibling
67- } else {
68- grandparent.right = sibling
69- }
70- } else {
71- wm.root = sibling
72- }
73 88
74- // STEP 4: Update GTK widgets (using GTK4 auto-unparenting)89+ // STEP 5: GTK updates leverage auto-unparenting
75- if grandparent != nil && grandparent.container != nil {90+ wm.swapContainers(grandparent, sibling)
76- grandparent.container.Execute(func(gpPtr uintptr) error {
77- sibling.container.Execute(func(sibPtr uintptr) error {
78- // GTK4 automatically unparents old child when setting new
79- if grandparent.left == sibling {
80- webkit.PanedSetStartChild(gpPtr, sibPtr)
81- } else {
82- webkit.PanedSetEndChild(gpPtr, sibPtr)
83- }
84- return nil
85- })
86- return nil
87- })
88- } else if sibling == wm.root {
89- // Sibling is new root
90- sibling.container.Execute(func(sibPtr uintptr) error {
91- wm.window.SetChild(sibPtr)
92- return nil
93- })
94- }
95-
96- // STEP 5: Cleanup (10 lines)
97- wm.cleanupPane(node)
98- wm.cleanupPane(parent) // Parent paned is no longer needed
99 91
100- // STEP 6: Update focus92+ // STEP 6: Cleanup & focus
93+ wm.cleanupPane(node, ctx.Generation())
94+ wm.decommissionParent(parent, ctx.Generation())
101 wm.setFocusToLeaf(sibling)95 wm.setFocusToLeaf(sibling)
102 96
103 return sibling, nil97 return sibling, nil
@@ -120,81 +114,163 @@ func (wm *WorkspaceManager) getSibling(node *paneNode) *paneNode {
120 }114 }
121 ```115 ```
122 116
123-#### promoteNewRoot - Handle Root Replacement117+#### beginClose / closeContext
118+```go
119+type closeContext struct {
120+ wm *WorkspaceManager
121+ target *paneNode
122+ remaining int
123+ err error
124+ generation uint
125+}
126+
127+func (wm *WorkspaceManager) beginClose(node *paneNode) closeContext {
128+ ctx := closeContext{wm: wm, target: node, generation: wm.nextCleanupGeneration()}
129+ switch {
130+ case wm == nil:
131+ ctx.err = errors.New("workspace manager nil")
132+ case node == nil || !node.isLeaf:
133+ ctx.err = errors.New("invalid close target")
134+ case node.pane == nil || node.pane.webView == nil:
135+ ctx.err = errors.New("close target missing webview")
136+ default:
137+ ctx.remaining = len(wm.app.panes)
138+ }
139+ return ctx
140+}
141+
142+func (ctx closeContext) Generation() uint { return ctx.generation }
143+
144+func (ctx closeContext) finish() {
145+ if ctx.err == nil {
146+ ctx.wm.updateMainPane()
147+ }
148+}
149+```
150+
151+#### ensureWidgets
152+```go
153+func (wm *WorkspaceManager) ensureWidgets(nodes ...*paneNode) {
154+ for _, n := range nodes {
155+ if n == nil || n.container == nil {
156+ continue
157+ }
158+ if n.container.IsValid() {
159+ continue
160+ }
161+ ptr := n.container.Ptr()
162+ n.container = wm.widgetRegistry.Recover(ptr, n.container.typeInfo)
163+ }
164+}
165+```
166+
167+#### promoteSibling
124 ```go168 ```go
125-func (wm *WorkspaceManager) promoteNewRoot(oldRoot *paneNode) (*paneNode, error) {169+func (wm *WorkspaceManager) promoteSibling(grand *paneNode, parent *paneNode, sibling *paneNode) {
126- // Find first non-root pane by traversing tree170+ if grand == nil {
127- var newRoot *paneNode171+ wm.root = sibling
128-172+ sibling.parent = nil
129- // Try left subtree first173+ return
130- if oldRoot.left != nil {174+ }
131- newRoot = oldRoot.left175+ sibling.parent = grand
132- } else if oldRoot.right != nil {176+ if grand.left == parent {
133- newRoot = oldRoot.right177+ grand.left = sibling
134 } else {178 } else {
135- // No other panes - shouldn't happen179+ grand.right = sibling
136- return nil, errors.New("no replacement root found")
137 }180 }
181+}
182+```
138 183
139- // Detach new root from its parent184+#### swapContainers
140- if newRoot.parent != nil {185+```go
141- sibling := wm.getSibling(newRoot)186+func (wm *WorkspaceManager) swapContainers(grand *paneNode, sibling *paneNode) {
142- parent := newRoot.parent187+ if grand == nil {
143-188+ wm.attachRoot(sibling)
144- // Promote sibling to parent's position189+ return
145- if parent.parent != nil {190+ }
146- // Has grandparent - attach sibling to it191+ grand.container.Execute(func(gPtr uintptr) error {
147- grand := parent.parent192+ sibling.container.Execute(func(sPtr uintptr) error {
148- sibling.parent = grand193+ if grand.left == sibling {
149- if grand.left == parent {194+ webkit.PanedSetStartChild(gPtr, sPtr)
150- grand.left = sibling
151 } else {195 } else {
152- grand.right = sibling196+ webkit.PanedSetEndChild(gPtr, sPtr)
153 }197 }
198+ return nil
199+ })
200+ return nil
201+ })
202+}
203+```
154 204
155- // Update GTK widget205+#### decommissionParent
156- grand.container.Execute(func(gPtr uintptr) error {206+```go
157- sibling.container.Execute(func(sPtr uintptr) error {207+func (wm *WorkspaceManager) decommissionParent(parent *paneNode, generation uint) {
158- if grand.left == sibling {208+ if parent == nil {
159- webkit.PanedSetStartChild(gPtr, sPtr)209+ return
160- } else {
161- webkit.PanedSetEndChild(gPtr, sPtr)
162- }
163- return nil
164- })
165- return nil
166- })
167- }
168 }210 }
211+ wm.cleanupPane(parent, generation)
212+}
213+```
169 214
170- // Make newRoot the root215+#### nextCleanupGeneration
171- newRoot.parent = nil216+```go
172- wm.root = newRoot217+func (wm *WorkspaceManager) nextCleanupGeneration() uint {
218+ wm.cleanupCounter++
219+ return wm.cleanupCounter
220+}
221+```
173 222
174- // Attach to window223+#### attachRoot
175- newRoot.container.Execute(func(ptr uintptr) error {224+```go
225+func (wm *WorkspaceManager) attachRoot(root *paneNode) {
226+ if root == nil || root.container == nil || wm.window == nil {
227+ return
228+ }
229+ root.container.Execute(func(ptr uintptr) error {
176 wm.window.SetChild(ptr)230 wm.window.SetChild(ptr)
231+ webkit.WidgetQueueAllocate(ptr)
232+ webkit.WidgetShow(ptr)
177 return nil233 return nil
178 })234 })
235+}
236+```
179 237
180- // Cleanup old root238+#### promoteNewRoot - Handle Root Replacement
181- wm.cleanupPane(oldRoot)239+```go
240+func (wm *WorkspaceManager) promoteNewRoot(ctx closeContext, oldRoot *paneNode) (*paneNode, error) {
241+ candidate := wm.findReplacementRoot(oldRoot)
242+ if candidate == nil {
243+ return wm.cleanupAndExit(oldRoot)
244+ }
182 245
183- return newRoot, nil246+ sibling := wm.getSibling(candidate)
247+ if sibling != nil {
248+ wm.promoteSibling(candidate.parent.parent, candidate.parent, sibling)
249+ }
250+
251+ candidate.parent = nil
252+ wm.root = candidate
253+
254+ wm.attachRoot(candidate)
255+
256+ wm.cleanupPane(oldRoot, ctx.Generation())
257+ return candidate, nil
184 }258 }
185 ```259 ```
186 260
187 #### cleanupPane - Safe Cleanup261 #### cleanupPane - Safe Cleanup
188 ```go262 ```go
189-func (wm *WorkspaceManager) cleanupPane(node *paneNode) {263+func (wm *WorkspaceManager) cleanupPane(node *paneNode, generation uint) {
190- if node == nil || node.widgetValid == false {264+ if node == nil {
191- return // Already cleaned265+ return
266+ }
267+ if !node.widgetValid {
268+ return
192 }269 }
193 270
194- // Mark as invalid immediately
195 node.widgetValid = false271 node.widgetValid = false
272+ node.cleanupGeneration = generation
196 273
197- // Clean up pane resources
198 if node.pane != nil {274 if node.pane != nil {
199 node.pane.CleanupFromWorkspace(wm)275 node.pane.CleanupFromWorkspace(wm)
200 if node.pane.webView != nil {276 if node.pane.webView != nil {
@@ -202,13 +278,11 @@ func (wm *WorkspaceManager) cleanupPane(node *paneNode) {
202 }278 }
203 }279 }
204 280
205- // Invalidate container
206 if node.container != nil {281 if node.container != nil {
207 node.container.Invalidate()282 node.container.Invalidate()
208 node.container = nil283 node.container = nil
209 }284 }
210 285
211- // Clear tree pointers
212 node.parent = nil286 node.parent = nil
213 node.left = nil287 node.left = nil
214 node.right = nil288 node.right = nil
@@ -217,9 +291,9 @@ func (wm *WorkspaceManager) cleanupPane(node *paneNode) {
217 291
218 ## PHASE 3: Stack Container Simplification292 ## PHASE 3: Stack Container Simplification
219 293
220-### 3.1 Simplified Stack Close294+### 3.1 Compatibility Layer for Stacked Panes
221 ```go295 ```go
222-func (wm *WorkspaceManager) closeStackedPaneSimple(node *paneNode) (*paneNode, error) {296+func (wm *WorkspaceManager) closeStackedPaneCompat(node *paneNode) (*paneNode, error) {
223 stack := node.parent297 stack := node.parent
224 298
225 // Find index299 // Find index
@@ -276,7 +350,7 @@ func (wm *WorkspaceManager) closeStackedPaneSimple(node *paneNode) (*paneNode, e
276 }350 }
277 351
278 // Cleanup stack container352 // Cleanup stack container
279- wm.cleanupPane(stack)353+ wm.cleanupPane(stack, wm.nextCleanupGeneration())
280 354
281 return remaining, nil355 return remaining, nil
282 }356 }
@@ -305,7 +379,7 @@ func (wm *WorkspaceManager) closeStackedPaneSimple(node *paneNode) (*paneNode, e
305 }379 }
306 380
307 // Cleanup closed pane381 // Cleanup closed pane
308- wm.cleanupPane(node)382+ wm.cleanupPane(node, wm.nextCleanupGeneration())
309 383
310 return stack, nil384 return stack, nil
311 }385 }
@@ -341,40 +415,13 @@ webkit.BoxAppend(box, widget) // Auto-unparents from old parent
341 ## PHASE 5: Migration Strategy415 ## PHASE 5: Migration Strategy
342 416
343 ### 5.1 Step-by-Step Migration417 ### 5.1 Step-by-Step Migration
344-1. **Week 1**: Implement new functions alongside old ones418+1. Land instrumentation (`beginClose` logging, `dumpTreeState`) behind `DebugPaneClose` flag.
345-2. **Week 2**: Add comprehensive logging to both paths419+2. Extend data structures (`widgetValid`, `cleanupGeneration`, `cleanupCounter`).
346-3. **Week 3**: A/B test with feature flag420+3. Introduce helper scaffolding (`closeContext`, `ensureWidgets`, `promoteSibling`, `swapContainers`).
347-4. **Week 4**: Full migration after stability proven421+4. Replace core `closePane` logic and keep compatibility helpers stubbed.
348-422+5. Rewrite stacked close handling to call `closeStackedPaneCompat` while keeping `StackedPaneManager` API surface intact.
349-### 5.2 Testing Scenarios423+6. Add regression tests + golden tree dumps; run `go test ./tests/browser/...` and `make build`.
350-```go424+7. Remove verbose logging, keep assertions + generation checks.
351-func TestClosePaneScenarios(t *testing.T) {
352- tests := []struct{
353- name string
354- setup func() *paneNode
355- target string // path to target node
356- expectPanes int
357- expectRoot string
358- }{
359- {
360- name: "close_right_child_simple_split",
361- // A[B,C] -> close C -> A becomes B
362- },
363- {
364- name: "close_middle_nested_split",
365- // A[B[D,E],C] -> close B -> A[sibling(D,E),C]
366- },
367- {
368- name: "close_in_stack",
369- // Stack[A,B,C] -> close B -> Stack[A,C]
370- },
371- {
372- name: "close_last_in_stack",
373- // A[Stack[B,C],D] -> close C -> A[B,D]
374- },
375- }
376-}
377-```
378 425
379 ## PHASE 6: Validation & Safety426 ## PHASE 6: Validation & Safety
380 427
@@ -394,51 +441,51 @@ if err := wm.validateTreeConsistency(); err != nil {
394 ### 6.2 Comprehensive Logging441 ### 6.2 Comprehensive Logging
395 ```go442 ```go
396 // Before operation:443 // Before operation:
397-log.Printf("[CLOSE] Starting: node=%p parent=%p sibling=%p",444+log.Printf("[pane-close] start node=%p parent=%p sibling=%p",
398 node, node.parent, sibling)445 node, node.parent, sibling)
399 446
400 // After tree update:447 // After tree update:
401-log.Printf("[CLOSE] Tree updated: new_root=%p promoted=%p",448+log.Printf("[pane-close] tree updated new_root=%p promoted=%p gen=%d",
402- wm.root, promoted)449+ wm.root, promoted, ctx.Generation())
403 450
404 // After GTK update:451 // After GTK update:
405-log.Printf("[CLOSE] GTK updated: widget=%#x parent=%#x",452+log.Printf("[pane-close] gtk updated widget=%#x parent=%#x",
406 widgetPtr, parentPtr)453 widgetPtr, parentPtr)
407 ```454 ```
408 455
409 ## Expected Results456 ## Expected Results
410 457
411 ### Before (Current Issues):458 ### Before (Current Issues):
412-- 400+ lines of complex code459+- 400+ lines of defensive code
413 - Segfaults on complex layouts460 - Segfaults on complex layouts
414 - Multiple panes closed incorrectly461 - Multiple panes closed incorrectly
415 - Widget corruption errors462 - Widget corruption errors
416 463
417 ### After (New Implementation):464 ### After (New Implementation):
418-- ~150 lines total465+- ≤200 lines of cohesive logic
419 - Clean GTK4 lifecycle adherence466 - Clean GTK4 lifecycle adherence
420 - Predictable single-pane closure467 - Predictable single-pane closure
421-- No widget corruption468+- Deterministic hover/focus cleanup using generations
422 469
423 ## Implementation Order470 ## Implementation Order
424-1. Implement `closePaneV2` function471+1. Land instrumentation + regression harness (Phase 0)
425-2. Implement helper functions472+2. Introduce data-structure additions (`widgetValid`, generation counter)
426-3. Add comprehensive logging473+3. Implement new `closePane` + helper set in-place (Phase 2)
427-4. Create test suite474+4. Wire stacked pane compatibility shim (Phase 3)
428-5. Run side-by-side with flag475+5. Run tests (`go test ./tests/browser/...`) and `make build`
429-6. Migrate after proven stable476+6. Trim temporary logging once confidence is regained
430 477
431 ## Key Files to Modify478 ## Key Files to Modify
432 479
433-1. `workspace_types.go:8` - Add `widgetValid bool` field to `paneNode`480+1. `internal/app/browser/workspace_types.go` - Add `widgetValid` and `cleanupGeneration`
434-2. `workspace_pane_ops.go:707` - Current `closePane` function (rename to `closePaneLegacy`)481+2. `internal/app/browser/workspace_manager.go` - Introduce `cleanupCounter` + helper
435-3. **NEW** `workspace_pane_close_v2.go` - Simplified implementation482+3. `internal/app/browser/workspace_pane_ops.go` - Replace `closePane`, add helpers, call into compat shim
436-4. **NEW** `workspace_pane_close_test.go` - Comprehensive test suite483+4. `internal/app/browser/stacked_panes.go` - Point manager to `closeStackedPaneCompat`
437-5. `stacked_panes.go` - Simplify `CloseStackedPane` function484+5. `tests/browser/workspace_close_test.go` - Add regression scenarios
438 485
439 ## Critical Success Metrics486 ## Critical Success Metrics
440 487
441 1. **Stability**: No segfaults in any close scenario488 1. **Stability**: No segfaults in any close scenario
442 2. **Correctness**: Only requested pane closes (never multiple)489 2. **Correctness**: Only requested pane closes (never multiple)
443 3. **Performance**: Close operation completes in <50ms490 3. **Performance**: Close operation completes in <50ms
444-4. **Maintainability**: Code is <200 lines and easily understood⏎̸491+4. **Maintainability**: Code is <200 lines and easily understood

Keyboard shortcuts

Single-key shortcuts are on. ctrl k and esc work whatever this says.

ctrl k :
go to a section, branch, tag, file or repository
?
this list
j k
move the row cursor down / up
enter
open the row under the cursor; on Files, a directory opens or closes in place
↑ ↓ ← → home end
in the file tree (focus on a row): previous / next row, close / open a directory or go to its parent / first entry, first / last row. They never open a file
l h
on Files: open a directory or a file / close a directory or go up to its parent, then to the parent page. Elsewhere: into the side panel (on a merge request, its files) / back to the content; there j k move between its controls
g g G
first / last row
/
filter the list; on Files, find a file anywhere in the repository (esc clears, enter opens the best match)
b
switch branch or tag
g s g f g c g b g m g i g r g a
tabs: summary, files, commits, branches & tags, merge requests, issues, releases, actions (when available)
g h
home
w
wide mode: use the whole width of the screen, or not (remembered)
y
copy a permalink pinned to the commit
d u x
diff: split / unified / collapse or expand all
e E
diff: the whole file in view / every file, with the lines the hunks hide
n
merge request: next note in the diff
esc
close a dialog, leave a field

Branches 8

Tags 42

↑↓ moveenter openesc close