fix: split startup shell from fresh-window command
- commit
- 10629bd96b08672f47aadaacb7496881c493a427
- author
- brice · · 2026-06-26 05:06 UTC
- parent
- ded7322
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
3 files · +31 -53
M internal/ui/app.go +1 -8
| @@ -343,7 +343,7 @@ func (a *App) onActivate(ctx context.Context) { | |||
| 343 | 343 | a.setupPoolBackgroundColor(ctx) | |
| 344 | 344 | a.initLayoutInfrastructure() | |
| 345 | 345 | ||
| 346 | - | if err := a.OpenFreshWindow(ctx, a.initialWindowURL()); err != nil { | |
| 346 | + | if err := a.openInitialBrowserWindowShell(ctx, a.initialWindowURL()); err != nil { | |
| 347 | 347 | log.Error().Err(err).Msg("failed to create main window") | |
| 348 | 348 | return | |
| 349 | 349 | } | |
| @@ -2456,13 +2456,6 @@ func (a *App) createInitialTab(ctx context.Context) { | |||
| 2456 | 2456 | } | |
| 2457 | 2457 | ||
| 2458 | 2458 | focusedWindow := a.lastFocusedBrowserWindow() | |
| 2459 | - | if tabs := a.tabListForBrowserWindow(focusedWindow); tabs != nil && tabs.Count() > 0 { | |
| 2460 | - | log.Debug(). | |
| 2461 | - | Str("window_id", focusedWindow.id). | |
| 2462 | - | Int("tab_count", tabs.Count()). | |
| 2463 | - | Msg("initial tab creation skipped; focused window already has tabs") | |
| 2464 | - | return | |
| 2465 | - | } | |
| 2466 | 2459 | ||
| 2467 | 2460 | // Create an initial tab using coordinator. | |
| 2468 | 2461 | target := a.ensureTabTargetForBrowserWindow(focusedWindow) | |
| @@ -343,7 +343,7 @@ func (a *App) onActivate(ctx context.Context) { | |||||
| 343 | a.setupPoolBackgroundColor(ctx) | 343 | a.setupPoolBackgroundColor(ctx) | ||
| 344 | a.initLayoutInfrastructure() | 344 | a.initLayoutInfrastructure() | ||
| 345 | 345 | ||||
| 346 | - | if err := a.OpenFreshWindow(ctx, a.initialWindowURL()); err != nil { | 346 | + | if err := a.openInitialBrowserWindowShell(ctx, a.initialWindowURL()); err != nil { |
| 347 | log.Error().Err(err).Msg("failed to create main window") | 347 | log.Error().Err(err).Msg("failed to create main window") | ||
| 348 | return | 348 | return | ||
| 349 | } | 349 | } | ||
| @@ -2456,13 +2456,6 @@ func (a *App) createInitialTab(ctx context.Context) { | |||||
| 2456 | } | 2456 | } | ||
| 2457 | 2457 | ||||
| 2458 | focusedWindow := a.lastFocusedBrowserWindow() | 2458 | focusedWindow := a.lastFocusedBrowserWindow() | ||
| 2459 | - | if tabs := a.tabListForBrowserWindow(focusedWindow); tabs != nil && tabs.Count() > 0 { | |||
| 2460 | - | log.Debug(). | |||
| 2461 | - | Str("window_id", focusedWindow.id). | |||
| 2462 | - | Int("tab_count", tabs.Count()). | |||
| 2463 | - | Msg("initial tab creation skipped; focused window already has tabs") | |||
| 2464 | - | return | |||
| 2465 | - | } | |||
| 2466 | 2459 | ||||
| 2467 | // Create an initial tab using coordinator. | 2460 | // Create an initial tab using coordinator. | ||
| 2468 | target := a.ensureTabTargetForBrowserWindow(focusedWindow) | 2461 | target := a.ensureTabTargetForBrowserWindow(focusedWindow) | ||
M internal/ui/app_window_lifecycle.go +19 -6
| @@ -83,6 +83,25 @@ func (a *App) createBrowserWindow(ctx context.Context, initialURL string) (*brow | |||
| 83 | 83 | return browserWindow, nil | |
| 84 | 84 | } | |
| 85 | 85 | ||
| 86 | + | func (a *App) openInitialBrowserWindowShell(ctx context.Context, initialURL string) error { | |
| 87 | + | log := logging.FromContext(ctx) | |
| 88 | + | created, err := a.createBrowserWindow(ctx, initialURL) | |
| 89 | + | if err != nil { | |
| 90 | + | log.Warn().Err(err). | |
| 91 | + | Str("url_host", logging.SafeURLHost(initialURL)). | |
| 92 | + | Int("window_count_after", len(a.browserWindows)). | |
| 93 | + | Msg("ui: initial browser window shell creation failed") | |
| 94 | + | return err | |
| 95 | + | } | |
| 96 | + | a.registerBrowserWindow(created) | |
| 97 | + | log.Debug(). | |
| 98 | + | Str("window_id", created.id). | |
| 99 | + | Str("url_host", logging.SafeURLHost(initialURL)). | |
| 100 | + | Msg("ui: initial browser window shell registered") | |
| 101 | + | a.activateBrowserWindow(created) | |
| 102 | + | return nil | |
| 103 | + | } | |
| 104 | + | ||
| 86 | 105 | func (a *App) openFreshWindow(ctx context.Context, url string) error { | |
| 87 | 106 | log := logging.FromContext(ctx) | |
| 88 | 107 | log.Debug(). | |
| @@ -110,12 +129,6 @@ func (a *App) openFreshWindow(ctx context.Context, url string) error { | |||
| 110 | 129 | Int("window_count_after_register", len(a.browserWindows)). | |
| 111 | 130 | Msg("ui: open fresh window registered browser window") | |
| 112 | 131 | ||
| 113 | - | if len(a.browserWindows) == 1 && (url == "" || (a.deps != nil && a.deps.RestoreSessionID != "")) { | |
| 114 | - | log.Debug().Str("window_id", created.id).Msg("ui: activating first browser window without initial tab") | |
| 115 | - | a.activateBrowserWindow(created) | |
| 116 | - | return nil | |
| 117 | - | } | |
| 118 | - | ||
| 119 | 132 | var openErr error | |
| 120 | 133 | path := "tabs_uc" | |
| 121 | 134 | if a.tabCoord != nil { | |
| @@ -83,6 +83,25 @@ func (a *App) createBrowserWindow(ctx context.Context, initialURL string) (*brow | |||||
| 83 | return browserWindow, nil | 83 | return browserWindow, nil | ||
| 84 | } | 84 | } | ||
| 85 | 85 | ||||
| 86 | + | func (a *App) openInitialBrowserWindowShell(ctx context.Context, initialURL string) error { | |||
| 87 | + | log := logging.FromContext(ctx) | |||
| 88 | + | created, err := a.createBrowserWindow(ctx, initialURL) | |||
| 89 | + | if err != nil { | |||
| 90 | + | log.Warn().Err(err). | |||
| 91 | + | Str("url_host", logging.SafeURLHost(initialURL)). | |||
| 92 | + | Int("window_count_after", len(a.browserWindows)). | |||
| 93 | + | Msg("ui: initial browser window shell creation failed") | |||
| 94 | + | return err | |||
| 95 | + | } | |||
| 96 | + | a.registerBrowserWindow(created) | |||
| 97 | + | log.Debug(). | |||
| 98 | + | Str("window_id", created.id). | |||
| 99 | + | Str("url_host", logging.SafeURLHost(initialURL)). | |||
| 100 | + | Msg("ui: initial browser window shell registered") | |||
| 101 | + | a.activateBrowserWindow(created) | |||
| 102 | + | return nil | |||
| 103 | + | } | |||
| 104 | + | ||||
| 86 | func (a *App) openFreshWindow(ctx context.Context, url string) error { | 105 | func (a *App) openFreshWindow(ctx context.Context, url string) error { | ||
| 87 | log := logging.FromContext(ctx) | 106 | log := logging.FromContext(ctx) | ||
| 88 | log.Debug(). | 107 | log.Debug(). | ||
| @@ -110,12 +129,6 @@ func (a *App) openFreshWindow(ctx context.Context, url string) error { | |||||
| 110 | Int("window_count_after_register", len(a.browserWindows)). | 129 | Int("window_count_after_register", len(a.browserWindows)). | ||
| 111 | Msg("ui: open fresh window registered browser window") | 130 | Msg("ui: open fresh window registered browser window") | ||
| 112 | 131 | ||||
| 113 | - | if len(a.browserWindows) == 1 && (url == "" || (a.deps != nil && a.deps.RestoreSessionID != "")) { | |||
| 114 | - | log.Debug().Str("window_id", created.id).Msg("ui: activating first browser window without initial tab") | |||
| 115 | - | a.activateBrowserWindow(created) | |||
| 116 | - | return nil | |||
| 117 | - | } | |||
| 118 | - | ||||
| 119 | var openErr error | 132 | var openErr error | ||
| 120 | path := "tabs_uc" | 133 | path := "tabs_uc" | ||
| 121 | if a.tabCoord != nil { | 134 | if a.tabCoord != nil { | ||
M internal/ui/browser_window_test.go +11 -39
| @@ -459,22 +459,18 @@ func TestOpenFreshWindow_DispatchesAndTracksBrowserWindow(t *testing.T) { | |||
| 459 | 459 | } | |
| 460 | 460 | } | |
| 461 | 461 | ||
| 462 | - | func TestOpenFreshWindow_FirstWindowWithEmptyURLRegistersShellWithoutCreatingTab(t *testing.T) { | |
| 462 | + | func TestOpenInitialBrowserWindowShellRegistersWithoutCreatingTab(t *testing.T) { | |
| 463 | 463 | app := &App{ | |
| 464 | 464 | tabs: entity.NewTabList(), | |
| 465 | 465 | tabsUC: usecase.NewManageTabsUseCase(func() string { return "unexpected-tab" }, nil), | |
| 466 | 466 | browserWindows: make(map[string]*browserWindow), | |
| 467 | 467 | } | |
| 468 | - | app.dispatchOnMainThread = func(label string, fn func()) syncdispatch.SyncDispatchResult { | |
| 469 | - | fn() | |
| 470 | - | return syncdispatch.SyncDispatchResult{Label: label, Status: syncdispatch.SyncDispatchCompleted} | |
| 471 | - | } | |
| 472 | - | app.browserWindowFactory = func(context.Context, string) (*browserWindow, error) { | |
| 473 | - | return &browserWindow{id: "window-1", tabs: entity.NewTabList()}, nil | |
| 468 | + | app.browserWindowFactory = func(_ context.Context, initialURL string) (*browserWindow, error) { | |
| 469 | + | return &browserWindow{id: "window-1", initialURL: initialURL, tabs: entity.NewTabList()}, nil | |
| 474 | 470 | } | |
| 475 | 471 | ||
| 476 | - | if err := app.OpenFreshWindow(context.Background(), ""); err != nil { | |
| 477 | - | t.Fatalf("OpenFreshWindow returned error: %v", err) | |
| 472 | + | if err := app.openInitialBrowserWindowShell(context.Background(), "https://example.com"); err != nil { | |
| 473 | + | t.Fatalf("openInitialBrowserWindowShell returned error: %v", err) | |
| 478 | 474 | } | |
| 479 | 475 | if got := len(app.browserWindows); got != 1 { | |
| 480 | 476 | t.Fatalf("browserWindows length = %d, want 1", got) | |
| @@ -483,11 +479,14 @@ func TestOpenFreshWindow_FirstWindowWithEmptyURLRegistersShellWithoutCreatingTab | |||
| 483 | 479 | t.Fatalf("lastFocusedWindowID = %q, want window-1", app.lastFocusedWindowID) | |
| 484 | 480 | } | |
| 485 | 481 | if got := app.tabs.Count(); got != 0 { | |
| 486 | - | t.Fatalf("tabs count = %d, want 0; first empty shell should wait for createInitialTab", got) | |
| 482 | + | t.Fatalf("tabs count = %d, want 0; initial shell should wait for createInitialTab", got) | |
| 487 | 483 | } | |
| 488 | - | if got := app.browserWindows["window-1"].tabs.Count(); got != 0 { | |
| 489 | - | t.Fatalf("window tabs count = %d, want 0; first empty shell should wait for createInitialTab", got) | |
| 484 | + | windowTabs := app.browserWindows["window-1"].tabs | |
| 485 | + | require.NotNil(t, windowTabs) | |
| 486 | + | if got := windowTabs.Count(); got != 0 { | |
| 487 | + | t.Fatalf("window tabs count = %d, want 0; initial shell should wait for createInitialTab", got) | |
| 490 | 488 | } | |
| 489 | + | require.Equal(t, "https://example.com", app.browserWindows["window-1"].initialURL) | |
| 491 | 490 | } | |
| 492 | 491 | ||
| 493 | 492 | func TestOpenFreshWindow_FirstWindowWithRequestedURLCreatesTabInAppAndWindow(t *testing.T) { | |
| @@ -528,33 +527,6 @@ func TestOpenFreshWindow_FirstWindowWithRequestedURLCreatesTabInAppAndWindow(t * | |||
| 528 | 527 | require.Same(t, app.browserWindows["window-1"], app.browserWindowForTab(tab.ID)) | |
| 529 | 528 | } | |
| 530 | 529 | ||
| 531 | - | func TestCreateInitialTabSkipsFocusedWindowThatAlreadyHasTabs(t *testing.T) { | |
| 532 | - | ctx := context.Background() | |
| 533 | - | existing := entity.NewTab(entity.TabID("existing-tab"), entity.WorkspaceID("existing-workspace"), entity.NewPane(entity.PaneID("existing-pane"))) | |
| 534 | - | windowTabs := entity.NewTabList() | |
| 535 | - | windowTabs.Add(existing) | |
| 536 | - | windowTabs.SetActive(existing.ID) | |
| 537 | - | bw := &browserWindow{id: "window-1", tabs: windowTabs} | |
| 538 | - | app := &App{ | |
| 539 | - | runtimeConfig: runtimeConfigStateForTest(config.DefaultConfig()), | |
| 540 | - | tabs: entity.NewTabList(), | |
| 541 | - | tabsUC: usecase.NewManageTabsUseCase(counterIDGen(), nil), | |
| 542 | - | browserWindows: map[string]*browserWindow{bw.id: bw}, | |
| 543 | - | lastFocusedWindowID: bw.id, | |
| 544 | - | workspaceViews: map[entity.TabID]*component.WorkspaceView{}, | |
| 545 | - | windowForTab: map[entity.TabID]*browserWindow{existing.ID: bw}, | |
| 546 | - | } | |
| 547 | - | app.tabs.Add(existing) | |
| 548 | - | app.initTabCoordinator(ctx) | |
| 549 | - | ||
| 550 | - | app.createInitialTab(ctx) | |
| 551 | - | ||
| 552 | - | require.Equal(t, 1, windowTabs.Count()) | |
| 553 | - | require.NotNil(t, windowTabs.Find(existing.ID)) | |
| 554 | - | require.Equal(t, 1, app.tabs.Count()) | |
| 555 | - | require.NotNil(t, app.tabs.Find(existing.ID)) | |
| 556 | - | } | |
| 557 | - | ||
| 558 | 530 | func TestOpenFreshWindow_PropagatesFactoryError(t *testing.T) { | |
| 559 | 531 | app := &App{browserWindows: make(map[string]*browserWindow)} | |
| 560 | 532 | app.dispatchOnMainThread = func(label string, fn func()) syncdispatch.SyncDispatchResult { | |
| @@ -459,22 +459,18 @@ func TestOpenFreshWindow_DispatchesAndTracksBrowserWindow(t *testing.T) { | |||||
| 459 | } | 459 | } | ||
| 460 | } | 460 | } | ||
| 461 | 461 | ||||
| 462 | - | func TestOpenFreshWindow_FirstWindowWithEmptyURLRegistersShellWithoutCreatingTab(t *testing.T) { | 462 | + | func TestOpenInitialBrowserWindowShellRegistersWithoutCreatingTab(t *testing.T) { |
| 463 | app := &App{ | 463 | app := &App{ | ||
| 464 | tabs: entity.NewTabList(), | 464 | tabs: entity.NewTabList(), | ||
| 465 | tabsUC: usecase.NewManageTabsUseCase(func() string { return "unexpected-tab" }, nil), | 465 | tabsUC: usecase.NewManageTabsUseCase(func() string { return "unexpected-tab" }, nil), | ||
| 466 | browserWindows: make(map[string]*browserWindow), | 466 | browserWindows: make(map[string]*browserWindow), | ||
| 467 | } | 467 | } | ||
| 468 | - | app.dispatchOnMainThread = func(label string, fn func()) syncdispatch.SyncDispatchResult { | 468 | + | app.browserWindowFactory = func(_ context.Context, initialURL string) (*browserWindow, error) { |
| 469 | - | fn() | 469 | + | return &browserWindow{id: "window-1", initialURL: initialURL, tabs: entity.NewTabList()}, nil |
| 470 | - | return syncdispatch.SyncDispatchResult{Label: label, Status: syncdispatch.SyncDispatchCompleted} | |||
| 471 | - | } | |||
| 472 | - | app.browserWindowFactory = func(context.Context, string) (*browserWindow, error) { | |||
| 473 | - | return &browserWindow{id: "window-1", tabs: entity.NewTabList()}, nil | |||
| 474 | } | 470 | } | ||
| 475 | 471 | ||||
| 476 | - | if err := app.OpenFreshWindow(context.Background(), ""); err != nil { | 472 | + | if err := app.openInitialBrowserWindowShell(context.Background(), "https://example.com"); err != nil { |
| 477 | - | t.Fatalf("OpenFreshWindow returned error: %v", err) | 473 | + | t.Fatalf("openInitialBrowserWindowShell returned error: %v", err) |
| 478 | } | 474 | } | ||
| 479 | if got := len(app.browserWindows); got != 1 { | 475 | if got := len(app.browserWindows); got != 1 { | ||
| 480 | t.Fatalf("browserWindows length = %d, want 1", got) | 476 | t.Fatalf("browserWindows length = %d, want 1", got) | ||
| @@ -483,11 +479,14 @@ func TestOpenFreshWindow_FirstWindowWithEmptyURLRegistersShellWithoutCreatingTab | |||||
| 483 | t.Fatalf("lastFocusedWindowID = %q, want window-1", app.lastFocusedWindowID) | 479 | t.Fatalf("lastFocusedWindowID = %q, want window-1", app.lastFocusedWindowID) | ||
| 484 | } | 480 | } | ||
| 485 | if got := app.tabs.Count(); got != 0 { | 481 | if got := app.tabs.Count(); got != 0 { | ||
| 486 | - | t.Fatalf("tabs count = %d, want 0; first empty shell should wait for createInitialTab", got) | 482 | + | t.Fatalf("tabs count = %d, want 0; initial shell should wait for createInitialTab", got) |
| 487 | } | 483 | } | ||
| 488 | - | if got := app.browserWindows["window-1"].tabs.Count(); got != 0 { | 484 | + | windowTabs := app.browserWindows["window-1"].tabs |
| 489 | - | t.Fatalf("window tabs count = %d, want 0; first empty shell should wait for createInitialTab", got) | 485 | + | require.NotNil(t, windowTabs) |
| 486 | + | if got := windowTabs.Count(); got != 0 { | |||
| 487 | + | t.Fatalf("window tabs count = %d, want 0; initial shell should wait for createInitialTab", got) | |||
| 490 | } | 488 | } | ||
| 489 | + | require.Equal(t, "https://example.com", app.browserWindows["window-1"].initialURL) | |||
| 491 | } | 490 | } | ||
| 492 | 491 | ||||
| 493 | func TestOpenFreshWindow_FirstWindowWithRequestedURLCreatesTabInAppAndWindow(t *testing.T) { | 492 | func TestOpenFreshWindow_FirstWindowWithRequestedURLCreatesTabInAppAndWindow(t *testing.T) { | ||
| @@ -528,33 +527,6 @@ func TestOpenFreshWindow_FirstWindowWithRequestedURLCreatesTabInAppAndWindow(t * | |||||
| 528 | require.Same(t, app.browserWindows["window-1"], app.browserWindowForTab(tab.ID)) | 527 | require.Same(t, app.browserWindows["window-1"], app.browserWindowForTab(tab.ID)) | ||
| 529 | } | 528 | } | ||
| 530 | 529 | ||||
| 531 | - | func TestCreateInitialTabSkipsFocusedWindowThatAlreadyHasTabs(t *testing.T) { | |||
| 532 | - | ctx := context.Background() | |||
| 533 | - | existing := entity.NewTab(entity.TabID("existing-tab"), entity.WorkspaceID("existing-workspace"), entity.NewPane(entity.PaneID("existing-pane"))) | |||
| 534 | - | windowTabs := entity.NewTabList() | |||
| 535 | - | windowTabs.Add(existing) | |||
| 536 | - | windowTabs.SetActive(existing.ID) | |||
| 537 | - | bw := &browserWindow{id: "window-1", tabs: windowTabs} | |||
| 538 | - | app := &App{ | |||
| 539 | - | runtimeConfig: runtimeConfigStateForTest(config.DefaultConfig()), | |||
| 540 | - | tabs: entity.NewTabList(), | |||
| 541 | - | tabsUC: usecase.NewManageTabsUseCase(counterIDGen(), nil), | |||
| 542 | - | browserWindows: map[string]*browserWindow{bw.id: bw}, | |||
| 543 | - | lastFocusedWindowID: bw.id, | |||
| 544 | - | workspaceViews: map[entity.TabID]*component.WorkspaceView{}, | |||
| 545 | - | windowForTab: map[entity.TabID]*browserWindow{existing.ID: bw}, | |||
| 546 | - | } | |||
| 547 | - | app.tabs.Add(existing) | |||
| 548 | - | app.initTabCoordinator(ctx) | |||
| 549 | - | ||||
| 550 | - | app.createInitialTab(ctx) | |||
| 551 | - | ||||
| 552 | - | require.Equal(t, 1, windowTabs.Count()) | |||
| 553 | - | require.NotNil(t, windowTabs.Find(existing.ID)) | |||
| 554 | - | require.Equal(t, 1, app.tabs.Count()) | |||
| 555 | - | require.NotNil(t, app.tabs.Find(existing.ID)) | |||
| 556 | - | } | |||
| 557 | - | ||||
| 558 | func TestOpenFreshWindow_PropagatesFactoryError(t *testing.T) { | 530 | func TestOpenFreshWindow_PropagatesFactoryError(t *testing.T) { | ||
| 559 | app := &App{browserWindows: make(map[string]*browserWindow)} | 531 | app := &App{browserWindows: make(map[string]*browserWindow)} | ||
| 560 | app.dispatchOnMainThread = func(label string, fn func()) syncdispatch.SyncDispatchResult { | 532 | app.dispatchOnMainThread = func(label string, fn func()) syncdispatch.SyncDispatchResult { | ||