Fix rodney start --show flag parsing bug

[jacobb]

Summary

• Bug: rodney start --show failed with unknown flag: --show because the argument parser had two sequential loops — the first rejected --show before the second (which handled it) could run
• Fix: Extract parseStartFlags() with a single merged switch statement
• Tests: 6 new unit tests covering --show, --show --insecure, --insecure, -k, no args, and unknown flags

Fixes #23

Demo

The --show flag for rodney start was documented in help.txt but broken — the argument parser rejected it as an unknown flag before reaching the code that handled it. Two sequential loops: the first validated flags (only knew --insecure/-k) and called fatal() on anything else; the second set headless=false for --show but was dead code.

The fix extracts flag parsing into a testable parseStartFlags function and merges both loops into a single switch statement. Six new tests cover --show alone, --show --insecure together, --insecure only, -k shorthand, no args, and unknown flags.

git diff main -- main.go

diff --git a/main.go b/main.go
index 0b2531a..79bb8a6 100644
--- a/main.go
+++ b/main.go
@@ -318,16 +318,33 @@ func withPage() (*State, *rod.Browser, *rod.Page) {
 
 // --- Commands ---
 
-func cmdStart(args []string) {
-	ignoreCertErrors := false
-	for i := 0; i < len(args); i++ {
-		switch args[i] {
+type startFlags struct {
+	headless         bool
+	ignoreCertErrors bool
+}
+
+// parseStartFlags parses the arguments to "rodney start".
+func parseStartFlags(args []string) (startFlags, error) {
+	f := startFlags{headless: true}
+	for _, arg := range args {
+		switch arg {
+		case "--show":
+			f.headless = false
 		case "--insecure", "-k":
-			ignoreCertErrors = true
+			f.ignoreCertErrors = true
 		default:
-			fatal("unknown flag: %s\nusage: rodney start [--insecure]", args[i])
+			return f, fmt.Errorf("unknown flag: %s\nusage: rodney start [--show] [--insecure]", arg)
 		}
 	}
+	return f, nil
+}
+
+func cmdStart(args []string) {
+	flags, err := parseStartFlags(args)
+	if err != nil {
+		fatal("%s", err)
+	}
+	ignoreCertErrors := flags.ignoreCertErrors
 
 	// Check if already running
 	if s, err := loadState(); err == nil {
@@ -339,13 +356,7 @@ func cmdStart(args []string) {
 		}
 	}
 
-	// Parse flags
-	headless := true
-	for _, arg := range args {
-		if arg == "--show" {
-			headless = false
-		}
-	}
+	headless := flags.headless
 
 	dataDir := filepath.Join(stateDir(), "chrome-data")
 	os.MkdirAll(dataDir, 0755)

go test -run TestParseStartFlags -v

=== RUN   TestParseStartFlags_ShowFlag
--- PASS: TestParseStartFlags_ShowFlag (0.00s)
=== RUN   TestParseStartFlags_ShowAndInsecure
--- PASS: TestParseStartFlags_ShowAndInsecure (0.00s)
=== RUN   TestParseStartFlags_InsecureOnly
--- PASS: TestParseStartFlags_InsecureOnly (0.00s)
=== RUN   TestParseStartFlags_KShorthand
--- PASS: TestParseStartFlags_KShorthand (0.00s)
=== RUN   TestParseStartFlags_NoArgs
--- PASS: TestParseStartFlags_NoArgs (0.00s)
=== RUN   TestParseStartFlags_UnknownFlag
--- PASS: TestParseStartFlags_UnknownFlag (0.00s)
PASS

All 6 new tests pass. The full suite (65 tests) also passes with no regressions.

Test plan

• TestParseStartFlags_ShowFlag — --show alone sets headless=false
• TestParseStartFlags_ShowAndInsecure — both flags work together
• TestParseStartFlags_InsecureOnly — --insecure regression test
• TestParseStartFlags_KShorthand — -k regression test
• TestParseStartFlags_NoArgs — defaults are correct
• TestParseStartFlags_UnknownFlag — unknown flags still rejected
• Full test suite (65 tests) passes

🤖 Generated with Claude Code

[dwt]

I frequently stumble on this bug when trying Rodney @simonw anything you need to get this merged?

[player1537]

@simonw I'd like to second this, I just ran into this problem myself while trying to test rodney out. I can help out if there's anything else that needs to be done.

[dwt]

Sadly @simonw is quite busy these days so it can take quite some time till he has time to come back to his creations. :/