fix(workspace): prevent domain/UI desync in stacked panes

- Add bounds check in syncStackedPaneTitles to handle desync gracefully
- Add rollback in insertPopupStacked when UI update fails
- Track stack conversion state for proper cleanup on failure

commit: 38a09237f8e0b2c0687e0099eef14f4bfb98e868
author: brice · 9mo ago · 2026-01-14 08:45 UTC
parent: d759bef
signature: GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email

1 file · +57 -6

M internal/ui/coordinator/workspace.go +57 -6


@@ -1135,11 +1135,24 @@ func (c *WorkspaceCoordinator) syncStackedPaneTitles(ctx context.Context, stacke
1135	1135		return
1136	1136		}
1137	1137
	1138	+	// Get UI pane count to avoid index out of bounds when domain and UI are out of sync
	1139	+	uiPaneCount := stackedView.Count()
	1140	+
1138	1141		for i, child := range stackNode.Children {
1139	1142		if child.Pane == nil {
1140	1143		continue
1141	1144		}
1142	1145
	1146	+	// Skip if index exceeds UI pane count (domain/UI desync)
	1147	+	if i >= uiPaneCount {
	1148	+	log.Debug().
	1149	+	Int("index", i).
	1150	+	Int("ui_count", uiPaneCount).
	1151	+	Str("pane_id", string(child.Pane.ID)).
	1152	+	Msg("skipping title sync: index exceeds UI pane count")
	1153	+	continue
	1154	+	}
	1155	+
1143	1156		title := c.contentCoord.GetTitle(child.Pane.ID)
1144	1157		if title == "" {
1145	1158		title = child.Pane.Title

@@ -1798,11 +1811,13 @@ func (c *WorkspaceCoordinator) insertPopupStacked(ctx context.Context, input Ins
1798	1811		return fmt.Errorf("parent pane not found: %s", input.ParentPaneID)
1799	1812		}
1800	1813
1801		-	// Resolve or create stack node
1802		-	stackNode := c.resolveOrCreateStackNode(ctx, parentNode, input.ParentPaneID)
	1814	+	// Resolve or create stack node (track conversion for potential rollback)
	1815	+	stackNode, conversionInfo := c.resolveOrCreateStackNode(ctx, parentNode, input.ParentPaneID)
1803	1816
1804	1817		// Add popup pane to stack using use case
1805	1818		if _, err := c.panesUC.AddToStack(ctx, ws, stackNode, input.PopupPane); err != nil {
	1819	+	// Rollback stack conversion if we created a new stack
	1820	+	conversionInfo.revert()
1806	1821		return fmt.Errorf("failed to add popup to stack: %w", err)
1807	1822		}
1808	1823

@@ -1812,6 +1827,17 @@ func (c *WorkspaceCoordinator) insertPopupStacked(ctx context.Context, input Ins
1812	1827		}
1813	1828
1814	1829		if err := c.attachPopupPaneView(ctx, input, wsView, stackNode, log); err != nil {
	1830	+	// UI update failed - rollback domain changes to maintain consistency
	1831	+	log.Warn().Err(err).Msg("rolling back domain changes due to UI failure")
	1832	+
	1833	+	// Remove popup pane from stack
	1834	+	if removeErr := c.panesUC.RemoveFromStack(ctx, stackNode, input.PopupPane.ID); removeErr != nil {
	1835	+	log.Error().Err(removeErr).Msg("failed to rollback popup pane addition")
	1836	+	}
	1837	+
	1838	+	// Revert stack conversion if we created a new stack
	1839	+	conversionInfo.revert()
	1840	+
1815	1841		return err
1816	1842		}
1817	1843

@@ -1823,14 +1849,35 @@ func (c *WorkspaceCoordinator) insertPopupStacked(ctx context.Context, input Ins
1823	1849		return nil
1824	1850		}
1825	1851
	1852	+	// stackConversionInfo holds state needed to revert a leaf-to-stack conversion.
	1853	+	type stackConversionInfo struct {
	1854	+	wasConverted bool // true if resolveOrCreateStackNode converted a leaf to a stack
	1855	+	originalPane *entity.Pane // the pane that was in the node before conversion
	1856	+	node *entity.PaneNode // the node that was converted
	1857	+	}
	1858	+
	1859	+	// revert undoes a leaf-to-stack conversion.
	1860	+	func (info *stackConversionInfo) revert() {
	1861	+	if !info.wasConverted \|\| info.node == nil {
	1862	+	return
	1863	+	}
	1864	+
	1865	+	// Restore node to leaf state
	1866	+	info.node.Pane = info.originalPane
	1867	+	info.node.IsStacked = false
	1868	+	info.node.ActiveStackIndex = 0
	1869	+	info.node.Children = nil
	1870	+	}
	1871	+
1826	1872		// resolveOrCreateStackNode finds an existing stack or creates one for popup insertion.
1827	1873		// Note: For popups, we can't use CreateStack use case since it creates a new pane,
1828	1874		// but we already have the popup pane. This conversion is a special case.
	1875	+	// Returns the stack node and conversion info for potential rollback.
1829	1876		func (c *WorkspaceCoordinator) resolveOrCreateStackNode(
1830	1877		ctx context.Context,
1831	1878		parentNode *entity.PaneNode,
1832	1879		parentPaneID entity.PaneID,
1833		-	) *entity.PaneNode {
	1880	+	) (*entity.PaneNode, stackConversionInfo) {
1834	1881		log := logging.FromContext(ctx)
1835	1882
1836	1883		// Check if parent is already in a stack

@@ -1838,7 +1885,7 @@ func (c *WorkspaceCoordinator) resolveOrCreateStackNode(
1838	1885		log.Debug().
1839	1886		Str("stack_id", parentNode.Parent.ID).
1840	1887		Msg("using existing parent stack for popup")
1841		-	return parentNode.Parent
	1888	+	return parentNode.Parent, stackConversionInfo{}
1842	1889		}
1843	1890
1844	1891		// Check if parent is already a stack container

@@ -1846,7 +1893,7 @@ func (c *WorkspaceCoordinator) resolveOrCreateStackNode(
1846	1893		log.Debug().
1847	1894		Str("stack_id", parentNode.ID).
1848	1895		Msg("parent is already a stack container")
1849		-	return parentNode
	1896	+	return parentNode, stackConversionInfo{}
1850	1897		}
1851	1898
1852	1899		// Need to convert leaf to stack for popup insertion.

@@ -1880,7 +1927,11 @@ func (c *WorkspaceCoordinator) resolveOrCreateStackNode(
1880	1927		Str("original_pane", string(originalPane.ID)).
1881	1928		Msg("converted leaf to stacked node for popup")
1882	1929
1883		-	return parentNode
	1930	+	return parentNode, stackConversionInfo{
	1931	+	wasConverted: true,
	1932	+	originalPane: originalPane,
	1933	+	node: parentNode,
	1934	+	}
1884	1935		}
1885	1936
1886	1937		func (c *WorkspaceCoordinator) attachPopupPaneView(

 		return
 	}
+	// Get UI pane count to avoid index out of bounds when domain and UI are out of sync
+	uiPaneCount := stackedView.Count()
 	for i, child := range stackNode.Children {
 		if child.Pane == nil {
 			continue
 		}
+		// Skip if index exceeds UI pane count (domain/UI desync)
+		if i >= uiPaneCount {
+			log.Debug().
+				Int("index", i).
+				Int("ui_count", uiPaneCount).
+				Str("pane_id", string(child.Pane.ID)).
+				Msg("skipping title sync: index exceeds UI pane count")
+			continue
+		}
 		title := c.contentCoord.GetTitle(child.Pane.ID)
 		if title == "" {
 			title = child.Pane.Title
 		return fmt.Errorf("parent pane not found: %s", input.ParentPaneID)
 	}
-	// Resolve or create stack node
+	// Resolve or create stack node (track conversion for potential rollback)
-	stackNode := c.resolveOrCreateStackNode(ctx, parentNode, input.ParentPaneID)
+	stackNode, conversionInfo := c.resolveOrCreateStackNode(ctx, parentNode, input.ParentPaneID)
 	// Add popup pane to stack using use case
 	if _, err := c.panesUC.AddToStack(ctx, ws, stackNode, input.PopupPane); err != nil {
+		// Rollback stack conversion if we created a new stack
+		conversionInfo.revert()
 		return fmt.Errorf("failed to add popup to stack: %w", err)
 	}
 	}
 	if err := c.attachPopupPaneView(ctx, input, wsView, stackNode, log); err != nil {
+		// UI update failed - rollback domain changes to maintain consistency
+		log.Warn().Err(err).Msg("rolling back domain changes due to UI failure")
+		// Remove popup pane from stack
+		if removeErr := c.panesUC.RemoveFromStack(ctx, stackNode, input.PopupPane.ID); removeErr != nil {
+			log.Error().Err(removeErr).Msg("failed to rollback popup pane addition")
+		}
+		// Revert stack conversion if we created a new stack
+		conversionInfo.revert()
 		return err
 	}
 	return nil
 }
+// stackConversionInfo holds state needed to revert a leaf-to-stack conversion.
+type stackConversionInfo struct {
+	wasConverted bool             // true if resolveOrCreateStackNode converted a leaf to a stack
+	originalPane *entity.Pane     // the pane that was in the node before conversion
+	node         *entity.PaneNode // the node that was converted
+}
+// revert undoes a leaf-to-stack conversion.
+func (info *stackConversionInfo) revert() {
+	if !info.wasConverted || info.node == nil {
+		return
+	}
+	// Restore node to leaf state
+	info.node.Pane = info.originalPane
+	info.node.IsStacked = false
+	info.node.ActiveStackIndex = 0
+	info.node.Children = nil
+}
 // resolveOrCreateStackNode finds an existing stack or creates one for popup insertion.
 // Note: For popups, we can't use CreateStack use case since it creates a new pane,
 // but we already have the popup pane. This conversion is a special case.
+// Returns the stack node and conversion info for potential rollback.
 func (c *WorkspaceCoordinator) resolveOrCreateStackNode(
 	ctx context.Context,
 	parentNode *entity.PaneNode,
 	parentPaneID entity.PaneID,
-) *entity.PaneNode {
+) (*entity.PaneNode, stackConversionInfo) {
 	log := logging.FromContext(ctx)
 	// Check if parent is already in a stack
 		log.Debug().
 			Str("stack_id", parentNode.Parent.ID).
 			Msg("using existing parent stack for popup")
-		return parentNode.Parent
+		return parentNode.Parent, stackConversionInfo{}
 	}
 	// Check if parent is already a stack container
 		log.Debug().
 			Str("stack_id", parentNode.ID).
 			Msg("parent is already a stack container")
-		return parentNode
+		return parentNode, stackConversionInfo{}
 	}
 	// Need to convert leaf to stack for popup insertion.
 		Str("original_pane", string(originalPane.ID)).
 		Msg("converted leaf to stacked node for popup")
-	return parentNode
+	return parentNode, stackConversionInfo{
+		wasConverted: true,
+		originalPane: originalPane,
+		node:         parentNode,
+	}
 }
 func (c *WorkspaceCoordinator) attachPopupPaneView(