Skip to content

Commit 573c4c4

Browse files
claudeapgiorgi
authored andcommitted
fix: align --agent/--no-agent parsing with pflag semantics
agentFlagOverride treated --agent=false as forcing human mode and --no-agent=false as forcing agent mode, and accepted yes/on/off tokens that pflag rejects — contradicting both pflag and the function's own 'agrees with cobra's later parse' contract. --agent and --no-agent are two independent pflag bool flags, so an explicit false just leaves that flag unset rather than flipping to the opposite mode. Track each flag's final value with last-write-wins, parse values with strconv.ParseBool (exactly as pflag does), and resolve a genuine --agent --no-agent conflict by most-recently-enabled-wins. parseBoolish stays lenient but is now used only for the DCI_AGENT_MODE env var.
1 parent 8a53f25 commit 573c4c4

2 files changed

Lines changed: 62 additions & 27 deletions

File tree

main.go

Lines changed: 45 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -585,44 +585,67 @@ func stdoutIsTTY() bool {
585585
return term.IsTerminal(int(os.Stdout.Fd()))
586586
}
587587

588-
// agentFlagOverride scans args for explicit --agent / --no-agent flags. Returns
589-
// +1 for agent mode, -1 for human mode, 0 if neither is present. The last
590-
// occurrence wins. Scanning stops at the "--" operand terminator. Both the bare
591-
// flag and any boolean value form cobra/pflag accepts (=true/false, =1/0, etc.)
592-
// are honored so this early scan agrees with cobra's later parse.
588+
// agentFlagOverride scans args for the explicit --agent / --no-agent flags and
589+
// returns +1 for agent mode, -1 for human mode, or 0 for no override. These are
590+
// two independent pflag bool flags, so this scan mirrors pflag's own semantics:
591+
// each flag's last occurrence wins, a bare flag means true, and the value (when
592+
// present) is parsed with strconv.ParseBool exactly as pflag does. Crucially, an
593+
// explicit false (--agent=false / --no-agent=0) just leaves that flag unset
594+
// rather than forcing the opposite mode — only a flag set to true is an
595+
// override. If both end up true, the most recently enabled one wins. Scanning
596+
// stops at the "--" operand terminator. Keeping this in step with pflag matters
597+
// because the result drives side effects (color, default output, User-Agent)
598+
// before cobra parses the flags itself.
593599
func agentFlagOverride(args []string) int {
594-
res := 0
600+
agent, noAgent := false, false
601+
last := 0 // +1 if --agent was most recently enabled, -1 if --no-agent was
595602
for _, a := range args {
596603
if a == "--" {
597604
break
598605
}
599606
name, val, hasVal := strings.Cut(a, "=")
600-
var sign int
601-
switch name {
602-
case "--agent":
603-
sign = 1
604-
case "--no-agent":
605-
sign = -1
606-
default:
607+
if name != "--agent" && name != "--no-agent" {
607608
continue
608609
}
610+
enabled := true
609611
if hasVal {
610-
b, ok := parseBoolish(val)
611-
if !ok {
612+
b, err := strconv.ParseBool(val)
613+
if err != nil {
612614
continue // let cobra surface the parse error later
613615
}
614-
if !b {
615-
sign = -sign // --agent=false / --no-agent=false
616+
enabled = b
617+
}
618+
switch name {
619+
case "--agent":
620+
agent = enabled
621+
case "--no-agent":
622+
noAgent = enabled
623+
}
624+
if enabled {
625+
if name == "--agent" {
626+
last = 1
627+
} else {
628+
last = -1
616629
}
617630
}
618-
res = sign
619631
}
620-
return res
632+
switch {
633+
case agent && noAgent:
634+
return last // conflicting flags: most recently enabled wins
635+
case agent:
636+
return 1
637+
case noAgent:
638+
return -1
639+
default:
640+
return 0
641+
}
621642
}
622643

623-
// parseBoolish interprets the boolean tokens cobra/pflag accept (plus a few
624-
// common aliases), case-insensitively. Returns (value, true) for a recognized
625-
// token and (false, false) otherwise.
644+
// parseBoolish interprets a boolean-ish env var value (DCI_AGENT_MODE),
645+
// case-insensitively and forgivingly: it accepts the strconv.ParseBool tokens
646+
// plus common aliases (yes/no, on/off, y/n). Returns (value, true) for a
647+
// recognized token and (false, false) otherwise. It is intentionally more
648+
// lenient than the flag parsing, which mirrors pflag exactly.
626649
func parseBoolish(v string) (bool, bool) {
627650
switch strings.ToLower(strings.TrimSpace(v)) {
628651
case "1", "t", "true", "yes", "y", "on":

main_test.go

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -442,16 +442,28 @@ func TestAgentFlagOverride(t *testing.T) {
442442
{name: "none", args: []string{"dci", "status"}, want: 0},
443443
{name: "agent", args: []string{"dci", "--agent", "status"}, want: 1},
444444
{name: "agent equals true", args: []string{"--agent=true"}, want: 1},
445-
{name: "agent equals false", args: []string{"--agent=false"}, want: -1},
446445
{name: "agent equals 1", args: []string{"--agent=1"}, want: 1},
447-
{name: "agent equals 0", args: []string{"--agent=0"}, want: -1},
448446
{name: "no-agent", args: []string{"--no-agent"}, want: -1},
449-
{name: "no-agent equals false", args: []string{"--no-agent=false"}, want: 1},
450447
{name: "no-agent equals 1", args: []string{"--no-agent=1"}, want: -1},
451-
{name: "no-agent equals 0", args: []string{"--no-agent=0"}, want: 1},
452448
{name: "agent uppercase TRUE", args: []string{"--agent=TRUE"}, want: 1},
449+
450+
// Explicit false leaves the flag unset (matching two independent pflag
451+
// bool flags), so it does NOT force the opposite mode — it clears to 0.
452+
{name: "agent equals false clears", args: []string{"--agent=false"}, want: 0},
453+
{name: "agent equals 0 clears", args: []string{"--agent=0"}, want: 0},
454+
{name: "no-agent equals false clears", args: []string{"--no-agent=false"}, want: 0},
455+
{name: "no-agent equals 0 clears", args: []string{"--no-agent=0"}, want: 0},
456+
{name: "agent then agent false clears", args: []string{"--agent", "--agent=false"}, want: 0},
457+
{name: "no-agent then no-agent false clears", args: []string{"--no-agent", "--no-agent=false"}, want: 0},
458+
459+
// pflag uses strconv.ParseBool, which rejects yes/on/etc. (parseBoolish
460+
// accepts them, but that's only for the DCI_AGENT_MODE env var).
461+
{name: "agent yes rejected (not pflag bool)", args: []string{"--agent=yes"}, want: 0},
453462
{name: "agent unrecognized value ignored", args: []string{"--agent=maybe"}, want: 0},
454-
{name: "last wins", args: []string{"--agent", "--no-agent"}, want: -1},
463+
464+
// Conflicting flags both true: most recently enabled wins.
465+
{name: "conflict no-agent last wins", args: []string{"--agent", "--no-agent"}, want: -1},
466+
{name: "conflict agent last wins", args: []string{"--no-agent", "--agent"}, want: 1},
455467
{name: "stop at terminator", args: []string{"--", "--agent"}, want: 0},
456468
}
457469

0 commit comments

Comments
 (0)