bug: mergeOptions always overwrites ShowBrowser regardless of intent #16

New Issue

Claude · 2026-02-14T16:06:49Z

Claude commented

2026-02-14 16:06:49 +00:00

Parent: #3

Description

In browser_init.go:128-158, mergeOptions() treats ShowBrowser differently from all other boolean fields:

func mergeOptions(base BrowserOptions, opts []BrowserOptions) BrowserOptions {
    for _, o := range opts {
        // ...
        if o.RequireServer {
            base.RequireServer = true  // only sets if true
        }
        if o.UseLocalOnly {
            base.UseLocalOnly = true   // only sets if true
        }
        base.ShowBrowser = o.ShowBrowser  // ALWAYS overwrites
    }
    return base
}

Unlike RequireServer and UseLocalOnly which only set to true, ShowBrowser is unconditionally overwritten. This means:

If you pass BrowserOptions{ShowBrowser: true} followed by BrowserOptions{UserAgent: "custom"}, the second option resets ShowBrowser to false even though it wasn't explicitly set.

Fix

Either:

Treat ShowBrowser consistently with other booleans: if o.ShowBrowser { base.ShowBrowser = true }
Or use a pointer *bool for all optional boolean fields to distinguish "not set" from "set to false"

**Parent:** #3 ## Description In `browser_init.go:128-158`, `mergeOptions()` treats `ShowBrowser` differently from all other boolean fields: ```go func mergeOptions(base BrowserOptions, opts []BrowserOptions) BrowserOptions { for _, o := range opts { // ... if o.RequireServer { base.RequireServer = true // only sets if true } if o.UseLocalOnly { base.UseLocalOnly = true // only sets if true } base.ShowBrowser = o.ShowBrowser // ALWAYS overwrites } return base } ``` Unlike `RequireServer` and `UseLocalOnly` which only set to `true`, `ShowBrowser` is unconditionally overwritten. This means: - If you pass `BrowserOptions{ShowBrowser: true}` followed by `BrowserOptions{UserAgent: "custom"}`, the second option resets `ShowBrowser` to `false` even though it wasn't explicitly set. ## Fix Either: 1. Treat `ShowBrowser` consistently with other booleans: `if o.ShowBrowser { base.ShowBrowser = true }` 2. Or use a pointer `*bool` for all optional boolean fields to distinguish "not set" from "set to false"

Claude added the bug priority/medium type/task labels 2026-02-14 16:07:34 +00:00

Claude referenced this issue

2026-02-14 16:09:05 +00:00

Epic: Code Quality & Duplication #3

Claude referenced this issue

2026-02-15 15:59:00 +00:00

Master plan: address all open issues (17 PRs across 5 phases) #31

Claude commented

2026-02-15 16:20:43 +00:00

Starting work on this as part of PR 7 (also includes #15). Will change ShowBrowser to *bool so nil means "don't override" in mergeOptions().

Starting work on this as part of PR 7 (also includes #15). Will change `ShowBrowser` to `*bool` so nil means "don't override" in `mergeOptions()`.

Claude referenced this issue