feat(cli): schema-aware sticky flag splitting and structured unknown-flag recovery (#272)

* chore: update coverage badge [skip ci] * chore: update coverage badge [skip ci] * chore: update coverage badge [skip ci] * chore: update coverage badge [skip ci] * chore: update coverage badge [skip ci] * 1.0.19 changelog * stick opt * fix(cli): utf-8 safe sticky suffix guard + changelog (#272) SuffixLooksLikeValue used to read suffix[0] (a single byte) for both the uuid format branch and the fallback "is the first rune a letter?" check. For multi-byte UTF-8 leading runes — common in dws because value text is often Chinese — this picked up only the first byte (0xE0..0xF4 lead), which is not a letter and not a hex digit, so the function silently returned true and let glued tokens like --name<CJK> get split into --name <CJK>... This switches both branches to utf8.DecodeRuneInString and adds a utf8.RuneError guard so invalid UTF-8 input is rejected too. Also locks down the new behaviour in CHANGELOG ## [Unreleased]: - Changed: schema-aware sticky flag splitting - Added: available_flags field on unknown-flag errors Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Cursor <cursoragent@cursor.com>
2026-05-13 10:56:02 +08:00
1 parent 2ba1dcdda4
commit 6707e56f9c
@@ -32,76 +32,101 @@ func TestFullPreParsePipeline(t *testing.T) {
 		ParamNameHandler{},
 	)

+	// Numeric / boolean flag typing matters for the sticky guard. The
+	// helper below declares known flags as int so digit-led suffixes
+	// pass the suffixLooksLikeValue check.
+	intSpecs := func(names ...string) []pipeline.FlagInfo {
+		out := make([]pipeline.FlagInfo, len(names))
+		for i, n := range names {
+			out[i] = pipeline.FlagInfo{Name: n, Type: "int"}
+		}
+		return out
+	}
+
 	tests := []struct {
 		name        string
 		args        []string
-		flags       []string
+		flags       []pipeline.FlagInfo
 		want        string
 		corrections int
 	}{
 		{
 			name:        "camelCase + sticky combined",
 			args:        []string{"--userId", "123", "--pageSize50"},
-			flags:       []string{"user-id", "page-size"},
+			flags:       intSpecs("user-id", "page-size"),
 			want:        "--user-id 123 --page-size 50",
 			corrections: 2, // alias(userId) + sticky(pageSize50)
 		},
 		{
 			name:        "camelCase + typo combined",
 			args:        []string{"--userId", "123", "--limt", "10"},
-			flags:       []string{"user-id", "limit"},
+			flags:       intSpecs("user-id", "limit"),
 			want:        "--user-id 123 --limit 10",
 			corrections: 2, // alias(userId) + fuzzy(limt)
 		},
 		{
 			name:        "triple error: case + sticky + typo",
 			args:        []string{"--UserName", "alice", "--limit100", "--offse", "0"},
-			flags:       []string{"user-name", "limit", "offset"},
+			flags:       append(flagSpecs("user-name"), intSpecs("limit", "offset")...),
 			want:        "--user-name alice --limit 100 --offset 0",
 			corrections: 3,
 		},
 		{
 			name:        "snake_case + sticky",
 			args:        []string{"--user_id", "42", "--pageSize20"},
-			flags:       []string{"user-id", "page-size"},
+			flags:       intSpecs("user-id", "page-size"),
 			want:        "--user-id 42 --page-size 20",
 			corrections: 2,
 		},
 		{
 			name:        "all correct — zero corrections",
 			args:        []string{"--user-id", "123", "--limit", "10"},
-			flags:       []string{"user-id", "limit"},
+			flags:       intSpecs("user-id", "limit"),
 			want:        "--user-id 123 --limit 10",
 			corrections: 0,
 		},
 		{
 			name:        "UPPER case flags",
 			args:        []string{"--USER-ID", "999"},
-			flags:       []string{"user-id"},
+			flags:       intSpecs("user-id"),
 			want:        "--user-id 999",
 			corrections: 1,
 		},
 		{
 			name:        "= syntax with camelCase",
 			args:        []string{"--userId=123", "--pageSize=50"},
-			flags:       []string{"user-id", "page-size"},
+			flags:       intSpecs("user-id", "page-size"),
 			want:        "--user-id=123 --page-size=50",
 			corrections: 2,
 		},
 		{
 			name:        "camelCase sticky split with normalisation",
 			args:        []string{"--limitValue100"},
-			flags:       []string{"limit-value"},
+			flags:       intSpecs("limit-value"),
 			want:        "--limit-value 100",
 			corrections: 1, // sticky handles both kebab-normalisation and split
 		},
+
+		// Hardening: a mistyped flag whose name happens to start with
+		// a real flag must NOT be split. The pipeline should leave the
+		// token untouched so Cobra can raise "unknown flag".
+		{
+			name: "typo --starttime1 not split (date-time format)",
+			args: []string{"--starttime1", "2026-02-07"},
+			flags: []pipeline.FlagInfo{
+				{Name: "start", Type: "string", Format: "date-time"},
+				{Name: "end", Type: "string", Format: "date-time"},
+			},
+			want:        "--starttime1 2026-02-07",
+			corrections: 0,
+		},
 	}

 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			ctx := &pipeline.Context{
 				Args:      append([]string{}, tt.args...),
-				FlagSpecs: flagSpecs(tt.flags...),
+				FlagSpecs: tt.flags,
 			}
 			if err := engine.RunPhase(pipeline.PreParse, ctx); err != nil {
 				t.Fatalf("RunPhase error: %v", err)
@@ -191,7 +216,11 @@ func TestFullPipelineEndToEnd(t *testing.T) {
 			"--pageSize50",
 			"--verbosetrue",
 		},
-		FlagSpecs: flagSpecs("user-id", "page-size", "verbose"),
+		FlagSpecs: []pipeline.FlagInfo{
+			{Name: "user-id", Type: "string"},
+			{Name: "page-size", Type: "int"},
+			{Name: "verbose", Type: "bool"},
+		},
 	}

 	if err := engine.RunPhase(pipeline.PreParse, ctx); err != nil {
@@ -287,7 +316,11 @@ func TestFullFivePhasePipeline(t *testing.T) {
 		"--pageSize50",
 		"--verbosetrue",
 	}
-	ctx.FlagSpecs = flagSpecs("user-id", "page-size", "verbose")
+	ctx.FlagSpecs = []pipeline.FlagInfo{
+		{Name: "user-id", Type: "string"},
+		{Name: "page-size", Type: "int"},
+		{Name: "verbose", Type: "bool"},
+	}

 	if err := engine.RunPhase(pipeline.PreParse, ctx); err != nil {
 		t.Fatalf("PreParse error: %v", err)
@@ -17,6 +17,7 @@ import (
 	"strings"

 	"github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/pipeline"
+	"github.com/DingTalk-Real-AI/dingtalk-workspace-cli/pkg/cmdutil"
 )

 // StickyHandler detects glued flag-value pairs in raw argv and splits
@@ -25,7 +26,12 @@ import (
 //
 // The handler only operates on tokens that start with "--" and do not
 // contain "=". It tries to match the longest known flag name prefix
-// and, if the remaining suffix is non-empty, splits the token.
+// and, if the remaining suffix is non-empty AND looks like a plausible
+// value for the matched flag's type/format, splits the token. The
+// type/format guard prevents misinterpreting mistyped flag names like
+// "--starttime1" as "--start time1" — when the suffix does not look
+// like a value, the original token is left untouched so Cobra can
+// report "unknown flag".
 type StickyHandler struct{}

 func (StickyHandler) Name() string          { return "sticky" }
@@ -36,11 +42,11 @@ func (StickyHandler) Handle(ctx *pipeline.Context) error {
 		return nil
 	}

-	known := buildFlagSet(ctx.FlagSpecs)
+	specByName := buildFlagSpecIndex(ctx.FlagSpecs)
 	result := make([]string, 0, len(ctx.Args))

 	for _, arg := range ctx.Args {
-		split, ok := trySplitSticky(arg, known)
+		split, ok := trySplitSticky(arg, specByName)
 		if ok {
 			ctx.AddCorrection("sticky", pipeline.PreParse, split.flag, arg, split.flag+" "+split.value, "sticky")
 			result = append(result, split.flag, split.value)
@@ -65,12 +71,14 @@ type stickyPair struct {
 //   - the prefix matches a known flag name (directly or after
 //     kebab-case normalisation)
 //   - the remaining suffix is non-empty
+//   - the suffix looks like a plausible value for the matched flag's
+//     declared type/format/enum (suffixLooksLikeValue)
 //
 // When multiple flag names match as prefixes, the longest one wins.
 // The handler also tries kebab-case normalisation of each prefix so
 // that camelCase+glued values like "--pageSize50" are correctly
 // split to "--page-size", "50".
-func trySplitSticky(arg string, known map[string]bool) (stickyPair, bool) {
+func trySplitSticky(arg string, specByName map[string]pipeline.FlagInfo) (stickyPair, bool) {
 	if !strings.HasPrefix(arg, "--") || strings.Contains(arg, "=") {
 		return stickyPair{}, false
 	}
@@ -83,7 +91,10 @@ func trySplitSticky(arg string, known map[string]bool) (stickyPair, bool) {

 	// If the whole token is a known flag, it is not sticky — it is
 	// a normal flag expecting a separate value token.
-	if known[bare] || known[toKebabCase(bare)] {
+	if _, ok := specByName[bare]; ok {
+		return stickyPair{}, false
+	}
+	if _, ok := specByName[toKebabCase(bare)]; ok {
 		return stickyPair{}, false
 	}

@@ -96,12 +107,14 @@ func trySplitSticky(arg string, known map[string]bool) (stickyPair, bool) {
 		prefix := bare[:i]

 		matchedFlag := ""
-		if known[prefix] {
+		if _, ok := specByName[prefix]; ok {
 			matchedFlag = prefix
 		} else {
 			kebab := toKebabCase(prefix)
-			if kebab != "" && known[kebab] {
-				matchedFlag = kebab
+			if kebab != "" {
+				if _, ok := specByName[kebab]; ok {
+					matchedFlag = kebab
+				}
 			}
 		}

@@ -120,14 +133,38 @@ func trySplitSticky(arg string, known map[string]bool) (stickyPair, bool) {
 		return stickyPair{}, false
 	}

+	// Guard: only split if the suffix plausibly looks like a value
+	// for this flag's declared type/format/enum. Otherwise leave the
+	// token untouched so Cobra reports "unknown flag" instead of
+	// silently corrupting the value.
+	fi := specByName[bestFlag]
+	if !cmdutil.SuffixLooksLikeValue(suffix, fi.Type, fi.Format, fi.Enum) {
+		return stickyPair{}, false
+	}
+
 	return stickyPair{
 		flag:  "--" + bestFlag,
 		value: suffix,
 	}, true
 }

+// buildFlagSpecIndex creates an index of known flag names (without "--"
+// prefix) to their FlagInfo entries from the context's FlagSpecs.
+func buildFlagSpecIndex(specs []pipeline.FlagInfo) map[string]pipeline.FlagInfo {
+	m := make(map[string]pipeline.FlagInfo, len(specs))
+	for _, spec := range specs {
+		if spec.Name != "" {
+			m[spec.Name] = spec
+		}
+	}
+	return m
+}
+
 // buildFlagSet creates a set of known flag names (without "--" prefix)
 // from the context's FlagSpecs.
+//
+// Retained for other PreParse handlers (alias, paramname) that only
+// need name presence and do not consume the richer FlagInfo metadata.
 func buildFlagSet(specs []pipeline.FlagInfo) map[string]bool {
 	m := make(map[string]bool, len(specs))
 	for _, spec := range specs {
@@ -20,6 +20,12 @@ import (
 	"github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/pipeline"
 )

+// flagSpecs is the legacy helper used by alias/paramname tests. It
+// produces string-typed FlagInfo entries which is fine for those
+// handlers because they don't consult the type.
+//
+// Sticky tests should use flagSpecsTyped (below) — the post-hardening
+// sticky decision depends on Type/Format/Enum.
 func flagSpecs(names ...string) []pipeline.FlagInfo {
 	specs := make([]pipeline.FlagInfo, len(names))
 	for i, name := range names {
@@ -28,126 +34,254 @@ func flagSpecs(names ...string) []pipeline.FlagInfo {
 	return specs
 }

+// flagSpec is a compact builder used by sticky tests to declare typed
+// flags inline. Type defaults to "string" when unset.
+type flagSpec struct {
+	name   string
+	typ    string
+	format string
+	enum   []string
+}
+
+func specs(in ...flagSpec) []pipeline.FlagInfo {
+	out := make([]pipeline.FlagInfo, len(in))
+	for i, s := range in {
+		t := s.typ
+		if t == "" {
+			t = "string"
+		}
+		out[i] = pipeline.FlagInfo{
+			Name:   s.name,
+			Type:   t,
+			Format: s.format,
+			Enum:   s.enum,
+		}
+	}
+	return out
+}
+
 func TestStickyHandler(t *testing.T) {
 	tests := []struct {
 		name        string
 		args        []string
-		flags       []string
+		flags       []pipeline.FlagInfo
 		want        string
 		corrections int
 	}{
 		{
-			name:        "basic split --limit100",
+			name:        "basic split --limit100 (int)",
 			args:        []string{"--limit100"},
-			flags:       []string{"limit"},
+			flags:       specs(flagSpec{name: "limit", typ: "int"}),
 			want:        "--limit 100",
 			corrections: 1,
 		},
 		{
 			name:        "no split when flag takes value separately",
 			args:        []string{"--limit", "100"},
-			flags:       []string{"limit"},
+			flags:       specs(flagSpec{name: "limit", typ: "int"}),
 			want:        "--limit 100",
 			corrections: 0,
 		},
 		{
 			name:        "no split when = syntax used",
 			args:        []string{"--limit=100"},
-			flags:       []string{"limit"},
+			flags:       specs(flagSpec{name: "limit", typ: "int"}),
 			want:        "--limit=100",
 			corrections: 0,
 		},
 		{
 			name:        "no split when flag name is not known",
 			args:        []string{"--unknown100"},
-			flags:       []string{"limit"},
+			flags:       specs(flagSpec{name: "limit", typ: "int"}),
 			want:        "--unknown100",
 			corrections: 0,
 		},
-		{
-			name:        "split with string value",
-			args:        []string{"--nameJohn"},
-			flags:       []string{"name"},
-			want:        "--name John",
-			corrections: 1,
-		},
 		{
 			name:        "longest prefix wins",
 			args:        []string{"--user-id123"},
-			flags:       []string{"user", "user-id"},
+			flags:       specs(flagSpec{name: "user"}, flagSpec{name: "user-id", typ: "int"}),
 			want:        "--user-id 123",
 			corrections: 1,
 		},
 		{
 			name:        "multiple sticky args in one invocation",
 			args:        []string{"--limit100", "--offset50"},
-			flags:       []string{"limit", "offset"},
+			flags:       specs(flagSpec{name: "limit", typ: "int"}, flagSpec{name: "offset", typ: "int"}),
 			want:        "--limit 100 --offset 50",
 			corrections: 2,
 		},
 		{
-			name:        "mixed sticky and normal args",
-			args:        []string{"--limit100", "--name", "test", "--offset50"},
-			flags:       []string{"limit", "name", "offset"},
+			name: "mixed sticky and normal args",
+			args: []string{"--limit100", "--name", "test", "--offset50"},
+			flags: specs(
+				flagSpec{name: "limit", typ: "int"},
+				flagSpec{name: "name"},
+				flagSpec{name: "offset", typ: "int"},
+			),
 			want:        "--limit 100 --name test --offset 50",
 			corrections: 2,
 		},
 		{
 			name:        "single dash prefix is ignored",
 			args:        []string{"-l100"},
-			flags:       []string{"l"},
+			flags:       specs(flagSpec{name: "l", typ: "int"}),
 			want:        "-l100",
 			corrections: 0,
 		},
 		{
 			name:        "empty args",
 			args:        []string{},
-			flags:       []string{"limit"},
+			flags:       specs(flagSpec{name: "limit", typ: "int"}),
 			want:        "",
 			corrections: 0,
 		},
 		{
 			name:        "bare double dash",
 			args:        []string{"--"},
-			flags:       []string{"limit"},
+			flags:       specs(flagSpec{name: "limit", typ: "int"}),
 			want:        "--",
 			corrections: 0,
 		},
 		{
 			name:        "no flag specs available",
 			args:        []string{"--limit100"},
-			flags:       []string{},
+			flags:       nil,
 			want:        "--limit100",
 			corrections: 0,
 		},
 		{
 			name:        "exact flag name is not split",
 			args:        []string{"--limit"},
-			flags:       []string{"limit"},
+			flags:       specs(flagSpec{name: "limit", typ: "int"}),
 			want:        "--limit",
 			corrections: 0,
 		},
 		{
-			name:        "boolean-like value still splits",
+			name:        "boolean-like value splits when type is bool",
 			args:        []string{"--verbosetrue"},
-			flags:       []string{"verbose"},
+			flags:       specs(flagSpec{name: "verbose", typ: "bool"}),
 			want:        "--verbose true",
 			corrections: 1,
 		},
 		{
 			name:        "hyphenated flag name with numeric suffix",
 			args:        []string{"--page-size50"},
-			flags:       []string{"page-size", "page"},
+			flags:       specs(flagSpec{name: "page-size", typ: "int"}, flagSpec{name: "page", typ: "int"}),
 			want:        "--page-size 50",
 			corrections: 1,
 		},
+
+		// --- Hardening cases: typo flags MUST NOT be misinterpreted ---
+		{
+			name: "typo --starttime1 not split (date-time format)",
+			args: []string{"--starttime1", "2026-02-07"},
+			flags: specs(flagSpec{
+				name: "start", typ: "string", format: "date-time",
+			}),
+			want:        "--starttime1 2026-02-07",
+			corrections: 0,
+		},
+		{
+			name: "glued ISO date splits cleanly (date-time format)",
+			args: []string{"--start2026-02-07"},
+			flags: specs(flagSpec{
+				name: "start", typ: "string", format: "date-time",
+			}),
+			want:        "--start 2026-02-07",
+			corrections: 1,
+		},
+		{
+			name:        "int flag rejects alpha suffix",
+			args:        []string{"--limitabc"},
+			flags:       specs(flagSpec{name: "limit", typ: "int"}),
+			want:        "--limitabc",
+			corrections: 0,
+		},
+		{
+			name:        "bool flag rejects non-literal suffix",
+			args:        []string{"--verbosehello"},
+			flags:       specs(flagSpec{name: "verbose", typ: "bool"}),
+			want:        "--verbosehello",
+			corrections: 0,
+		},
+		{
+			name: "string + enum: suffix hits enum splits",
+			args: []string{"--statusapproved"},
+			flags: specs(flagSpec{
+				name: "status", typ: "string", enum: []string{"approved", "pending"},
+			}),
+			want:        "--status approved",
+			corrections: 1,
+		},
+		{
+			name: "string + enum: suffix not in enum refuses split",
+			args: []string{"--statusunknown"},
+			flags: specs(flagSpec{
+				name: "status", typ: "string", enum: []string{"approved", "pending"},
+			}),
+			want:        "--statusunknown",
+			corrections: 0,
+		},
+		{
+			name: "email format: suffix containing @ splits",
+			args: []string{"--emailfoo@bar.com"},
+			flags: specs(flagSpec{
+				name: "email", typ: "string", format: "email",
+			}),
+			want:        "--email foo@bar.com",
+			corrections: 1,
+		},
+		{
+			name: "email format: suffix without @ refuses split",
+			args: []string{"--emailalice"},
+			flags: specs(flagSpec{
+				name: "email", typ: "string", format: "email",
+			}),
+			want:        "--emailalice",
+			corrections: 0,
+		},
+		{
+			name:        "stringSlice: refuses to split (ambiguous)",
+			args:        []string{"--tagsfoo,bar"},
+			flags:       specs(flagSpec{name: "tags", typ: "stringSlice"}),
+			want:        "--tagsfoo,bar",
+			corrections: 0,
+		},
+		{
+			name:        "plain string + no metadata: alpha suffix refuses split",
+			args:        []string{"--nameJohn"},
+			flags:       specs(flagSpec{name: "name"}),
+			want:        "--nameJohn",
+			corrections: 0,
+		},
+		{
+			name:        "plain string + no metadata: digit-led suffix splits",
+			args:        []string{"--name123"},
+			flags:       specs(flagSpec{name: "name"}),
+			want:        "--name 123",
+			corrections: 1,
+		},
+		{
+			name:        "duration type: digit-led suffix splits",
+			args:        []string{"--timeout30s"},
+			flags:       specs(flagSpec{name: "timeout", typ: "duration"}),
+			want:        "--timeout 30s",
+			corrections: 1,
+		},
+		{
+			name:        "duration type: alpha suffix refuses split",
+			args:        []string{"--timeoutever"},
+			flags:       specs(flagSpec{name: "timeout", typ: "duration"}),
+			want:        "--timeoutever",
+			corrections: 0,
+		},
 	}

 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			ctx := &pipeline.Context{
 				Args:      append([]string{}, tt.args...),
-				FlagSpecs: flagSpecs(tt.flags...),
+				FlagSpecs: tt.flags,
 			}
 			h := StickyHandler{}
 			if err := h.Handle(ctx); err != nil {