From 6707e56f9cb93dae3b41f6a46bcc386a42db2443 Mon Sep 17 00:00:00 2001 From: xianfeng wang Date: Wed, 13 May 2026 10:56:02 +0800 Subject: [PATCH] feat(cli): schema-aware sticky flag splitting and structured unknown-flag recovery (#272) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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 get split into --name ... 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 --------- Co-authored-by: github-actions[bot] Co-authored-by: Cursor --- CHANGELOG.md | 8 + internal/app/flag_error_test.go | 75 +++++++ internal/app/root.go | 24 ++- internal/compat/dynamic_commands.go | 21 +- internal/compat/dynamic_commands_test.go | 83 ++++++++ internal/errors/errors.go | 71 +++++-- internal/errors/errors_test.go | 21 ++ internal/pipeline/cobra.go | 35 +++- .../pipeline/handlers/integration_test.go | 57 ++++-- internal/pipeline/handlers/sticky.go | 53 ++++- internal/pipeline/handlers/sticky_test.go | 188 +++++++++++++++--- internal/pipeline/phase.go | 14 +- pkg/cmdutil/flagfix.go | 84 +++++++- pkg/cmdutil/flagfix_test.go | 106 ++++++++++ pkg/cmdutil/sticky_suffix.go | 133 +++++++++++++ pkg/cmdutil/sticky_suffix_test.go | 91 +++++++++ 16 files changed, 982 insertions(+), 82 deletions(-) create mode 100644 internal/app/flag_error_test.go create mode 100644 pkg/cmdutil/flagfix_test.go create mode 100644 pkg/cmdutil/sticky_suffix.go create mode 100644 pkg/cmdutil/sticky_suffix_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 3e32e5bc..e253fc9d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,14 @@ Platform-stability round: Windows PAT-auth browser opener no longer truncates UR - **`-f ndjson` and `-f csv` global output formats** (#259, closes #252) — `ndjson` emits one compact JSON record per line (works straight with `jq -c` / `while read` / log pipelines); `csv` goes through `encoding/csv` (RFC-4180 — quoting, embedded newlines, CJK all handled by stdlib) and reuses the existing `-f table` column resolver (`normalizePayload` / `unwrapPrimaryObject` / `extractRowsFromMap` / `rowsFromSlice` / `formatValue`) so table and csv stay visually aligned. After a 7-product real-traffic sweep (contact / chat / doc / mail / todo / minutes / schema), the `preferredListKeys` whitelist was extended to cover the actual DingTalk envelope shapes — `contact user search` (`result`), `chat search` (`result.value`), `doc search` (`documents`), `mail mailbox list` (`emailAccounts`), `todo task list` (`result.todoCards`) — so these commands now degrade into a proper row stream instead of collapsing to a single-line `key,value` blob. Lives in `internal/output/ndjson.go` + `internal/output/csv.go`; `--format` help in `internal/app/flags.go` now lists `ndjson|csv` alongside `json|table|raw|pretty`. +### Changed + +- **Sticky flag splitting is now schema-aware** (#272) — PreParse `StickyHandler` 此前会把任何前缀命中已知 flag 的 `--flagsuffix` 一律切成 `--flag suffix`，于是 `--starttime20260507` 这类拼错被静默改写成 `--start time20260507`，把假值传到下游。新行为按 flag 的 pflag 类型 / JSON Schema `format` / `enum` 校验 suffix 是否像合法 value（共享逻辑见 `pkg/cmdutil/sticky_suffix.go`），不像就保留原 token 让 cobra 报 `unknown flag`。slice/array/object 类型的 flag 永不切分。首 rune 读取使用 `utf8.DecodeRuneInString`，对中文等多字节 value 安全。 + +### Added + +- **`available_flags` field on unknown-flag errors** (#272) — `dws -f json` 的 unknown-flag 错误体里新增 `available_flags`（已排序、过滤掉 hidden 与内部 `json` / `params`），方便 agent 不解析 `--help` 就能恢复。Human-readable 输出会附 `Flags: ...` 行，截断在 200 字节内。 + ### Fixed - **`dws chat message send` 单聊缺 `--title` 时前置校验** (#250) — 单聊（`--user` / `--open-dingtalk-id`）的底层工具 `send_direct_message_as_user` 在 API 层强制要求 title，缺失时返回误导性的 `发群服务窗会话消息失败`。CLI 现在在 `buildChatMessageSendInvocation` 里前置校验，直接返回 `--title is required for direct messages (--user / --open-dingtalk-id)`；同时把 `Long` help、`--title` flag 描述、Example 和 `skills/references/products/chat.md` 全部对齐为「单聊必填，群聊可选」。群聊行为不变。 diff --git a/internal/app/flag_error_test.go b/internal/app/flag_error_test.go new file mode 100644 index 00000000..9a8025c1 --- /dev/null +++ b/internal/app/flag_error_test.go @@ -0,0 +1,75 @@ +// Copyright 2026 Alibaba Group +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package app + +import ( + stderrors "errors" + "fmt" + "strings" + "testing" + + apperrors "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/errors" + "github.com/spf13/cobra" +) + +func TestFlagErrorWithSuggestions_authStructured(t *testing.T) { + t.Parallel() + cmd := &cobra.Command{Use: "login", Run: func(*cobra.Command, []string) {}} + orig := fmt.Errorf("unknown flag: --json") + err := flagErrorWithSuggestions(cmd, orig) + var ae *apperrors.Error + if !stderrors.As(err, &ae) { + t.Fatalf("want *apperrors.Error, got %T", err) + } + if ae.Message != orig.Error() { + t.Fatalf("Message = %q, want %q", ae.Message, orig.Error()) + } + if ae.Reason != "unknown_flag" { + t.Fatalf("Reason = %q, want unknown_flag", ae.Reason) + } + if ae.Hint == "" || !strings.Contains(ae.Hint, "format json") { + t.Fatalf("Hint = %q", ae.Hint) + } + if ae.Cause != orig { + t.Fatalf("Cause = %v, want orig", ae.Cause) + } + if !stderrors.Is(err, orig) { + t.Fatal("errors.Is(err, orig) should hold via unwrap") + } +} + +func TestFlagErrorWithSuggestions_unknownFlagHintAndFlags(t *testing.T) { + t.Parallel() + cmd := &cobra.Command{Use: "list", Run: func(*cobra.Command, []string) {}} + cmd.Flags().String("start", "", "begin time") + _ = cmd.Flags().SetAnnotation("start", "x-cli-format", []string{"date-time"}) + orig := fmt.Errorf("unknown flag: --starttime1") + err := flagErrorWithSuggestions(cmd, orig) + var ae *apperrors.Error + if !stderrors.As(err, &ae) { + t.Fatalf("want *apperrors.Error, got %T", err) + } + if ae.Reason != "unknown_flag" { + t.Fatalf("Reason = %q", ae.Reason) + } + if strings.Contains(ae.Hint, "Space required") { + t.Fatalf("false glue must not suggest space: %q", ae.Hint) + } + if !strings.Contains(ae.Hint, "help") { + t.Fatalf("expected help fallback in hint, got %q", ae.Hint) + } + if len(ae.AvailableFlags) != 1 || ae.AvailableFlags[0] != "start" { + t.Fatalf("AvailableFlags = %v, want [start]", ae.AvailableFlags) + } +} diff --git a/internal/app/root.go b/internal/app/root.go index 1f969a08..adb32a01 100644 --- a/internal/app/root.go +++ b/internal/app/root.go @@ -47,6 +47,7 @@ import ( "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/plugin" "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/recovery" "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/transport" + "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/pkg/cmdutil" "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/pkg/config" "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/pkg/edition" "github.com/spf13/cobra" @@ -133,7 +134,28 @@ func flagErrorWithSuggestions(cmd *cobra.Command, err error) error { for flag, suggestion := range suggestions { if strings.Contains(errMsg, "unknown flag: "+flag) { - return fmt.Errorf("%w\n%s", err, suggestion) + return apperrors.NewValidation( + errMsg, + apperrors.WithHint(suggestion), + apperrors.WithReason("unknown_flag"), + apperrors.WithCause(err), + apperrors.WithActions(fmt.Sprintf("Run '%s --help' for valid flags", cmd.CommandPath())), + apperrors.WithAvailableFlags(cmdutil.VisibleFlagNames(cmd)...), + ) + } + } + + if strings.Contains(errMsg, "unknown flag:") { + fix := cmdutil.SuggestFlagFix(cmd, err) + if fix.Suggestion != "" { + return apperrors.NewValidation( + errMsg, + apperrors.WithHint(fix.Suggestion), + apperrors.WithReason("unknown_flag"), + apperrors.WithCause(err), + apperrors.WithActions(fmt.Sprintf("Run '%s --help' for valid flags", cmd.CommandPath())), + apperrors.WithAvailableFlags(cmdutil.VisibleFlagNames(cmd)...), + ) } } diff --git a/internal/compat/dynamic_commands.go b/internal/compat/dynamic_commands.go index 6d0c0ea7..c964c61a 100644 --- a/internal/compat/dynamic_commands.go +++ b/internal/compat/dynamic_commands.go @@ -283,10 +283,12 @@ type toolRequestSchema struct { } type toolRequestProp struct { - Type string `json:"type"` - Title string `json:"title"` - Description string `json:"description"` - Default string `json:"default,omitempty"` + Type string `json:"type"` + Title string `json:"title"` + Description string `json:"description"` + Default string `json:"default,omitempty"` + Format string `json:"format,omitempty"` + Enum []string `json:"enum,omitempty"` } // buildFlagsFromDetailSchema adds properly-typed cobra flags to cmd based on @@ -371,6 +373,17 @@ func buildFlagsFromDetailSchema(cmd *cobra.Command, schemaJSON string, flagOverr cmd.Flags().String(flagName, defaultVal, help) } + // Carry schema "format" / "enum" hints onto the cobra flag via + // pflag annotations so PreParse handlers (e.g. StickyHandler) + // can reason about whether a glued suffix looks like a real + // value. The annotation keys are read by FlagInfoFromCommand. + if prop.Format != "" { + _ = cmd.Flags().SetAnnotation(flagName, "x-cli-format", []string{prop.Format}) + } + if len(prop.Enum) > 0 { + _ = cmd.Flags().SetAnnotation(flagName, "x-cli-enum", append([]string{}, prop.Enum...)) + } + if requiredSet[key] { _ = cmd.MarkFlagRequired(flagName) } diff --git a/internal/compat/dynamic_commands_test.go b/internal/compat/dynamic_commands_test.go index 1bd06621..1aed79f5 100644 --- a/internal/compat/dynamic_commands_test.go +++ b/internal/compat/dynamic_commands_test.go @@ -1945,3 +1945,86 @@ func TestBuildDynamicCommands_ParentMergeLeafCollision(t *testing.T) { t.Fatalf("expected exactly one 'send' leaf, got %d", sendCount) } } + +// TestBuildFlagsFromDetailSchema_FormatEnumAnnotations verifies that the +// JSON Schema "format" and "enum" hints are copied onto the cobra flag's +// pflag annotations under x-cli-format / x-cli-enum, so PreParse +// handlers can use them when deciding whether to split glued tokens. +func TestBuildFlagsFromDetailSchema_FormatEnumAnnotations(t *testing.T) { + t.Parallel() + + servers := []market.ServerDescriptor{ + { + Endpoint: "https://endpoint-calendar", + CLI: market.CLIOverlay{ + ID: "calendar", + Command: "calendar", + ToolOverrides: map[string]market.CLIToolOverride{ + "event_list": {CLIName: "list"}, + }, + }, + }, + } + + details := map[string][]market.DetailTool{ + "calendar": { + { + ToolName: "event_list", + ToolRequest: `{"properties":{` + + `"start":{"type":"string","format":"date-time","description":"开始时间"},` + + `"end":{"type":"string","format":"date-time","description":"结束时间"},` + + `"status":{"type":"string","enum":["confirmed","tentative","cancelled"]}` + + `}}`, + }, + }, + } + + cmds := BuildDynamicCommands(servers, executor.EchoRunner{}, details) + list := findChild(cmds[0], "list") + if list == nil { + t.Fatal("list leaf not found") + } + + startFlag := list.Flags().Lookup("start") + if startFlag == nil { + t.Fatal("--start flag missing") + } + if got := startFlag.Annotations["x-cli-format"]; len(got) != 1 || got[0] != "date-time" { + t.Errorf("--start x-cli-format = %v, want [date-time]", got) + } + + endFlag := list.Flags().Lookup("end") + if endFlag == nil { + t.Fatal("--end flag missing") + } + if got := endFlag.Annotations["x-cli-format"]; len(got) != 1 || got[0] != "date-time" { + t.Errorf("--end x-cli-format = %v, want [date-time]", got) + } + + statusFlag := list.Flags().Lookup("status") + if statusFlag == nil { + t.Fatal("--status flag missing") + } + gotEnum := statusFlag.Annotations["x-cli-enum"] + wantEnum := []string{"confirmed", "tentative", "cancelled"} + if !equalStringSlice(gotEnum, wantEnum) { + t.Errorf("--status x-cli-enum = %v, want %v", gotEnum, wantEnum) + } + // Status has no format and should not carry x-cli-format. + if got := statusFlag.Annotations["x-cli-format"]; len(got) != 0 { + t.Errorf("--status should not have x-cli-format, got %v", got) + } +} + +// equalStringSlice is a small helper for slice comparison in tests. +func equalStringSlice(a, b []string) bool { + if len(a) != len(b) { + return false + } + for i := range a { + if a[i] != b[i] { + return false + } + } + return true +} diff --git a/internal/errors/errors.go b/internal/errors/errors.go index 56cfe6a4..6f0be969 100644 --- a/internal/errors/errors.go +++ b/internal/errors/errors.go @@ -37,19 +37,20 @@ const ( // Error is the structured repository-local error model for the Go rewrite. type Error struct { - Category Category - Message string - Operation string - ServerKey string - Retryable bool - Reason string - Hint string - Actions []string - Snapshot string - RPCCode int `json:"rpc_code,omitempty"` - RPCData json.RawMessage `json:"rpc_data,omitempty"` - ServerDiag ServerDiagnostics `json:"-"` - Cause error `json:"-"` + Category Category + Message string + Operation string + ServerKey string + Retryable bool + Reason string + Hint string + Actions []string + AvailableFlags []string + Snapshot string + RPCCode int `json:"rpc_code,omitempty"` + RPCData json.RawMessage `json:"rpc_data,omitempty"` + ServerDiag ServerDiagnostics `json:"-"` + Cause error `json:"-"` } func (e *Error) Error() string { @@ -135,6 +136,16 @@ func WithActions(actions ...string) Option { } } +// WithAvailableFlags records visible local flag names for agent recovery. +func WithAvailableFlags(names ...string) Option { + return func(err *Error) { + if len(names) == 0 { + return + } + err.AvailableFlags = append([]string{}, names...) + } +} + // WithSnapshot records the recovery snapshot path associated with the failure. func WithSnapshot(path string) Option { return func(err *Error) { @@ -260,6 +271,9 @@ func PrintJSON(w io.Writer, err error) error { if len(typed.Actions) > 0 { errorPayload["actions"] = typed.Actions } + if len(typed.AvailableFlags) > 0 { + errorPayload["available_flags"] = typed.AvailableFlags + } if typed.Snapshot != "" { errorPayload["snapshot_path"] = typed.Snapshot } @@ -359,6 +373,9 @@ func PrintHumanAt(w io.Writer, err error, v Verbosity) error { lines = append(lines, fmt.Sprintf("Action: %s", action)) } } + if line := formatAvailableFlagsHumanLine(typed.AvailableFlags); line != "" { + lines = append(lines, line) + } if typed.Retryable { lines = append(lines, "Retryable: true") } @@ -414,3 +431,31 @@ func category(err error) string { } return string(CategoryInternal) } + +const availableFlagsHumanMaxRunes = 200 + +func formatAvailableFlagsHumanLine(flags []string) string { + if len(flags) == 0 { + return "" + } + b := strings.Builder{} + b.WriteString("Flags: ") + written := 0 + for i, name := range flags { + if i > 0 { + if written+2 > availableFlagsHumanMaxRunes { + b.WriteString("...") + return b.String() + } + b.WriteString(", ") + written += 2 + } + if written+len(name) > availableFlagsHumanMaxRunes { + b.WriteString("...") + return b.String() + } + b.WriteString(name) + written += len(name) + } + return b.String() +} diff --git a/internal/errors/errors_test.go b/internal/errors/errors_test.go index 293480f4..fc7c2ae3 100644 --- a/internal/errors/errors_test.go +++ b/internal/errors/errors_test.go @@ -77,6 +77,27 @@ func TestPrintJSON(t *testing.T) { } } +func TestPrintJSON_AvailableFlags(t *testing.T) { + t.Parallel() + + var b strings.Builder + if err := PrintJSON(&b, NewValidation( + "unknown flag: --foo", + WithReason("unknown_flag"), + WithHint("Did you mean --bar?"), + WithAvailableFlags("bar", "baz"), + )); err != nil { + t.Fatalf("PrintJSON() error = %v", err) + } + got := b.String() + if !strings.Contains(got, `"available_flags"`) { + t.Fatalf("expected available_flags in output, got %q", got) + } + if !strings.Contains(got, `"bar"`) || !strings.Contains(got, `"baz"`) { + t.Fatalf("expected flag names in output, got %q", got) + } +} + func TestPrintHuman(t *testing.T) { t.Parallel() diff --git a/internal/pipeline/cobra.go b/internal/pipeline/cobra.go index 7c401735..16f348e5 100644 --- a/internal/pipeline/cobra.go +++ b/internal/pipeline/cobra.go @@ -79,6 +79,12 @@ func RunPreParse(root *cobra.Command, engine *Engine) { // FlagInfoFromCommand extracts FlagInfo entries from a Cobra // command's registered flags (both local and inherited). +// +// JSON Schema "format" and "enum" hints injected via pflag +// annotations (x-cli-format / x-cli-enum, see +// internal/compat/dynamic_commands.go) are surfaced on FlagInfo +// so PreParse handlers can validate sticky-split candidates against +// the actual schema, not just the pflag type. func FlagInfoFromCommand(cmd *cobra.Command) []FlagInfo { if cmd == nil { return nil @@ -92,11 +98,7 @@ func FlagInfoFromCommand(cmd *cobra.Command) []FlagInfo { return } seen[f.Name] = true - infos = append(infos, FlagInfo{ - Name: f.Name, - PropertyName: f.Name, - Type: f.Value.Type(), - }) + infos = append(infos, flagInfoFromPflag(f)) }) cmd.InheritedFlags().VisitAll(func(f *pflag.Flag) { @@ -104,12 +106,25 @@ func FlagInfoFromCommand(cmd *cobra.Command) []FlagInfo { return } seen[f.Name] = true - infos = append(infos, FlagInfo{ - Name: f.Name, - PropertyName: f.Name, - Type: f.Value.Type(), - }) + infos = append(infos, flagInfoFromPflag(f)) }) return infos } + +// flagInfoFromPflag builds a FlagInfo from a pflag.Flag, copying +// schema metadata stashed in the flag's annotations map. +func flagInfoFromPflag(f *pflag.Flag) FlagInfo { + fi := FlagInfo{ + Name: f.Name, + PropertyName: f.Name, + Type: f.Value.Type(), + } + if v := f.Annotations["x-cli-format"]; len(v) > 0 { + fi.Format = v[0] + } + if v := f.Annotations["x-cli-enum"]; len(v) > 0 { + fi.Enum = append([]string{}, v...) + } + return fi +} diff --git a/internal/pipeline/handlers/integration_test.go b/internal/pipeline/handlers/integration_test.go index dd8ef601..6132781c 100644 --- a/internal/pipeline/handlers/integration_test.go +++ b/internal/pipeline/handlers/integration_test.go @@ -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) diff --git a/internal/pipeline/handlers/sticky.go b/internal/pipeline/handlers/sticky.go index af99aed9..51d43c3a 100644 --- a/internal/pipeline/handlers/sticky.go +++ b/internal/pipeline/handlers/sticky.go @@ -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 { diff --git a/internal/pipeline/handlers/sticky_test.go b/internal/pipeline/handlers/sticky_test.go index 434c356f..b46b57de 100644 --- a/internal/pipeline/handlers/sticky_test.go +++ b/internal/pipeline/handlers/sticky_test.go @@ -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 { diff --git a/internal/pipeline/phase.go b/internal/pipeline/phase.go index 92acb766..2f3efbaa 100644 --- a/internal/pipeline/phase.go +++ b/internal/pipeline/phase.go @@ -120,8 +120,20 @@ type FlagInfo struct { // PropertyName is the original schema property key (e.g. "userId"). PropertyName string - // Type is the JSON Schema type ("string", "integer", etc.). + // Type is the JSON Schema / pflag type ("string", "integer", + // "bool", "stringSlice", "duration", etc.). Type string + + // Format carries the JSON Schema "format" hint when present — + // e.g. "date", "date-time", "duration", "email", "uri", "ipv4". + // PreParse handlers use this to decide whether a suffix in a + // glued token (e.g. "--starttime1") looks like a plausible value. + Format string + + // Enum carries the JSON Schema "enum" string values when present. + // PreParse handlers use this for sticky-split validation: a glued + // suffix is accepted only if it matches one of the enum entries. + Enum []string } // Correction records a single input correction applied by a handler. diff --git a/pkg/cmdutil/flagfix.go b/pkg/cmdutil/flagfix.go index 610937a9..ccd09c83 100644 --- a/pkg/cmdutil/flagfix.go +++ b/pkg/cmdutil/flagfix.go @@ -15,6 +15,7 @@ package cmdutil import ( "fmt" + "sort" "strings" "github.com/spf13/cobra" @@ -75,6 +76,41 @@ func DetectNumericTypeError(err error) (flagName, badValue string, ok bool) { return rest[:endIdx], badVal, true } +// flagFixCandidate reports whether f should participate in unknown-flag +// suggestion candidates and Flags: listings. Hidden flags (e.g. wukong's +// MarkHidden compatibility aliases) and internal json/params merge flags +// are skipped so the hint candidate set stays a subset of what --help shows. +func flagFixCandidate(f *pflag.Flag) bool { + if f == nil || f.Hidden { + return false + } + switch f.Name { + case "json", "params": + return false + } + return true +} + +// VisibleFlagNames returns sorted candidate flag names for cmd.Flags() +// using flagFixCandidate. Intended for agent-facing error recovery +// (available_flags). +func VisibleFlagNames(cmd *cobra.Command) []string { + if cmd == nil { + return nil + } + seen := make(map[string]bool) + var names []string + cmd.Flags().VisitAll(func(f *pflag.Flag) { + if !flagFixCandidate(f) || seen[f.Name] { + return + } + seen[f.Name] = true + names = append(names, f.Name) + }) + sort.Strings(names) + return names +} + // SuggestFlagFix detects flag-value concatenation errors, common flag aliases, // and Levenshtein-close typos. func SuggestFlagFix(cmd *cobra.Command, flagErr error) FlagFixResult { @@ -92,6 +128,9 @@ func SuggestFlagFix(cmd *cobra.Command, flagErr error) FlagFixResult { var bestFlag, bestValue string cmd.Flags().VisitAll(func(f *pflag.Flag) { + if !flagFixCandidate(f) { + return + } name := f.Name if strings.HasPrefix(body, name) && len(body) > len(name) { if len(name) > len(bestFlag) { @@ -101,16 +140,28 @@ func SuggestFlagFix(cmd *cobra.Command, flagErr error) FlagFixResult { } }) if bestFlag != "" { - canAutoFix := len(bestValue) > 0 - suggestion := fmt.Sprintf("Space required between flag and value: --%s %s", bestFlag, bestValue) - if canAutoFix { - return FlagFixResult{Suggestion: suggestion, AutoFixFlag: bestFlag, AutoFixValue: bestValue} + lf := cmd.Flags().Lookup(bestFlag) + if lf != nil { + fmtStr := "" + if v := lf.Annotations["x-cli-format"]; len(v) > 0 { + fmtStr = v[0] + } + var enumCopy []string + if v := lf.Annotations["x-cli-enum"]; len(v) > 0 { + enumCopy = append([]string{}, v...) + } + if SuffixLooksLikeValue(bestValue, lf.Value.Type(), fmtStr, enumCopy) { + suggestion := fmt.Sprintf("Space required between flag and value: --%s %s", bestFlag, bestValue) + return FlagFixResult{Suggestion: suggestion, AutoFixFlag: bestFlag, AutoFixValue: bestValue} + } } - return FlagFixResult{Suggestion: suggestion} } bestName, bestDist := "", 999 cmd.Flags().VisitAll(func(f *pflag.Flag) { + if !flagFixCandidate(f) { + return + } d := LevenshteinDist(body, f.Name) if d < bestDist { bestDist = d @@ -119,12 +170,33 @@ func SuggestFlagFix(cmd *cobra.Command, flagErr error) FlagFixResult { }) threshold := LevenshteinThreshold(len(body)) if bestDist > 0 && bestDist <= threshold && bestName != "" { - return FlagFixResult{Suggestion: fmt.Sprintf("Did you mean --%s?", bestName)} + suf := formatFlagHintSuffix(cmd.Flags().Lookup(bestName)) + return FlagFixResult{Suggestion: fmt.Sprintf("Did you mean --%s?%s", bestName, suf)} } return FlagFixResult{Suggestion: fmt.Sprintf("Run '%s --help' to see available options", cmd.CommandPath())} } +func formatFlagHintSuffix(f *pflag.Flag) string { + if f == nil { + return "" + } + var parts []string + if u := strings.TrimSpace(f.Usage); u != "" { + if len(u) > 100 { + u = u[:97] + "..." + } + parts = append(parts, u) + } + if v := f.Annotations["x-cli-format"]; len(v) > 0 && v[0] != "" { + parts = append(parts, "format="+v[0]) + } + if len(parts) == 0 { + return "" + } + return " (" + strings.Join(parts, ", ") + ")" +} + // LevenshteinThreshold returns the max edit distance allowed based on string length. func LevenshteinThreshold(nameLen int) int { if nameLen <= 3 { diff --git a/pkg/cmdutil/flagfix_test.go b/pkg/cmdutil/flagfix_test.go new file mode 100644 index 00000000..e68b2509 --- /dev/null +++ b/pkg/cmdutil/flagfix_test.go @@ -0,0 +1,106 @@ +// Copyright 2026 Alibaba Group +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package cmdutil + +import ( + "strings" + "testing" + + "github.com/spf13/cobra" +) + +func TestSuggestFlagFix_falseGlue_starttime1(t *testing.T) { + cmd := &cobra.Command{Use: "list"} + cmd.Flags().String("start", "", "begin time") + _ = cmd.Flags().SetAnnotation("start", "x-cli-format", []string{"date-time"}) + + err := errUnknownFlag("starttime1") + fix := SuggestFlagFix(cmd, err) + if strings.Contains(fix.Suggestion, "Space required") { + t.Fatalf("should not treat as glued value, got %q", fix.Suggestion) + } + if fix.AutoFixFlag != "" { + t.Fatalf("AutoFixFlag = %q, want empty", fix.AutoFixFlag) + } + if !strings.Contains(fix.Suggestion, "--help") { + t.Fatalf("expected help fallback, got %q", fix.Suggestion) + } +} + +func TestSuggestFlagFix_trueGlue_isoDateSuffix(t *testing.T) { + cmd := &cobra.Command{Use: "list"} + cmd.Flags().String("start", "", "begin time") + _ = cmd.Flags().SetAnnotation("start", "x-cli-format", []string{"date-time"}) + + err := errUnknownFlag("start2026-02-07") + fix := SuggestFlagFix(cmd, err) + wantSub := "Space required between flag and value: --start 2026-02-07" + if fix.Suggestion != wantSub { + t.Fatalf("Suggestion = %q, want %q", fix.Suggestion, wantSub) + } + if fix.AutoFixFlag != "start" || fix.AutoFixValue != "2026-02-07" { + t.Fatalf("AutoFix = %q/%q, want start/2026-02-07", fix.AutoFixFlag, fix.AutoFixValue) + } +} + +func TestSuggestFlagFix_levenshteinAddsUsage(t *testing.T) { + cmd := &cobra.Command{Use: "send"} + cmd.Flags().String("conversation-id", "", "Conversation id") + + err := errUnknownFlag("conversaton-id") + fix := SuggestFlagFix(cmd, err) + if !strings.HasPrefix(fix.Suggestion, "Did you mean --conversation-id?") { + t.Fatalf("unexpected: %q", fix.Suggestion) + } + if !strings.Contains(fix.Suggestion, "Conversation id") { + t.Fatalf("expected usage in hint, got %q", fix.Suggestion) + } +} + +func TestSuggestFlagFix_skipsHiddenAlias(t *testing.T) { + cmd := &cobra.Command{Use: "list"} + cmd.Flags().String("start", "", "begin time") + _ = cmd.Flags().SetAnnotation("start", "x-cli-format", []string{"date-time"}) + cmd.Flags().String("start-time", "", "") + _ = cmd.Flags().MarkHidden("start-time") + + fix := SuggestFlagFix(cmd, errUnknownFlag("starttime1")) + if strings.Contains(fix.Suggestion, "start-time") { + t.Fatalf("must not recommend hidden alias, got %q", fix.Suggestion) + } + if !strings.Contains(fix.Suggestion, "--help") { + t.Fatalf("expected help fallback, got %q", fix.Suggestion) + } +} + +func TestVisibleFlagNames_skipsHiddenAndInternal(t *testing.T) { + cmd := &cobra.Command{Use: "x"} + cmd.Flags().String("alpha", "", "") + cmd.Flags().String("json", "", "") + cmd.Flags().String("beta", "", "") + _ = cmd.Flags().MarkHidden("beta") + + names := VisibleFlagNames(cmd) + if len(names) != 1 || names[0] != "alpha" { + t.Fatalf("got %v, want [alpha]", names) + } +} + +func errUnknownFlag(body string) error { + return &stubFlagErr{msg: "unknown flag: --" + body} +} + +type stubFlagErr struct{ msg string } + +func (e *stubFlagErr) Error() string { return e.msg } diff --git a/pkg/cmdutil/sticky_suffix.go b/pkg/cmdutil/sticky_suffix.go new file mode 100644 index 00000000..ad1c721e --- /dev/null +++ b/pkg/cmdutil/sticky_suffix.go @@ -0,0 +1,133 @@ +// Copyright 2026 Alibaba Group +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package cmdutil + +import ( + "strings" + "unicode" + "unicode/utf8" +) + +// SuffixLooksLikeValue decides whether a candidate suffix from a glued +// "--flagsuffix" token plausibly represents a value for a flag's +// declared type/format/enum. Shared by StickyHandler (PreParse) and +// SuggestFlagFix (unknown-flag recovery). +// +// typ is a pflag value type string (e.g. "int", "bool", "string"); +// format is JSON Schema "format" when present (e.g. "date-time"); +// enum is the schema enum list when present. +func SuffixLooksLikeValue(suffix, typ, format string, enum []string) bool { + if suffix == "" { + return false + } + + switch typ { + case "int", "int8", "int16", "int32", "int64", + "uint", "uint8", "uint16", "uint32", "uint64", + "integer", "number", "float", "float32", "float64", + "count": + return startsWithNumericSuffix(suffix) + + case "bool", "boolean": + return isBoolLiteralSuffix(suffix) + + case "duration": + return startsWithDigitSuffix(suffix) || startsWithSignSuffix(suffix) + + case "stringSlice", "stringArray", "intSlice", "boolSlice", "float32Slice", + "float64Slice", "uintSlice", "durationSlice", "ipSlice", "array": + return false + + case "object": + return false + } + + if len(enum) > 0 { + return matchesEnumSuffix(suffix, enum) + } + + switch strings.ToLower(format) { + case "date", "date-time", "datetime", "time": + return startsWithDigitSuffix(suffix) + case "duration": + return startsWithDigitSuffix(suffix) || startsWithSignSuffix(suffix) + case "email": + return strings.Contains(suffix, "@") + case "uri", "url": + lower := strings.ToLower(suffix) + return strings.HasPrefix(lower, "http") || + strings.HasPrefix(lower, "ftp") || + strings.HasPrefix(lower, "mailto:") + case "ipv4", "ipv6", "hostname": + return startsWithDigitSuffix(suffix) + case "uuid": + first, _ := utf8.DecodeRuneInString(suffix) + return isHexRuneSuffix(first) + } + + first, _ := utf8.DecodeRuneInString(suffix) + if first == utf8.RuneError || unicode.IsLetter(first) { + return false + } + return true +} + +func startsWithDigitSuffix(s string) bool { + if s == "" { + return false + } + c := s[0] + return c >= '0' && c <= '9' +} + +func startsWithSignSuffix(s string) bool { + if s == "" { + return false + } + c := s[0] + return c == '+' || c == '-' +} + +func startsWithNumericSuffix(s string) bool { + if startsWithDigitSuffix(s) { + return true + } + if startsWithSignSuffix(s) && len(s) > 1 { + c := s[1] + return c >= '0' && c <= '9' + } + return false +} + +func isBoolLiteralSuffix(s string) bool { + switch strings.ToLower(s) { + case "true", "false", "1", "0", "t", "f", "yes", "no", "on", "off", "y", "n": + return true + } + return false +} + +func matchesEnumSuffix(s string, enum []string) bool { + lower := strings.ToLower(s) + for _, e := range enum { + if strings.ToLower(e) == lower { + return true + } + } + return false +} + +func isHexRuneSuffix(r rune) bool { + return (r >= '0' && r <= '9') || (r >= 'a' && r <= 'f') || (r >= 'A' && r <= 'F') +} diff --git a/pkg/cmdutil/sticky_suffix_test.go b/pkg/cmdutil/sticky_suffix_test.go new file mode 100644 index 00000000..0539a3ee --- /dev/null +++ b/pkg/cmdutil/sticky_suffix_test.go @@ -0,0 +1,91 @@ +// Copyright 2026 Alibaba Group +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package cmdutil + +import "testing" + +// TestSuffixLooksLikeValue_UTF8FirstRune locks in the UTF-8 first-rune +// reading contract on SuffixLooksLikeValue. The function previously read +// suffix[0] (a single byte) which produced incorrect splits / matches for +// any multi-byte first rune — relevant because dws is a Chinese-language +// CLI and value text often starts with CJK characters. +func TestSuffixLooksLikeValue_UTF8FirstRune(t *testing.T) { + t.Parallel() + + cases := []struct { + name string + suffix string + typ string + format string + enum []string + want bool + }{ + { + // --name张三 must NOT be split: with no metadata the fallback + // branch should treat a CJK letter as "not a value-looking + // suffix" so cobra reports unknown flag instead of cutting + // the user's typo into --name + 张三. + name: "plain string + CJK letter suffix refuses split", + suffix: "张三", + typ: "string", + want: false, + }, + { + // CJK leading rune is fine as long as the email-format guard + // ('@' anywhere in suffix) still passes. + name: "email format + CJK leading + @ allows split", + suffix: "张三@example.com", + typ: "string", + format: "email", + want: true, + }, + { + // uuid format: first rune must be hex. CJK starting char is + // not hex so the suffix must be rejected. + name: "uuid format + CJK leading rejects split", + suffix: "张abcd", + typ: "string", + format: "uuid", + want: false, + }, + { + // Baseline: digit-led suffix on int still splits — guards + // against accidental over-tightening of the fallback path. + name: "int + digit-led baseline still splits", + suffix: "100", + typ: "int", + want: true, + }, + { + // Invalid UTF-8 (lone continuation byte 0x80) decodes as + // utf8.RuneError; the RuneError guard makes the fallback + // reject it instead of treating it as "non-letter, splittable". + name: "plain string + invalid UTF-8 leading byte refuses split", + suffix: "\x80abc", + typ: "string", + want: false, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + got := SuffixLooksLikeValue(tc.suffix, tc.typ, tc.format, tc.enum) + if got != tc.want { + t.Errorf("SuffixLooksLikeValue(%q, %q, %q, %v) = %v, want %v", + tc.suffix, tc.typ, tc.format, tc.enum, got, tc.want) + } + }) + } +}