5b9fd74 fix: address second round of code review findings · dumber

M internal/bootstrap/crash_report_unix.go +2 -2


@@ -14,8 +14,8 @@ func collectCoreDumpDiagnostics() unexpectedCloseCoreDump {
14	14		}
15	15		var limit unix.Rlimit
16	16		if err := unix.Getrlimit(unix.RLIMIT_CORE, &limit); err != nil {
17		-	d.RLimitCoreSoft = "unknown"
18		-	d.RLimitCoreHard = "unknown"
	17	+	d.RLimitCoreSoft = string(SessionExitUnknown)
	18	+	d.RLimitCoreHard = string(SessionExitUnknown)
19	19		return d
20	20		}
21	21		d.RLimitCoreSoft = formatRlimitCore(limit.Cur)


@@ -14,8 +14,8 @@ func collectCoreDumpDiagnostics() unexpectedCloseCoreDump {
14		}	14		}
15		var limit unix.Rlimit	15		var limit unix.Rlimit
16		if err := unix.Getrlimit(unix.RLIMIT_CORE, &limit); err != nil {	16		if err := unix.Getrlimit(unix.RLIMIT_CORE, &limit); err != nil {
17	-	d.RLimitCoreSoft = "unknown"	17	+	d.RLimitCoreSoft = string(SessionExitUnknown)
18	-	d.RLimitCoreHard = "unknown"	18	+	d.RLimitCoreHard = string(SessionExitUnknown)
19		return d	19		return d
20		}	20		}
21		d.RLimitCoreSoft = formatRlimitCore(limit.Cur)	21		d.RLimitCoreSoft = formatRlimitCore(limit.Cur)

M internal/domain/entity/config_types.go +8 -8


@@ -6,13 +6,13 @@ package entity
6	6
7	7		// AppearanceConfig holds visual appearance settings.
8	8		type AppearanceConfig struct {
9		-	SansFont string `mapstructure:"sans_font" yaml:"sans_font" toml:"sans_font"`
10		-	SerifFont string `mapstructure:"serif_font" yaml:"serif_font" toml:"serif_font"`
11		-	MonospaceFont string `mapstructure:"monospace_font" yaml:"monospace_font" toml:"monospace_font"`
12		-	DefaultFontSize int `mapstructure:"default_font_size" yaml:"default_font_size" toml:"default_font_size"`
13		-	LightPalette ColorPalette `mapstructure:"light_palette" yaml:"light_palette" toml:"light_palette"`
14		-	DarkPalette ColorPalette `mapstructure:"dark_palette" yaml:"dark_palette" toml:"dark_palette"`
15		-	ColorScheme string `mapstructure:"color_scheme" yaml:"color_scheme" toml:"color_scheme"`
	9	+	SansFont string `mapstructure:"sans_font" yaml:"sans_font" toml:"sans_font" json:"sans_font"`
	10	+	SerifFont string `mapstructure:"serif_font" yaml:"serif_font" toml:"serif_font" json:"serif_font"`
	11	+	MonospaceFont string `mapstructure:"monospace_font" yaml:"monospace_font" toml:"monospace_font" json:"monospace_font"`
	12	+	DefaultFontSize int `mapstructure:"default_font_size" yaml:"default_font_size" toml:"default_font_size" json:"default_font_size"`
	13	+	LightPalette ColorPalette `mapstructure:"light_palette" yaml:"light_palette" toml:"light_palette" json:"light_palette"`
	14	+	DarkPalette ColorPalette `mapstructure:"dark_palette" yaml:"dark_palette" toml:"dark_palette" json:"dark_palette"`
	15	+	ColorScheme string `mapstructure:"color_scheme" yaml:"color_scheme" toml:"color_scheme" json:"color_scheme"`
16	16		}
17	17
18	18		// ColorPalette defines the color scheme for a theme variant (light/dark).

@@ -176,7 +176,7 @@ type PopupBehaviorConfig struct {
176	176
177	177		// WorkspaceConfig holds all workspace layout and behavior settings.
178	178		type WorkspaceConfig struct {
179		-	NewPaneURL string `mapstructure:"new_pane_url" yaml:"new_pane_url" toml:"new_pane_url"`
	179	+	NewPaneURL string `mapstructure:"new_pane_url" yaml:"new_pane_url" toml:"new_pane_url" json:"new_pane_url"`
180	180		PaneMode PaneModeConfig `mapstructure:"pane_mode" yaml:"pane_mode" toml:"pane_mode" json:"pane_mode"`
181	181		TabMode TabModeConfig `mapstructure:"tab_mode" yaml:"tab_mode" toml:"tab_mode" json:"tab_mode"`
182	182		ResizeMode ResizeModeConfig `mapstructure:"resize_mode" yaml:"resize_mode" toml:"resize_mode" json:"resize_mode"`


@@ -6,13 +6,13 @@ package entity
6			6
7		// AppearanceConfig holds visual appearance settings.	7		// AppearanceConfig holds visual appearance settings.
8		type AppearanceConfig struct {	8		type AppearanceConfig struct {
9	-	SansFont string `mapstructure:"sans_font" yaml:"sans_font" toml:"sans_font"`	9	+	SansFont string `mapstructure:"sans_font" yaml:"sans_font" toml:"sans_font" json:"sans_font"`
10	-	SerifFont string `mapstructure:"serif_font" yaml:"serif_font" toml:"serif_font"`	10	+	SerifFont string `mapstructure:"serif_font" yaml:"serif_font" toml:"serif_font" json:"serif_font"`
11	-	MonospaceFont string `mapstructure:"monospace_font" yaml:"monospace_font" toml:"monospace_font"`	11	+	MonospaceFont string `mapstructure:"monospace_font" yaml:"monospace_font" toml:"monospace_font" json:"monospace_font"`
12	-	DefaultFontSize int `mapstructure:"default_font_size" yaml:"default_font_size" toml:"default_font_size"`	12	+	DefaultFontSize int `mapstructure:"default_font_size" yaml:"default_font_size" toml:"default_font_size" json:"default_font_size"`
13	-	LightPalette ColorPalette `mapstructure:"light_palette" yaml:"light_palette" toml:"light_palette"`	13	+	LightPalette ColorPalette `mapstructure:"light_palette" yaml:"light_palette" toml:"light_palette" json:"light_palette"`
14	-	DarkPalette ColorPalette `mapstructure:"dark_palette" yaml:"dark_palette" toml:"dark_palette"`	14	+	DarkPalette ColorPalette `mapstructure:"dark_palette" yaml:"dark_palette" toml:"dark_palette" json:"dark_palette"`
15	-	ColorScheme string `mapstructure:"color_scheme" yaml:"color_scheme" toml:"color_scheme"`	15	+	ColorScheme string `mapstructure:"color_scheme" yaml:"color_scheme" toml:"color_scheme" json:"color_scheme"`
16		}	16		}
17			17
18		// ColorPalette defines the color scheme for a theme variant (light/dark).	18		// ColorPalette defines the color scheme for a theme variant (light/dark).

@@ -176,7 +176,7 @@ type PopupBehaviorConfig struct {
176			176
177		// WorkspaceConfig holds all workspace layout and behavior settings.	177		// WorkspaceConfig holds all workspace layout and behavior settings.
178		type WorkspaceConfig struct {	178		type WorkspaceConfig struct {
179	-	NewPaneURL string `mapstructure:"new_pane_url" yaml:"new_pane_url" toml:"new_pane_url"`	179	+	NewPaneURL string `mapstructure:"new_pane_url" yaml:"new_pane_url" toml:"new_pane_url" json:"new_pane_url"`
180		PaneMode PaneModeConfig `mapstructure:"pane_mode" yaml:"pane_mode" toml:"pane_mode" json:"pane_mode"`	180		PaneMode PaneModeConfig `mapstructure:"pane_mode" yaml:"pane_mode" toml:"pane_mode" json:"pane_mode"`
181		TabMode TabModeConfig `mapstructure:"tab_mode" yaml:"tab_mode" toml:"tab_mode" json:"tab_mode"`	181		TabMode TabModeConfig `mapstructure:"tab_mode" yaml:"tab_mode" toml:"tab_mode" json:"tab_mode"`
182		ResizeMode ResizeModeConfig `mapstructure:"resize_mode" yaml:"resize_mode" toml:"resize_mode" json:"resize_mode"`	182		ResizeMode ResizeModeConfig `mapstructure:"resize_mode" yaml:"resize_mode" toml:"resize_mode" json:"resize_mode"`

M internal/infrastructure/webkit/handlers/config.go +3 -0


@@ -18,6 +18,9 @@ type ConfigHandler struct {
18	18		// NewConfigHandler creates a new ConfigHandler.
19	19		// saveConfig is called to persist config changes (typically usecase.SaveWebUIConfigUseCase.Execute).
20	20		func NewConfigHandler(saveConfig func(context.Context, port.WebUIConfig) error) *ConfigHandler {
	21	+	if saveConfig == nil {
	22	+	panic("NewConfigHandler: saveConfig must not be nil")
	23	+	}
21	24		return &ConfigHandler{saveConfig: saveConfig}
22	25		}
23	26


@@ -18,6 +18,9 @@ type ConfigHandler struct {
18	// NewConfigHandler creates a new ConfigHandler.	18		// NewConfigHandler creates a new ConfigHandler.
19	// saveConfig is called to persist config changes (typically usecase.SaveWebUIConfigUseCase.Execute).	19		// saveConfig is called to persist config changes (typically usecase.SaveWebUIConfigUseCase.Execute).
20	func NewConfigHandler(saveConfig func(context.Context, port.WebUIConfig) error) *ConfigHandler {	20		func NewConfigHandler(saveConfig func(context.Context, port.WebUIConfig) error) *ConfigHandler {
		21	+	if saveConfig == nil {
		22	+	panic("NewConfigHandler: saveConfig must not be nil")
		23	+	}
21	return &ConfigHandler{saveConfig: saveConfig}	24		return &ConfigHandler{saveConfig: saveConfig}
22	}	25		}
23		26

M internal/infrastructure/webkit/handlers/homepage/folders.go +4 -1


@@ -67,9 +67,12 @@ func (h *FolderHandlers) HandleCreate() webkit.MessageHandler {
67	67		return NewErrorResponse(req.RequestID, err), nil
68	68		}
69	69
70		-	// Set icon if provided
	70	+	// Set icon if provided and persist it
71	71		if req.Icon != nil && *req.Icon != "" {
72	72		folder.Icon = *req.Icon
	73	+	if err := h.favoritesUC.UpdateFolder(ctx, folder.ID, folder.Name, folder.Icon); err != nil {
	74	+	return NewErrorResponse(req.RequestID, err), nil
	75	+	}
73	76		}
74	77
75	78		return NewSuccessResponse(req.RequestID, folder), nil

 			return NewErrorResponse(req.RequestID, err), nil
 		}
-		// Set icon if provided
+		// Set icon if provided and persist it
 		if req.Icon != nil && *req.Icon != "" {
 			folder.Icon = *req.Icon
+			if err := h.favoritesUC.UpdateFolder(ctx, folder.ID, folder.Name, folder.Icon); err != nil {
+				return NewErrorResponse(req.RequestID, err), nil
+			}
 		}
 		return NewSuccessResponse(req.RequestID, folder), nil

M internal/infrastructure/webkit/handlers/register.go +4 -2


@@ -43,8 +43,10 @@ func RegisterAll(ctx context.Context, router *webkit.MessageRouter, cfg Config)
43	43		}
44	44
45	45		// Configuration handlers
46		-	if err := RegisterConfigHandlers(ctx, router, cfg.SaveConfig); err != nil {
47		-	return err
	46	+	if cfg.SaveConfig != nil {
	47	+	if err := RegisterConfigHandlers(ctx, router, cfg.SaveConfig); err != nil {
	48	+	return err
	49	+	}
48	50		}
49	51
50	52		// Keybindings handlers (always available)

 	}
 	// Configuration handlers
-	if err := RegisterConfigHandlers(ctx, router, cfg.SaveConfig); err != nil {
+	if cfg.SaveConfig != nil {
-		return err
+		if err := RegisterConfigHandlers(ctx, router, cfg.SaveConfig); err != nil {
+			return err
+		}
 	}
 	// Keybindings handlers (always available)

M internal/infrastructure/webkit/settings.go +3 -6


@@ -51,7 +51,6 @@ func (sm SettingsManager) applySettings(ctx context.Context, settings webkit.S
51	51		log := logging.FromContext(ctx)
52	52		applyJavaScriptSettings(settings)
53	53		applyFontSettings(settings, cfg)
54		-	applyRenderingSettings(settings, cfg)
55	54		applyDebugSettings(settings, cfg)
56	55		applyBrowsingSettings(settings)
57	56		applyMediaSettings(settings, cfg, log)

@@ -92,11 +91,6 @@ func applyFontSettings(settings webkit.Settings, cfg config.Config) {
92	91		}
93	92		}
94	93
95		-	func applyRenderingSettings(_ webkit.Settings, _ config.Config) {
96		-	// Hardware acceleration policy is now managed by the engine layer via
97		-	// cfg.Engine.WebKit. No WebKit settings to apply here at the settings layer.
98		-	}
99		-
100	94		func applyDebugSettings(settings webkit.Settings, cfg config.Config) {
101	95		settings.SetEnableDeveloperExtras(cfg.Debug.EnableDevTools)
102	96		settings.SetEnableWriteConsoleMessagesToStdout(cfg.Logging.CaptureConsole)

@@ -171,6 +165,9 @@ func (sm SettingsManager) UpdateFromConfig(ctx context.Context, cfg config.Con
171	165		// ApplyToWebView applies current settings to an existing WebView.
172	166		// This can be used to update a WebView's settings after config hot-reload.
173	167		func (sm SettingsManager) ApplyToWebView(ctx context.Context, wv webkit.WebView) {
	168	+	if sm == nil {
	169	+	return
	170	+	}
174	171		if wv == nil {
175	172		return
176	173		}

 	log := logging.FromContext(ctx)
 	applyJavaScriptSettings(settings)
 	applyFontSettings(settings, cfg)
-	applyRenderingSettings(settings, cfg)
 	applyDebugSettings(settings, cfg)
 	applyBrowsingSettings(settings)
 	applyMediaSettings(settings, cfg, log)
 	}
 }
-func applyRenderingSettings(_ *webkit.Settings, _ *config.Config) {
-	// Hardware acceleration policy is now managed by the engine layer via
-	// cfg.Engine.WebKit. No WebKit settings to apply here at the settings layer.
-}
 func applyDebugSettings(settings *webkit.Settings, cfg *config.Config) {
 	settings.SetEnableDeveloperExtras(cfg.Debug.EnableDevTools)
 	settings.SetEnableWriteConsoleMessagesToStdout(cfg.Logging.CaptureConsole)
 // ApplyToWebView applies current settings to an existing WebView.
 // This can be used to update a WebView's settings after config hot-reload.
 func (sm *SettingsManager) ApplyToWebView(ctx context.Context, wv *webkit.WebView) {
+	if sm == nil {
+		return
+	}
 	if wv == nil {
 		return
 	}

M internal/ui/app.go +1 -1


@@ -1635,7 +1635,7 @@ func (a *App) initCoordinators(ctx context.Context) {
1635	1635		// Set theme background color on the engine's popup factory to eliminate white flash.
1636	1636		if a.engine != nil && a.deps.Theme != nil {
1637	1637		r, g, b, alpha := a.deps.Theme.GetBackgroundRGBA()
1638		-	_ = a.engine.UpdateAppearance(context.Background(), float64(r), float64(g), float64(b), float64(alpha))
	1638	+	_ = a.engine.UpdateAppearance(ctx, float64(r), float64(g), float64(b), float64(alpha))
1639	1639		}
1640	1640		a.contentCoord.SetPopupConfig(
1641	1641		a.engine.Factory(),


@@ -1635,7 +1635,7 @@ func (a *App) initCoordinators(ctx context.Context) {
1635		// Set theme background color on the engine's popup factory to eliminate white flash.	1635		// Set theme background color on the engine's popup factory to eliminate white flash.
1636		if a.engine != nil && a.deps.Theme != nil {	1636		if a.engine != nil && a.deps.Theme != nil {
1637		r, g, b, alpha := a.deps.Theme.GetBackgroundRGBA()	1637		r, g, b, alpha := a.deps.Theme.GetBackgroundRGBA()
1638	-	_ = a.engine.UpdateAppearance(context.Background(), float64(r), float64(g), float64(b), float64(alpha))	1638	+	_ = a.engine.UpdateAppearance(ctx, float64(r), float64(g), float64(b), float64(alpha))
1639		}	1639		}
1640		a.contentCoord.SetPopupConfig(	1640		a.contentCoord.SetPopupConfig(
1641		a.engine.Factory(),	1641		a.engine.Factory(),

M internal/ui/coordinator/content/oauth.go +22 -17


@@ -119,31 +119,36 @@ func shouldForceCloseOnSafetyTimeout() bool {
119	119		}
120	120
121	121		// IsOAuthSuccess checks if the callback indicates successful authentication.
122		-	func IsOAuthSuccess(url string) bool {
123		-	if url == "" {
	122	+	// It parses query and fragment parameters (like IsOAuthCallback) to avoid
	123	+	// false positives from substring matching.
	124	+	func IsOAuthSuccess(rawURL string) bool {
	125	+	if rawURL == "" {
124	126		return false
125	127		}
126		-	lower := strings.ToLower(url)
127		-
128		-	// Check for success indicators
129		-	successPatterns := []string{
130		-	"code=",
131		-	"access_token=",
132		-	"id_token=",
	128	+	u, err := neturl.Parse(rawURL)
	129	+	if err != nil {
	130	+	return false
133	131		}
134		-
135		-	for _, pattern := range successPatterns {
136		-	if strings.Contains(lower, pattern) {
137		-	// Make sure it's not an error response
138		-	if !strings.Contains(lower, "error=") {
139		-	return true
140		-	}
	132	+	if hasOAuthSuccessParam(u.Query()) {
	133	+	return true
	134	+	}
	135	+	if u.Fragment != "" {
	136	+	if fragVals, err := neturl.ParseQuery(u.Fragment); err == nil {
	137	+	return hasOAuthSuccessParam(fragVals)
141	138		}
142	139		}
143		-
144	140		return false
145	141		}
146	142
	143	+	// hasOAuthSuccessParam returns true if params contain a success token
	144	+	// (code, access_token, id_token) and no error key.
	145	+	func hasOAuthSuccessParam(params neturl.Values) bool {
	146	+	if params.Has("error") {
	147	+	return false
	148	+	}
	149	+	return params.Has("code") \|\| params.Has("access_token") \|\| params.Has("id_token")
	150	+	}
	151	+
147	152		// IsOAuthError checks if the callback indicates an authentication error.
148	153		func IsOAuthError(url string) bool {
149	154		if url == "" {


@@ -119,31 +119,36 @@ func shouldForceCloseOnSafetyTimeout() bool {
119		}	119		}
120			120
121		// IsOAuthSuccess checks if the callback indicates successful authentication.	121		// IsOAuthSuccess checks if the callback indicates successful authentication.
122	-	func IsOAuthSuccess(url string) bool {	122	+	// It parses query and fragment parameters (like IsOAuthCallback) to avoid
123	-	if url == "" {	123	+	// false positives from substring matching.
			124	+	func IsOAuthSuccess(rawURL string) bool {
			125	+	if rawURL == "" {
124		return false	126		return false
125		}	127		}
126	-	lower := strings.ToLower(url)	128	+	u, err := neturl.Parse(rawURL)
127	-		129	+	if err != nil {
128	-	// Check for success indicators	130	+	return false
129	-	successPatterns := []string{
130	-	"code=",
131	-	"access_token=",
132	-	"id_token=",
133		}	131		}
134	-		132	+	if hasOAuthSuccessParam(u.Query()) {
135	-	for _, pattern := range successPatterns {	133	+	return true
136	-	if strings.Contains(lower, pattern) {	134	+	}
137	-	// Make sure it's not an error response	135	+	if u.Fragment != "" {
138	-	if !strings.Contains(lower, "error=") {	136	+	if fragVals, err := neturl.ParseQuery(u.Fragment); err == nil {
139	-	return true	137	+	return hasOAuthSuccessParam(fragVals)
140	-	}
141		}	138		}
142		}	139		}
143	-
144		return false	140		return false
145		}	141		}
146			142
			143	+	// hasOAuthSuccessParam returns true if params contain a success token
			144	+	// (code, access_token, id_token) and no error key.
			145	+	func hasOAuthSuccessParam(params neturl.Values) bool {
			146	+	if params.Has("error") {
			147	+	return false
			148	+	}
			149	+	return params.Has("code") \|\| params.Has("access_token") \|\| params.Has("id_token")
			150	+	}
			151	+
147		// IsOAuthError checks if the callback indicates an authentication error.	152		// IsOAuthError checks if the callback indicates an authentication error.
148		func IsOAuthError(url string) bool {	153		func IsOAuthError(url string) bool {
149		if url == "" {	154		if url == "" {

M internal/ui/coordinator/content/oauth_test.go +1 -7


@@ -108,13 +108,7 @@ func TestIsOAuthURL_DetectsOAuthAuthorizeRequest(t *testing.T) {
108	108		}
109	109
110	110		func TestShouldForceCloseOnSafetyTimeout(t *testing.T) {
111		-	t.Run("does not force close during in-progress auth challenge", func(t *testing.T) {
112		-	assert.False(t, shouldForceCloseOnSafetyTimeout())
113		-	})
114		-
115		-	t.Run("does not force close when callback is reached", func(t *testing.T) {
116		-	assert.False(t, shouldForceCloseOnSafetyTimeout())
117		-	})
	111	+	assert.False(t, shouldForceCloseOnSafetyTimeout())
118	112		}
119	113
120	114		func waitFor(t *testing.T, timeout time.Duration, condition func() bool) {

 }
 func TestShouldForceCloseOnSafetyTimeout(t *testing.T) {
-	t.Run("does not force close during in-progress auth challenge", func(t *testing.T) {
+	assert.False(t, shouldForceCloseOnSafetyTimeout())
-		assert.False(t, shouldForceCloseOnSafetyTimeout())
-	})
-	t.Run("does not force close when callback is reached", func(t *testing.T) {
-		assert.False(t, shouldForceCloseOnSafetyTimeout())
-	})
 }
 func waitFor(t *testing.T, timeout time.Duration, condition func() bool) {

M internal/ui/coordinator/content/popup_test.go +5 -1


@@ -6,6 +6,7 @@ import (
6	6		"testing"
7	7
8	8		"github.com/stretchr/testify/assert"
	9	+	"github.com/stretchr/testify/require"
9	10
10	11		"github.com/bnema/dumber/internal/application/port"
11	12		"github.com/bnema/dumber/internal/domain/entity"

@@ -143,7 +144,8 @@ func TestTrackOAuthPopup_StoresState(t *testing.T) {
143	144		state, ok := c.popupOAuth[popupID]
144	145		c.popupMu.RUnlock()
145	146
146		-	assert.True(t, ok, "oauth state should be tracked after trackOAuthPopup")
	147	+	require.True(t, ok, "oauth state should be tracked after trackOAuthPopup")
	148	+	require.NotNil(t, state)
147	149		assert.Equal(t, parentPaneID, state.ParentPaneID)
148	150		assert.False(t, state.Seen, "Seen should be false before capturing callback URI")
149	151		}

@@ -164,6 +166,7 @@ func TestCapturePopupOAuthState_SuccessCallback(t *testing.T) {
164	166		state := c.popupOAuth[popupID]
165	167		c.popupMu.RUnlock()
166	168
	169	+	require.NotNil(t, state)
167	170		assert.True(t, state.Seen)
168	171		assert.True(t, state.Success)
169	172		assert.False(t, state.Error)

@@ -186,6 +189,7 @@ func TestCapturePopupOAuthState_ErrorCallback(t *testing.T) {
186	189		state := c.popupOAuth[popupID]
187	190		c.popupMu.RUnlock()
188	191
	192	+	require.NotNil(t, state)
189	193		assert.True(t, state.Seen)
190	194		assert.False(t, state.Success)
191	195		assert.True(t, state.Error)

 	"testing"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 	"github.com/bnema/dumber/internal/application/port"
 	"github.com/bnema/dumber/internal/domain/entity"
 	state, ok := c.popupOAuth[popupID]
 	c.popupMu.RUnlock()
-	assert.True(t, ok, "oauth state should be tracked after trackOAuthPopup")
+	require.True(t, ok, "oauth state should be tracked after trackOAuthPopup")
+	require.NotNil(t, state)
 	assert.Equal(t, parentPaneID, state.ParentPaneID)
 	assert.False(t, state.Seen, "Seen should be false before capturing callback URI")
 }
 	state := c.popupOAuth[popupID]
 	c.popupMu.RUnlock()
+	require.NotNil(t, state)
 	assert.True(t, state.Seen)
 	assert.True(t, state.Success)
 	assert.False(t, state.Error)
 	state := c.popupOAuth[popupID]
 	c.popupMu.RUnlock()
+	require.NotNil(t, state)
 	assert.True(t, state.Seen)
 	assert.False(t, state.Success)
 	assert.True(t, state.Error)

M internal/ui/dialog/permission.go +6 -5


@@ -227,13 +227,14 @@ func (d *PermissionDialog) buildBody(
227	227		action = "access your device"
228	228		case len(parts) == 1:
229	229		action = parts[0]
230		-	case f.mic && f.cam:
231		-	// When mic+cam are combined into one phrase containing "and",
232		-	// use ", and" before the next part for clarity.
	230	+	case len(parts) == 2 && (!f.mic \|\| !f.cam):
	231	+	// Simple two-part join when neither part contains "and".
	232	+	action = parts[0] + " and " + parts[1]
	233	+	default:
	234	+	// Oxford-comma join for 3+ parts, or when a part already
	235	+	// contains "and" (e.g. "access your microphone and camera").
233	236		action = strings.Join(parts[:len(parts)-1], ", ") +
234	237		", and " + parts[len(parts)-1]
235		-	default:
236		-	action = parts[0] + " and " + parts[1]
237	238		}
238	239
239	240		return fmt.Sprintf("%s wants to %s.", origin, action)

 		action = "access your device"
 	case len(parts) == 1:
 		action = parts[0]
-	case f.mic && f.cam:
+	case len(parts) == 2 && (!f.mic || !f.cam):
-		// When mic+cam are combined into one phrase containing "and",
+		// Simple two-part join when neither part contains "and".
-		// use ", and" before the next part for clarity.
+		action = parts[0] + " and " + parts[1]
+	default:
+		// Oxford-comma join for 3+ parts, or when a part already
+		// contains "and" (e.g. "access your microphone and camera").
 		action = strings.Join(parts[:len(parts)-1], ", ") +
 			", and " + parts[len(parts)-1]
-	default:
-		action = parts[0] + " and " + parts[1]
 	}
 	return fmt.Sprintf("%s wants to %s.", origin, action)

M internal/ui/input/shortcuts.go +20 -0


@@ -287,6 +287,19 @@ func (s *ShortcutSet) registerActivationShortcutsFromParts(
287	287		) {
288	288		log := logging.FromContext(ctx)
289	289		if workspace == nil {
	290	+	// Still register session mode if available.
	291	+	if session != nil {
	292	+	if binding, ok := ParseKeyString(session.SessionMode.ActivationShortcut); ok {
	293	+	s.Global[binding] = ActionEnterSessionMode
	294	+	log.Trace().
	295	+	Str("shortcut", session.SessionMode.ActivationShortcut).
	296	+	Uint("keyval", binding.Keyval).
	297	+	Uint("mod", uint(binding.Modifiers)).
	298	+	Msg("session mode activation registered")
	299	+	} else {
	300	+	log.Warn().Str("shortcut", session.SessionMode.ActivationShortcut).Msg("failed to parse session mode activation shortcut")
	301	+	}
	302	+	}
290	303		return
291	304		}
292	305		if binding, ok := ParseKeyString(workspace.TabMode.ActivationShortcut); ok {

@@ -352,12 +365,19 @@ func (s ShortcutSet) registerConfiguredShortcuts(cfg entity.WorkspaceConfig) {
352	365		"toggle_floating_pane": ActionToggleFloatingPane,
353	366		"toggle-floating-pane": ActionToggleFloatingPane,
354	367		"close_pane": ActionClosePane,
	368	+	"close-pane": ActionClosePane,
355	369		"next_tab": ActionNextTab,
	370	+	"next-tab": ActionNextTab,
356	371		"previous_tab": ActionPreviousTab,
	372	+	"previous-tab": ActionPreviousTab,
357	373		"consume_or_expel_left": ActionConsumeOrExpelLeft,
	374	+	"consume-or-expel-left": ActionConsumeOrExpelLeft,
358	375		"consume_or_expel_right": ActionConsumeOrExpelRight,
	376	+	"consume-or-expel-right": ActionConsumeOrExpelRight,
359	377		"consume_or_expel_up": ActionConsumeOrExpelUp,
	378	+	"consume-or-expel-up": ActionConsumeOrExpelUp,
360	379		"consume_or_expel_down": ActionConsumeOrExpelDown,
	380	+	"consume-or-expel-down": ActionConsumeOrExpelDown,
361	381		}
362	382
363	383		for actionName, actionBinding := range cfg.Shortcuts.Actions {


@@ -287,6 +287,19 @@ func (s *ShortcutSet) registerActivationShortcutsFromParts(
287	) {	287		) {
288	log := logging.FromContext(ctx)	288		log := logging.FromContext(ctx)
289	if workspace == nil {	289		if workspace == nil {
		290	+	// Still register session mode if available.
		291	+	if session != nil {
		292	+	if binding, ok := ParseKeyString(session.SessionMode.ActivationShortcut); ok {
		293	+	s.Global[binding] = ActionEnterSessionMode
		294	+	log.Trace().
		295	+	Str("shortcut", session.SessionMode.ActivationShortcut).
		296	+	Uint("keyval", binding.Keyval).
		297	+	Uint("mod", uint(binding.Modifiers)).
		298	+	Msg("session mode activation registered")
		299	+	} else {
		300	+	log.Warn().Str("shortcut", session.SessionMode.ActivationShortcut).Msg("failed to parse session mode activation shortcut")
		301	+	}
		302	+	}
290	return	303		return
291	}	304		}
292	if binding, ok := ParseKeyString(workspace.TabMode.ActivationShortcut); ok {	305		if binding, ok := ParseKeyString(workspace.TabMode.ActivationShortcut); ok {

@@ -352,12 +365,19 @@ func (s ShortcutSet) registerConfiguredShortcuts(cfg entity.WorkspaceConfig) {
352	"toggle_floating_pane": ActionToggleFloatingPane,	365		"toggle_floating_pane": ActionToggleFloatingPane,
353	"toggle-floating-pane": ActionToggleFloatingPane,	366		"toggle-floating-pane": ActionToggleFloatingPane,
354	"close_pane": ActionClosePane,	367		"close_pane": ActionClosePane,
		368	+	"close-pane": ActionClosePane,
355	"next_tab": ActionNextTab,	369		"next_tab": ActionNextTab,
		370	+	"next-tab": ActionNextTab,
356	"previous_tab": ActionPreviousTab,	371		"previous_tab": ActionPreviousTab,
		372	+	"previous-tab": ActionPreviousTab,
357	"consume_or_expel_left": ActionConsumeOrExpelLeft,	373		"consume_or_expel_left": ActionConsumeOrExpelLeft,
		374	+	"consume-or-expel-left": ActionConsumeOrExpelLeft,
358	"consume_or_expel_right": ActionConsumeOrExpelRight,	375		"consume_or_expel_right": ActionConsumeOrExpelRight,
		376	+	"consume-or-expel-right": ActionConsumeOrExpelRight,
359	"consume_or_expel_up": ActionConsumeOrExpelUp,	377		"consume_or_expel_up": ActionConsumeOrExpelUp,
		378	+	"consume-or-expel-up": ActionConsumeOrExpelUp,
360	"consume_or_expel_down": ActionConsumeOrExpelDown,	379		"consume_or_expel_down": ActionConsumeOrExpelDown,
		380	+	"consume-or-expel-down": ActionConsumeOrExpelDown,
361	}	381		}
362		382
363	for actionName, actionBinding := range cfg.Shortcuts.Actions {	383		for actionName, actionBinding := range cfg.Shortcuts.Actions {