fix(ui): stabilize external link routing checks

commit: f108dcf2e607f20b29a7342eea617cc976484155
author: brice · 2mo ago · 2026-08-13 04:15 UTC
committer: brice · 2mo ago · 2026-08-13 04:39 UTC
parent: cf4e69e
signature: GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email

3 files · +84 -81

M internal/ui/browser_window.go +34 -19


@@ -629,32 +629,17 @@ func (a *App) openExternalURLOnMainThread(ctx context.Context, url string, cfg e
629	629		}
630	630
631	631		func (a *App) openExternalURLInFocusedWindow(ctx context.Context, url string, cfg entity.ExternalLinksConfig) error {
632		-	target := a.lastFocusedBrowserWindow()
633		-	if target == nil \|\| target.id == "" \|\| a.browserWindows[target.id] != target {
634		-	return fmt.Errorf("last-focused browser window is missing or stale")
635		-	}
636		-	activeTab := a.activeTabForBrowserWindow(target)
637		-	if activeTab == nil \|\| activeTab.Workspace == nil \|\| activeTab.Workspace.ActivePane() == nil {
638		-	return fmt.Errorf("last-focused browser window has no active tab and pane")
	632	+	target, activeTab, err := a.externalURLTarget()
	633	+	if err != nil {
	634	+	return err
639	635		}
640	636
641	637		a.activateBrowserWindow(target)
642	638		switch cfg.Behavior {
643	639		case entity.ExternalLinkBehaviorTabbed:
644		-	if a.tabCoord == nil {
645		-	return fmt.Errorf("tab coordinator not available")
646		-	}
647		-	created, err := a.tabCoord.Create(ctx, a.tabTargetForBrowserWindow(target), url)
648		-	if err != nil {
649		-	a.rollbackExternalTabCreation(ctx, target, created)
	640	+	if err := a.createExternalURLTab(ctx, target, url); err != nil {
650	641		return err
651	642		}
652		-	if created == nil \|\| created.Workspace == nil \|\| created.Workspace.ActivePane() == nil \|\|
653		-	created.Workspace.ActivePane().Pane == nil \|\| created.Workspace.ActivePane().Pane.URI != url \|\|
654		-	a.activeTabForBrowserWindow(target) != created \|\| a.workspaceViews[created.ID] == nil {
655		-	a.rollbackExternalTabCreation(ctx, target, created)
656		-	return fmt.Errorf("created tab did not become an active UI target")
657		-	}
658	643		case entity.ExternalLinkBehaviorSplit:
659	644		if err := a.placeExternalURLInPane(ctx, activeTab.Workspace, url, func(ctx context.Context) error {
660	645		return a.wsCoord.SplitWithURL(ctx, externalLinkSplitDirection(cfg.Placement), url)

@@ -678,6 +663,36 @@ func (a *App) openExternalURLInFocusedWindow(ctx context.Context, url string, cf
678	663		return nil
679	664		}
680	665
	666	+	func (a App) externalURLTarget() (browserWindow, *entity.Tab, error) {
	667	+	target := a.lastFocusedBrowserWindow()
	668	+	if target == nil \|\| target.id == "" \|\| a.browserWindows[target.id] != target {
	669	+	return nil, nil, fmt.Errorf("last-focused browser window is missing or stale")
	670	+	}
	671	+	activeTab := a.activeTabForBrowserWindow(target)
	672	+	if activeTab == nil \|\| activeTab.Workspace == nil \|\| activeTab.Workspace.ActivePane() == nil {
	673	+	return nil, nil, fmt.Errorf("last-focused browser window has no active tab and pane")
	674	+	}
	675	+	return target, activeTab, nil
	676	+	}
	677	+
	678	+	func (a App) createExternalURLTab(ctx context.Context, target browserWindow, url string) error {
	679	+	if a.tabCoord == nil {
	680	+	return fmt.Errorf("tab coordinator not available")
	681	+	}
	682	+	created, err := a.tabCoord.Create(ctx, a.tabTargetForBrowserWindow(target), url)
	683	+	if err != nil {
	684	+	a.rollbackExternalTabCreation(ctx, target, created)
	685	+	return err
	686	+	}
	687	+	if created == nil \|\| created.Workspace == nil \|\| created.Workspace.ActivePane() == nil \|\|
	688	+	created.Workspace.ActivePane().Pane == nil \|\| created.Workspace.ActivePane().Pane.URI != url \|\|
	689	+	a.activeTabForBrowserWindow(target) != created \|\| a.workspaceViews[created.ID] == nil {
	690	+	a.rollbackExternalTabCreation(ctx, target, created)
	691	+	return fmt.Errorf("created tab did not become an active UI target")
	692	+	}
	693	+	return nil
	694	+	}
	695	+
681	696		func (a App) placeExternalURLInPane(ctx context.Context, ws entity.Workspace, url string, place func(context.Context) error) error {
682	697		if a.wsCoord == nil {
683	698		return fmt.Errorf("workspace coordinator not available")

 }
 func (a *App) openExternalURLInFocusedWindow(ctx context.Context, url string, cfg entity.ExternalLinksConfig) error {
-	target := a.lastFocusedBrowserWindow()
+	target, activeTab, err := a.externalURLTarget()
-	if target == nil || target.id == "" || a.browserWindows[target.id] != target {
+	if err != nil {
-		return fmt.Errorf("last-focused browser window is missing or stale")
+		return err
-	}
-	activeTab := a.activeTabForBrowserWindow(target)
-	if activeTab == nil || activeTab.Workspace == nil || activeTab.Workspace.ActivePane() == nil {
-		return fmt.Errorf("last-focused browser window has no active tab and pane")
 	}
 	a.activateBrowserWindow(target)
 	switch cfg.Behavior {
 	case entity.ExternalLinkBehaviorTabbed:
-		if a.tabCoord == nil {
+		if err := a.createExternalURLTab(ctx, target, url); err != nil {
-			return fmt.Errorf("tab coordinator not available")
-		}
-		created, err := a.tabCoord.Create(ctx, a.tabTargetForBrowserWindow(target), url)
-		if err != nil {
-			a.rollbackExternalTabCreation(ctx, target, created)
 			return err
 		}
-		if created == nil || created.Workspace == nil || created.Workspace.ActivePane() == nil ||
-			created.Workspace.ActivePane().Pane == nil || created.Workspace.ActivePane().Pane.URI != url ||
-			a.activeTabForBrowserWindow(target) != created || a.workspaceViews[created.ID] == nil {
-			a.rollbackExternalTabCreation(ctx, target, created)
-			return fmt.Errorf("created tab did not become an active UI target")
-		}
 	case entity.ExternalLinkBehaviorSplit:
 		if err := a.placeExternalURLInPane(ctx, activeTab.Workspace, url, func(ctx context.Context) error {
 			return a.wsCoord.SplitWithURL(ctx, externalLinkSplitDirection(cfg.Placement), url)
 	return nil
 }
+func (a *App) externalURLTarget() (*browserWindow, *entity.Tab, error) {
+	target := a.lastFocusedBrowserWindow()
+	if target == nil || target.id == "" || a.browserWindows[target.id] != target {
+		return nil, nil, fmt.Errorf("last-focused browser window is missing or stale")
+	}
+	activeTab := a.activeTabForBrowserWindow(target)
+	if activeTab == nil || activeTab.Workspace == nil || activeTab.Workspace.ActivePane() == nil {
+		return nil, nil, fmt.Errorf("last-focused browser window has no active tab and pane")
+	}
+	return target, activeTab, nil
+}
+func (a *App) createExternalURLTab(ctx context.Context, target *browserWindow, url string) error {
+	if a.tabCoord == nil {
+		return fmt.Errorf("tab coordinator not available")
+	}
+	created, err := a.tabCoord.Create(ctx, a.tabTargetForBrowserWindow(target), url)
+	if err != nil {
+		a.rollbackExternalTabCreation(ctx, target, created)
+		return err
+	}
+	if created == nil || created.Workspace == nil || created.Workspace.ActivePane() == nil ||
+		created.Workspace.ActivePane().Pane == nil || created.Workspace.ActivePane().Pane.URI != url ||
+		a.activeTabForBrowserWindow(target) != created || a.workspaceViews[created.ID] == nil {
+		a.rollbackExternalTabCreation(ctx, target, created)
+		return fmt.Errorf("created tab did not become an active UI target")
+	}
+	return nil
+}
 func (a *App) placeExternalURLInPane(ctx context.Context, ws *entity.Workspace, url string, place func(context.Context) error) error {
 	if a.wsCoord == nil {
 		return fmt.Errorf("workspace coordinator not available")

M internal/ui/coordinator/workspace.go +48 -61


@@ -198,7 +198,10 @@ func (c *WorkspaceCoordinator) splitWithInitialURL(ctx context.Context, directio
198	198
199	199		// Update the workspace view
200	200		if splitCtx.wsView != nil {
201		-	if err := c.applySplitToView(ctx, splitCtx.wsView, splitCtx.ws, output, direction, splitCtx.existingWidget, splitCtx.isStackSplit, oldActivePaneID); err != nil {
	201	+	if err := c.applySplitToView(
	202	+	ctx, splitCtx.wsView, splitCtx.ws, output, direction,
	203	+	splitCtx.existingWidget, splitCtx.isStackSplit, oldActivePaneID,
	204	+	); err != nil {
202	205		return err
203	206		}
204	207		splitCtx.wsView.NotifyNewPaneCreated(ctx)

@@ -1472,64 +1475,11 @@ func (c *WorkspaceCoordinator) StackPaneWithURL(ctx context.Context, initialURL
1472	1475		return err
1473	1476		}
1474	1477
1475		-	// Determine if we need to create a new stack or add to existing.
1476		-	var stackNode *entity.PaneNode
1477		-	var newPane *entity.Pane
1478		-	var newPaneID entity.PaneID
1479		-	var needsFirstPaneTitleUpdate bool
1480		-
1481		-	if stackCtx.activeNode.Parent != nil && stackCtx.activeNode.Parent.IsStacked {
1482		-	// Already in a stack - use AddToStack use case
1483		-	stackNode = stackCtx.activeNode.Parent
1484		-	output, err := c.panesUC.AddToStack(ctx, stackCtx.ws, stackNode, nil, initialURL)
1485		-	if err != nil {
1486		-	log.Error().Err(err).Msg("failed to add pane to stack via use case")
1487		-	return err
1488		-	}
1489		-	newPane = output.NewPaneNode.Pane
1490		-	newPaneID = newPane.ID
1491		-	newPane.Title = defaultPaneTitle
1492		-	log.Debug().
1493		-	Int("stack_size", len(stackNode.Children)).
1494		-	Int("insert_index", output.StackIndex).
1495		-	Msg("added to existing stack via use case")
1496		-	} else if stackCtx.activeNode.IsStacked {
1497		-	// Active node is already a stack container - add to it
1498		-	stackNode = stackCtx.activeNode
1499		-	output, err := c.panesUC.AddToStack(ctx, stackCtx.ws, stackNode, nil, initialURL)
1500		-	if err != nil {
1501		-	log.Error().Err(err).Msg("failed to add pane to stack via use case")
1502		-	return err
1503		-	}
1504		-	newPane = output.NewPaneNode.Pane
1505		-	newPaneID = newPane.ID
1506		-	newPane.Title = defaultPaneTitle
1507		-	log.Debug().
1508		-	Int("stack_size", len(stackNode.Children)).
1509		-	Int("insert_index", output.StackIndex).
1510		-	Msg("added to stack container via use case")
1511		-	} else {
1512		-	// Need to create a new stack - use CreateStack use case.
1513		-	output, err := c.panesUC.CreateStack(ctx, stackCtx.ws, stackCtx.activeNode, initialURL)
1514		-	if err != nil {
1515		-	log.Error().Err(err).Msg("failed to create stack via use case")
1516		-	return err
1517		-	}
1518		-	stackNode = output.StackNode
1519		-	newPane = output.NewPane
1520		-	newPaneID = newPane.ID
1521		-	newPane.Title = defaultPaneTitle
1522		-	needsFirstPaneTitleUpdate = true
1523		-
1524		-	// Update the original pane's title in the domain
1525		-	if output.OriginalNode != nil && output.OriginalNode.Pane != nil {
1526		-	output.OriginalNode.Pane.Title = stackCtx.originalTitle
1527		-	}
1528		-
1529		-	log.Debug().
1530		-	Int("stack_size", len(stackNode.Children)).
1531		-	Msg("created new stack via use case")
	1478	+	stackNode, newPane, needsFirstPaneTitleUpdate, err := c.createOrAddStackPane(ctx, stackCtx, initialURL)
	1479	+	if err != nil {
	1480	+	return err
1532	1481		}
	1482	+	newPaneID := newPane.ID
1533	1483
1534	1484		// Create PaneView for the new pane
1535	1485		newPaneView := component.NewPaneView(ctx, c.widgetFactory, newPaneID, nil)

@@ -1537,9 +1487,10 @@ func (c *WorkspaceCoordinator) StackPaneWithURL(ctx context.Context, initialURL
1537	1487		stackCtx.wsView.RegisterPaneView(newPaneID, newPaneView)
1538	1488
1539	1489		// Add to the UI StackedView
1540		-	if err := c.stackedPaneMgr.AddPaneToStack(ctx, stackCtx.wsView, stackCtx.activePaneID, newPaneView, defaultPaneTitle); err != nil {
1541		-	log.Error().Err(err).Msg("failed to add pane to stack")
1542		-	return err
	1490	+	addErr := c.stackedPaneMgr.AddPaneToStack(ctx, stackCtx.wsView, stackCtx.activePaneID, newPaneView, defaultPaneTitle)
	1491	+	if addErr != nil {
	1492	+	log.Error().Err(addErr).Msg("failed to add pane to stack")
	1493	+	return addErr
1543	1494		}
1544	1495
1545	1496		// Update the first pane's title if we just converted from leaf to stacked

@@ -1603,6 +1554,42 @@ func (c *WorkspaceCoordinator) StackPaneWithURL(ctx context.Context, initialURL
1603	1554		return nil
1604	1555		}
1605	1556
	1557	+	func (c *WorkspaceCoordinator) createOrAddStackPane(
	1558	+	ctx context.Context,
	1559	+	stackCtx *stackPaneContext,
	1560	+	initialURL string,
	1561	+	) (entity.PaneNode, entity.Pane, bool, error) {
	1562	+	log := logging.FromContext(ctx)
	1563	+	stackNode := stackCtx.activeNode
	1564	+	if stackNode.Parent != nil && stackNode.Parent.IsStacked {
	1565	+	stackNode = stackNode.Parent
	1566	+	}
	1567	+	if stackNode.IsStacked {
	1568	+	output, err := c.panesUC.AddToStack(ctx, stackCtx.ws, stackNode, nil, initialURL)
	1569	+	if err != nil {
	1570	+	log.Error().Err(err).Msg("failed to add pane to stack via use case")
	1571	+	return nil, nil, false, err
	1572	+	}
	1573	+	newPane := output.NewPaneNode.Pane
	1574	+	newPane.Title = defaultPaneTitle
	1575	+	log.Debug().Int("stack_size", len(stackNode.Children)).Int("insert_index", output.StackIndex).
	1576	+	Msg("added to stack via use case")
	1577	+	return stackNode, newPane, false, nil
	1578	+	}
	1579	+
	1580	+	output, err := c.panesUC.CreateStack(ctx, stackCtx.ws, stackCtx.activeNode, initialURL)
	1581	+	if err != nil {
	1582	+	log.Error().Err(err).Msg("failed to create stack via use case")
	1583	+	return nil, nil, false, err
	1584	+	}
	1585	+	output.NewPane.Title = defaultPaneTitle
	1586	+	if output.OriginalNode != nil && output.OriginalNode.Pane != nil {
	1587	+	output.OriginalNode.Pane.Title = stackCtx.originalTitle
	1588	+	}
	1589	+	log.Debug().Int("stack_size", len(output.StackNode.Children)).Msg("created new stack via use case")
	1590	+	return output.StackNode, output.NewPane, true, nil
	1591	+	}
	1592	+
1606	1593		func (c *WorkspaceCoordinator) prepareStackPane(
1607	1594		ctx context.Context,
1608	1595		) (*stackPaneContext, error) {

 	// Update the workspace view
 	if splitCtx.wsView != nil {
-		if err := c.applySplitToView(ctx, splitCtx.wsView, splitCtx.ws, output, direction, splitCtx.existingWidget, splitCtx.isStackSplit, oldActivePaneID); err != nil {
+		if err := c.applySplitToView(
+			ctx, splitCtx.wsView, splitCtx.ws, output, direction,
+			splitCtx.existingWidget, splitCtx.isStackSplit, oldActivePaneID,
+		); err != nil {
 			return err
 		}
 		splitCtx.wsView.NotifyNewPaneCreated(ctx)
 		return err
 	}
-	// Determine if we need to create a new stack or add to existing.
+	stackNode, newPane, needsFirstPaneTitleUpdate, err := c.createOrAddStackPane(ctx, stackCtx, initialURL)
-	var stackNode *entity.PaneNode
+	if err != nil {
-	var newPane *entity.Pane
+		return err
-	var newPaneID entity.PaneID
-	var needsFirstPaneTitleUpdate bool
-	if stackCtx.activeNode.Parent != nil && stackCtx.activeNode.Parent.IsStacked {
-		// Already in a stack - use AddToStack use case
-		stackNode = stackCtx.activeNode.Parent
-		output, err := c.panesUC.AddToStack(ctx, stackCtx.ws, stackNode, nil, initialURL)
-		if err != nil {
-			log.Error().Err(err).Msg("failed to add pane to stack via use case")
-			return err
-		}
-		newPane = output.NewPaneNode.Pane
-		newPaneID = newPane.ID
-		newPane.Title = defaultPaneTitle
-		log.Debug().
-			Int("stack_size", len(stackNode.Children)).
-			Int("insert_index", output.StackIndex).
-			Msg("added to existing stack via use case")
-	} else if stackCtx.activeNode.IsStacked {
-		// Active node is already a stack container - add to it
-		stackNode = stackCtx.activeNode
-		output, err := c.panesUC.AddToStack(ctx, stackCtx.ws, stackNode, nil, initialURL)
-		if err != nil {
-			log.Error().Err(err).Msg("failed to add pane to stack via use case")
-			return err
-		}
-		newPane = output.NewPaneNode.Pane
-		newPaneID = newPane.ID
-		newPane.Title = defaultPaneTitle
-		log.Debug().
-			Int("stack_size", len(stackNode.Children)).
-			Int("insert_index", output.StackIndex).
-			Msg("added to stack container via use case")
-	} else {
-		// Need to create a new stack - use CreateStack use case.
-		output, err := c.panesUC.CreateStack(ctx, stackCtx.ws, stackCtx.activeNode, initialURL)
-		if err != nil {
-			log.Error().Err(err).Msg("failed to create stack via use case")
-			return err
-		}
-		stackNode = output.StackNode
-		newPane = output.NewPane
-		newPaneID = newPane.ID
-		newPane.Title = defaultPaneTitle
-		needsFirstPaneTitleUpdate = true
-		// Update the original pane's title in the domain
-		if output.OriginalNode != nil && output.OriginalNode.Pane != nil {
-			output.OriginalNode.Pane.Title = stackCtx.originalTitle
-		}
-		log.Debug().
-			Int("stack_size", len(stackNode.Children)).
-			Msg("created new stack via use case")
 	}
+	newPaneID := newPane.ID
 	// Create PaneView for the new pane
 	newPaneView := component.NewPaneView(ctx, c.widgetFactory, newPaneID, nil)
 	stackCtx.wsView.RegisterPaneView(newPaneID, newPaneView)
 	// Add to the UI StackedView
-	if err := c.stackedPaneMgr.AddPaneToStack(ctx, stackCtx.wsView, stackCtx.activePaneID, newPaneView, defaultPaneTitle); err != nil {
+	addErr := c.stackedPaneMgr.AddPaneToStack(ctx, stackCtx.wsView, stackCtx.activePaneID, newPaneView, defaultPaneTitle)
-		log.Error().Err(err).Msg("failed to add pane to stack")
+	if addErr != nil {
-		return err
+		log.Error().Err(addErr).Msg("failed to add pane to stack")
+		return addErr
 	}
 	// Update the first pane's title if we just converted from leaf to stacked
 	return nil
 }
+func (c *WorkspaceCoordinator) createOrAddStackPane(
+	ctx context.Context,
+	stackCtx *stackPaneContext,
+	initialURL string,
+) (*entity.PaneNode, *entity.Pane, bool, error) {
+	log := logging.FromContext(ctx)
+	stackNode := stackCtx.activeNode
+	if stackNode.Parent != nil && stackNode.Parent.IsStacked {
+		stackNode = stackNode.Parent
+	}
+	if stackNode.IsStacked {
+		output, err := c.panesUC.AddToStack(ctx, stackCtx.ws, stackNode, nil, initialURL)
+		if err != nil {
+			log.Error().Err(err).Msg("failed to add pane to stack via use case")
+			return nil, nil, false, err
+		}
+		newPane := output.NewPaneNode.Pane
+		newPane.Title = defaultPaneTitle
+		log.Debug().Int("stack_size", len(stackNode.Children)).Int("insert_index", output.StackIndex).
+			Msg("added to stack via use case")
+		return stackNode, newPane, false, nil
+	}
+	output, err := c.panesUC.CreateStack(ctx, stackCtx.ws, stackCtx.activeNode, initialURL)
+	if err != nil {
+		log.Error().Err(err).Msg("failed to create stack via use case")
+		return nil, nil, false, err
+	}
+	output.NewPane.Title = defaultPaneTitle
+	if output.OriginalNode != nil && output.OriginalNode.Pane != nil {
+		output.OriginalNode.Pane.Title = stackCtx.originalTitle
+	}
+	log.Debug().Int("stack_size", len(output.StackNode.Children)).Msg("created new stack via use case")
+	return output.StackNode, output.NewPane, true, nil
+}
 func (c *WorkspaceCoordinator) prepareStackPane(
 	ctx context.Context,
 ) (*stackPaneContext, error) {

M internal/ui/coordinator/workspace_external_url_test.go +2 -1


@@ -9,6 +9,7 @@ import (
9	9		"github.com/bnema/dumber/internal/ui/component"
10	10		contentcoord "github.com/bnema/dumber/internal/ui/coordinator/content"
11	11		"github.com/bnema/dumber/internal/ui/layout"
	12	+	"github.com/bnema/puregotk/v4/gdk"
12	13		"github.com/bnema/puregotk/v4/gtk"
13	14		"github.com/stretchr/testify/assert"
14	15		"github.com/stretchr/testify/require"

@@ -68,7 +69,7 @@ func TestWorkspaceCoordinator_StackPaneWithURLUsesExplicitURL(t *testing.T) {
68	69
69	70		func stackURLTestCoordinator(t testing.T, legacyURL string) (WorkspaceCoordinator, *entity.Workspace) {
70	71		t.Helper()
71		-	if !gtk.InitCheck() {
	72	+	if !gtk.InitCheck() \|\| gdk.DisplayGetDefault() == nil {
72	73		t.Skip("GTK native display prerequisite unavailable")
73	74		}
74	75		ctx := context.Background()

 	"github.com/bnema/dumber/internal/ui/component"
 	contentcoord "github.com/bnema/dumber/internal/ui/coordinator/content"
 	"github.com/bnema/dumber/internal/ui/layout"
+	"github.com/bnema/puregotk/v4/gdk"
 	"github.com/bnema/puregotk/v4/gtk"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 func stackURLTestCoordinator(t *testing.T, legacyURL string) (*WorkspaceCoordinator, *entity.Workspace) {
 	t.Helper()
-	if !gtk.InitCheck() {
+	if !gtk.InitCheck() || gdk.DisplayGetDefault() == nil {
 		t.Skip("GTK native display prerequisite unavailable")
 	}
 	ctx := context.Background()