From 969203f17ce5ef28c8c8759d9ac0772cea3e38e0 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 12 Mar 2026 16:16:44 +0000 Subject: [PATCH 1/2] Fix --show flag rejected as unknown in rodney start The first flag-parsing loop in cmdStart only recognized --insecure/-k, causing --show to hit the default case and exit with "unknown flag" error. The second loop that handled --show was unreachable. Extract flag parsing into testable parseStartArgs function that handles all flags (--show, --insecure, -k) in a single pass. Add tests covering each flag individually, combined flags, and unknown flag rejection. https://claude.ai/code/session_019dG8c5MD6DibdiFCXcSq7C --- main.go | 30 ++++++++++++--------- main_test.go | 76 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 93 insertions(+), 13 deletions(-) diff --git a/main.go b/main.go index 0b2531a..5719d40 100644 --- a/main.go +++ b/main.go @@ -318,16 +318,28 @@ func withPage() (*State, *rod.Browser, *rod.Page) { // --- Commands --- -func cmdStart(args []string) { - ignoreCertErrors := false - for i := 0; i < len(args); i++ { - switch args[i] { +// parseStartArgs parses the flags for the "start" command. +// Returns ignoreCertErrors, headless, and an error for unknown flags. +func parseStartArgs(args []string) (ignoreCertErrors bool, headless bool, err error) { + headless = true + for _, arg := range args { + switch arg { case "--insecure", "-k": ignoreCertErrors = true + case "--show": + headless = false default: - fatal("unknown flag: %s\nusage: rodney start [--insecure]", args[i]) + return false, true, fmt.Errorf("unknown flag: %s\nusage: rodney start [--show] [--insecure]", arg) } } + return ignoreCertErrors, headless, nil +} + +func cmdStart(args []string) { + ignoreCertErrors, headless, err := parseStartArgs(args) + if err != nil { + fatal("%s", err) + } // Check if already running if s, err := loadState(); err == nil { @@ -339,14 +351,6 @@ func cmdStart(args []string) { } } - // Parse flags - headless := true - for _, arg := range args { - if arg == "--show" { - headless = false - } - } - dataDir := filepath.Join(stateDir(), "chrome-data") os.MkdirAll(dataDir, 0755) diff --git a/main_test.go b/main_test.go index 79ee87c..1753da9 100644 --- a/main_test.go +++ b/main_test.go @@ -1074,6 +1074,82 @@ func TestFormatAssertFail_EqualityWithMessage(t *testing.T) { } } +// ===================== +// parseStartArgs tests +// ===================== + +func TestParseStartArgs_NoFlags(t *testing.T) { + insecure, headless, err := parseStartArgs([]string{}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if insecure { + t.Error("expected insecure=false with no flags") + } + if !headless { + t.Error("expected headless=true with no flags") + } +} + +func TestParseStartArgs_ShowFlag(t *testing.T) { + insecure, headless, err := parseStartArgs([]string{"--show"}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if insecure { + t.Error("expected insecure=false") + } + if headless { + t.Error("expected headless=false when --show is passed") + } +} + +func TestParseStartArgs_InsecureFlag(t *testing.T) { + insecure, headless, err := parseStartArgs([]string{"--insecure"}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !insecure { + t.Error("expected insecure=true when --insecure is passed") + } + if !headless { + t.Error("expected headless=true when only --insecure is passed") + } +} + +func TestParseStartArgs_InsecureShortFlag(t *testing.T) { + insecure, _, err := parseStartArgs([]string{"-k"}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !insecure { + t.Error("expected insecure=true when -k is passed") + } +} + +func TestParseStartArgs_ShowAndInsecure(t *testing.T) { + insecure, headless, err := parseStartArgs([]string{"--show", "--insecure"}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !insecure { + t.Error("expected insecure=true") + } + if headless { + t.Error("expected headless=false when --show is passed") + } +} + +func TestParseStartArgs_UnknownFlag(t *testing.T) { + _, _, err := parseStartArgs([]string{"--bogus"}) + if err == nil { + t.Fatal("expected error for unknown flag --bogus") + } + if !strings.Contains(err.Error(), "--bogus") { + t.Errorf("error should mention the unknown flag, got: %v", err) + } +} + func TestInsecureFlag_WithSelfSignedCert(t *testing.T) { // Create HTTPS server with self-signed certificate mux := http.NewServeMux() From 4050b391c1b260fe1591179578802151d02349b5 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 12 Mar 2026 18:17:40 +0000 Subject: [PATCH 2/2] Migrate command arg parsing from hand-rolled loops to flag.FlagSet Port 6 functions to use Go's standard flag package: - parseStartArgs: --show, --insecure/-k - cmdReload: --hard - cmdScreenshot: -w/--width, -h/--height + positional filename - cmdAXTree: --depth, --json - cmdAXFind: --name, --role, --json - cmdAXNode: --json + positional selector (hybrid pre-extraction) Leave parseAssertArgs and extractScopeArgs as-is because their flags-anywhere-in-the-arg-list semantics are incompatible with flag.Parse which stops at the first non-flag argument. Add findUnknownFlag helper for backward-compatible error messages. https://claude.ai/code/session_019dG8c5MD6DibdiFCXcSq7C --- main.go | 199 +++++++++++++++++++++++++++----------------------------- 1 file changed, 95 insertions(+), 104 deletions(-) diff --git a/main.go b/main.go index 5719d40..a5188db 100644 --- a/main.go +++ b/main.go @@ -4,6 +4,7 @@ import ( _ "embed" "encoding/base64" "encoding/json" + "flag" "fmt" "io" "net" @@ -162,6 +163,23 @@ func fatal(format string, args ...interface{}) { os.Exit(2) } +// findUnknownFlag returns the first arg not registered in fs, preserving original form (e.g. --bogus). +func findUnknownFlag(args []string, fs *flag.FlagSet) string { + for _, a := range args { + if !strings.HasPrefix(a, "-") { + continue + } + name := strings.TrimLeft(a, "-") + if fs.Lookup(name) == nil { + return a + } + } + if len(args) > 0 { + return args[0] + } + return "" +} + func main() { if len(os.Args) < 2 { printUsage() @@ -321,17 +339,19 @@ func withPage() (*State, *rod.Browser, *rod.Page) { // parseStartArgs parses the flags for the "start" command. // Returns ignoreCertErrors, headless, and an error for unknown flags. func parseStartArgs(args []string) (ignoreCertErrors bool, headless bool, err error) { - headless = true - for _, arg := range args { - switch arg { - case "--insecure", "-k": - ignoreCertErrors = true - case "--show": - headless = false - default: - return false, true, fmt.Errorf("unknown flag: %s\nusage: rodney start [--show] [--insecure]", arg) - } + fs := flag.NewFlagSet("start", flag.ContinueOnError) + fs.SetOutput(io.Discard) + fs.BoolVar(&ignoreCertErrors, "insecure", false, "") + fs.BoolVar(&ignoreCertErrors, "k", false, "") + show := fs.Bool("show", false, "") + + if parseErr := fs.Parse(args); parseErr != nil { + return false, true, fmt.Errorf("unknown flag: %s\nusage: rodney start [--show] [--insecure]", findUnknownFlag(args, fs)) + } + if fs.NArg() > 0 { + return false, true, fmt.Errorf("unknown flag: %s\nusage: rodney start [--show] [--insecure]", fs.Arg(0)) } + headless = !*show return ignoreCertErrors, headless, nil } @@ -593,14 +613,12 @@ func cmdForward(args []string) { } func cmdReload(args []string) { - hard := false - for _, a := range args { - if a == "--hard" { - hard = true - } - } + fs := flag.NewFlagSet("reload", flag.ContinueOnError) + fs.SetOutput(io.Discard) + hard := fs.Bool("hard", false, "") + fs.Parse(args) _, _, page := withPage() - if hard { + if *hard { // CDP Page.reload with ignoreCache (equivalent to Shift+Refresh) err := (proto.PageReload{IgnoreCache: true}).Call(page) if err != nil { @@ -1106,43 +1124,27 @@ func nextAvailableFile(base, ext string) string { } func cmdScreenshot(args []string) { - var file string - width := 1280 - height := 0 - fullPage := true + fs := flag.NewFlagSet("screenshot", flag.ContinueOnError) + fs.SetOutput(io.Discard) + width := fs.Int("width", 1280, "") + fs.IntVar(width, "w", 1280, "") + height := fs.Int("height", 0, "") + fs.IntVar(height, "h", 0, "") + + if err := fs.Parse(args); err != nil { + fatal("%v", err) + } - // Parse flags and positional args - var positional []string - for i := 0; i < len(args); i++ { - switch args[i] { - case "-w", "--width": - i++ - if i >= len(args) { - fatal("missing value for %s", args[i-1]) - } - v, err := strconv.Atoi(args[i]) - if err != nil { - fatal("invalid width: %v", err) - } - width = v - case "-h", "--height": - i++ - if i >= len(args) { - fatal("missing value for %s", args[i-1]) - } - v, err := strconv.Atoi(args[i]) - if err != nil { - fatal("invalid height: %v", err) - } - height = v + fullPage := true + fs.Visit(func(f *flag.Flag) { + if f.Name == "height" || f.Name == "h" { fullPage = false - default: - positional = append(positional, args[i]) } - } + }) - if len(positional) > 0 { - file = positional[0] + var file string + if fs.NArg() > 0 { + file = fs.Arg(0) } else { file = nextAvailableFile("screenshot", ".png") } @@ -1150,12 +1152,12 @@ func cmdScreenshot(args []string) { _, _, page := withPage() // Set viewport size - viewportHeight := height + viewportHeight := *height if viewportHeight == 0 { viewportHeight = 720 } err := proto.EmulationSetDeviceMetricsOverride{ - Width: width, + Width: *width, Height: viewportHeight, DeviceScaleFactor: 1, }.Call(page) @@ -1500,35 +1502,32 @@ func init() { // --- Accessibility commands --- func cmdAXTree(args []string) { - var depth *int - jsonOutput := false + fs := flag.NewFlagSet("ax-tree", flag.ContinueOnError) + fs.SetOutput(io.Discard) + depthVal := fs.Int("depth", 0, "") + jsonOutput := fs.Bool("json", false, "") - for i := 0; i < len(args); i++ { - switch args[i] { - case "--depth": - i++ - if i >= len(args) { - fatal("missing value for --depth") - } - v, err := strconv.Atoi(args[i]) - if err != nil { - fatal("invalid depth: %v", err) - } - depth = &v - case "--json": - jsonOutput = true - default: - fatal("unknown flag: %s\nusage: rodney ax-tree [--depth N] [--json]", args[i]) - } + if err := fs.Parse(args); err != nil { + fatal("unknown flag: %s\nusage: rodney ax-tree [--depth N] [--json]", findUnknownFlag(args, fs)) + } + if fs.NArg() > 0 { + fatal("unknown flag: %s\nusage: rodney ax-tree [--depth N] [--json]", fs.Arg(0)) } + var depth *int + fs.Visit(func(f *flag.Flag) { + if f.Name == "depth" { + depth = depthVal + } + }) + _, _, page := withPage() result, err := proto.AccessibilityGetFullAXTree{Depth: depth}.Call(page) if err != nil { fatal("failed to get accessibility tree: %v", err) } - if jsonOutput { + if *jsonOutput { fmt.Println(formatAXTreeJSON(result.Nodes)) } else { fmt.Print(formatAXTree(result.Nodes)) @@ -1536,32 +1535,21 @@ func cmdAXTree(args []string) { } func cmdAXFind(args []string) { - var name, role string - jsonOutput := false + fs := flag.NewFlagSet("ax-find", flag.ContinueOnError) + fs.SetOutput(io.Discard) + name := fs.String("name", "", "") + role := fs.String("role", "", "") + jsonOutput := fs.Bool("json", false, "") - for i := 0; i < len(args); i++ { - switch args[i] { - case "--name": - i++ - if i >= len(args) { - fatal("missing value for --name") - } - name = args[i] - case "--role": - i++ - if i >= len(args) { - fatal("missing value for --role") - } - role = args[i] - case "--json": - jsonOutput = true - default: - fatal("unknown flag: %s\nusage: rodney ax-find [--name N] [--role R] [--json]", args[i]) - } + if err := fs.Parse(args); err != nil { + fatal("unknown flag: %s\nusage: rodney ax-find [--name N] [--role R] [--json]", findUnknownFlag(args, fs)) + } + if fs.NArg() > 0 { + fatal("unknown flag: %s\nusage: rodney ax-find [--name N] [--role R] [--json]", fs.Arg(0)) } _, _, page := withPage() - nodes, err := queryAXNodes(page, name, role) + nodes, err := queryAXNodes(page, *name, *role) if err != nil { fatal("query failed: %v", err) } @@ -1571,7 +1559,7 @@ func cmdAXFind(args []string) { os.Exit(1) } - if jsonOutput { + if *jsonOutput { data, _ := json.MarshalIndent(nodes, "", " ") fmt.Println(string(data)) } else { @@ -1580,22 +1568,25 @@ func cmdAXFind(args []string) { } func cmdAXNode(args []string) { + // Pre-extract --json since it may appear after the positional selector jsonOutput := false - var positional []string - - for i := 0; i < len(args); i++ { - switch args[i] { - case "--json": + var filtered []string + for _, a := range args { + if a == "--json" { jsonOutput = true - default: - positional = append(positional, args[i]) + } else { + filtered = append(filtered, a) } } - if len(positional) < 1 { + fs := flag.NewFlagSet("ax-node", flag.ContinueOnError) + fs.SetOutput(io.Discard) + fs.Parse(filtered) + + if fs.NArg() < 1 { fatal("usage: rodney ax-node [--json]") } - selector := positional[0] + selector := fs.Arg(0) _, _, page := withPage() node, err := getAXNode(page, selector)