From 230187296fae5253cb1773f16c2f1d1a39499ed0 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Mon, 26 Oct 2026 14:37:33 +0000 Subject: [PATCH] Stop flag interpretation at unknown flags --- docs/roadmap/remediation.md | 27 +++++++++++++++++++++--- internal/cobra/flags.go | 40 ++++++++++-------------------------- internal/cobra/flags_test.go | 28 +++++++++++++++++++++++++ 3 files changed, 63 insertions(+), 32 deletions(-) diff --git a/docs/roadmap/remediation.md b/docs/roadmap/remediation.md index e0f9da9..71e2a8d 100644 --- a/docs/roadmap/remediation.md +++ b/docs/roadmap/remediation.md @@ -1,6 +1,6 @@ # Invocation reporting audit remediation -Status: remediation in progress; 6 of 7 findings complete. +Status: remediation complete; all 7 findings addressed. ## Review scope @@ -31,7 +31,8 @@ native reports, and maintained consumers. Representative successful invocations covered command and scoped flag aliases with explicit zero and repeated values, quiet help/version, aggregate configuration YAML, quiet comparison, concrete request validation, and offline aggregate smoke testing. Findings 4 and 5 record -the additional gaps; findings 1–3 remain open and are not duplicated below. +the additional gaps; findings 1–3 were still open at that review and were not +duplicated below. The algorithmic audit at `ee76b0f` examined the reporting-specific parser passes, delivery bookkeeping, deferred storage, native diagnostic partitioning, and @@ -444,7 +445,13 @@ storage analysis is sufficient; no benchmark or allocation threshold is proposed ## 7. Stop normalizing an argument tail that parsing cannot reach -Priority: P3. Status: for consideration. +Priority: P3. Status: complete. + +- [x] Completed: interpretation returns the first fatal unknown-flag prefix, + retaining earlier canonical/alias occurrences and short-cluster members. + Temporary slices grow with that prefix; unreachable values and spellings are + no longer normalized or copied. Existing error precedence, values, and + terminator coverage passes. Validation: root module suite passes. ### Finding and complexity @@ -500,3 +507,17 @@ the extra work statically. Existing prefix-order, missing-value precedence, short-cluster, and terminator/value tests provide relevant behavior coverage if this is selected. Preserve the backend error and consumed occurrence prefix on early failure; no timing threshold or real-world performance data is needed. + +## Final remediation validation + +All seven items have completion markers and focused regression evidence. The +following checks pass for the assembled changes: + +- `GOTOOLCHAIN=local go test ./...` at the repository root and in each of + `integrations/ckconfig` and `integrations/ckprompt`, including maintained examples. +- `GOWORK=off GOTOOLCHAIN=local go test ./...` at the repository root. +- `GOTOOLCHAIN=local go test -race -run '^TestReporter|^TestHidden' .` at root. +- Go formatting, staged/unstaged whitespace review, and local document-link review. + +The ignored local workspace is preserved. No dependency upgrade, release, +publication, or change to upstream domain behavior was part of this work. diff --git a/internal/cobra/flags.go b/internal/cobra/flags.go index 540386a..2dc706a 100644 --- a/internal/cobra/flags.go +++ b/internal/cobra/flags.go @@ -27,9 +27,8 @@ func interpretFlagTokens(argv []string, flags []commandmeta.Flag, aliases []comm } spellings := make(map[string][]string) - occurrences := make([]commandmeta.FlagOccurrence, 0, len(argv)) - interpreted := make([]string, 0, len(argv)) - recordingOccurrences := true + var occurrences []commandmeta.FlagOccurrence + var interpreted []string for index := 0; index < len(argv); { token := argv[index] if token == "--" { @@ -50,35 +49,27 @@ func interpretFlagTokens(argv []string, flags []commandmeta.Flag, aliases []comm } if !ok { interpreted = append(interpreted, token) - recordingOccurrences = false - index++ - continue + return interpreted, spellings, occurrences, nil } spellings[flag.Name] = append(spellings[flag.Name], original) if equal >= 0 { value := body[equal+1:] interpreted = append(interpreted, "--"+flag.Name+"="+value) - if recordingOccurrences { - occurrences = append(occurrences, commandmeta.FlagOccurrence{Name: flag.Name, Spelling: original, RawValue: value}) - } + occurrences = append(occurrences, commandmeta.FlagOccurrence{Name: flag.Name, Spelling: original, RawValue: value}) index++ continue } if !flag.Boolean && index+1 < len(argv) { value := argv[index+1] interpreted = append(interpreted, "--"+flag.Name+"="+value) - if recordingOccurrences { - occurrences = append(occurrences, commandmeta.FlagOccurrence{Name: flag.Name, Spelling: original, RawValue: value}) - } + occurrences = append(occurrences, commandmeta.FlagOccurrence{Name: flag.Name, Spelling: original, RawValue: value}) index += 2 continue } if !flag.Boolean { return interpreted, spellings, occurrences, &commandmeta.Failure{Message: fmt.Sprintf("flag needs an argument: %s", original)} } - if recordingOccurrences { - occurrences = append(occurrences, commandmeta.FlagOccurrence{Name: flag.Name, Spelling: original, RawValue: "true"}) - } + occurrences = append(occurrences, commandmeta.FlagOccurrence{Name: flag.Name, Spelling: original, RawValue: "true"}) interpreted = append(interpreted, "--"+flag.Name) index++ continue @@ -98,8 +89,7 @@ func interpretFlagTokens(argv []string, flags []commandmeta.Flag, aliases []comm } if !ok { interpreted = append(interpreted, "-"+token[offset:]) - recordingOccurrences = false - break + return interpreted, spellings, occurrences, nil } spellings[flag.Name] = append(spellings[flag.Name], original) suffix := token[offset+1:] @@ -107,14 +97,10 @@ func interpretFlagTokens(argv []string, flags []commandmeta.Flag, aliases []comm if strings.HasPrefix(suffix, "=") { value := suffix[1:] interpreted = append(interpreted, "--"+flag.Name+"="+value) - if recordingOccurrences { - occurrences = append(occurrences, commandmeta.FlagOccurrence{Name: flag.Name, Spelling: original, RawValue: value}) - } + occurrences = append(occurrences, commandmeta.FlagOccurrence{Name: flag.Name, Spelling: original, RawValue: value}) break } - if recordingOccurrences { - occurrences = append(occurrences, commandmeta.FlagOccurrence{Name: flag.Name, Spelling: original, RawValue: "true"}) - } + occurrences = append(occurrences, commandmeta.FlagOccurrence{Name: flag.Name, Spelling: original, RawValue: "true"}) interpreted = append(interpreted, "--"+flag.Name) offset++ continue @@ -125,17 +111,13 @@ func interpretFlagTokens(argv []string, flags []commandmeta.Flag, aliases []comm value = value[1:] } interpreted = append(interpreted, "--"+flag.Name+"="+value) - if recordingOccurrences { - occurrences = append(occurrences, commandmeta.FlagOccurrence{Name: flag.Name, Spelling: original, RawValue: value}) - } + occurrences = append(occurrences, commandmeta.FlagOccurrence{Name: flag.Name, Spelling: original, RawValue: value}) break } if index+1 < len(argv) { value := argv[index+1] interpreted = append(interpreted, "--"+flag.Name+"="+value) - if recordingOccurrences { - occurrences = append(occurrences, commandmeta.FlagOccurrence{Name: flag.Name, Spelling: original, RawValue: value}) - } + occurrences = append(occurrences, commandmeta.FlagOccurrence{Name: flag.Name, Spelling: original, RawValue: value}) index += 2 consumedNext = true break diff --git a/internal/cobra/flags_test.go b/internal/cobra/flags_test.go index 0ffebb7..eac7f90 100644 --- a/internal/cobra/flags_test.go +++ b/internal/cobra/flags_test.go @@ -264,3 +264,31 @@ func TestParseFailuresRetainEnteredFlagSpellings(t *testing.T) { t.Fatalf("earlier parser failure = %#v", result.Failure) } } + +func TestFlagInterpretationStopsAtTheBackendFailurePrefix(t *testing.T) { + for _, test := range []struct { + name string + args []string + want []string + }{ + {"long", []string{"--debug", "--unknown"}, []string{"--debug", "--unknown"}}, + {"short cluster", []string{"-dqz"}, []string{"--debug", "--quiet", "-z"}}, + } { + t.Run(test.name, func(t *testing.T) { + // The later value must not be canonicalized or retained in spellings. + args := append(append([]string(nil), test.args...), "--message="+strings.Repeat("value", 1024), "--quiet=false") + flags := append(commandmeta.ReportingFlags(), commandmeta.Flag{Name: "message"}) + interpreted, spellings, occurrences, failure := interpretFlagTokens(args, flags, reportingFlagOptions().FlagAliases) + if failure != nil || !reflect.DeepEqual(interpreted, test.want) || len(spellings["message"]) != 0 { + t.Fatalf("interpreted=%q spellings=%v failure=%v", interpreted, spellings, failure) + } + if len(occurrences) != len(test.want)-1 || occurrences[0].Name != "debug" { + t.Fatalf("consumed occurrences=%#v", occurrences) + } + result := Parse(reportingFlagTree(), append([]string{"run"}, args...), reportingFlagOptions()) + if result.Failure == nil || !strings.Contains(result.Failure.Message, "unknown") || !reflect.DeepEqual(result.FlagOccurrences, occurrences) { + t.Fatalf("backend result=%#v", result) + } + }) + } +}