detached a5ef447

Harden stale pane restore paths

commit
a5ef447bb6498fd5e7f33343773fffd6aaaa9630
author
brice · · 2026-05-11 17:42 UTC
parent
7bf171b
signature
GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email
4 files · +87 -9
Unified
UnifiedSplit
M internal/ui/app.go +9 -5
@@ -3826,11 +3826,15 @@ func (a *App) attachPopupToTab(ctx context.Context, tabID entity.TabID, pane *en
38263826
38273827 // Wrap and attach widget
38283828 widget := a.contentCoord.WrapWidget(ctx, wv)
3829- if widget != nil {
3830- if err := wsView.SetWebViewWidget(pane.ID, widget); err != nil {
3831- log.Warn().Err(err).Str("pane_id", string(pane.ID)).Msg("pane view not found for popup")
3832- return
3833- }
3829+ if widget == nil {
3830+ a.contentCoord.ReleaseWebView(ctx, pane.ID)
3831+ log.Warn().Str("pane_id", string(pane.ID)).Msg("failed to wrap popup webview")
3832+ return
3833+ }
3834+ if err := wsView.SetWebViewWidget(pane.ID, widget); err != nil {
3835+ a.contentCoord.ReleaseWebView(ctx, pane.ID)
3836+ log.Warn().Err(err).Str("pane_id", string(pane.ID)).Msg("pane view not found for popup")
3837+ return
38343838 }
38353839 }
38363840 log.Debug().
@@ -3826,11 +3826,15 @@ func (a *App) attachPopupToTab(ctx context.Context, tabID entity.TabID, pane *en
3826 3826
3827 // Wrap and attach widget3827 // Wrap and attach widget
3828 widget := a.contentCoord.WrapWidget(ctx, wv)3828 widget := a.contentCoord.WrapWidget(ctx, wv)
3829- if widget != nil {3829+ if widget == nil {
3830- if err := wsView.SetWebViewWidget(pane.ID, widget); err != nil {3830+ a.contentCoord.ReleaseWebView(ctx, pane.ID)
3831- log.Warn().Err(err).Str("pane_id", string(pane.ID)).Msg("pane view not found for popup")3831+ log.Warn().Str("pane_id", string(pane.ID)).Msg("failed to wrap popup webview")
3832- return3832+ return
3833- }3833+ }
3834+ if err := wsView.SetWebViewWidget(pane.ID, widget); err != nil {
3835+ a.contentCoord.ReleaseWebView(ctx, pane.ID)
3836+ log.Warn().Err(err).Str("pane_id", string(pane.ID)).Msg("pane view not found for popup")
3837+ return
3834 }3838 }
3835 }3839 }
3836 log.Debug().3840 log.Debug().
M internal/ui/app_browser_launch_test.go +37 -0
@@ -1854,6 +1854,43 @@ func TestApp_AttachPopupToTabSkipsRegistrationWhenPaneViewMissing(t *testing.T)
18541854 }
18551855 }
18561856
1857+func TestApp_AttachPopupToTabReleasesRegistrationWhenWrapFails(t *testing.T) {
1858+ ctx := context.Background()
1859+ contentCoord := contentcoord.NewCoordinator(ctx, nil, nil, nil, nil, nil, nil, nil)
1860+ tabID := entity.TabID("tab-1")
1861+ pane := entity.NewPane(entity.PaneID("popup-pane"))
1862+
1863+ factory := layoutmocks.NewMockWidgetFactory(t)
1864+ container := layoutmocks.NewMockBoxWidget(t)
1865+ overlay := layoutmocks.NewMockOverlayWidget(t)
1866+ factory.EXPECT().NewBox(layout.OrientationVertical, 0).Return(container).Once()
1867+ container.EXPECT().SetHexpand(true).Once()
1868+ container.EXPECT().SetVexpand(true).Once()
1869+ container.EXPECT().SetVisible(true).Once()
1870+ factory.EXPECT().NewOverlay().Return(overlay).Once()
1871+ overlay.EXPECT().SetHexpand(true).Once()
1872+ overlay.EXPECT().SetVexpand(true).Once()
1873+ overlay.EXPECT().SetChild(container).Once()
1874+ overlay.EXPECT().SetVisible(true).Once()
1875+
1876+ wsView := component.NewWorkspaceView(ctx, factory)
1877+ container.EXPECT().AddCssClass("single-pane").Once()
1878+ wsView.RegisterPaneView(pane.ID, &component.PaneView{})
1879+
1880+ app := &App{
1881+ contentCoord: contentCoord,
1882+ workspaceViews: map[entity.TabID]*component.WorkspaceView{
1883+ tabID: wsView,
1884+ },
1885+ }
1886+
1887+ app.attachPopupToTab(ctx, tabID, pane, &fakeRecordingWebView{id: 1})
1888+
1889+ if got := contentCoord.GetWebView(pane.ID); got != nil {
1890+ t.Fatalf("popup webview registration remained after wrap failure: %v", got)
1891+ }
1892+}
1893+
18571894 type fakeZoomRepo struct{}
18581895
18591896 func (f *fakeZoomRepo) Get(context.Context, string) (*entity.ZoomLevel, error) { return nil, nil }
@@ -1854,6 +1854,43 @@ func TestApp_AttachPopupToTabSkipsRegistrationWhenPaneViewMissing(t *testing.T)
1854 }1854 }
1855 }1855 }
1856 1856
1857+func TestApp_AttachPopupToTabReleasesRegistrationWhenWrapFails(t *testing.T) {
1858+ ctx := context.Background()
1859+ contentCoord := contentcoord.NewCoordinator(ctx, nil, nil, nil, nil, nil, nil, nil)
1860+ tabID := entity.TabID("tab-1")
1861+ pane := entity.NewPane(entity.PaneID("popup-pane"))
1862+
1863+ factory := layoutmocks.NewMockWidgetFactory(t)
1864+ container := layoutmocks.NewMockBoxWidget(t)
1865+ overlay := layoutmocks.NewMockOverlayWidget(t)
1866+ factory.EXPECT().NewBox(layout.OrientationVertical, 0).Return(container).Once()
1867+ container.EXPECT().SetHexpand(true).Once()
1868+ container.EXPECT().SetVexpand(true).Once()
1869+ container.EXPECT().SetVisible(true).Once()
1870+ factory.EXPECT().NewOverlay().Return(overlay).Once()
1871+ overlay.EXPECT().SetHexpand(true).Once()
1872+ overlay.EXPECT().SetVexpand(true).Once()
1873+ overlay.EXPECT().SetChild(container).Once()
1874+ overlay.EXPECT().SetVisible(true).Once()
1875+
1876+ wsView := component.NewWorkspaceView(ctx, factory)
1877+ container.EXPECT().AddCssClass("single-pane").Once()
1878+ wsView.RegisterPaneView(pane.ID, &component.PaneView{})
1879+
1880+ app := &App{
1881+ contentCoord: contentCoord,
1882+ workspaceViews: map[entity.TabID]*component.WorkspaceView{
1883+ tabID: wsView,
1884+ },
1885+ }
1886+
1887+ app.attachPopupToTab(ctx, tabID, pane, &fakeRecordingWebView{id: 1})
1888+
1889+ if got := contentCoord.GetWebView(pane.ID); got != nil {
1890+ t.Fatalf("popup webview registration remained after wrap failure: %v", got)
1891+ }
1892+}
1893+
1857 type fakeZoomRepo struct{}1894 type fakeZoomRepo struct{}
1858 1895
1859 func (f *fakeZoomRepo) Get(context.Context, string) (*entity.ZoomLevel, error) { return nil, nil }1896 func (f *fakeZoomRepo) Get(context.Context, string) (*entity.ZoomLevel, error) { return nil, nil }
M internal/ui/component/workspace_view.go +9 -4
@@ -217,9 +217,10 @@ func (wv *WorkspaceView) SetWorkspace(ctx context.Context, ws *entity.Workspace)
217217 }
218218 }
219219
220- // Set initial active pane
220+ // Set initial active pane. A persisted workspace can contain a stale active
221+ // pane ID after restore/rebuild; that should not abort initialization.
221222 if ws.ActivePaneID != "" {
222- if err := wv.setActivePaneIDInternal(ws.ActivePaneID); err != nil {
223+ if err := wv.setActivePaneIDInternal(ws.ActivePaneID, true); err != nil {
223224 return fmt.Errorf("set initial active pane: %w", err)
224225 }
225226 }
@@ -234,7 +235,7 @@ func (wv *WorkspaceView) SetWorkspace(ctx context.Context, ws *entity.Workspace)
234235 func (wv *WorkspaceView) SetActivePaneID(paneID entity.PaneID) error {
235236 wv.mu.Lock()
236237 currentActiveID := wv.getActivePaneIDInternal()
237- err := wv.setActivePaneIDInternal(paneID)
238+ err := wv.setActivePaneIDInternal(paneID, false)
238239 callback := wv.onActivePaneChanged
239240 wv.mu.Unlock()
240241 if err != nil {
@@ -248,7 +249,7 @@ func (wv *WorkspaceView) SetActivePaneID(paneID entity.PaneID) error {
248249
249250 // setActivePaneIDInternal updates active pane without locking.
250251 // Updates the domain model as the single source of truth.
251-func (wv *WorkspaceView) setActivePaneIDInternal(paneID entity.PaneID) error {
252+func (wv *WorkspaceView) setActivePaneIDInternal(paneID entity.PaneID, allowMissing bool) error {
252253 currentActiveID := wv.getActivePaneIDInternal()
253254
254255 wv.logger.Debug().
@@ -260,6 +261,10 @@ func (wv *WorkspaceView) setActivePaneIDInternal(paneID entity.PaneID) error {
260261 // should leave the current active pane styling and overlays untouched.
261262 newPV, ok := wv.paneViews[paneID]
262263 if !ok {
264+ if allowMissing {
265+ wv.logger.Debug().Str("pane_id", string(paneID)).Msg("stale active pane ignored")
266+ return nil
267+ }
263268 return ErrPaneNotFound
264269 }
265270
@@ -217,9 +217,10 @@ func (wv *WorkspaceView) SetWorkspace(ctx context.Context, ws *entity.Workspace)
217 }217 }
218 }218 }
219 219
220- // Set initial active pane220+ // Set initial active pane. A persisted workspace can contain a stale active
221+ // pane ID after restore/rebuild; that should not abort initialization.
221 if ws.ActivePaneID != "" {222 if ws.ActivePaneID != "" {
222- if err := wv.setActivePaneIDInternal(ws.ActivePaneID); err != nil {223+ if err := wv.setActivePaneIDInternal(ws.ActivePaneID, true); err != nil {
223 return fmt.Errorf("set initial active pane: %w", err)224 return fmt.Errorf("set initial active pane: %w", err)
224 }225 }
225 }226 }
@@ -234,7 +235,7 @@ func (wv *WorkspaceView) SetWorkspace(ctx context.Context, ws *entity.Workspace)
234 func (wv *WorkspaceView) SetActivePaneID(paneID entity.PaneID) error {235 func (wv *WorkspaceView) SetActivePaneID(paneID entity.PaneID) error {
235 wv.mu.Lock()236 wv.mu.Lock()
236 currentActiveID := wv.getActivePaneIDInternal()237 currentActiveID := wv.getActivePaneIDInternal()
237- err := wv.setActivePaneIDInternal(paneID)238+ err := wv.setActivePaneIDInternal(paneID, false)
238 callback := wv.onActivePaneChanged239 callback := wv.onActivePaneChanged
239 wv.mu.Unlock()240 wv.mu.Unlock()
240 if err != nil {241 if err != nil {
@@ -248,7 +249,7 @@ func (wv *WorkspaceView) SetActivePaneID(paneID entity.PaneID) error {
248 249
249 // setActivePaneIDInternal updates active pane without locking.250 // setActivePaneIDInternal updates active pane without locking.
250 // Updates the domain model as the single source of truth.251 // Updates the domain model as the single source of truth.
251-func (wv *WorkspaceView) setActivePaneIDInternal(paneID entity.PaneID) error {252+func (wv *WorkspaceView) setActivePaneIDInternal(paneID entity.PaneID, allowMissing bool) error {
252 currentActiveID := wv.getActivePaneIDInternal()253 currentActiveID := wv.getActivePaneIDInternal()
253 254
254 wv.logger.Debug().255 wv.logger.Debug().
@@ -260,6 +261,10 @@ func (wv *WorkspaceView) setActivePaneIDInternal(paneID entity.PaneID) error {
260 // should leave the current active pane styling and overlays untouched.261 // should leave the current active pane styling and overlays untouched.
261 newPV, ok := wv.paneViews[paneID]262 newPV, ok := wv.paneViews[paneID]
262 if !ok {263 if !ok {
264+ if allowMissing {
265+ wv.logger.Debug().Str("pane_id", string(paneID)).Msg("stale active pane ignored")
266+ return nil
267+ }
263 return ErrPaneNotFound268 return ErrPaneNotFound
264 }269 }
265 270
M internal/ui/component/workspace_view_test.go +32 -0
@@ -259,6 +259,38 @@ func TestSetWorkspace_SinglePane_CreatesPaneView(t *testing.T) {
259259 assert.NotNil(t, wv.GetPaneView(pane.ID))
260260 }
261261
262+func TestSetWorkspace_StaleActivePaneID_DoesNotFailInitialization(t *testing.T) {
263+ // Arrange
264+ mockFactory := mocks.NewMockWidgetFactory(t)
265+ ctx, mockBox := setupWorkspaceViewBase(t, mockFactory)
266+
267+ wv := component.NewWorkspaceView(ctx, mockFactory)
268+
269+ mockOverlay, _ := setupWorkspacePaneViewMocks(t, mockFactory)
270+ mockStackBox := setupStackedLeafMocks(t, mockFactory, mockOverlay)
271+
272+ mockStackBox.EXPECT().SetVisible(true).Once()
273+ mockBox.EXPECT().Append(mockStackBox).Once()
274+ mockBox.EXPECT().AddCssClass("single-pane").Once()
275+
276+ pane := entity.NewPane(entity.PaneID("pane-1"))
277+ node := &entity.PaneNode{ID: "node-1", Pane: pane}
278+ ws := &entity.Workspace{
279+ ID: "ws-1",
280+ Root: node,
281+ ActivePaneID: entity.PaneID("stale-pane"),
282+ }
283+
284+ // Act
285+ err := wv.SetWorkspace(ctx, ws)
286+
287+ // Assert
288+ require.NoError(t, err)
289+ assert.Equal(t, 1, wv.PaneCount())
290+ assert.Equal(t, entity.PaneID("stale-pane"), wv.GetActivePaneID())
291+ assert.False(t, wv.GetPaneView(pane.ID).IsActive())
292+}
293+
262294 func TestWorkspaceView_HoverFocusLockState(t *testing.T) {
263295 // Intentional zero-value check: hoverFocusLock is an atomic.Bool and should
264296 // be safe with its language-level zero value before constructor wiring.
@@ -259,6 +259,38 @@ func TestSetWorkspace_SinglePane_CreatesPaneView(t *testing.T) {
259 assert.NotNil(t, wv.GetPaneView(pane.ID))259 assert.NotNil(t, wv.GetPaneView(pane.ID))
260 }260 }
261 261
262+func TestSetWorkspace_StaleActivePaneID_DoesNotFailInitialization(t *testing.T) {
263+ // Arrange
264+ mockFactory := mocks.NewMockWidgetFactory(t)
265+ ctx, mockBox := setupWorkspaceViewBase(t, mockFactory)
266+
267+ wv := component.NewWorkspaceView(ctx, mockFactory)
268+
269+ mockOverlay, _ := setupWorkspacePaneViewMocks(t, mockFactory)
270+ mockStackBox := setupStackedLeafMocks(t, mockFactory, mockOverlay)
271+
272+ mockStackBox.EXPECT().SetVisible(true).Once()
273+ mockBox.EXPECT().Append(mockStackBox).Once()
274+ mockBox.EXPECT().AddCssClass("single-pane").Once()
275+
276+ pane := entity.NewPane(entity.PaneID("pane-1"))
277+ node := &entity.PaneNode{ID: "node-1", Pane: pane}
278+ ws := &entity.Workspace{
279+ ID: "ws-1",
280+ Root: node,
281+ ActivePaneID: entity.PaneID("stale-pane"),
282+ }
283+
284+ // Act
285+ err := wv.SetWorkspace(ctx, ws)
286+
287+ // Assert
288+ require.NoError(t, err)
289+ assert.Equal(t, 1, wv.PaneCount())
290+ assert.Equal(t, entity.PaneID("stale-pane"), wv.GetActivePaneID())
291+ assert.False(t, wv.GetPaneView(pane.ID).IsActive())
292+}
293+
262 func TestWorkspaceView_HoverFocusLockState(t *testing.T) {294 func TestWorkspaceView_HoverFocusLockState(t *testing.T) {
263 // Intentional zero-value check: hoverFocusLock is an atomic.Bool and should295 // Intentional zero-value check: hoverFocusLock is an atomic.Bool and should
264 // be safe with its language-level zero value before constructor wiring.296 // be safe with its language-level zero value before constructor wiring.

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