a86e62f fix(ui): release vim page capture without re-entering the modal lock · dumber

M internal/ui/app.go +11 -8


@@ -2258,7 +2258,7 @@ func (a App) captureVimPageKeys(ctx context.Context, bw browserWindow, wv port
2258	2258		bw.keyboardHandler.SetPageKeyCapture(func(key string) {
2259	2259		if err := a.vimNavigationUseCase().SendPageKey(ctx, wv, key); err != nil {
2260	2260		logging.FromContext(ctx).Debug().Err(err).Msg("vim page key forwarding failed")
2261		-	a.endVimPageInteraction(ctx, bw)
	2261	+	a.endVimPageInteraction(ctx, bw, a.vimModeActiveForBrowserWindow(bw))
2262	2262		}
2263	2263		})
2264	2264		a.showVimModeToast(ctx, bw, label)

@@ -2272,9 +2272,11 @@ func (a App) captureVimPageKeys(ctx context.Context, bw browserWindow, wv port
2272	2272		}
2273	2273
2274	2274		// releaseVimPageKeys returns this window's keys to Vim Mode bindings and
2275		-	// restores the plain indicator and legend when the window is still in Vim
2276		-	// Mode. It reports the WebView that owned the interaction, if any.
2277		-	func (a App) releaseVimPageKeys(ctx context.Context, bw browserWindow, reason string) port.WebView {
	2275	+	// restores the plain indicator and legend when stillVim reports the window
	2276	+	// remains in Vim Mode. The caller supplies it because the mode-change callback
	2277	+	// runs under the modal lock, where reading the mode back would deadlock.
	2278	+	// It reports the WebView that owned the interaction, if any.
	2279	+	func (a App) releaseVimPageKeys(ctx context.Context, bw browserWindow, reason string, stillVim bool) port.WebView {
2278	2280		wv := bw.vimPageInteractionWebView
2279	2281		bw.vimPageInteractionWebView = nil
2280	2282		if bw.keyboardHandler != nil {

@@ -2283,7 +2285,7 @@ func (a App) releaseVimPageKeys(ctx context.Context, bw browserWindow, reason
2283	2285		if wv == nil {
2284	2286		return nil
2285	2287		}
2286		-	if bw.keyboardHandler != nil && bw.keyboardHandler.Mode() == input.ModeVim {
	2288	+	if stillVim {
2287	2289		a.showVimModeToast(ctx, bw, "")
2288	2290		if bw.modeFrame != nil {
2289	2291		bw.modeFrame.setSuspended(false)

@@ -2306,7 +2308,7 @@ func (a *App) handleVimPageInteractionEnded(ctx context.Context, paneID entity.P
2306	2308		if a.contentCoord.GetWebView(paneID) != bw.vimPageInteractionWebView {
2307	2309		return
2308	2310		}
2309		-	a.releaseVimPageKeys(ctx, bw, "page-reported-end")
	2311	+	a.releaseVimPageKeys(ctx, bw, "page-reported-end", a.vimModeActiveForBrowserWindow(bw))
2310	2312		}
2311	2313
2312	2314		// handleVimPageModeChanged repaints the indicator when the page moves its

@@ -2365,11 +2367,12 @@ func vimPageInteractionLabel(action string) string {
2365	2367		}
2366	2368
2367	2369		// endVimPageInteraction cancels any in-page interaction owned by this window.
2368		-	func (a App) endVimPageInteraction(ctx context.Context, bw browserWindow) {
	2370	+	// stillVim reports whether the window stays in Vim Mode (see releaseVimPageKeys).
	2371	+	func (a App) endVimPageInteraction(ctx context.Context, bw browserWindow, stillVim bool) {
2369	2372		if bw == nil {
2370	2373		return
2371	2374		}
2372		-	wv := a.releaseVimPageKeys(ctx, bw, "canceled")
	2375	+	wv := a.releaseVimPageKeys(ctx, bw, "canceled", stillVim)
2373	2376		if wv == nil {
2374	2377		return
2375	2378		}

 	bw.keyboardHandler.SetPageKeyCapture(func(key string) {
 		if err := a.vimNavigationUseCase().SendPageKey(ctx, wv, key); err != nil {
 			logging.FromContext(ctx).Debug().Err(err).Msg("vim page key forwarding failed")
-			a.endVimPageInteraction(ctx, bw)
+			a.endVimPageInteraction(ctx, bw, a.vimModeActiveForBrowserWindow(bw))
 		}
 	})
 	a.showVimModeToast(ctx, bw, label)
 }
 // releaseVimPageKeys returns this window's keys to Vim Mode bindings and
-// restores the plain indicator and legend when the window is still in Vim
+// restores the plain indicator and legend when stillVim reports the window
-// Mode. It reports the WebView that owned the interaction, if any.
+// remains in Vim Mode. The caller supplies it because the mode-change callback
-func (a *App) releaseVimPageKeys(ctx context.Context, bw *browserWindow, reason string) port.WebView {
+// runs under the modal lock, where reading the mode back would deadlock.
+// It reports the WebView that owned the interaction, if any.
+func (a *App) releaseVimPageKeys(ctx context.Context, bw *browserWindow, reason string, stillVim bool) port.WebView {
 	wv := bw.vimPageInteractionWebView
 	bw.vimPageInteractionWebView = nil
 	if bw.keyboardHandler != nil {
 	if wv == nil {
 		return nil
 	}
-	if bw.keyboardHandler != nil && bw.keyboardHandler.Mode() == input.ModeVim {
+	if stillVim {
 		a.showVimModeToast(ctx, bw, "")
 		if bw.modeFrame != nil {
 			bw.modeFrame.setSuspended(false)
 	if a.contentCoord.GetWebView(paneID) != bw.vimPageInteractionWebView {
 		return
 	}
-	a.releaseVimPageKeys(ctx, bw, "page-reported-end")
+	a.releaseVimPageKeys(ctx, bw, "page-reported-end", a.vimModeActiveForBrowserWindow(bw))
 }
 // handleVimPageModeChanged repaints the indicator when the page moves its
 }
 // endVimPageInteraction cancels any in-page interaction owned by this window.
-func (a *App) endVimPageInteraction(ctx context.Context, bw *browserWindow) {
+// stillVim reports whether the window stays in Vim Mode (see releaseVimPageKeys).
+func (a *App) endVimPageInteraction(ctx context.Context, bw *browserWindow, stillVim bool) {
 	if bw == nil {
 		return
 	}
-	wv := a.releaseVimPageKeys(ctx, bw, "canceled")
+	wv := a.releaseVimPageKeys(ctx, bw, "canceled", stillVim)
 	if wv == nil {
 		return
 	}

M internal/ui/app_vim_page_test.go +36 -1


@@ -3,6 +3,7 @@ package ui
3	3		import (
4	4		"context"
5	5		"testing"
	6	+	"time"
6	7
7	8		"github.com/stretchr/testify/mock"
8	9		"github.com/stretchr/testify/require"

@@ -84,7 +85,7 @@ func TestVimPageInteractionHidesLegendWhileCaptured(t *testing.T) {
84	85		require.True(t, frame.suspended, "legend must be suspended while the page captures keys")
85	86
86	87		wv.EXPECT().ID().Return(1).Maybe()
87		-	app.endVimPageInteraction(context.Background(), bw)
	88	+	app.endVimPageInteraction(context.Background(), bw, true)
88	89		require.False(t, frame.suspended, "legend must be restored when the interaction ends")
89	90		require.False(t, bw.keyboardHandler.PageKeyCaptureActive())
90	91		}

@@ -121,3 +122,37 @@ func TestVimPageModeChangeIgnoredWithoutInteraction(t *testing.T) {
121	122
122	123		require.Empty(t, *shown, "a late mode report must not repaint the indicator once the interaction ended")
123	124		}
	125	+
	126	+	// exitModeWithin runs fn and fails the test if it does not return in time,
	127	+	// so a lock re-entry deadlock shows up as a failure instead of a hung run.
	128	+	func exitModeWithin(t *testing.T, fn func()) {
	129	+	t.Helper()
	130	+	done := make(chan struct{})
	131	+	go func() {
	132	+	defer close(done)
	133	+	fn()
	134	+	}()
	135	+	select {
	136	+	case <-done:
	137	+	case <-time.After(3 * time.Second):
	138	+	t.Fatal("ExitMode did not return: mode-change callback re-entered the modal lock")
	139	+	}
	140	+	}
	141	+
	142	+	// ExitMode runs the mode-change callback under the modal lock. Releasing the
	143	+	// page capture from that callback must not read the mode back through the lock.
	144	+	func TestExitModeWithPageCaptureDoesNotDeadlock(t *testing.T) {
	145	+	app, bw, _ := newVimPageTestWindow(t)
	146	+	bw.keyboardHandler.SetOnModeChange(func(from, to input.Mode) {
	147	+	app.handleModeChange(context.Background(), bw, from, to)
	148	+	})
	149	+	wv := portmocks.NewMockWebView(t)
	150	+	wv.EXPECT().ID().Return(1).Maybe()
	151	+	app.captureVimPageKeys(context.Background(), bw, wv, "VISUAL")
	152	+
	153	+	exitModeWithin(t, bw.keyboardHandler.ExitMode)
	154	+
	155	+	require.False(t, bw.keyboardHandler.PageKeyCaptureActive())
	156	+	require.Nil(t, bw.vimPageInteractionWebView)
	157	+	require.Equal(t, input.ModeNormal, bw.keyboardHandler.Mode())
	158	+	}


@@ -3,6 +3,7 @@ package ui
3		import (	3		import (
4		"context"	4		"context"
5		"testing"	5		"testing"
			6	+	"time"
6			7
7		"github.com/stretchr/testify/mock"	8		"github.com/stretchr/testify/mock"
8		"github.com/stretchr/testify/require"	9		"github.com/stretchr/testify/require"

@@ -84,7 +85,7 @@ func TestVimPageInteractionHidesLegendWhileCaptured(t *testing.T) {
84		require.True(t, frame.suspended, "legend must be suspended while the page captures keys")	85		require.True(t, frame.suspended, "legend must be suspended while the page captures keys")
85			86
86		wv.EXPECT().ID().Return(1).Maybe()	87		wv.EXPECT().ID().Return(1).Maybe()
87	-	app.endVimPageInteraction(context.Background(), bw)	88	+	app.endVimPageInteraction(context.Background(), bw, true)
88		require.False(t, frame.suspended, "legend must be restored when the interaction ends")	89		require.False(t, frame.suspended, "legend must be restored when the interaction ends")
89		require.False(t, bw.keyboardHandler.PageKeyCaptureActive())	90		require.False(t, bw.keyboardHandler.PageKeyCaptureActive())
90		}	91		}

@@ -121,3 +122,37 @@ func TestVimPageModeChangeIgnoredWithoutInteraction(t *testing.T) {
121			122
122		require.Empty(t, *shown, "a late mode report must not repaint the indicator once the interaction ended")	123		require.Empty(t, *shown, "a late mode report must not repaint the indicator once the interaction ended")
123		}	124		}
			125	+
			126	+	// exitModeWithin runs fn and fails the test if it does not return in time,
			127	+	// so a lock re-entry deadlock shows up as a failure instead of a hung run.
			128	+	func exitModeWithin(t *testing.T, fn func()) {
			129	+	t.Helper()
			130	+	done := make(chan struct{})
			131	+	go func() {
			132	+	defer close(done)
			133	+	fn()
			134	+	}()
			135	+	select {
			136	+	case <-done:
			137	+	case <-time.After(3 * time.Second):
			138	+	t.Fatal("ExitMode did not return: mode-change callback re-entered the modal lock")
			139	+	}
			140	+	}
			141	+
			142	+	// ExitMode runs the mode-change callback under the modal lock. Releasing the
			143	+	// page capture from that callback must not read the mode back through the lock.
			144	+	func TestExitModeWithPageCaptureDoesNotDeadlock(t *testing.T) {
			145	+	app, bw, _ := newVimPageTestWindow(t)
			146	+	bw.keyboardHandler.SetOnModeChange(func(from, to input.Mode) {
			147	+	app.handleModeChange(context.Background(), bw, from, to)
			148	+	})
			149	+	wv := portmocks.NewMockWebView(t)
			150	+	wv.EXPECT().ID().Return(1).Maybe()
			151	+	app.captureVimPageKeys(context.Background(), bw, wv, "VISUAL")
			152	+
			153	+	exitModeWithin(t, bw.keyboardHandler.ExitMode)
			154	+
			155	+	require.False(t, bw.keyboardHandler.PageKeyCaptureActive())
			156	+	require.Nil(t, bw.vimPageInteractionWebView)
			157	+	require.Equal(t, input.ModeNormal, bw.keyboardHandler.Mode())
			158	+	}

M internal/ui/input/vim_page_capture.go +21 -1


@@ -35,7 +35,23 @@ func (h *KeyboardHandler) PageKeyCaptureActive() bool {
35	35		return h.pageKeyCapture != nil
36	36		}
37	37
38		-	// forwardPageKey sends a Vim Mode key to the active page capture. Escape is
	38	+	// isVimModeToggleKey reports whether the key is the configured Vim Mode
	39	+	// activation shortcut. While Vim Mode is active it exits the mode, so it must
	40	+	// stay with Go even when a page interaction owns every other key.
	41	+	func (h *KeyboardHandler) isVimModeToggleKey(keyval uint, state gdk.ModifierType) bool {
	42	+	h.mu.RLock()
	43	+	shortcuts := h.shortcuts
	44	+	h.mu.RUnlock()
	45	+	if shortcuts == nil {
	46	+	return false
	47	+	}
	48	+	binding := KeyBinding{Keyval: normalizeKeyval(keyval), Modifiers: Modifier(state) & modifierMask}
	49	+	action, found := shortcuts.Lookup(binding, ModeVim)
	50	+	return found && action == ActionEnterVimMode
	51	+	}
	52	+
	53	+	// forwardPageKey sends a Vim Mode key to the active page capture, except the
	54	+	// Vim Mode toggle shortcut, which always falls through. Escape is
39	55		// forwarded like any other key: the page decides whether it steps back (visual
40	56		// to caret) or ends the interaction, and reports the end so the capture is
41	57		// released. A page that cannot be reached ends the capture through the

@@ -53,6 +69,10 @@ func (h *KeyboardHandler) forwardPageKey(mode Mode, keyval uint, state gdk.Modif
53	69		if isModifierKeyval(keyval) {
54	70		return true
55	71		}
	72	+	if h.isVimModeToggleKey(keyval, state) {
	73	+	// Escape hatch: a page can never swallow the shortcut that leaves Vim Mode.
	74	+	return false
	75	+	}
56	76		key, ok := KeyvalToVimKey(keyval, state)
57	77		if !ok {
58	78		return true


@@ -35,7 +35,23 @@ func (h *KeyboardHandler) PageKeyCaptureActive() bool {
35		return h.pageKeyCapture != nil	35		return h.pageKeyCapture != nil
36		}	36		}
37			37
38	-	// forwardPageKey sends a Vim Mode key to the active page capture. Escape is	38	+	// isVimModeToggleKey reports whether the key is the configured Vim Mode
			39	+	// activation shortcut. While Vim Mode is active it exits the mode, so it must
			40	+	// stay with Go even when a page interaction owns every other key.
			41	+	func (h *KeyboardHandler) isVimModeToggleKey(keyval uint, state gdk.ModifierType) bool {
			42	+	h.mu.RLock()
			43	+	shortcuts := h.shortcuts
			44	+	h.mu.RUnlock()
			45	+	if shortcuts == nil {
			46	+	return false
			47	+	}
			48	+	binding := KeyBinding{Keyval: normalizeKeyval(keyval), Modifiers: Modifier(state) & modifierMask}
			49	+	action, found := shortcuts.Lookup(binding, ModeVim)
			50	+	return found && action == ActionEnterVimMode
			51	+	}
			52	+
			53	+	// forwardPageKey sends a Vim Mode key to the active page capture, except the
			54	+	// Vim Mode toggle shortcut, which always falls through. Escape is
39		// forwarded like any other key: the page decides whether it steps back (visual	55		// forwarded like any other key: the page decides whether it steps back (visual
40		// to caret) or ends the interaction, and reports the end so the capture is	56		// to caret) or ends the interaction, and reports the end so the capture is
41		// released. A page that cannot be reached ends the capture through the	57		// released. A page that cannot be reached ends the capture through the

@@ -53,6 +69,10 @@ func (h *KeyboardHandler) forwardPageKey(mode Mode, keyval uint, state gdk.Modif
53		if isModifierKeyval(keyval) {	69		if isModifierKeyval(keyval) {
54		return true	70		return true
55		}	71		}
			72	+	if h.isVimModeToggleKey(keyval, state) {
			73	+	// Escape hatch: a page can never swallow the shortcut that leaves Vim Mode.
			74	+	return false
			75	+	}
56		key, ok := KeyvalToVimKey(keyval, state)	76		key, ok := KeyvalToVimKey(keyval, state)
57		if !ok {	77		if !ok {
58		return true	78		return true

M internal/ui/input/vim_page_capture_test.go +32 -0


@@ -84,3 +84,35 @@ func TestSetPageKeyCaptureDoesNotNotifyPendingListeners(t *testing.T) {
84	84		assert.Empty(t, *pending)
85	85		assert.Empty(t, h.PendingSequence())
86	86		}
	87	+
	88	+	// The Vim Mode toggle shortcut is the escape hatch from a page interaction:
	89	+	// it must reach Go, leave Vim Mode and release the capture, never the page.
	90	+	func TestPageKeyCaptureLeavesVimModeToggleShortcutToGo(t *testing.T) {
	91	+	h := NewKeyboardHandler(context.Background(), vimModeSequenceWorkspace(nil), newTestSession())
	92	+	enterVimMode(t, h)
	93	+	var forwarded []string
	94	+	h.SetPageKeyCapture(func(key string) { forwarded = append(forwarded, key) })
	95	+	h.handleKeyRelease(uint('y')) // the activation press is over; this is a new press
	96	+
	97	+	require.True(t, h.handleKeyPress(uint('y'), 0, gdk.ControlMaskValue))
	98	+
	99	+	assert.Empty(t, forwarded, "the toggle shortcut must not be forwarded to the page")
	100	+	assert.Equal(t, ModeNormal, h.Mode())
	101	+	assert.False(t, h.PageKeyCaptureActive(), "leaving Vim Mode releases the capture")
	102	+	}
	103	+
	104	+	func TestPageKeyCaptureHonorsConfiguredToggleShortcut(t *testing.T) {
	105	+	ws := vimModeSequenceWorkspace(nil)
	106	+	ws.VimMode.ActivationShortcut = "ctrl+g"
	107	+	h := NewKeyboardHandler(context.Background(), ws, newTestSession())
	108	+	h.EnterVimMode()
	109	+	var forwarded []string
	110	+	h.SetPageKeyCapture(func(key string) { forwarded = append(forwarded, key) })
	111	+
	112	+	require.True(t, h.handleKeyPress(uint('y'), 0, gdk.ControlMaskValue), "ctrl+y is an ordinary key now")
	113	+	assert.Equal(t, []string{"<C-y>"}, forwarded)
	114	+	require.True(t, h.handleKeyPress(uint('g'), 0, gdk.ControlMaskValue))
	115	+
	116	+	assert.Equal(t, []string{"<C-y>"}, forwarded)
	117	+	assert.Equal(t, ModeNormal, h.Mode())
	118	+	}


@@ -84,3 +84,35 @@ func TestSetPageKeyCaptureDoesNotNotifyPendingListeners(t *testing.T) {
84	assert.Empty(t, *pending)	84		assert.Empty(t, *pending)
85	assert.Empty(t, h.PendingSequence())	85		assert.Empty(t, h.PendingSequence())
86	}	86		}
		87	+
		88	+	// The Vim Mode toggle shortcut is the escape hatch from a page interaction:
		89	+	// it must reach Go, leave Vim Mode and release the capture, never the page.
		90	+	func TestPageKeyCaptureLeavesVimModeToggleShortcutToGo(t *testing.T) {
		91	+	h := NewKeyboardHandler(context.Background(), vimModeSequenceWorkspace(nil), newTestSession())
		92	+	enterVimMode(t, h)
		93	+	var forwarded []string
		94	+	h.SetPageKeyCapture(func(key string) { forwarded = append(forwarded, key) })
		95	+	h.handleKeyRelease(uint('y')) // the activation press is over; this is a new press
		96	+
		97	+	require.True(t, h.handleKeyPress(uint('y'), 0, gdk.ControlMaskValue))
		98	+
		99	+	assert.Empty(t, forwarded, "the toggle shortcut must not be forwarded to the page")
		100	+	assert.Equal(t, ModeNormal, h.Mode())
		101	+	assert.False(t, h.PageKeyCaptureActive(), "leaving Vim Mode releases the capture")
		102	+	}
		103	+
		104	+	func TestPageKeyCaptureHonorsConfiguredToggleShortcut(t *testing.T) {
		105	+	ws := vimModeSequenceWorkspace(nil)
		106	+	ws.VimMode.ActivationShortcut = "ctrl+g"
		107	+	h := NewKeyboardHandler(context.Background(), ws, newTestSession())
		108	+	h.EnterVimMode()
		109	+	var forwarded []string
		110	+	h.SetPageKeyCapture(func(key string) { forwarded = append(forwarded, key) })
		111	+
		112	+	require.True(t, h.handleKeyPress(uint('y'), 0, gdk.ControlMaskValue), "ctrl+y is an ordinary key now")
		113	+	assert.Equal(t, []string{"<C-y>"}, forwarded)
		114	+	require.True(t, h.handleKeyPress(uint('g'), 0, gdk.ControlMaskValue))
		115	+
		116	+	assert.Equal(t, []string{"<C-y>"}, forwarded)
		117	+	assert.Equal(t, ModeNormal, h.Mode())
		118	+	}

M internal/ui/vim_mode.go +4 -2


@@ -117,7 +117,9 @@ func (a App) handleModeChange(ctx context.Context, bw browserWindow, from, to
117	117		a.enableAccessibilityForVimMode(ctx, bw)
118	118		}
119	119		if from == input.ModeVim && to != input.ModeVim {
120		-	a.endVimPageInteraction(ctx, bw)
	120	+	// Runs under the modal lock: the window is leaving Vim Mode, so do not
	121	+	// read the mode back.
	122	+	a.endVimPageInteraction(ctx, bw, false)
121	123		a.clearVimNavigationHighlight(ctx, bw)
122	124		}
123	125

@@ -144,7 +146,7 @@ func (a App) transferVimModeOwnershipToPane(ctx context.Context, bw browserWin
144	146		return
145	147		}
146	148		// A page interaction belongs to the previous owner's WebView.
147		-	a.endVimPageInteraction(ctx, bw)
	149	+	a.endVimPageInteraction(ctx, bw, a.vimModeActiveForBrowserWindow(bw))
148	150
149	151		// Check if this window is actually in vim mode.
150	152		if bw.keyboardHandler == nil \|\| bw.keyboardHandler.Mode() != input.ModeVim {

 		a.enableAccessibilityForVimMode(ctx, bw)
 	}
 	if from == input.ModeVim && to != input.ModeVim {
-		a.endVimPageInteraction(ctx, bw)
+		// Runs under the modal lock: the window is leaving Vim Mode, so do not
+		// read the mode back.
+		a.endVimPageInteraction(ctx, bw, false)
 		a.clearVimNavigationHighlight(ctx, bw)
 	}
 		return
 	}
 	// A page interaction belongs to the previous owner's WebView.
-	a.endVimPageInteraction(ctx, bw)
+	a.endVimPageInteraction(ctx, bw, a.vimModeActiveForBrowserWindow(bw))
 	// Check if this window is actually in vim mode.
 	if bw.keyboardHandler == nil || bw.keyboardHandler.Mode() != input.ModeVim {