5967a65 fix: address CodeRabbit review findings and fix media config migration loop · dumber

M internal/bootstrap/gui.go +4 -0


@@ -332,6 +332,10 @@ func RunParallelDBEngine(input ParallelDBEngineInput) (*ParallelDBEngineResult,
332	332		Logger: *log,
333	333		})
334	334		if err != nil {
	335	+	dbRes := <-dbCh
	336	+	if dbRes.err == nil && dbRes.cleanup != nil {
	337	+	dbRes.cleanup()
	338	+	}
335	339		return nil, fmt.Errorf("engine initialization: %w", err)
336	340		}
337	341


@@ -332,6 +332,10 @@ func RunParallelDBEngine(input ParallelDBEngineInput) (*ParallelDBEngineResult,
332	Logger: *log,	332		Logger: *log,
333	})	333		})
334	if err != nil {	334		if err != nil {
		335	+	dbRes := <-dbCh
		336	+	if dbRes.err == nil && dbRes.cleanup != nil {
		337	+	dbRes.cleanup()
		338	+	}
335	return nil, fmt.Errorf("engine initialization: %w", err)	339		return nil, fmt.Errorf("engine initialization: %w", err)
336	}	340		}
337		341

M internal/infrastructure/config/loader.go +3 -16


@@ -158,15 +158,15 @@ func (m *Manager) transformLegacyActionBindings() {
158	158		// checkLegacyFormat detects old config format and returns an error directing user to migrate.
159	159		func (m *Manager) checkLegacyFormat() error {
160	160		// Check if old sections exist by looking for known keys.
161		-	// IsSet returns true only for explicitly set values (not SetDefault), so this
162		-	// correctly identifies keys present in the config file, env vars, or overrides.
	161	+	// IsSet works for legacy keys because they do not have defaults, while
	162	+	// engine.type has a default and must be checked with InConfig.
163	163		hasOldSections := m.viper.IsSet("rendering.mode") \|\|
164	164		m.viper.IsSet("rendering.disable_dmabuf_renderer") \|\|
165	165		m.viper.IsSet("performance.profile") \|\|
166	166		m.viper.IsSet("privacy.cookie_policy") \|\|
167	167		m.viper.IsSet("runtime.prefix")
168	168
169		-	hasEngineSection := m.viper.IsSet("engine.type")
	169	+	hasEngineSection := m.viper.InConfig("engine.type")
170	170
171	171		if hasOldSections && !hasEngineSection {
172	172		return fmt.Errorf(

@@ -278,19 +278,6 @@ func normalizeMedia(config *Config) {
278	278		default:
279	279		config.Media.HardwareDecodingMode = HardwareDecodingAuto
280	280		}
281		-
282		-	switch strings.ToLower(string(config.Media.GLRenderingMode)) {
283		-	case "", string(GLRenderingModeAuto):
284		-	config.Media.GLRenderingMode = GLRenderingModeAuto
285		-	case string(GLRenderingModeGLES2):
286		-	config.Media.GLRenderingMode = GLRenderingModeGLES2
287		-	case string(GLRenderingModeGL3):
288		-	config.Media.GLRenderingMode = GLRenderingModeGL3
289		-	case string(GLRenderingModeNone):
290		-	config.Media.GLRenderingMode = GLRenderingModeNone
291		-	default:
292		-	config.Media.GLRenderingMode = GLRenderingModeAuto
293		-	}
294	281		}
295	282
296	283		// Get returns the current configuration (thread-safe).

 // checkLegacyFormat detects old config format and returns an error directing user to migrate.
 func (m *Manager) checkLegacyFormat() error {
 	// Check if old sections exist by looking for known keys.
-	// IsSet returns true only for explicitly set values (not SetDefault), so this
+	// IsSet works for legacy keys because they do not have defaults, while
-	// correctly identifies keys present in the config file, env vars, or overrides.
+	// engine.type has a default and must be checked with InConfig.
 	hasOldSections := m.viper.IsSet("rendering.mode") ||
 		m.viper.IsSet("rendering.disable_dmabuf_renderer") ||
 		m.viper.IsSet("performance.profile") ||
 		m.viper.IsSet("privacy.cookie_policy") ||
 		m.viper.IsSet("runtime.prefix")
-	hasEngineSection := m.viper.IsSet("engine.type")
+	hasEngineSection := m.viper.InConfig("engine.type")
 	if hasOldSections && !hasEngineSection {
 		return fmt.Errorf(
 	default:
 		config.Media.HardwareDecodingMode = HardwareDecodingAuto
 	}
-	switch strings.ToLower(string(config.Media.GLRenderingMode)) {
-	case "", string(GLRenderingModeAuto):
-		config.Media.GLRenderingMode = GLRenderingModeAuto
-	case string(GLRenderingModeGLES2):
-		config.Media.GLRenderingMode = GLRenderingModeGLES2
-	case string(GLRenderingModeGL3):
-		config.Media.GLRenderingMode = GLRenderingModeGL3
-	case string(GLRenderingModeNone):
-		config.Media.GLRenderingMode = GLRenderingModeNone
-	default:
-		config.Media.GLRenderingMode = GLRenderingModeAuto
-	}
 }
 // Get returns the current configuration (thread-safe).

M internal/infrastructure/config/loader_legacy_test.go +3 -2


@@ -17,14 +17,15 @@ func TestCheckLegacyFormat_OldSectionsNoEngine_ReturnsError(t *testing.T) {
17	17		require.Contains(t, err.Error(), "dumber migrate")
18	18		}
19	19
20		-	func TestCheckLegacyFormat_OldSectionsWithEngine_ReturnsNoError(t *testing.T) {
	20	+	func TestCheckLegacyFormat_OldSectionsWithEngine_ReturnsError(t *testing.T) {
21	21		m := &Manager{viper: viper.New()}
22	22		m.viper.Set("rendering.mode", "gpu")
23	23		m.viper.Set("engine.type", "webkit")
24	24
25	25		err := m.checkLegacyFormat()
26	26
27		-	require.NoError(t, err)
	27	+	require.Error(t, err)
	28	+	require.Contains(t, err.Error(), "dumber migrate")
28	29		}
29	30
30	31		func TestCheckLegacyFormat_FreshConfig_ReturnsNoError(t *testing.T) {

 	require.Contains(t, err.Error(), "dumber migrate")
 }
-func TestCheckLegacyFormat_OldSectionsWithEngine_ReturnsNoError(t *testing.T) {
+func TestCheckLegacyFormat_OldSectionsWithEngine_ReturnsError(t *testing.T) {
 	m := &Manager{viper: viper.New()}
 	m.viper.Set("rendering.mode", "gpu")
 	m.viper.Set("engine.type", "webkit")
 	err := m.checkLegacyFormat()
-	require.NoError(t, err)
+	require.Error(t, err)
+	require.Contains(t, err.Error(), "dumber migrate")
 }
 func TestCheckLegacyFormat_FreshConfig_ReturnsNoError(t *testing.T) {

M internal/infrastructure/config/migrate_engine.go +22 -0

@@ -1,6 +1,7 @@
1	1		package config
2	2
3	3		import (
	4	+	"fmt"
4	5		"io/fs"
5	6		"os"
6	7

@@ -93,6 +94,27 @@ func MigrateToEngineConfig(configFile string) (bool, error) {
93	94		}
94	95
95	96		if _, exists := raw["engine"]; exists {
	97	+	for _, section := range []string{"rendering", "performance", "privacy", "runtime"} {
	98	+	if _, hasLegacy := raw[section]; hasLegacy {
	99	+	return false, fmt.Errorf(
	100	+	"mixed old/new config: [engine] coexists with legacy [%s]; "+
	101	+	"remove [rendering], [performance], [privacy], [runtime]",
	102	+	section,
	103	+	)
	104	+	}
	105	+	}
	106	+	// Check for deprecated media keys that moved to [engine.webkit].
	107	+	if mediaRaw, ok := raw["media"].(map[string]any); ok {
	108	+	for _, key := range []string{"force_vsync", "gl_rendering_mode", "gstreamer_debug_level"} {
	109	+	if _, has := mediaRaw[key]; has {
	110	+	return false, fmt.Errorf(
	111	+	"mixed old/new config: [engine] coexists with deprecated "+
	112	+	"media.%s; remove it from [media] (now in [engine.webkit])",
	113	+	key,
	114	+	)
	115	+	}
	116	+	}
	117	+	}
96	118		return false, nil // already migrated
97	119		}
98	120

@@ -1,6 +1,7 @@
1	package config	1		package config
2		2
3	import (	3		import (
		4	+	"fmt"
4	"io/fs"	5		"io/fs"
5	"os"	6		"os"
6		7

@@ -93,6 +94,27 @@ func MigrateToEngineConfig(configFile string) (bool, error) {
93	}	94		}
94		95
95	if _, exists := raw["engine"]; exists {	96		if _, exists := raw["engine"]; exists {
		97	+	for _, section := range []string{"rendering", "performance", "privacy", "runtime"} {
		98	+	if _, hasLegacy := raw[section]; hasLegacy {
		99	+	return false, fmt.Errorf(
		100	+	"mixed old/new config: [engine] coexists with legacy [%s]; "+
		101	+	"remove [rendering], [performance], [privacy], [runtime]",
		102	+	section,
		103	+	)
		104	+	}
		105	+	}
		106	+	// Check for deprecated media keys that moved to [engine.webkit].
		107	+	if mediaRaw, ok := raw["media"].(map[string]any); ok {
		108	+	for _, key := range []string{"force_vsync", "gl_rendering_mode", "gstreamer_debug_level"} {
		109	+	if _, has := mediaRaw[key]; has {
		110	+	return false, fmt.Errorf(
		111	+	"mixed old/new config: [engine] coexists with deprecated "+
		112	+	"media.%s; remove it from [media] (now in [engine.webkit])",
		113	+	key,
		114	+	)
		115	+	}
		116	+	}
		117	+	}
96	return false, nil // already migrated	118		return false, nil // already migrated
97	}	119		}
98		120

M internal/infrastructure/config/schema.go +12 -6


@@ -116,13 +116,19 @@ type MediaConfig struct {
116	116		PreferAV1 bool `mapstructure:"prefer_av1" yaml:"prefer_av1" toml:"prefer_av1"`
117	117		// ShowDiagnosticsOnStartup shows media capability warnings at startup
118	118		ShowDiagnosticsOnStartup bool `mapstructure:"show_diagnostics" yaml:"show_diagnostics" toml:"show_diagnostics"`
119		-	// ForceVSync forces vertical sync for video playback (may help with tearing)
120		-	ForceVSync bool `mapstructure:"force_vsync" yaml:"force_vsync" toml:"force_vsync"`
121		-	// GLRenderingMode controls OpenGL API selection for video rendering
	119	+	// ForceVSync forces vertical sync for video playback (may help with tearing).
	120	+	//
	121	+	// Deprecated: moved to [engine.webkit]. Kept for read compatibility during migration.
	122	+	ForceVSync bool `mapstructure:"force_vsync" yaml:"force_vsync" toml:"-"`
	123	+	// GLRenderingMode controls OpenGL API selection for video rendering.
122	124		// Values: "auto" (default), "gles2", "gl3", "none"
123		-	GLRenderingMode GLRenderingMode `mapstructure:"gl_rendering_mode" yaml:"gl_rendering_mode" toml:"gl_rendering_mode"`
124		-	// GStreamerDebugLevel sets GStreamer debug verbosity (0=off, 1-5=increasing verbosity)
125		-	GStreamerDebugLevel int `mapstructure:"gstreamer_debug_level" yaml:"gstreamer_debug_level" toml:"gstreamer_debug_level"`
	125	+	//
	126	+	// Deprecated: moved to [engine.webkit]. Kept for read compatibility during migration.
	127	+	GLRenderingMode GLRenderingMode `mapstructure:"gl_rendering_mode" yaml:"gl_rendering_mode" toml:"-"`
	128	+	// GStreamerDebugLevel sets GStreamer debug verbosity (0=off, 1-5=increasing verbosity).
	129	+	//
	130	+	// Deprecated: moved to [engine.webkit]. Kept for read compatibility during migration.
	131	+	GStreamerDebugLevel int `mapstructure:"gstreamer_debug_level" yaml:"gstreamer_debug_level" toml:"-"`
126	132		}
127	133
128	134		// DatabaseConfig holds database-related configuration.

 	PreferAV1 bool `mapstructure:"prefer_av1" yaml:"prefer_av1" toml:"prefer_av1"`
 	// ShowDiagnosticsOnStartup shows media capability warnings at startup
 	ShowDiagnosticsOnStartup bool `mapstructure:"show_diagnostics" yaml:"show_diagnostics" toml:"show_diagnostics"`
-	// ForceVSync forces vertical sync for video playback (may help with tearing)
+	// ForceVSync forces vertical sync for video playback (may help with tearing).
-	ForceVSync bool `mapstructure:"force_vsync" yaml:"force_vsync" toml:"force_vsync"`
+	//
-	// GLRenderingMode controls OpenGL API selection for video rendering
+	// Deprecated: moved to [engine.webkit]. Kept for read compatibility during migration.
+	ForceVSync bool `mapstructure:"force_vsync" yaml:"force_vsync" toml:"-"`
+	// GLRenderingMode controls OpenGL API selection for video rendering.
 	// Values: "auto" (default), "gles2", "gl3", "none"
-	GLRenderingMode GLRenderingMode `mapstructure:"gl_rendering_mode" yaml:"gl_rendering_mode" toml:"gl_rendering_mode"`
+	//
-	// GStreamerDebugLevel sets GStreamer debug verbosity (0=off, 1-5=increasing verbosity)
+	// Deprecated: moved to [engine.webkit]. Kept for read compatibility during migration.
-	GStreamerDebugLevel int `mapstructure:"gstreamer_debug_level" yaml:"gstreamer_debug_level" toml:"gstreamer_debug_level"`
+	GLRenderingMode GLRenderingMode `mapstructure:"gl_rendering_mode" yaml:"gl_rendering_mode" toml:"-"`
+	// GStreamerDebugLevel sets GStreamer debug verbosity (0=off, 1-5=increasing verbosity).
+	//
+	// Deprecated: moved to [engine.webkit]. Kept for read compatibility during migration.
+	GStreamerDebugLevel int `mapstructure:"gstreamer_debug_level" yaml:"gstreamer_debug_level" toml:"-"`
 }
 // DatabaseConfig holds database-related configuration.

M internal/infrastructure/config/schema_provider.go +0 -23


@@ -795,14 +795,6 @@ func (SchemaProvider) getMediaKeys(defaults Config) []entity.ConfigKeyInfo {
795	795		Values: []string{"auto", "force", "disable"},
796	796		Section: SectionMedia,
797	797		},
798		-	{
799		-	Key: "media.gl_rendering_mode",
800		-	Type: "string",
801		-	Default: string(defaults.Media.GLRenderingMode),
802		-	Description: "OpenGL API selection for video rendering",
803		-	Values: []string{"auto", "gles2", "gl3", "none"},
804		-	Section: SectionMedia,
805		-	},
806	798		{
807	799		Key: "media.prefer_av1",
808	800		Type: "bool",

@@ -817,21 +809,6 @@ func (SchemaProvider) getMediaKeys(defaults Config) []entity.ConfigKeyInfo {
817	809		Description: "Show media capability warnings at startup",
818	810		Section: SectionMedia,
819	811		},
820		-	{
821		-	Key: "media.force_vsync",
822		-	Type: "bool",
823		-	Default: fmt.Sprintf("%t", defaults.Media.ForceVSync),
824		-	Description: "Force vertical sync for video playback",
825		-	Section: SectionMedia,
826		-	},
827		-	{
828		-	Key: "media.gstreamer_debug_level",
829		-	Type: "int",
830		-	Default: fmt.Sprintf("%d", defaults.Media.GStreamerDebugLevel),
831		-	Description: "GStreamer debug verbosity (0=off, 1-5)",
832		-	Range: "0-5",
833		-	Section: SectionMedia,
834		-	},
835	812		}
836	813		}
837	814


@@ -795,14 +795,6 @@ func (SchemaProvider) getMediaKeys(defaults Config) []entity.ConfigKeyInfo {
795		Values: []string{"auto", "force", "disable"},	795	Values: []string{"auto", "force", "disable"},
796		Section: SectionMedia,	796	Section: SectionMedia,
797		},	797	},
798	-	{
799	-	Key: "media.gl_rendering_mode",
800	-	Type: "string",
801	-	Default: string(defaults.Media.GLRenderingMode),
802	-	Description: "OpenGL API selection for video rendering",
803	-	Values: []string{"auto", "gles2", "gl3", "none"},
804	-	Section: SectionMedia,
805	-	},
806		{	798	{
807		Key: "media.prefer_av1",	799	Key: "media.prefer_av1",
808		Type: "bool",	800	Type: "bool",

@@ -817,21 +809,6 @@ func (SchemaProvider) getMediaKeys(defaults Config) []entity.ConfigKeyInfo {
817		Description: "Show media capability warnings at startup",	809	Description: "Show media capability warnings at startup",
818		Section: SectionMedia,	810	Section: SectionMedia,
819		},	811	},
820	-	{
821	-	Key: "media.force_vsync",
822	-	Type: "bool",
823	-	Default: fmt.Sprintf("%t", defaults.Media.ForceVSync),
824	-	Description: "Force vertical sync for video playback",
825	-	Section: SectionMedia,
826	-	},
827	-	{
828	-	Key: "media.gstreamer_debug_level",
829	-	Type: "int",
830	-	Default: fmt.Sprintf("%d", defaults.Media.GStreamerDebugLevel),
831	-	Description: "GStreamer debug verbosity (0=off, 1-5)",
832	-	Range: "0-5",
833	-	Section: SectionMedia,
834	-	},
835		}	812	}
836		}	813	}
837			814

M internal/infrastructure/webkit/context.go +3 -1


@@ -193,8 +193,10 @@ func mapCookiePolicy(policy cookiePolicy) (webkit.CookieAcceptPolicy, string) {
193	193		return webkit.CookiePolicyAcceptAlwaysValue, string(cookiePolicyAlways)
194	194		case cookiePolicyNever:
195	195		return webkit.CookiePolicyAcceptNeverValue, string(cookiePolicyNever)
196		-	case cookiePolicyNoThirdParty, "":
	196	+	case cookiePolicyNoThirdParty:
197	197		return webkit.CookiePolicyAcceptNoThirdPartyValue, string(cookiePolicyNoThirdParty)
	198	+	case "":
	199	+	return webkit.CookiePolicyAcceptNoThirdPartyValue, ""
198	200		default:
199	201		return webkit.CookiePolicyAcceptNoThirdPartyValue, string(cookiePolicyNoThirdParty)
200	202		}

 		return webkit.CookiePolicyAcceptAlwaysValue, string(cookiePolicyAlways)
 	case cookiePolicyNever:
 		return webkit.CookiePolicyAcceptNeverValue, string(cookiePolicyNever)
-	case cookiePolicyNoThirdParty, "":
+	case cookiePolicyNoThirdParty:
 		return webkit.CookiePolicyAcceptNoThirdPartyValue, string(cookiePolicyNoThirdParty)
+	case "":
+		return webkit.CookiePolicyAcceptNoThirdPartyValue, ""
 	default:
 		return webkit.CookiePolicyAcceptNoThirdPartyValue, string(cookiePolicyNoThirdParty)
 	}

M internal/infrastructure/webkit/engine_init.go +1 -4


@@ -202,10 +202,7 @@ func engineBuildContextOptions(
202	202		wkCfg WebKitEngineConfig,
203	203		perfSettings *config.ResolvedPerformanceSettings,
204	204		) webKitContextOptions {
205		-	cp := cookiePolicyNoThirdParty
206		-	if opts.CookiePolicy != "" {
207		-	cp = opts.CookiePolicy
208		-	}
	205	+	cp := opts.CookiePolicy // empty preserves runtime default per port contract
209	206
210	207		wkOpts := webKitContextOptions{
211	208		DataDir: opts.DataDir,

 	wkCfg WebKitEngineConfig,
 	perfSettings *config.ResolvedPerformanceSettings,
 ) webKitContextOptions {
-	cp := cookiePolicyNoThirdParty
+	cp := opts.CookiePolicy // empty preserves runtime default per port contract
-	if opts.CookiePolicy != "" {
-		cp = opts.CookiePolicy
-	}
 	wkOpts := webKitContextOptions{
 		DataDir:      opts.DataDir,

M internal/ui/coordinator/content/navigation.go +9 -0


@@ -120,6 +120,15 @@ func (c *Coordinator) notifyActiveNavigation(paneID entity.PaneID, uri string) {
120	120		if c.onActiveNavigationCommitted == nil {
121	121		return
122	122		}
	123	+	c.activePaneOverrideMu.RLock()
	124	+	override := c.activePaneOverride
	125	+	c.activePaneOverrideMu.RUnlock()
	126	+	if override != "" {
	127	+	if override == paneID {
	128	+	c.onActiveNavigationCommitted(uri)
	129	+	}
	130	+	return
	131	+	}
123	132		ws, _ := c.getActiveWS()
124	133		if ws != nil && ws.ActivePaneID == paneID {
125	134		c.onActiveNavigationCommitted(uri)


@@ -120,6 +120,15 @@ func (c *Coordinator) notifyActiveNavigation(paneID entity.PaneID, uri string) {
120	if c.onActiveNavigationCommitted == nil {	120		if c.onActiveNavigationCommitted == nil {
121	return	121		return
122	}	122		}
		123	+	c.activePaneOverrideMu.RLock()
		124	+	override := c.activePaneOverride
		125	+	c.activePaneOverrideMu.RUnlock()
		126	+	if override != "" {
		127	+	if override == paneID {
		128	+	c.onActiveNavigationCommitted(uri)
		129	+	}
		130	+	return
		131	+	}
123	ws, _ := c.getActiveWS()	132		ws, _ := c.getActiveWS()
124	if ws != nil && ws.ActivePaneID == paneID {	133		if ws != nil && ws.ActivePaneID == paneID {
125	c.onActiveNavigationCommitted(uri)	134		c.onActiveNavigationCommitted(uri)

M internal/ui/coordinator/content/oauth.go +35 -24


@@ -2,6 +2,7 @@ package content
2	2
3	3		import (
4	4		"context"
	5	+	neturl "net/url"
5	6		"strings"
6	7		"sync"
7	8		"time"

@@ -33,21 +34,6 @@ var oauthFlowPatterns = []string{
33	34		"/cb",
34	35		}
35	36
36		-	// oauthCallbackPatterns are URL query parameters that indicate an OAuth callback.
37		-	// These appear when the OAuth provider redirects back to the application.
38		-	var oauthCallbackPatterns = []string{
39		-	// Success parameters
40		-	"code=",
41		-	"access_token=",
42		-	"id_token=",
43		-	"token_type=",
44		-	"refresh_token=",
45		-	// Error parameters
46		-	"error=",
47		-	"error_description=",
48		-	"error_uri=",
49		-	}
50		-
51	37		// oauthRequestPatterns are query parameters found in OAuth authorization requests.
52	38		var oauthRequestPatterns = []string{
53	39		"response_type=",

@@ -97,15 +83,18 @@ func IsOAuthCallback(url string) bool {
97	83		if url == "" {
98	84		return false
99	85		}
100		-	lower := strings.ToLower(url)
101		-
102		-	// Must contain callback patterns (code=, access_token=, error=, etc.)
103		-	for _, pattern := range oauthCallbackPatterns {
104		-	if strings.Contains(lower, pattern) {
105		-	return true
	86	+	u, err := neturl.Parse(url)
	87	+	if err != nil {
	88	+	return false
	89	+	}
	90	+	if hasOAuthCallbackParam(u.Query()) {
	91	+	return true
	92	+	}
	93	+	if u.Fragment != "" {
	94	+	if fragVals, err := neturl.ParseQuery(u.Fragment); err == nil {
	95	+	return hasOAuthCallbackParam(fragVals)
106	96		}
107	97		}
108		-
109	98		return false
110	99		}
111	100

@@ -161,8 +150,30 @@ func IsOAuthError(url string) bool {
161	150		if url == "" {
162	151		return false
163	152		}
164		-	lower := strings.ToLower(url)
165		-	return strings.Contains(lower, "error=")
	153	+	u, err := neturl.Parse(url)
	154	+	if err != nil {
	155	+	return false
	156	+	}
	157	+	if _, ok := u.Query()["error"]; ok {
	158	+	return true
	159	+	}
	160	+	if u.Fragment != "" {
	161	+	if fragVals, err := neturl.ParseQuery(u.Fragment); err == nil {
	162	+	if _, ok := fragVals["error"]; ok {
	163	+	return true
	164	+	}
	165	+	}
	166	+	}
	167	+	return false
	168	+	}
	169	+
	170	+	func hasOAuthCallbackParam(v neturl.Values) bool {
	171	+	for _, k := range []string{"code", "access_token", "id_token", "token_type", "refresh_token", "error", "error_description", "error_uri"} {
	172	+	if _, ok := v[k]; ok {
	173	+	return true
	174	+	}
	175	+	}
	176	+	return false
166	177		}
167	178
168	179		func composeOnClose(existing, next func()) func() {


@@ -2,6 +2,7 @@ package content
2			2
3		import (	3		import (
4		"context"	4		"context"
			5	+	neturl "net/url"
5		"strings"	6		"strings"
6		"sync"	7		"sync"
7		"time"	8		"time"

@@ -33,21 +34,6 @@ var oauthFlowPatterns = []string{
33		"/cb",	34		"/cb",
34		}	35		}
35			36
36	-	// oauthCallbackPatterns are URL query parameters that indicate an OAuth callback.
37	-	// These appear when the OAuth provider redirects back to the application.
38	-	var oauthCallbackPatterns = []string{
39	-	// Success parameters
40	-	"code=",
41	-	"access_token=",
42	-	"id_token=",
43	-	"token_type=",
44	-	"refresh_token=",
45	-	// Error parameters
46	-	"error=",
47	-	"error_description=",
48	-	"error_uri=",
49	-	}
50	-
51		// oauthRequestPatterns are query parameters found in OAuth authorization requests.	37		// oauthRequestPatterns are query parameters found in OAuth authorization requests.
52		var oauthRequestPatterns = []string{	38		var oauthRequestPatterns = []string{
53		"response_type=",	39		"response_type=",

@@ -97,15 +83,18 @@ func IsOAuthCallback(url string) bool {
97		if url == "" {	83		if url == "" {
98		return false	84		return false
99		}	85		}
100	-	lower := strings.ToLower(url)	86	+	u, err := neturl.Parse(url)
101	-		87	+	if err != nil {
102	-	// Must contain callback patterns (code=, access_token=, error=, etc.)	88	+	return false
103	-	for _, pattern := range oauthCallbackPatterns {	89	+	}
104	-	if strings.Contains(lower, pattern) {	90	+	if hasOAuthCallbackParam(u.Query()) {
105	-	return true	91	+	return true
			92	+	}
			93	+	if u.Fragment != "" {
			94	+	if fragVals, err := neturl.ParseQuery(u.Fragment); err == nil {
			95	+	return hasOAuthCallbackParam(fragVals)
106		}	96		}
107		}	97		}
108	-
109		return false	98		return false
110		}	99		}
111			100

@@ -161,8 +150,30 @@ func IsOAuthError(url string) bool {
161		if url == "" {	150		if url == "" {
162		return false	151		return false
163		}	152		}
164	-	lower := strings.ToLower(url)	153	+	u, err := neturl.Parse(url)
165	-	return strings.Contains(lower, "error=")	154	+	if err != nil {
			155	+	return false
			156	+	}
			157	+	if _, ok := u.Query()["error"]; ok {
			158	+	return true
			159	+	}
			160	+	if u.Fragment != "" {
			161	+	if fragVals, err := neturl.ParseQuery(u.Fragment); err == nil {
			162	+	if _, ok := fragVals["error"]; ok {
			163	+	return true
			164	+	}
			165	+	}
			166	+	}
			167	+	return false
			168	+	}
			169	+
			170	+	func hasOAuthCallbackParam(v neturl.Values) bool {
			171	+	for _, k := range []string{"code", "access_token", "id_token", "token_type", "refresh_token", "error", "error_description", "error_uri"} {
			172	+	if _, ok := v[k]; ok {
			173	+	return true
			174	+	}
			175	+	}
			176	+	return false
166		}	177		}
167			178
168		func composeOnClose(existing, next func()) func() {	179		func composeOnClose(existing, next func()) func() {

M internal/ui/coordinator/content/popup.go +15 -7


@@ -49,6 +49,9 @@ func DetectPopupType(frameName string) PopupType {
49	49		// inserted into the workspace. This is used during the three-phase popup
50	50		// lifecycle: create → ready-to-show → (insert into workspace).
51	51		type PendingPopup struct {
	52	+	// PaneID is the popup pane created during the create phase.
	53	+	PaneID entity.PaneID
	54	+
52	55		// WebView is the related WebView created in the "create" phase.
53	56		// It shares cookies/session with the parent for OAuth support.
54	57		WebView port.WebView

@@ -205,7 +208,7 @@ func (c *Coordinator) handlePopupCreate(
205	208
206	209		log.Debug().
207	210		Str("parent_pane", string(parentPaneID)).
208		-	Str("target_uri", req.TargetURI).
	211	+	Str("target_uri", logging.TruncateURL(req.TargetURI, logURLMaxLen)).
209	212		Str("frame_name", req.FrameName).
210	213		Bool("user_gesture", req.IsUserGesture).
211	214		Msg("popup create request")

@@ -291,6 +294,7 @@ func (c *Coordinator) handlePopupCreate(
291	294
292	295		// Store pending popup for ready-to-show handling (just visibility now)
293	296		pending := &PendingPopup{
	297	+	PaneID: paneID,
294	298		WebView: popupWV,
295	299		ParentPaneID: parentPaneID,
296	300		ParentWebViewID: parentID,

@@ -380,10 +384,14 @@ func (c *Coordinator) handlePopupClose(ctx context.Context, popupID port.WebView
380	384		c.popupMu.Unlock()
381	385
382	386		if wasPending && pending != nil {
	387	+	if c.onClosePane != nil {
	388	+	if err := c.onClosePane(ctx, pending.PaneID); err != nil {
	389	+	log.Warn().Err(err).Str("pane_id", string(pending.PaneID)).Msg("failed to close pending popup pane")
	390	+	}
	391	+	}
383	392		c.handlePopupOAuthClose(ctx, popupID)
384		-	// Was never shown, just destroy
385		-	pending.WebView.Destroy()
386		-	log.Debug().Msg("destroyed pending popup that was never shown")
	393	+	c.ReleaseWebView(ctx, pending.PaneID)
	394	+	log.Debug().Str("pane_id", string(pending.PaneID)).Msg("cleaned up pending popup that was never shown")
387	395		return
388	396		}
389	397

@@ -426,7 +434,7 @@ func (c *Coordinator) handleLinkMiddleClick(ctx context.Context, parentPaneID en
426	434
427	435		log.Info().
428	436		Str("parent_pane", string(parentPaneID)).
429		-	Str("uri", uri).
	437	+	Str("uri", logging.TruncateURL(uri, logURLMaxLen)).
430	438		Msg("middle-click/ctrl+click on link")
431	439
432	440		// Check if popups are enabled

@@ -500,13 +508,13 @@ func (c *Coordinator) handleLinkMiddleClick(ctx context.Context, parentPaneID en
500	508
501	509		// Load the URI after insertion
502	510		if err := newWV.LoadURI(ctx, uri); err != nil {
503		-	log.Error().Err(err).Str("uri", uri).Msg("failed to load URI in new pane")
	511	+	log.Error().Err(err).Str("uri", logging.TruncateURL(uri, logURLMaxLen)).Msg("failed to load URI in new pane")
504	512		}
505	513
506	514		log.Info().
507	515		Str("pane_id", string(paneID)).
508	516		Str("behavior", string(behavior)).
509		-	Str("uri", uri).
	517	+	Str("uri", logging.TruncateURL(uri, logURLMaxLen)).
510	518		Msg("middle-click link opened in new pane")
511	519
512	520		return true

 // inserted into the workspace. This is used during the three-phase popup
 // lifecycle: create → ready-to-show → (insert into workspace).
 type PendingPopup struct {
+	// PaneID is the popup pane created during the create phase.
+	PaneID entity.PaneID
 	// WebView is the related WebView created in the "create" phase.
 	// It shares cookies/session with the parent for OAuth support.
 	WebView port.WebView
 	log.Debug().
 		Str("parent_pane", string(parentPaneID)).
-		Str("target_uri", req.TargetURI).
+		Str("target_uri", logging.TruncateURL(req.TargetURI, logURLMaxLen)).
 		Str("frame_name", req.FrameName).
 		Bool("user_gesture", req.IsUserGesture).
 		Msg("popup create request")
 	// Store pending popup for ready-to-show handling (just visibility now)
 	pending := &PendingPopup{
+		PaneID:          paneID,
 		WebView:         popupWV,
 		ParentPaneID:    parentPaneID,
 		ParentWebViewID: parentID,
 	c.popupMu.Unlock()
 	if wasPending && pending != nil {
+		if c.onClosePane != nil {
+			if err := c.onClosePane(ctx, pending.PaneID); err != nil {
+				log.Warn().Err(err).Str("pane_id", string(pending.PaneID)).Msg("failed to close pending popup pane")
+			}
+		}
 		c.handlePopupOAuthClose(ctx, popupID)
-		// Was never shown, just destroy
+		c.ReleaseWebView(ctx, pending.PaneID)
-		pending.WebView.Destroy()
+		log.Debug().Str("pane_id", string(pending.PaneID)).Msg("cleaned up pending popup that was never shown")
-		log.Debug().Msg("destroyed pending popup that was never shown")
 		return
 	}
 	log.Info().
 		Str("parent_pane", string(parentPaneID)).
-		Str("uri", uri).
+		Str("uri", logging.TruncateURL(uri, logURLMaxLen)).
 		Msg("middle-click/ctrl+click on link")
 	// Check if popups are enabled
 	// Load the URI after insertion
 	if err := newWV.LoadURI(ctx, uri); err != nil {
-		log.Error().Err(err).Str("uri", uri).Msg("failed to load URI in new pane")
+		log.Error().Err(err).Str("uri", logging.TruncateURL(uri, logURLMaxLen)).Msg("failed to load URI in new pane")
 	}
 	log.Info().
 		Str("pane_id", string(paneID)).
 		Str("behavior", string(behavior)).
-		Str("uri", uri).
+		Str("uri", logging.TruncateURL(uri, logURLMaxLen)).
 		Msg("middle-click link opened in new pane")
 	return true

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


@@ -299,7 +299,7 @@ func TestPendingPopups_ConcurrentAccess(t *testing.T) {
299	299		count := len(c.pendingPopups)
300	300		c.popupMu.RUnlock()
301	301
302		-	assert.LessOrEqual(t, count, workers, "pending popups should not exceed number of writers")
	302	+	assert.Equal(t, workers, count, "each writer uses a unique key, so all inserts should be present")
303	303		}
304	304
305	305		func TestPendingPopups_ConcurrentDeleteAndRead(t *testing.T) {

@@ -338,4 +338,8 @@ func TestPendingPopups_ConcurrentDeleteAndRead(t *testing.T) {
338	338		}
339	339
340	340		wg.Wait()
	341	+
	342	+	c.popupMu.RLock()
	343	+	defer c.popupMu.RUnlock()
	344	+	assert.Empty(t, c.pendingPopups, "all preloaded popups should have been deleted")
341	345		}

 	count := len(c.pendingPopups)
 	c.popupMu.RUnlock()
-	assert.LessOrEqual(t, count, workers, "pending popups should not exceed number of writers")
+	assert.Equal(t, workers, count, "each writer uses a unique key, so all inserts should be present")
 }
 func TestPendingPopups_ConcurrentDeleteAndRead(t *testing.T) {
 	}
 	wg.Wait()
+	c.popupMu.RLock()
+	defer c.popupMu.RUnlock()
+	assert.Empty(t, c.pendingPopups, "all preloaded popups should have been deleted")
 }

M internal/ui/coordinator/content/title.go +9 -0


@@ -31,6 +31,9 @@ func (c *Coordinator) onTitleChanged(ctx context.Context, paneID entity.PaneID,
31	31
32	32		// Update domain model and check if this is the active pane
33	33		isActivePaneTitle := false
	34	+	if c.getActiveWS == nil {
	35	+	return
	36	+	}
34	37		ws, wsView := c.getActiveWS()
35	38		if ws != nil {
36	39		paneNode := ws.FindPane(paneID)

@@ -101,6 +104,9 @@ func (c *Coordinator) updateStackedPaneTitle(
101	104		// syncStackedTitle updates the stacked title bar for a pane if it's in a stack.
102	105		// Called from onLoadCommitted to keep titles in sync during navigation.
103	106		func (c *Coordinator) syncStackedTitle(ctx context.Context, paneID entity.PaneID, title string) {
	107	+	if c.getActiveWS == nil {
	108	+	return
	109	+	}
104	110		_, wsView := c.getActiveWS()
105	111		if wsView == nil {
106	112		return

@@ -172,6 +178,9 @@ func (c *Coordinator) updateStackedPaneFavicon(
172	178
173	179		// updateStackedFaviconForPane updates the stacked title bar favicon for a pane.
174	180		func (c Coordinator) updateStackedFaviconForPane(ctx context.Context, paneID entity.PaneID, texture gdk.Texture) {
	181	+	if c.getActiveWS == nil {
	182	+	return
	183	+	}
175	184		_, wsView := c.getActiveWS()
176	185		if wsView == nil {
177	186		return


@@ -31,6 +31,9 @@ func (c *Coordinator) onTitleChanged(ctx context.Context, paneID entity.PaneID,
31		31
32	// Update domain model and check if this is the active pane	32		// Update domain model and check if this is the active pane
33	isActivePaneTitle := false	33		isActivePaneTitle := false
		34	+	if c.getActiveWS == nil {
		35	+	return
		36	+	}
34	ws, wsView := c.getActiveWS()	37		ws, wsView := c.getActiveWS()
35	if ws != nil {	38		if ws != nil {
36	paneNode := ws.FindPane(paneID)	39		paneNode := ws.FindPane(paneID)

@@ -101,6 +104,9 @@ func (c *Coordinator) updateStackedPaneTitle(
101	// syncStackedTitle updates the stacked title bar for a pane if it's in a stack.	104		// syncStackedTitle updates the stacked title bar for a pane if it's in a stack.
102	// Called from onLoadCommitted to keep titles in sync during navigation.	105		// Called from onLoadCommitted to keep titles in sync during navigation.
103	func (c *Coordinator) syncStackedTitle(ctx context.Context, paneID entity.PaneID, title string) {	106		func (c *Coordinator) syncStackedTitle(ctx context.Context, paneID entity.PaneID, title string) {
		107	+	if c.getActiveWS == nil {
		108	+	return
		109	+	}
104	_, wsView := c.getActiveWS()	110		_, wsView := c.getActiveWS()
105	if wsView == nil {	111		if wsView == nil {
106	return	112		return

@@ -172,6 +178,9 @@ func (c *Coordinator) updateStackedPaneFavicon(
172		178
173	// updateStackedFaviconForPane updates the stacked title bar favicon for a pane.	179		// updateStackedFaviconForPane updates the stacked title bar favicon for a pane.
174	func (c Coordinator) updateStackedFaviconForPane(ctx context.Context, paneID entity.PaneID, texture gdk.Texture) {	180		func (c Coordinator) updateStackedFaviconForPane(ctx context.Context, paneID entity.PaneID, texture gdk.Texture) {
		181	+	if c.getActiveWS == nil {
		182	+	return
		183	+	}
175	_, wsView := c.getActiveWS()	184		_, wsView := c.getActiveWS()
176	if wsView == nil {	185		if wsView == nil {
177	return	186		return

M internal/ui/coordinator/tab.go +3 -0


@@ -387,6 +387,9 @@ func (c *TabCoordinator) CreateWithPane(
387	387		c.onTabSwitched(ctx, output.Tab)
388	388		}
389	389
	390	+	// Notify state change for session snapshots
	391	+	c.notifyStateChanged()
	392	+
390	393		log.Debug().
391	394		Str("tab_id", string(output.Tab.ID)).
392	395		Str("pane_id", string(pane.ID)).


@@ -387,6 +387,9 @@ func (c *TabCoordinator) CreateWithPane(
387	c.onTabSwitched(ctx, output.Tab)	387		c.onTabSwitched(ctx, output.Tab)
388	}	388		}
389		389
		390	+	// Notify state change for session snapshots
		391	+	c.notifyStateChanged()
		392	+
390	log.Debug().	393		log.Debug().
391	Str("tab_id", string(output.Tab.ID)).	394		Str("tab_id", string(output.Tab.ID)).
392	Str("pane_id", string(pane.ID)).	395		Str("pane_id", string(pane.ID)).

M internal/ui/coordinator/workspace.go +4 -3


@@ -476,13 +476,13 @@ func (c *WorkspaceCoordinator) doIncrementalSplit(
476	476		// 2. Get the active pane's StackedView widget (what we're actually splitting)
477	477		tr := wsView.TreeRenderer()
478	478		if tr == nil {
479		-	return nil
	479	+	return fmt.Errorf("tree renderer not available")
480	480		}
481	481
482	482		activeStackedView := tr.GetStackedViewForPane(string(oldActivePaneID))
483	483		if activeStackedView == nil {
484	484		log.Warn().Msg("no stacked view found for active pane")
485		-	return nil
	485	+	return fmt.Errorf("no stacked view found for active pane")
486	486		}
487	487		activePaneWidget := activeStackedView.Widget()
488	488

@@ -512,7 +512,7 @@ func (c *WorkspaceCoordinator) doIncrementalSplit(
512	512
513	513		orientation, existingFirst, ok := splitOrientation(direction)
514	514		if !ok {
515		-	return nil
	515	+	return fmt.Errorf("unsupported split direction: %s", direction)
516	516		}
517	517
518	518		// 7. Handle root vs non-root split differently

@@ -2054,6 +2054,7 @@ func (c *WorkspaceCoordinator) attachPopupPaneView(
2054	2054		wsView.RegisterPaneView(input.PopupPane.ID, newPaneView)
2055	2055
2056	2056		if err := c.stackedPaneMgr.AddPaneToStack(ctx, wsView, input.ParentPaneID, newPaneView, input.PopupPane.Title); err != nil {
	2057	+	wsView.UnregisterPaneView(input.PopupPane.ID)
2057	2058		log.Error().Err(err).Msg("failed to add popup pane to stack")
2058	2059		return err
2059	2060		}

 	// 2. Get the active pane's StackedView widget (what we're actually splitting)
 	tr := wsView.TreeRenderer()
 	if tr == nil {
-		return nil
+		return fmt.Errorf("tree renderer not available")
 	}
 	activeStackedView := tr.GetStackedViewForPane(string(oldActivePaneID))
 	if activeStackedView == nil {
 		log.Warn().Msg("no stacked view found for active pane")
-		return nil
+		return fmt.Errorf("no stacked view found for active pane")
 	}
 	activePaneWidget := activeStackedView.Widget()
 	orientation, existingFirst, ok := splitOrientation(direction)
 	if !ok {
-		return nil
+		return fmt.Errorf("unsupported split direction: %s", direction)
 	}
 	// 7. Handle root vs non-root split differently
 	wsView.RegisterPaneView(input.PopupPane.ID, newPaneView)
 	if err := c.stackedPaneMgr.AddPaneToStack(ctx, wsView, input.ParentPaneID, newPaneView, input.PopupPane.Title); err != nil {
+		wsView.UnregisterPaneView(input.PopupPane.ID)
 		log.Error().Err(err).Msg("failed to add popup pane to stack")
 		return err
 	}

M internal/ui/deps.go +3 -1


@@ -84,7 +84,9 @@ func (d *Dependencies) Validate() error {
84	84		if d.Config == nil {
85	85		return ErrMissingDependency("Config")
86	86		}
87		-	// WebKit dependencies are optional for testing without WebViews
	87	+	if d.Engine == nil {
	88	+	return ErrMissingDependency("Engine")
	89	+	}
88	90		// Use cases are optional - can be nil if not needed
89	91		return nil
90	92		}

 	if d.Config == nil {
 		return ErrMissingDependency("Config")
 	}
-	// WebKit dependencies are optional for testing without WebViews
+	if d.Engine == nil {
+		return ErrMissingDependency("Engine")
+	}
 	// Use cases are optional - can be nil if not needed
 	return nil
 }