From be0242f90de744a8c2fae5c8e96feae51697d720 Mon Sep 17 00:00:00 2001 From: ZhiJiaXing Date: Mon, 14 Sep 2026 19:23:38 +0800 Subject: [PATCH 1/5] fix: unify product error suggestions across human and AI modes - keep the original casing of the unknown product/command token in error messages instead of lowercasing it - route human and AI suggestions through one shared pipeline (close typo, then prefix) so prefix typos like "openapiex" also surface Did-you-mean hints in human mode - reverse-look PascalCase/kebab root tokens up as API names across all products and suggest runnable full commands in the input's own style (e.g. "aliyun ecs DescribeRegions"), capped at the default suggest limit - guard GetSuggestions against nil product and skip the overflow hint when no recovery command exists - drop the unused removeDuplicates helper --- openapi/agent_error_test.go | 12 +++ openapi/errors.go | 43 ++++------- openapi/errors_additional_test.go | 4 +- openapi/errors_test.go | 122 +++++++++++++++++++++++------- openapi/product_suggestion.go | 108 ++++++++++++++++++++++++++ 5 files changed, 230 insertions(+), 59 deletions(-) create mode 100644 openapi/product_suggestion.go diff --git a/openapi/agent_error_test.go b/openapi/agent_error_test.go index 279bb5d75..47d6a5d1f 100644 --- a/openapi/agent_error_test.go +++ b/openapi/agent_error_test.go @@ -118,6 +118,18 @@ func TestNormalizeAgentErrorSupportedLocalRecoveries(t *testing.T) { assert.Equal(t, RecoverySearchRequest{Keyword: "ecs"}, request) }) + t.Run("unknown PascalCase token reverse-looks up the owning product", func(t *testing.T) { + repo, err := meta.MockLoadRepository([]meta.Product{{Code: "ecs", ApiNames: []string{"DescribeRegions"}}}) + require.NoError(t, err) + cause := &InvalidProductError{Code: "DescribeRegions", library: &Library{builtinRepo: repo}} + envelope := requireAgentEnvelope(t, cause, []string{"DescribeRegions"}, func(RecoverySearchRequest) bool { + return true + }) + + assert.Equal(t, `"DescribeRegions" is not a valid command or product.`, envelope.Message) + assert.Equal(t, []string{"aliyun ecs DescribeRegions"}, envelope.DidYouMean) + }) + t.Run("unknown API derives a resource keyword from a real candidate", func(t *testing.T) { cause := &InvalidApiError{ Name: "DescribeInstnaces", diff --git a/openapi/errors.go b/openapi/errors.go index d5f46bae8..4da64471b 100644 --- a/openapi/errors.go +++ b/openapi/errors.go @@ -65,32 +65,23 @@ type InvalidProductError struct { } func (e *InvalidProductError) Error() string { - return fmt.Sprintf("%q is not a valid command or product. See `aliyun help`.", strings.ToLower(e.Code)) + return fmt.Sprintf("%q is not a valid command or product. See `aliyun help`.", e.Code) } func (e *InvalidProductError) AgentMessage() string { - return fmt.Sprintf("%q is not a valid command or product.", strings.ToLower(e.Code)) + return fmt.Sprintf("%q is not a valid command or product.", e.Code) } func (*InvalidProductError) AIRecoveryEligible() {} +// GetSuggestions and AgentSuggestions must render the same candidate list in +// both output modes, so both delegate to the shared productSuggestions pipeline. func (e *InvalidProductError) GetSuggestions() []string { - sr := cli.NewSuggester(strings.ToLower(e.Code), 2) - for _, p := range e.library.GetProducts() { - sr.Apply(strings.ToLower(p.Code)) - } - return sr.GetResults() + return productSuggestions(e.Code, e.library) } func (e *InvalidProductError) AgentSuggestions() []string { - if e.library == nil { - return nil - } - candidates := make([]string, 0) - for _, product := range e.library.GetProducts() { - candidates = append(candidates, strings.ToLower(product.Code)) - } - return apiSuggestions(strings.ToLower(e.Code), candidates) + return productSuggestions(e.Code, e.library) } // return when use unknown api @@ -114,6 +105,9 @@ func (e *InvalidApiError) AgentMessage() string { func (*InvalidApiError) AIRecoveryEligible() {} func (e *InvalidApiError) GetSuggestions() []string { + if e.product == nil { + return nil + } return humanAPISuggestions(e.Name, e.product.ApiNames, apiRecoveryCommand(e.Name, e.product.GetLowerCode(), e.product.ApiNames)) } @@ -365,6 +359,9 @@ func explicitLocalErrorText(err error, fallback string) string { } func (e *InvalidUnifiedApiError) GetSuggestions() []string { + if e.product == nil { + return nil + } candidates := append(append([]string(nil), e.product.ApiNames...), e.lPlugin.CmdNames...) return humanAPISuggestions(e.Name, candidates, apiRecoveryCommand(e.Name, e.product.GetLowerCode(), candidates)) @@ -379,20 +376,6 @@ func (e *InvalidUnifiedApiError) AgentSuggestions() []string { return apiSuggestions(e.Name, candidates) } -func removeDuplicates(slice []string) []string { - seen := make(map[string]bool) - result := []string{} - - for _, item := range slice { - if !seen[item] { - seen[item] = true - result = append(result, item) - } - } - - return result -} - // sameStyleCandidates keeps only candidates written in the input's command // style: mixed-case input keeps PascalCase candidates and all-lowercase // input keeps kebab candidates. Suggestions must not cross styles — a @@ -410,7 +393,7 @@ func sameStyleCandidates(input string, candidates []string) []string { func prefixSuggestionsWithOverflow(input string, candidates []string, helpCommand string) []string { results, total := cli.PrefixSuggestions(input, sameStyleCandidates(input, candidates), cli.DefaultSuggestLimit) - if total > len(results) { + if total > len(results) && helpCommand != "" { results = append(results, fmt.Sprintf("... and %d more, run `%s`", total-len(results), helpCommand)) } return results diff --git a/openapi/errors_additional_test.go b/openapi/errors_additional_test.go index fbb5a13cb..fed1966c6 100644 --- a/openapi/errors_additional_test.go +++ b/openapi/errors_additional_test.go @@ -51,7 +51,7 @@ func TestLocalErrorContractsCoverMessagesMarkersAndUnwrap(t *testing.T) { func TestProductAPIAndParameterAgentContracts(t *testing.T) { product := &InvalidProductError{Code: "ECX"} - assert.Equal(t, `"ecx" is not a valid command or product.`, product.AgentMessage()) + assert.Equal(t, `"ECX" is not a valid command or product.`, product.AgentMessage()) product.AIRecoveryEligible() assert.Nil(t, product.AgentSuggestions()) @@ -62,6 +62,8 @@ func TestProductAPIAndParameterAgentContracts(t *testing.T) { api.AIRecoveryEligible() assert.Contains(t, api.AgentSuggestions(), "DescribeInstances") assert.Nil(t, (&InvalidApiError{Name: "missing"}).AgentSuggestions()) + assert.Nil(t, (&InvalidApiError{Name: "missing"}).GetSuggestions()) + assert.Nil(t, (&InvalidUnifiedApiError{Name: "missing"}).GetSuggestions()) flags := cli.NewFlagSet() flags.Add(&cli.Flag{Name: "region"}) diff --git a/openapi/errors_test.go b/openapi/errors_test.go index e104b502b..b2c58b6b7 100644 --- a/openapi/errors_test.go +++ b/openapi/errors_test.go @@ -14,6 +14,7 @@ package openapi import ( + "fmt" "strings" "testing" @@ -31,7 +32,7 @@ func TestInvalidProductError_Error(t *testing.T) { } str := err.Error() assert.Equal(t, `"ecs" is not a valid command or product. See `+"`aliyun help`"+`.`, str) - assert.Equal(t, `"ec's" is not a valid command or product. See `+"`aliyun help`"+`.`, (&InvalidProductError{Code: "EC'S"}).Error()) + assert.Equal(t, `"EC'S" is not a valid command or product. See `+"`aliyun help`"+`.`, (&InvalidProductError{Code: "EC'S"}).Error()) } func TestInvalidProductError_GetSuggestions(t *testing.T) { @@ -255,33 +256,6 @@ func TestInvalidUnifiedApiError_GetSuggestions(t *testing.T) { }) } -func TestRemoveDuplicates(t *testing.T) { - t.Run("No duplicates", func(t *testing.T) { - result := removeDuplicates([]string{"a", "b", "c"}) - assert.Equal(t, []string{"a", "b", "c"}, result) - }) - - t.Run("With duplicates", func(t *testing.T) { - result := removeDuplicates([]string{"a", "b", "a", "c", "b"}) - assert.Equal(t, []string{"a", "b", "c"}, result) - }) - - t.Run("All same", func(t *testing.T) { - result := removeDuplicates([]string{"x", "x", "x"}) - assert.Equal(t, []string{"x"}, result) - }) - - t.Run("Empty", func(t *testing.T) { - result := removeDuplicates([]string{}) - assert.Empty(t, result) - }) - - t.Run("Nil", func(t *testing.T) { - result := removeDuplicates(nil) - assert.Empty(t, result) - }) -} - func TestInvalidApiError_GetSuggestions_PrefixFallback(t *testing.T) { err := &InvalidApiError{ Name: "Get", @@ -433,3 +407,95 @@ func TestInvalidBaselineCommandError_Suggestions(t *testing.T) { assert.Nil(t, err.AgentSuggestions()) }) } + +func newProductSuggestionLibrary(products ...meta.Product) *Library { + return &Library{builtinRepo: &meta.Repository{Products: products}} +} + +func TestInvalidProductError_ErrorPreservesInputCase(t *testing.T) { + err := &InvalidProductError{Code: "DescribeRegions"} + assert.Contains(t, err.Error(), `"DescribeRegions" is not a valid command or product`) + assert.Contains(t, err.AgentMessage(), `"DescribeRegions" is not a valid command or product`) +} + +func TestInvalidProductError_GetSuggestions_ProductPrefixParity(t *testing.T) { + // "openapiex" is a prefix of "openapiexplorer" (edit distance 5): the + // human path must surface it through the same prefix tier AI mode uses. + err := &InvalidProductError{ + Code: "openapiex", + library: newProductSuggestionLibrary(meta.Product{Code: "ecs"}, meta.Product{Code: "openapiexplorer"}), + } + assert.Equal(t, []string{"openapiexplorer"}, err.GetSuggestions()) +} + +func TestInvalidProductError_SuggestionsFromAPIReverseLookup(t *testing.T) { + library := newProductSuggestionLibrary( + meta.Product{Code: "ecs", ApiNames: []string{"DescribeRegions", "DescribeInstances"}}, + meta.Product{Code: "ecd", ApiNames: []string{"DescribeRegions"}}, + meta.Product{Code: "sts", ApiNames: []string{"GetCallerIdentity"}}, + meta.Product{Code: "oss", ApiNames: []string{"PutBucket"}}, + ) + + t.Run("pascal case input suggests runnable full commands", func(t *testing.T) { + err := &InvalidProductError{Code: "DescribeRegions", library: library} + assert.Equal(t, []string{"aliyun ecd DescribeRegions", "aliyun ecs DescribeRegions"}, err.GetSuggestions()) + }) + + t.Run("kebab case input keeps the kebab style", func(t *testing.T) { + err := &InvalidProductError{Code: "describe-regions", library: library} + assert.Equal(t, []string{"aliyun ecd describe-regions", "aliyun ecs describe-regions"}, err.GetSuggestions()) + }) + + t.Run("camel case api name resolves its owning product", func(t *testing.T) { + err := &InvalidProductError{Code: "getCallerIdentity", library: library} + assert.Equal(t, []string{"aliyun sts GetCallerIdentity"}, err.GetSuggestions()) + }) + + t.Run("singular api name matches its plural candidate", func(t *testing.T) { + err := &InvalidProductError{Code: "DescribeRegion", library: library} + assert.Equal(t, []string{"aliyun ecd DescribeRegions", "aliyun ecs DescribeRegions"}, err.GetSuggestions()) + }) + + t.Run("unknown garbage yields nothing", func(t *testing.T) { + err := &InvalidProductError{Code: "zzzznotexist", library: library} + assert.Nil(t, err.GetSuggestions()) + }) + + t.Run("results are capped at the default suggest limit", func(t *testing.T) { + products := make([]meta.Product, 0, cli.DefaultSuggestLimit+1) + for i := 0; i < cli.DefaultSuggestLimit+1; i++ { + products = append(products, meta.Product{Code: fmt.Sprintf("prod%d", i), ApiNames: []string{"DescribeRegions"}}) + } + err := &InvalidProductError{Code: "DescribeRegions", library: newProductSuggestionLibrary(products...)} + assert.Len(t, err.GetSuggestions(), cli.DefaultSuggestLimit) + }) +} + +func TestInvalidProductError_HumanAndAISuggestionsMatch(t *testing.T) { + library := newProductSuggestionLibrary( + meta.Product{Code: "ecs", ApiNames: []string{"DescribeRegions", "DescribeInstances"}}, + meta.Product{Code: "ahas-openapi", ApiNames: []string{"GetToken"}}, + ) + for _, code := range []string{"openapiex", "Ecsx", "ahas-openap", "DescribeRegions", "describe-regions", "getCallerIdentity"} { + err := &InvalidProductError{Code: code, library: library} + assert.Equal(t, err.GetSuggestions(), err.AgentSuggestions(), code) + } +} + +func TestInvalidApiError_OverflowHintOmittedWithoutRecoveryCommand(t *testing.T) { + // An all-verb input such as "Describe" yields no usable search keyword, so + // apiRecoveryCommand returns "". The overflow hint must be dropped rather + // than rendered as "... and N more, run ``". + err := &InvalidApiError{ + Name: "Describe", + product: &meta.Product{Code: "ecs", ApiNames: []string{ + "DescribeAccessPoints", "DescribeAccountAttributes", "DescribeActivations", + "DescribeAddresses", "DescribeAdviserCapacity", "DescribeAggregateCompliancePacks", + }}, + } + results := err.GetSuggestions() + assert.Len(t, results, cli.DefaultSuggestLimit) + for _, r := range results { + assert.NotContains(t, r, "run ``") + } +} diff --git a/openapi/product_suggestion.go b/openapi/product_suggestion.go new file mode 100644 index 000000000..6f991aeeb --- /dev/null +++ b/openapi/product_suggestion.go @@ -0,0 +1,108 @@ +// Copyright (c) 2009-present, Alibaba Cloud All rights reserved. +// +// 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 openapi + +import ( + "strings" + + "github.com/aliyun/aliyun-cli/v3/cli" + "github.com/aliyun/aliyun-cli/v3/meta" +) + +// productSuggestions is the single suggestion pipeline for unknown products, +// shared by human and AI rendering so both output modes report the same +// candidates. It matches product codes first (close typo, then prefix) and +// falls back to reverse-looking the input up as an API name, because a +// PascalCase or kebab token at the root position is usually an API whose +// product prefix was omitted. +func productSuggestions(input string, library *Library) []string { + if library == nil { + return nil + } + products := library.GetProducts() + candidates := make([]string, 0, len(products)) + for _, product := range products { + candidates = append(candidates, strings.ToLower(product.Code)) + } + // Product-code tiers keep the historical lowercased input: product codes + // carry no case style, and lowercasing preserves today's AI behavior for + // mixed-case typos such as "Openapiex". + if suggestions := apiSuggestions(strings.ToLower(input), candidates); len(suggestions) > 0 { + return suggestions + } + return apiNameProductSuggestions(input, products) +} + +// apiNameProductSuggestions reverse-looks the failed token up as an API name +// across every product's API list, in the input's own command style, and +// returns runnable full commands such as "aliyun ecs DescribeRegions". +// Matching compares compact forms (case and separator insensitive) in three +// precision passes — compact-equal, prefix, then edit distance — short +// circuiting on the first pass with results. The list is capped at +// DefaultSuggestLimit without an overflow hint, mirroring the product-code +// tiers. +func apiNameProductSuggestions(input string, products []meta.Product) []string { + if len(products) == 0 || strings.TrimSpace(input) == "" { + return nil + } + kebab := commandStyle(input) == "kebab" + commands := make([]string, 0, 1024) + compacts := make([]string, 0, 1024) + for _, product := range products { + code := strings.ToLower(product.Code) + for _, apiName := range product.ApiNames { + name := apiName + if kebab { + name = apiNameToKebab(apiName) + } + compact := compactAPITokens(name) + if compact == "" { + continue + } + commands = append(commands, "aliyun "+code+" "+name) + compacts = append(compacts, compact) + } + } + needle := compactAPITokens(input) + if needle == "" || len(commands) == 0 { + return nil + } + for _, match := range []func(candidate, needle string) bool{ + func(candidate, needle string) bool { return candidate == needle }, + func(candidate, needle string) bool { + return len(needle) >= 2 && strings.HasPrefix(candidate, needle) + }, + func(candidate, needle string) bool { + return cli.CalculateStringDistance(needle, candidate) <= cli.DefaultSuggestDistance + }, + } { + var results []string + for index, compact := range compacts { + if match(compact, needle) { + results = append(results, commands[index]) + } + } + if len(results) > 0 { + return truncateSuggestions(stableStrings(results)) + } + } + return nil +} + +func truncateSuggestions(values []string) []string { + if len(values) > cli.DefaultSuggestLimit { + return values[:cli.DefaultSuggestLimit] + } + return values +} From 84d193aea427702ef22587e9a5c074ad27b79648 Mon Sep 17 00:00:00 2001 From: ZhiJiaXing Date: Tue, 15 Sep 2026 10:22:09 +0800 Subject: [PATCH 2/5] feat: style-aware parameter suggestions with equivalent commands (R12) - PascalCase command + kebab flag: InvalidParameterError now maps the flag through the API's metadata rename table (biz-region-id -> RegionId) and, when every assigned flag maps cleanly, shows the directly runnable equivalent kebab command - kebab command + PascalCase API flag: the engine's UnknownFlagError is adapted host-side into a did-you-mean suggestion (verified against the engine's own option list) plus the equivalent PascalCase command, in both human and AI modes - human and AI parameter suggestions now share one candidate computation - missing-required errors on kebab commands clarify that --region only selects the signing/endpoint region when the user passed --region and the missing parameter is the region-ish one - all style-mixing paths fail closed: unverifiable mappings fall back to the previous output unchanged --- openapi/agent_error.go | 46 ++++- openapi/commando.go | 2 + openapi/errors.go | 101 +++++++-- openapi/local_command_validation.go | 7 +- openapi/style_migration.go | 306 ++++++++++++++++++++++++++++ openapi/style_migration_test.go | 295 +++++++++++++++++++++++++++ 6 files changed, 735 insertions(+), 22 deletions(-) create mode 100644 openapi/style_migration.go create mode 100644 openapi/style_migration_test.go diff --git a/openapi/agent_error.go b/openapi/agent_error.go index 195382f6d..b9efbf0a6 100644 --- a/openapi/agent_error.go +++ b/openapi/agent_error.go @@ -76,6 +76,23 @@ func normalizeAgentErrorWithSearch(err error, args []string, validate RecoverySe context := newRecoveryContext(args) + var styleMixed *styleMixedFlagError + if errors.As(err, &styleMixed) { + recovery := cli.AgentErrorRecovery{ + Action: "inspect_action_help", + Command: context.actionHelpCommand(), + Hint: fmt.Sprintf("--%s is a PascalCase parameter name; the kebab command accepts --%s.", styleMixed.flag, strings.TrimPrefix(styleMixed.suggestion, "--")), + } + if styleMixed.equivalent != "" { + recovery = cli.AgentErrorRecovery{ + Action: "switch_command_style", + Command: styleMixed.equivalent, + Hint: "The flag belongs to the PascalCase command style. Run the equivalent PascalCase command, or keep the kebab command and use the suggested flag.", + } + } + return newLocalAgentError(err, styleMixed.AgentMessage(), styleMixed.AgentSuggestions(), recovery) + } + var unknownFlag *argparser.UnknownFlagError if errors.As(err, &unknownFlag) { suggestions := flagSuggestions(unknownFlag.Flag, unknownFlag.Known) @@ -84,17 +101,25 @@ func normalizeAgentErrorWithSearch(err error, args []string, validate RecoverySe var missing *runtime.MissingRequiredError if errors.As(err, &missing) { - return missingRequiredAgentError(err, missingRequiredAgentMessage(missing), context) + // The --region routing-flag clarification is computed once in + // finishCommandRun (where the parsed flags are available) and carried + // by the wrapper; here it enriches the hint without a second parse. + note := "" + var regionErr *regionConfusionError + if errors.As(err, ®ionErr) { + note = regionErr.note + } + return missingRequiredAgentError(err, missingRequiredAgentMessage(missing), context, note) } var legacyDocRequired *LegacyDocRequiredError if errors.As(err, &legacyDocRequired) { - return missingRequiredAgentError(err, legacyDocRequired.Error(), context) + return missingRequiredAgentError(err, legacyDocRequired.Error(), context, "") } var legacyMissingRequired *LegacyMissingRequiredError if errors.As(err, &legacyMissingRequired) { - return missingRequiredAgentError(err, legacyMissingRequired.Error(), context) + return missingRequiredAgentError(err, legacyMissingRequired.Error(), context, "") } var runtimeConstraint *runtime.ConstraintViolationError @@ -111,6 +136,13 @@ func normalizeAgentErrorWithSearch(err error, args []string, validate RecoverySe if errors.As(err, &invalidParameter) { parameterContext := context.withProductAPI(invalidParameter.ProductCode, invalidParameter.ApiName) suggestions := invalidParameter.AgentSuggestions() + if invalidParameter.equivalentCommand != "" { + return newLocalAgentError(err, invalidParameter.AgentMessage(), suggestions, cli.AgentErrorRecovery{ + Action: "switch_command_style", + Command: invalidParameter.equivalentCommand, + Hint: "The flag belongs to the kebab command style. Run the equivalent kebab command, or keep the PascalCase command and use the suggested flag.", + }) + } return parameterSearchAgentError(err, invalidParameter.AgentMessage(), suggestions, invalidParameter.Name, parameterContext, validate) } @@ -732,11 +764,15 @@ func missingRequiredAgentMessage(err *runtime.MissingRequiredError) string { return "missing required parameter(s): " + strings.Join(err.Flags, ", ") } -func missingRequiredAgentError(cause error, message string, context recoveryContext) error { +func missingRequiredAgentError(cause error, message string, context recoveryContext, note string) error { + hint := "Inspect the API help for request parameters and provide every required value." + if note != "" { + hint = hint + " " + note + } return newLocalAgentError(cause, message, nil, cli.AgentErrorRecovery{ Action: "inspect_request_help", Command: context.actionHelpCommand(), - Hint: "Inspect the API help for request parameters and provide every required value.", + Hint: hint, }) } diff --git a/openapi/commando.go b/openapi/commando.go index 0eef06f24..e03331286 100644 --- a/openapi/commando.go +++ b/openapi/commando.go @@ -190,6 +190,8 @@ func (c *Commando) finishCommandRun(ctx *cli.Context, args []string, err error) // the AI gate so non-AI output is protected too. err = sanitizeNetworkTransportError(err) err = suggestKebabProfileFlagCase(err, args) + err = c.adaptStyleMixedFlagError(err, args, ctx) + err = annotateRegionConfusion(err, ctx) enabled := c.applyEffectiveAIModeForArgs(ctx, args) diff --git a/openapi/errors.go b/openapi/errors.go index 4da64471b..78345c6ee 100644 --- a/openapi/errors.go +++ b/openapi/errors.go @@ -22,6 +22,7 @@ import ( "github.com/aliyun/aliyun-cli/v3/cli" "github.com/aliyun/aliyun-cli/v3/cli/plugin" "github.com/aliyun/aliyun-cli/v3/meta" + "github.com/aliyun/aliyun-cli/v3/openapi/runtimehost" ) // LegacyMissingRequiredError marks required-parameter validation failures from @@ -127,6 +128,14 @@ type InvalidParameterError struct { ParameterNames []string ParameterExamples map[string]string flags *cli.FlagSet + // kebabToRaw/rawToKebab carry the metadata rename tables of this API's + // parameters (e.g. biz-region-id <-> RegionId); they power cross-style + // suggestions. kebabCommand/equivalentCommand are filled only when the + // unknown flag is a confirmed kebab-style name (style mixing). + kebabToRaw map[string]string + rawToKebab map[string]string + kebabCommand string + equivalentCommand string } func (e *InvalidParameterError) Error() string { @@ -141,33 +150,49 @@ func (e *InvalidParameterError) AgentMessage() string { func (*InvalidParameterError) AIRecoveryEligible() {} func (e *InvalidParameterError) GetSuggestions() []string { - sr := cli.NewSuggester(e.Name, 2) - for _, name := range e.ParameterNames { - sr.Apply(name) - } - if e.flags != nil { - for _, f := range e.flags.Flags() { - sr.Apply(f.Name) - } - } - - results := sr.GetResults() - for i, name := range results { + names := e.candidateNames() + results := make([]string, 0, len(names)) + for _, name := range names { if example := e.ParameterExamples[name]; example != "" { - results[i] = fmt.Sprintf("%s (example: %s)", name, example) + results = append(results, fmt.Sprintf("%s (example: %s)", name, example)) + } else { + results = append(results, name) } } return results } func (e *InvalidParameterError) AgentSuggestions() []string { + names := e.candidateNames() + results := make([]string, 0, len(names)) + for _, name := range names { + results = append(results, "--"+strings.TrimLeft(name, "-")) + } + return results +} + +// candidateNames computes the suggestion names shared by human and AI output: +// a metadata-driven cross-style rename hit comes first (the flag is a valid +// kebab-style parameter name of this API), then the usual typo suggestions. +func (e *InvalidParameterError) candidateNames() []string { + name := strings.TrimLeft(e.Name, "-") + if raw, ok := e.kebabToRaw[name]; ok { + return []string{raw} + } candidates := append([]string(nil), e.ParameterNames...) if e.flags != nil { - for _, flag := range e.flags.Flags() { - candidates = append(candidates, flag.Name) + for _, f := range e.flags.Flags() { + candidates = append(candidates, f.Name) } } - return flagSuggestions(e.Name, candidates) + if suggestions := closeSuggestions(name, candidates, false); len(suggestions) > 0 { + return suggestions + } + suggestions := crossStyleFlagSuggestions(name, candidates) + for i := range suggestions { + suggestions[i] = strings.TrimLeft(suggestions[i], "-") + } + return suggestions } // NewInvalidParameterErrorFromCanonical creates error from canonical API @@ -186,6 +211,7 @@ func NewInvalidParameterErrorFromCanonical(name string, api *canonicalmeta.API, examples[name] = example } } + kebabToRaw, rawToKebab := styleRenameMaps(api) return &InvalidParameterError{ Name: name, ProductCode: productCode, @@ -193,7 +219,50 @@ func NewInvalidParameterErrorFromCanonical(name string, api *canonicalmeta.API, ParameterNames: params, ParameterExamples: examples, flags: flags, + kebabToRaw: kebabToRaw, + rawToKebab: rawToKebab, + } +} + +// attachStyleMigration precomputes the kebab command name and the equivalent +// kebab command when the unknown flag is a confirmed kebab-style rename of +// one of this API's parameters (i.e. the caller mixed command styles). Plain +// typos keep both fields empty. +func (e *InvalidParameterError) attachStyleMigration(api *canonicalmeta.API, ctx *cli.Context) { + name := strings.TrimLeft(e.Name, "-") + if _, ok := e.kebabToRaw[name]; !ok { + return + } + e.kebabCommand = "" + if api != nil { + e.kebabCommand = api.CmdName + } + if e.kebabCommand == "" { + e.kebabCommand = apiNameToKebab(e.ApiName) + } + // The kebab command name is only useful when the engine actually serves + // it for this product; otherwise the equivalent would advise a command + // that cannot run. + if !containsString(runtimehost.ProductCommands(e.ProductCode), e.kebabCommand) { + e.kebabCommand = "" + return + } + e.equivalentCommand = rebuildStyleEquivalentCommand(e.ProductCode, e.kebabCommand, ctx, e.rawToKebab, keySet(e.kebabToRaw)) +} + +// styleMigrationTip renders the cross-style hint for human output, or "" when +// the flag is not a style-mixing rename. Without a served kebab command it +// degrades to naming the style-correct flag only. +func (e *InvalidParameterError) styleMigrationTip() string { + name := strings.TrimLeft(e.Name, "-") + raw, ok := e.kebabToRaw[name] + if !ok { + return "" + } + if e.equivalentCommand != "" { + return fmt.Sprintf("--%s is the kebab-style name of --%s. Equivalent command:\n %s", name, raw, e.equivalentCommand) } + return fmt.Sprintf("--%s is the kebab-style name of --%s; use --%s here.", name, raw, raw) } type InvalidProductOrPluginError struct { diff --git a/openapi/local_command_validation.go b/openapi/local_command_validation.go index b60c50198..94ae94590 100644 --- a/openapi/local_command_validation.go +++ b/openapi/local_command_validation.go @@ -37,7 +37,12 @@ func (c *Commando) validateCanonicalAPICommand(args []string, ctx *cli.Context) } name := strings.TrimSuffix(flag.Name, "-FILE") if resolved.API.FindLegacyParameter(name) == nil { - return NewInvalidParameterErrorFromCanonical(name, resolved.API, args[0], ctx.Flags()) + paramErr := NewInvalidParameterErrorFromCanonical(name, resolved.API, args[0], ctx.Flags()) + paramErr.attachStyleMigration(resolved.API, ctx) + if tip := paramErr.styleMigrationTip(); tip != "" { + return cli.NewErrorWithTip(paramErr, "%s", tip) + } + return paramErr } } return nil diff --git a/openapi/style_migration.go b/openapi/style_migration.go new file mode 100644 index 000000000..c26e667db --- /dev/null +++ b/openapi/style_migration.go @@ -0,0 +1,306 @@ +// Copyright (c) 2009-present, Alibaba Cloud All rights reserved. +// +// 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 openapi + +import ( + "errors" + "fmt" + "strings" + + "github.com/aliyun/aliyun-cli/v3/canonicalmeta" + "github.com/aliyun/aliyun-cli/v3/cli" + "github.com/aliyun/aliyun-cli/v3/config" + "github.com/aliyun/aliyun-openapi-runtime/argparser" + "github.com/aliyun/aliyun-openapi-runtime/runtime" +) + +// styleRenameMaps builds the bidirectional rename tables between the kebab +// option names the engine serves and the legacy raw parameter names of one +// API's top-level parameters. The rename is metadata-driven — a kebab name +// such as "biz-region-id" cannot be derived from "RegionId" by string +// conversion — so both directions come from the same source of truth. +func styleRenameMaps(api *canonicalmeta.API) (kebabToRaw, rawToKebab map[string]string) { + if api == nil { + return nil, nil + } + kebabToRaw = make(map[string]string) + rawToKebab = make(map[string]string) + for i := range api.Parameters { + p := &api.Parameters[i] + switch strings.ToLower(p.Location) { + case "domain", "header": + continue + } + if p.RawName == "" { + continue + } + kebab := kebabOptionName(p) + if kebab == "" { + continue + } + kebabToRaw[kebab] = p.RawName + rawToKebab[p.RawName] = kebab + } + return kebabToRaw, rawToKebab +} + +// kebabOptionName returns the parameter's kebab flag spelling. Options carry +// the authoritative form ("--biz-region-id"); the snake-case Name converted to +// kebab is the fallback for parameters without declared options. +func kebabOptionName(p *canonicalmeta.Parameter) string { + for _, opt := range p.Options { + if name := strings.TrimPrefix(opt, "--"); name != "" { + return name + } + } + return strings.ReplaceAll(p.Name, "_", "-") +} + +func keySet(m map[string]string) map[string]bool { + set := make(map[string]bool, len(m)) + for k := range m { + set[k] = true + } + return set +} + +// rebuildStyleEquivalentCommand rewrites the current invocation in the target +// command style: host flags pass through unchanged, API parameter flags are +// renamed through the metadata tables, and values keep their assignments. It +// fails closed — any unmappable API flag yields "" so callers fall back to a +// static example instead of printing a wrong command. +func rebuildStyleEquivalentCommand(product, targetCommand string, ctx *cli.Context, rename map[string]string, validTarget map[string]bool) string { + if targetCommand == "" || ctx == nil { + return "" + } + parts := []string{"aliyun", strings.ToLower(product), targetCommand} + if ctx.Flags() != nil { + for _, f := range ctx.Flags().Flags() { + if f == nil || !f.IsAssigned() { + continue + } + emitFlagValues(&parts, f.Name, flagAssignedValues(f)) + } + } + if ctx.UnknownFlags() != nil { + for _, f := range ctx.UnknownFlags().Flags() { + if f == nil || !f.IsAssigned() { + continue + } + name := strings.TrimSuffix(f.Name, "-FILE") + switch { + case validTarget[name]: + // already spelled in the target style + case rename[name] != "": + name = rename[name] + default: + return "" + } + emitFlagValues(&parts, name, flagAssignedValues(f)) + } + } + return strings.Join(parts, " ") +} + +// flagAssignedValues returns the assigned values of a flag: every value for +// repeatable flags, the single value otherwise, and nil for valueless +// (boolean) flags. +func flagAssignedValues(f *cli.Flag) []string { + if values := f.GetValues(); len(values) > 0 { + return values + } + if v, ok := f.GetValue(); ok && v != "" { + return []string{v} + } + return nil +} + +func emitFlagValues(parts *[]string, name string, values []string) { + if len(values) == 0 { + *parts = append(*parts, "--"+name) + return + } + for _, v := range values { + *parts = append(*parts, "--"+name, quoteShellValue(v)) + } +} + +// quoteShellValue keeps shell-safe values bare and quotes the rest, so the +// rebuilt command stays readable and copy-pasteable. +func quoteShellValue(v string) string { + if safeCommandToken(v) { + return v + } + return shellSingleQuote(v) +} + +// styleMixedFlagError adapts the engine's unknown-flag failure when the flag +// is the other command style's parameter name — e.g. the PascalCase wire name +// --RegionId on a kebab command whose metadata-renamed flag is +// --biz-region-id. It carries the style-correct suggestion and, when every +// assigned flag can be mapped, the directly runnable equivalent command. +type styleMixedFlagError struct { + cause error + product string + command string // kebab command name + flag string // offending flag without dashes + suggestion string // kebab option with dashes, e.g. "--biz-region-id" + equivalent string // equivalent PascalCase command; "" when fail-closed +} + +func (e *styleMixedFlagError) Error() string { + msg := e.cause.Error() + kebabName := strings.TrimPrefix(e.suggestion, "--") + if e.equivalent != "" { + return fmt.Sprintf("%s\n\n--%s is a PascalCase parameter name; kebab commands use --%s. Equivalent command:\n %s", + msg, e.flag, kebabName, e.equivalent) + } + return fmt.Sprintf("%s\n\n--%s is a PascalCase parameter name; kebab commands use --%s.", msg, e.flag, kebabName) +} + +func (e *styleMixedFlagError) Unwrap() error { return e.cause } + +func (*styleMixedFlagError) AIRecoveryEligible() {} + +func (e *styleMixedFlagError) GetSuggestions() []string { + if e.suggestion == "" { + return nil + } + return []string{e.suggestion} +} + +func (e *styleMixedFlagError) AgentMessage() string { + return fmt.Sprintf("unknown flag --%s", e.flag) +} + +func (e *styleMixedFlagError) AgentSuggestions() []string { + return e.GetSuggestions() +} + +// adaptStyleMixedFlagError converts the engine's unknown-flag failure into a +// style-aware suggestion when the flag is a PascalCase wire name of the same +// API. Every verification step fails closed to the original error: unknown +// product, missing canonical metadata, unmappable flag, or a kebab suggestion +// the engine does not actually serve. +func (c *Commando) adaptStyleMixedFlagError(err error, args []string, ctx *cli.Context) error { + if err == nil || len(args) < 2 || commandStyle(args[1]) != "kebab" { + return err + } + var profileCase *kebabProfileFlagCaseError + if errors.As(err, &profileCase) { + return err + } + var unknown *argparser.UnknownFlagError + if !errors.As(err, &unknown) { + return err + } + if c.library == nil || c.library.canonicalRepo == nil { + return err + } + product, ok := c.library.GetProduct(args[0]) + if !ok { + return err + } + apiName, api := resolveKebabCommandAPI(c.library.canonicalRepo, product.Code, product.Version, args[1]) + if api == nil { + return err + } + kebabToRaw, rawToKebab := styleRenameMaps(api) + kebabName, ok := rawToKebab[unknown.Flag] + if !ok || !containsString(unknown.Known, kebabName) { + return err + } + equivalent := rebuildStyleEquivalentCommand(product.Code, apiName, ctx, kebabToRaw, keySet(rawToKebab)) + return &styleMixedFlagError{ + cause: err, + product: strings.ToLower(product.Code), + command: args[1], + flag: unknown.Flag, + suggestion: "--" + kebabName, + equivalent: equivalent, + } +} + +// resolveKebabCommandAPI finds the canonical API behind an engine-served +// kebab command name, matching on the declared cmd_name first and the +// converted API name as fallback. +func resolveKebabCommandAPI(repo canonicalAPIRepository, productCode, version, kebabCommand string) (string, *canonicalmeta.API) { + index, err := repo.GetVersionIndex(productCode, version) + if err != nil || index == nil { + return "", nil + } + for apiName, entry := range index.APIs { + if entry.CmdName == kebabCommand || apiNameToKebab(apiName) == kebabCommand { + api, err := repo.GetAPI(productCode, version, apiName) + if err == nil && api != nil { + return apiName, api + } + } + } + return "", nil +} + +// regionConfusionNote detects the routing-flag mix-up: on kebab commands +// --region only selects the signing/endpoint region, while the API's region +// parameter (e.g. --biz-region-id) must be passed explicitly. It fires only +// when the user did pass --region and a missing required parameter is the +// region-ish one. The PascalCase chain needs no note: there --region feeds +// the wire RegionId through the legacy invoker. +func regionConfusionNote(regionAssigned bool, missingFlags []string) string { + if !regionAssigned { + return "" + } + for _, flag := range missingFlags { + if strings.Contains(compactAPITokens(strings.TrimLeft(flag, "-")), "regionid") { + return fmt.Sprintf("Note: --region only selects the region for signing and endpoint resolution; it does not set the API parameter %s. Pass %s explicitly to set it.", flag, flag) + } + } + return "" +} + +// regionConfusionError appends the --region clarification to the engine's +// missing-required message in human output. AI mode reads the note off the +// wrapper and enriches the recovery hint with it. +type regionConfusionError struct { + cause error + note string +} + +func (e *regionConfusionError) Error() string { return e.cause.Error() + "\n\n" + e.note } + +func (e *regionConfusionError) Unwrap() error { return e.cause } + +func (*regionConfusionError) AIRecoveryEligible() {} + +// annotateRegionConfusion attaches the --region clarification to the engine's +// missing-required failure when the mix-up pattern matches; otherwise the +// error passes through unchanged. --region assignment is read from the parsed +// flags, which is authoritative, rather than from a raw argv scan. +func annotateRegionConfusion(err error, ctx *cli.Context) error { + var missing *runtime.MissingRequiredError + if !errors.As(err, &missing) { + return err + } + regionAssigned := false + if ctx != nil && ctx.Flags() != nil { + if f := config.RegionFlag(ctx.Flags()); f != nil && f.IsAssigned() { + regionAssigned = true + } + } + note := regionConfusionNote(regionAssigned, missing.Flags) + if note == "" { + return err + } + return ®ionConfusionError{cause: err, note: note} +} diff --git a/openapi/style_migration_test.go b/openapi/style_migration_test.go new file mode 100644 index 000000000..1d3f80337 --- /dev/null +++ b/openapi/style_migration_test.go @@ -0,0 +1,295 @@ +// Copyright (c) 2009-present, Alibaba Cloud All rights reserved. +// +// 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 openapi + +import ( + "bytes" + "fmt" + "testing" + + "github.com/aliyun/aliyun-cli/v3/canonicalmeta" + "github.com/aliyun/aliyun-cli/v3/cli" + "github.com/aliyun/aliyun-cli/v3/meta" + "github.com/aliyun/aliyun-openapi-runtime/argparser" + "github.com/aliyun/aliyun-openapi-runtime/engine" + "github.com/aliyun/aliyun-openapi-runtime/runtime" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func styleMigrationTestAPI() *canonicalmeta.API { + return &canonicalmeta.API{ + Name: "DescribeInstances", + CmdName: "describe-instances", + Parameters: []canonicalmeta.Parameter{ + {Name: "biz_region_id", RawName: "RegionId", Options: []string{"--biz-region-id"}, Location: "query"}, + {Name: "instance_id", RawName: "InstanceId", Options: []string{"--instance-id"}, Location: "query"}, + {Name: "x_trace", RawName: "XTrace", Options: []string{"--x-trace"}, Location: "header"}, + }, + } +} + +func newAssignedFlag(name string, values ...string) *cli.Flag { + f := &cli.Flag{Name: name, AssignedMode: cli.AssignedOnce} + f.SetAssigned(true) + for _, v := range values { + f.SetValues(append(f.GetValues(), v)) + } + if len(values) > 0 { + f.SetValue(values[0]) + } + return f +} + +func TestStyleRenameMaps(t *testing.T) { + kebabToRaw, rawToKebab := styleRenameMaps(styleMigrationTestAPI()) + assert.Equal(t, "RegionId", kebabToRaw["biz-region-id"]) + assert.Equal(t, "biz-region-id", rawToKebab["RegionId"]) + assert.Equal(t, "InstanceId", kebabToRaw["instance-id"]) + // header-position parameters are excluded from the suggestion space + _, ok := kebabToRaw["x-trace"] + assert.False(t, ok) + + k2r, r2k := styleRenameMaps(nil) + assert.Nil(t, k2r) + assert.Nil(t, r2k) +} + +func TestInvalidParameterError_CrossStyleRenameSuggestion(t *testing.T) { + err := NewInvalidParameterErrorFromCanonical("biz-region-id", styleMigrationTestAPI(), "ecs", cli.NewFlagSet()) + // The rename table maps the kebab flag to its PascalCase raw name; a plain + // edit-distance matcher could never bridge "biz-region-id" and "RegionId". + assert.Equal(t, []string{"RegionId"}, err.GetSuggestions()) + assert.Equal(t, []string{"--RegionId"}, err.AgentSuggestions()) +} + +func TestInvalidParameterError_TypoSuggestionStillWorks(t *testing.T) { + err := NewInvalidParameterErrorFromCanonical("InstnaceId", styleMigrationTestAPI(), "ecs", cli.NewFlagSet()) + assert.Contains(t, err.GetSuggestions(), "InstanceId") + assert.Contains(t, err.AgentSuggestions(), "--InstanceId") +} + +func TestRebuildStyleEquivalentCommand(t *testing.T) { + api := styleMigrationTestAPI() + kebabToRaw, rawToKebab := styleRenameMaps(api) + + newContext := func() *cli.Context { + ctx := cli.NewCommandContext(new(bytes.Buffer), new(bytes.Buffer)) + ctx.Flags().Add(newAssignedFlag("region", "cn-hangzhou")) + return ctx + } + + t.Run("pascal to kebab maps raw names and keeps kebab flags", func(t *testing.T) { + ctx := newContext() + unknown := cli.NewFlagSet() + unknown.Add(newAssignedFlag("biz-region-id", "cn-hangzhou")) + unknown.Add(newAssignedFlag("InstanceId", "i-123")) + ctx.SetUnknownFlags(unknown) + got := rebuildStyleEquivalentCommand("ecs", "describe-instances", ctx, rawToKebab, keySet(kebabToRaw)) + assert.Equal(t, "aliyun ecs describe-instances --region cn-hangzhou --biz-region-id cn-hangzhou --instance-id i-123", got) + }) + + t.Run("kebab to pascal maps kebab names and keeps raw names", func(t *testing.T) { + ctx := newContext() + unknown := cli.NewFlagSet() + unknown.Add(newAssignedFlag("biz-region-id", "cn-hangzhou")) + ctx.SetUnknownFlags(unknown) + got := rebuildStyleEquivalentCommand("ecs", "DescribeInstances", ctx, kebabToRaw, keySet(rawToKebab)) + assert.Equal(t, "aliyun ecs DescribeInstances --region cn-hangzhou --RegionId cn-hangzhou", got) + }) + + t.Run("valueless flags stay bare", func(t *testing.T) { + ctx := newContext() + unknown := cli.NewFlagSet() + unknown.Add(newAssignedFlag("biz-region-id", "cn-hangzhou")) + ctx.SetUnknownFlags(unknown) + ctx.Flags().Add(newAssignedFlag("force")) + got := rebuildStyleEquivalentCommand("ecs", "describe-instances", ctx, rawToKebab, keySet(kebabToRaw)) + assert.Equal(t, "aliyun ecs describe-instances --region cn-hangzhou --force --biz-region-id cn-hangzhou", got) + }) + + t.Run("values with spaces are quoted", func(t *testing.T) { + ctx := newContext() + unknown := cli.NewFlagSet() + unknown.Add(newAssignedFlag("InstanceId", "hello world")) + ctx.SetUnknownFlags(unknown) + got := rebuildStyleEquivalentCommand("ecs", "describe-instances", ctx, rawToKebab, keySet(kebabToRaw)) + assert.Contains(t, got, "--instance-id 'hello world'") + }) + + t.Run("unmappable api flag fails closed", func(t *testing.T) { + ctx := newContext() + unknown := cli.NewFlagSet() + unknown.Add(newAssignedFlag("totally-unknown", "x")) + ctx.SetUnknownFlags(unknown) + got := rebuildStyleEquivalentCommand("ecs", "describe-instances", ctx, rawToKebab, keySet(kebabToRaw)) + assert.Equal(t, "", got) + }) +} + +func TestInvalidParameterError_StyleMigrationTipWithoutServedKebabCommand(t *testing.T) { + // In the test environment the engine serves no product, so the tip + // degrades to naming the style-correct flag only. + err := NewInvalidParameterErrorFromCanonical("biz-region-id", styleMigrationTestAPI(), "ecs", cli.NewFlagSet()) + err.attachStyleMigration(styleMigrationTestAPI(), cli.NewCommandContext(new(bytes.Buffer), new(bytes.Buffer))) + assert.Equal(t, "--biz-region-id is the kebab-style name of --RegionId; use --RegionId here.", err.styleMigrationTip()) + + plain := NewInvalidParameterErrorFromCanonical("InstnaceId", styleMigrationTestAPI(), "ecs", cli.NewFlagSet()) + plain.attachStyleMigration(styleMigrationTestAPI(), cli.NewCommandContext(new(bytes.Buffer), new(bytes.Buffer))) + assert.Equal(t, "", plain.styleMigrationTip()) +} + +func newStyleMixedTestCommando(t *testing.T) *Commando { + t.Helper() + repo := newFakeCanonicalRepo() + repo.AddVersionIndex("ecs", "2014-05-26", &canonicalmeta.VersionIndex{APIs: map[string]canonicalmeta.VersionAPIEntry{ + "DescribeInstances": {CmdName: "describe-instances"}, + }}) + repo.AddAPI("ecs", "2014-05-26", styleMigrationTestAPI()) + products, err := meta.MockLoadRepository([]meta.Product{{Code: "ecs", Version: "2014-05-26"}}) + require.NoError(t, err) + return &Commando{library: &Library{builtinRepo: products, canonicalRepo: repo}} +} + +func unknownFlagUsageError(flag string, known ...string) error { + return &engine.UsageError{ + Code: "UNKNOWN_FLAG", + Err: fmt.Errorf("%w (run `aliyun ecs describe-instances --help` for accepted flags)", &argparser.UnknownFlagError{Flag: flag, Known: known}), + } +} + +func TestAdaptStyleMixedFlagError(t *testing.T) { + args := []string{"ecs", "describe-instances"} + + t.Run("pascal flag on kebab command gets suggestion and equivalent", func(t *testing.T) { + c := newStyleMixedTestCommando(t) + ctx := cli.NewCommandContext(new(bytes.Buffer), new(bytes.Buffer)) + ctx.Flags().Add(newAssignedFlag("RegionId", "cn-hangzhou")) + unknown := cli.NewFlagSet() + unknown.Add(newAssignedFlag("instance-id", "i-123")) + ctx.SetUnknownFlags(unknown) + + err := c.adaptStyleMixedFlagError(unknownFlagUsageError("RegionId", "biz-region-id", "instance-id"), args, ctx) + var mixed *styleMixedFlagError + require.ErrorAs(t, err, &mixed) + assert.Equal(t, []string{"--biz-region-id"}, mixed.GetSuggestions()) + assert.Contains(t, mixed.Error(), "--RegionId is a PascalCase parameter name; kebab commands use --biz-region-id") + assert.Equal(t, "aliyun ecs DescribeInstances --RegionId cn-hangzhou --InstanceId i-123", mixed.equivalent) + }) + + t.Run("unmappable flag passes through unchanged", func(t *testing.T) { + c := newStyleMixedTestCommando(t) + cause := unknownFlagUsageError("NotARealParam", "biz-region-id") + err := c.adaptStyleMixedFlagError(cause, args, cli.NewCommandContext(new(bytes.Buffer), new(bytes.Buffer))) + assert.Equal(t, cause, err) + }) + + t.Run("rename not served by the engine fails closed", func(t *testing.T) { + c := newStyleMixedTestCommando(t) + // RegionId maps to biz-region-id in metadata, but the engine's Known + // list does not contain it: do not suggest what the engine rejects. + cause := unknownFlagUsageError("RegionId", "instance-id") + err := c.adaptStyleMixedFlagError(cause, args, cli.NewCommandContext(new(bytes.Buffer), new(bytes.Buffer))) + assert.Equal(t, cause, err) + }) + + t.Run("profile case error is left to its dedicated adapter", func(t *testing.T) { + c := newStyleMixedTestCommando(t) + cause := &kebabProfileFlagCaseError{cause: unknownFlagUsageError("Profile"), product: "ecs", command: "describe-instances"} + err := c.adaptStyleMixedFlagError(cause, args, cli.NewCommandContext(new(bytes.Buffer), new(bytes.Buffer))) + assert.Equal(t, cause, err) + }) +} + +func TestAdaptStyleMixedFlagErrorAgentEnvelope(t *testing.T) { + mixed := &styleMixedFlagError{ + cause: unknownFlagUsageError("RegionId", "biz-region-id"), + product: "ecs", + command: "describe-instances", + flag: "RegionId", + suggestion: "--biz-region-id", + equivalent: "aliyun ecs DescribeInstances --RegionId cn-hangzhou", + } + envelope := requireAgentEnvelope(t, mixed, []string{"ecs", "describe-instances"}, nil) + assert.Equal(t, `unknown flag --RegionId`, envelope.Message) + assert.Equal(t, []string{"--biz-region-id"}, envelope.DidYouMean) + assert.Equal(t, "switch_command_style", envelope.Recovery.Action) + assert.Equal(t, "aliyun ecs DescribeInstances --RegionId cn-hangzhou", envelope.Recovery.Command) +} + +func TestInvalidParameterError_AgentEnvelopeWithEquivalentCommand(t *testing.T) { + err := &InvalidParameterError{ + Name: "biz-region-id", + ProductCode: "ecs", + ApiName: "DescribeInstances", + kebabToRaw: map[string]string{"biz-region-id": "RegionId"}, + equivalentCommand: "aliyun ecs describe-instances --biz-region-id cn-hangzhou", + } + envelope := requireAgentEnvelope(t, err, []string{"ecs", "DescribeInstances"}, nil) + assert.Equal(t, `"--biz-region-id" is not a valid parameter or flag.`, envelope.Message) + assert.Equal(t, []string{"--RegionId"}, envelope.DidYouMean) + assert.Equal(t, "switch_command_style", envelope.Recovery.Action) + assert.Equal(t, "aliyun ecs describe-instances --biz-region-id cn-hangzhou", envelope.Recovery.Command) +} + +func TestInvalidParameterError_AgentEnvelopeWithoutEquivalentKeepsSearchRecovery(t *testing.T) { + err := &InvalidParameterError{ + Name: "InstnaceId", + ProductCode: "ecs", + ApiName: "DescribeInstances", + ParameterNames: []string{"InstanceId"}, + } + envelope := requireAgentEnvelope(t, err, []string{"ecs", "DescribeInstances"}, nil) + assert.Equal(t, []string{"--InstanceId"}, envelope.DidYouMean) + assert.NotEqual(t, "switch_command_style", envelope.Recovery.Action) +} + +func TestRegionConfusionNote(t *testing.T) { + // fires only when --region was passed AND a missing parameter is region-ish + assert.NotEmpty(t, regionConfusionNote(true, []string{"--biz-region-id"})) + assert.NotEmpty(t, regionConfusionNote(true, []string{"--region-id"})) + assert.Empty(t, regionConfusionNote(false, []string{"--biz-region-id"})) + assert.Empty(t, regionConfusionNote(true, []string{"--instance-id"})) +} + +func TestAnnotateRegionConfusion(t *testing.T) { + cause := &engine.UsageError{ + Code: "MISSING_REQUIRED_PARAMETER", + Err: &runtime.MissingRequiredError{Flags: []string{"--biz-region-id"}}, + } + ctxWithRegion := cli.NewCommandContext(new(bytes.Buffer), new(bytes.Buffer)) + ctxWithRegion.Flags().Add(newAssignedFlag("region", "cn-hangzhou")) + + err := annotateRegionConfusion(cause, ctxWithRegion) + var wrapped *regionConfusionError + require.ErrorAs(t, err, &wrapped) + assert.Contains(t, wrapped.Error(), "missing required parameter(s): --biz-region-id") + assert.Contains(t, wrapped.Error(), "--region only selects the region for signing and endpoint resolution") + + ctxNoRegion := cli.NewCommandContext(new(bytes.Buffer), new(bytes.Buffer)) + same := annotateRegionConfusion(cause, ctxNoRegion) + assert.Equal(t, cause, same) +} + +func TestRegionConfusionAgentHint(t *testing.T) { + cause := &runtime.MissingRequiredError{Flags: []string{"--biz-region-id"}} + note := regionConfusionNote(true, cause.Flags) + wrapped := ®ionConfusionError{cause: cause, note: note} + envelope := requireAgentEnvelope(t, wrapped, []string{"ecs", "describe-instances"}, nil) + assert.Equal(t, "missing required parameter(s): --biz-region-id", envelope.Message) + assert.Contains(t, envelope.Recovery.Hint, "--region only selects the region") + + noNote := requireAgentEnvelope(t, cause, []string{"ecs", "describe-instances"}, nil) + assert.NotContains(t, noNote.Recovery.Hint, "--region only selects the region") +} From fd634ce5e5b41bf952d9efe5ecf344e3fd219d0f Mon Sep 17 00:00:00 2001 From: ZhiJiaXing Date: Tue, 15 Sep 2026 16:33:23 +0800 Subject: [PATCH 3/5] feat: version the agent error envelope and document the output contract (R08) - AgentErrorEnvelope gains an always-present integer schema_version (protocol metadata, not error content), injected at the single NewAgentError construction point - integrations.md (zh/en) now defines the field guarantees, the five-level AI-mode precedence chain, the version evolution rule, the JSON key-order non-contract, and the separation from the Machine Help v1 protocol - flag help for --cli-ai-mode/--no-cli-ai-mode describes the real behavior instead of the historical User-Agent-segment wording - tests lock the precedence chain, the explicit opt-out path, and the envelope shape; assertions parse JSON semantically rather than byte order --- cli/agent_error.go | 12 +++- cli/agent_error_test.go | 26 ++++++++ cli/command_test.go | 2 +- docs/en/integrations.md | 17 +++++- docs/en/usage.md | 2 +- docs/zh-CN/integrations.md | 17 +++++- docs/zh-CN/usage.md | 2 +- openapi/agent_aimode_test.go | 113 +++++++++++++++++++++++++++++++++++ openapi/agent_error_test.go | 5 +- openapi/flags.go | 8 +-- 10 files changed, 190 insertions(+), 14 deletions(-) diff --git a/cli/agent_error.go b/cli/agent_error.go index 7f53ca2df..7e08399e9 100644 --- a/cli/agent_error.go +++ b/cli/agent_error.go @@ -45,8 +45,15 @@ type AgentErrorRecovery struct { Hint string `json:"hint"` } +// AgentErrorSchemaVersion is the current schema version of the agent error +// envelope. It is protocol metadata, not error content; consumers must ignore +// unknown fields. Backward-compatible field additions do not bump the +// version; only breaking shape changes do. +const AgentErrorSchemaVersion = 1 + type AgentErrorEnvelope struct { - Message string `json:"message"` + Message string `json:"message"` + SchemaVersion int `json:"schema_version"` // Structured server-error facts, populated only for remote server errors so // agents can branch on them instead of parsing the message string. ErrorCode string `json:"error_code,omitempty"` @@ -64,6 +71,9 @@ type AgentError struct { // NewAgentError returns nil when the required compact-envelope fields are // incomplete. Optional data is normalized before it can reach JSON output. func NewAgentError(envelope AgentErrorEnvelope, cause error) *AgentError { + if envelope.SchemaVersion == 0 { + envelope.SchemaVersion = AgentErrorSchemaVersion + } envelope.DidYouMean = compactStrings(envelope.DidYouMean) envelope.Recovery.Command = strings.TrimSpace(envelope.Recovery.Command) if strings.TrimSpace(envelope.Message) == "" || diff --git a/cli/agent_error_test.go b/cli/agent_error_test.go index d1652d714..e7e9b49ee 100644 --- a/cli/agent_error_test.go +++ b/cli/agent_error_test.go @@ -48,6 +48,7 @@ func TestAgentErrorPreservesCompactLocalEnvelope(t *testing.T) { require.NoError(t, marshalErr) assert.JSONEq(t, `{ "message":"unknown flag --instnace-type", + "schema_version":1, "did_you_mean":["--instance-type"], "recovery":{ "action":"search_parameter", @@ -113,6 +114,7 @@ func TestAgentErrorRecursivelyOmitsEmptyOptionalValues(t *testing.T) { require.NoError(t, marshalErr) assert.JSONEq(t, `{ "message":"invalid local usage", + "schema_version":1, "did_you_mean":["--instance-id"], "recovery":{ "action":"inspect_request_help", @@ -135,6 +137,7 @@ func TestAgentErrorOmitsEmptyOptionalFields(t *testing.T) { require.NoError(t, marshalErr) assert.JSONEq(t, `{ "message":"missing required parameter(s): --region-id", + "schema_version":1, "recovery":{ "action":"inspect_request_help", "hint":"Inspect the complete request help." @@ -144,6 +147,29 @@ func TestAgentErrorOmitsEmptyOptionalFields(t *testing.T) { assert.NotContains(t, string(encoded), "command") } +// The schema version is protocol metadata injected at the single construction +// point: unset envelopes get the current version, explicitly pinned producer +// versions are preserved. +func TestAgentErrorSchemaVersion(t *testing.T) { + valid := AgentErrorEnvelope{ + Message: "invalid local usage", + Recovery: AgentErrorRecovery{ + Action: "inspect_request_help", + Hint: "Inspect the request help.", + }, + } + + err := NewAgentError(valid, errors.New("cause")) + require.NotNil(t, err) + assert.Equal(t, AgentErrorSchemaVersion, err.Envelope().SchemaVersion) + + pinned := valid + pinned.SchemaVersion = AgentErrorSchemaVersion + 1 + err = NewAgentError(pinned, errors.New("cause")) + require.NotNil(t, err) + assert.Equal(t, AgentErrorSchemaVersion+1, err.Envelope().SchemaVersion) +} + func TestAIModeEnableHintsShareStableContent(t *testing.T) { assert.Equal(t, "export ALIBABA_CLOUD_CLI_AI_MODE=1", NewAIModeHint().Command) assert.Equal(t, "Enable AI Mode for compact Help, structured JSON errors, and actionable recovery guidance.", NewAIModeHint().Message) diff --git a/cli/command_test.go b/cli/command_test.go index 6e68332c4..7d4767ee4 100644 --- a/cli/command_test.go +++ b/cli/command_test.go @@ -351,7 +351,7 @@ func TestProcessAgentErrorWritesOneJSONLineToStderr(t *testing.T) { cmd.processError(ctx, err) assert.Empty(t, stdout.String()) - assert.Equal(t, "{\"message\":\"unknown flag --instnace-type\",\"did_you_mean\":[\"--instance-type\"],\"recovery\":{\"action\":\"search_parameter\",\"command\":\"aliyun ecs describe-instances --help-search instance-type\",\"hint\":\"Search request parameters related to instance-type.\"}}\n", stderr.String()) + assert.Equal(t, "{\"message\":\"unknown flag --instnace-type\",\"schema_version\":1,\"did_you_mean\":[\"--instance-type\"],\"recovery\":{\"action\":\"search_parameter\",\"command\":\"aliyun ecs describe-instances --help-search instance-type\",\"hint\":\"Search request parameters related to instance-type.\"}}\n", stderr.String()) assert.NotContains(t, stderr.String(), AIModeEnableTextHint) } diff --git a/docs/en/integrations.md b/docs/en/integrations.md index 849c30f36..e21677375 100644 --- a/docs/en/integrations.md +++ b/docs/en/integrations.md @@ -129,11 +129,20 @@ export ALIBABA_CLOUD_CLI_AI_MODE=1 aliyun ecs describe-instances --cli-ai-mode ``` +AI mode takes effect in this order of precedence (highest first): + +1. `--no-cli-ai-mode` on a single command (explicit opt-out, always wins) +2. `--cli-ai-mode` on a single command (explicit opt-in) +3. The `ALIBABA_CLOUD_CLI_AI_MODE=1/0` environment variable (explicit value) +4. Agent-environment auto-detection (gated by `ALIBABA_CLOUD_CLI_AGENT_INTEGRATION`, enabled by default) +5. The global `configure ai-mode` setting (`~/.aliyun/ai-mode.json`, off by default) + Supported local usage, query, transport, OAuth, and server errors are written as one compact JSON object to stderr. Success output remains on stdout. Optional fields are omitted when unavailable: ```json { "message": "unknown flag --instnace-type", + "schema_version": 1, "did_you_mean": ["--instance-type"], "recovery": { "action": "search_parameter", @@ -143,9 +152,13 @@ Supported local usage, query, transport, OAuth, and server errors are written as } ``` -Remote server errors may additionally include `error_code`, `status_code`, and `request_id`. `did_you_mean` and `recovery.command` are also optional; `message`, `recovery.action`, and `recovery.hint` are present in every structured Agent error. +Remote server errors may additionally include `error_code`, `status_code`, and `request_id`. `did_you_mean` and `recovery.command` are also optional; `message`, `schema_version`, `recovery.action`, and `recovery.hint` are present in every structured Agent error. + +`schema_version` is the envelope's protocol version (currently `1`) — protocol metadata, not error content. Backward-compatible field additions do not bump it; only breaking shape changes do. Consumers should ignore unknown fields and branch on `schema_version`. + +In AI mode, API responses are written as compact single-line JSON preserving the server's key order; other modes pretty-print with sorted keys. JSON key order is not part of the contract — never parse it byte-wise. -The Agent error object is a separate compact interface and currently has no `schemaVersion`; the Machine Help `v1` contract does not apply to it. Not every error is normalized yet, so consumers must also tolerate human-readable stderr. +The Agent error object is a compact protocol separate from Machine Help: Machine Help uses `schemaVersion: "v1"` (camelCase, on stdout), while Agent errors use `schema_version: 1` (snake_case, on stderr). Not every error is normalized yet, so consumers must also tolerate human-readable stderr. | Exit status | Meaning | | --- | --- | diff --git a/docs/en/usage.md b/docs/en/usage.md index e5ad8169a..d860dd856 100644 --- a/docs/en/usage.md +++ b/docs/en/usage.md @@ -274,7 +274,7 @@ aliyun configure ai-mode --help aliyun ecs describe-instances --cli-ai-mode ``` -When a supported agent environment is detected, in-process OpenAPI commands automatically enable agent-oriented interaction and execution optimizations. These optimizations currently include stricter metadata-based validation and more structured error output, but the exact behavior may change and is not a stable compatibility contract. +When a supported agent environment is detected, in-process OpenAPI commands automatically enable agent-oriented interaction and execution optimizations. These optimizations currently include stricter metadata-based validation and more structured error output; the Agent error envelope is versioned via `schema_version` (see [MCP proxy, OpenTelemetry, and machine-readable interfaces](./integrations.md)), while other optimizations may change across versions. Requests made through this automatically enabled mode append the following generic User-Agent marker: diff --git a/docs/zh-CN/integrations.md b/docs/zh-CN/integrations.md index 642e0a598..a176a668b 100644 --- a/docs/zh-CN/integrations.md +++ b/docs/zh-CN/integrations.md @@ -129,11 +129,20 @@ export ALIBABA_CLOUD_CLI_AI_MODE=1 aliyun ecs describe-instances --cli-ai-mode ``` +AI mode 的生效优先级（从高到低）： + +1. 单次命令 `--no-cli-ai-mode`（显式关闭，永远优先） +2. 单次命令 `--cli-ai-mode`（显式开启） +3. 环境变量 `ALIBABA_CLOUD_CLI_AI_MODE=1/0`（显式值） +4. Agent 环境自动探测（受 `ALIBABA_CLOUD_CLI_AGENT_INTEGRATION` 门控，默认开启） +5. 全局配置 `configure ai-mode`（`~/.aliyun/ai-mode.json`，兜底默认关闭） + 目前支持的本地用法错误、查询错误、传输错误、OAuth 错误和服务端错误会以单个紧凑 JSON 对象写入 stderr；成功结果仍写入 stdout。没有值的可选字段会被省略： ```json { "message": "unknown flag --instnace-type", + "schema_version": 1, "did_you_mean": ["--instance-type"], "recovery": { "action": "search_parameter", @@ -143,9 +152,13 @@ aliyun ecs describe-instances --cli-ai-mode } ``` -远端服务错误还可能包含 `error_code`、`status_code` 和 `request_id`。`did_you_mean` 和 `recovery.command` 也是可选字段；每个结构化 Agent 错误都会包含 `message`、`recovery.action` 和 `recovery.hint`。 +远端服务错误还可能包含 `error_code`、`status_code` 和 `request_id`。`did_you_mean` 和 `recovery.command` 也是可选字段；每个结构化 Agent 错误都会包含 `message`、`schema_version`、`recovery.action` 和 `recovery.hint`。 + +`schema_version` 是信封的协议版本（当前为 `1`），属于协议元数据而非错误内容。向后兼容的新增字段不升版本；只有破坏性变更才会递增。调用方应忽略未知字段，并按 `schema_version` 分支处理。 + +Agent 模式下的 API 响应输出为紧凑单行 JSON（保留服务端返回的 key 顺序）；非 Agent 模式为美化缩进并按 key 排序。JSON key 顺序不属于契约，解析时请不要依赖字节顺序。 -Agent 错误对象是一套独立的紧凑接口，目前没有 `schemaVersion`；机器 Help 的 `v1` 协议不适用于它。并非所有错误都已经结构化，因此调用方还必须兼容 stderr 中的人类可读错误。 +Agent 错误对象是独立于机器 Help 的紧凑协议：机器 Help 使用 `schemaVersion: "v1"`（camelCase，写入 stdout），Agent 错误使用 `schema_version: 1`（snake_case，写入 stderr），两者不混用。并非所有错误都已经结构化，因此调用方还必须兼容 stderr 中的人类可读错误。 | 退出状态 | 含义 | | --- | --- | diff --git a/docs/zh-CN/usage.md b/docs/zh-CN/usage.md index dc1b81f27..50f1c5583 100644 --- a/docs/zh-CN/usage.md +++ b/docs/zh-CN/usage.md @@ -274,7 +274,7 @@ aliyun configure ai-mode --help aliyun ecs describe-instances --cli-ai-mode ``` -检测到受支持的 Agent 环境时，进程内 OpenAPI 命令会自动启用面向 Agent 的交互和执行优化。目前包括更严格的 metadata 参数校验和更结构化的错误输出，但具体优化行为可能随版本调整，不属于稳定兼容协议。 +检测到受支持的 Agent 环境时，进程内 OpenAPI 命令会自动启用面向 Agent 的交互和执行优化。目前包括更严格的 metadata 参数校验和更结构化的错误输出；其中 Agent 错误信封已通过 `schema_version` 版本化（见 [MCP 代理、OpenTelemetry 与机器可读接口](./integrations.md)），其余优化行为可能随版本调整。 通过自动探测启用后，请求会增加以下通用 UA 标记： diff --git a/openapi/agent_aimode_test.go b/openapi/agent_aimode_test.go index 86913c253..0d60dea15 100644 --- a/openapi/agent_aimode_test.go +++ b/openapi/agent_aimode_test.go @@ -1,11 +1,18 @@ package openapi import ( + "bytes" + "errors" + "fmt" "io" "testing" "github.com/aliyun/aliyun-cli/v3/cli" + "github.com/aliyun/aliyun-cli/v3/config" "github.com/aliyun/aliyun-cli/v3/sysconfig/aimode" + "github.com/aliyun/aliyun-openapi-runtime/argparser" + "github.com/aliyun/aliyun-openapi-runtime/engine" + "github.com/stretchr/testify/assert" ) func TestCliAIOverridesForOpenAPIIncludesDetectedAgent(t *testing.T) { @@ -84,3 +91,109 @@ func TestDetectedAgentAddsLegacyOpenAPIUserAgent(t *testing.T) { t.Fatalf("force-off legacy OpenAPI suffix = %q", suffix) } } + +// TestAIModeEffectivePrecedenceChain locks the documented AI-mode precedence: +// --no-cli-ai-mode > --cli-ai-mode > ALIBABA_CLOUD_CLI_AI_MODE > agent +// auto-detection (gated by ALIBABA_CLOUD_CLI_AGENT_INTEGRATION) > ai-mode.json. +// The contract is frozen by test so later refactors cannot silently reorder it. +func TestAIModeEffectivePrecedenceChain(t *testing.T) { + newCtx := func(agent bool) *cli.Context { + ctx := cli.NewCommandContext(io.Discard, io.Discard) + AddFlags(ctx.Flags()) + if agent { + ctx.SetAgentName("codex") + } + return ctx + } + enabledFor := func(ctx *cli.Context, cfg *aimode.AiConfig) bool { + on, off := CliAIOverridesForOpenAPI(ctx) + return aimode.EnabledForCommand(cfg, on, off) + } + + t.Run("default off", func(t *testing.T) { + t.Setenv(aimode.EnvAIMode, "") + t.Setenv(aimode.EnvAgentIntegration, "") + assert.False(t, enabledFor(newCtx(false), &aimode.AiConfig{Enabled: false})) + }) + + t.Run("config file on", func(t *testing.T) { + t.Setenv(aimode.EnvAIMode, "") + t.Setenv(aimode.EnvAgentIntegration, "") + assert.True(t, enabledFor(newCtx(false), &aimode.AiConfig{Enabled: true})) + }) + + t.Run("env on beats config off", func(t *testing.T) { + t.Setenv(aimode.EnvAIMode, "1") + assert.True(t, enabledFor(newCtx(false), &aimode.AiConfig{Enabled: false})) + }) + + t.Run("env off beats config on", func(t *testing.T) { + t.Setenv(aimode.EnvAIMode, "0") + assert.False(t, enabledFor(newCtx(false), &aimode.AiConfig{Enabled: true})) + }) + + t.Run("agent auto-detection turns on", func(t *testing.T) { + t.Setenv(aimode.EnvAIMode, "") + t.Setenv(aimode.EnvAgentIntegration, "") + assert.True(t, enabledFor(newCtx(true), &aimode.AiConfig{Enabled: false})) + }) + + t.Run("agent auto-detection gated off by integration switch", func(t *testing.T) { + t.Setenv(aimode.EnvAIMode, "") + t.Setenv(aimode.EnvAgentIntegration, "disabled") + assert.False(t, enabledFor(newCtx(true), &aimode.AiConfig{Enabled: false})) + }) + + t.Run("env opt-out beats agent detection", func(t *testing.T) { + t.Setenv(aimode.EnvAIMode, "0") + assert.False(t, enabledFor(newCtx(true), &aimode.AiConfig{Enabled: false})) + }) + + t.Run("flag on beats env off", func(t *testing.T) { + t.Setenv(aimode.EnvAIMode, "0") + ctx := newCtx(false) + CliAIModeFlag(ctx.Flags()).SetAssigned(true) + assert.True(t, enabledFor(ctx, &aimode.AiConfig{Enabled: false})) + }) + + t.Run("flag off beats everything", func(t *testing.T) { + t.Setenv(aimode.EnvAIMode, "1") + t.Setenv(aimode.EnvAgentIntegration, "") + ctx := newCtx(true) + CliNoAIModeFlag(ctx.Flags()).SetAssigned(true) + assert.False(t, enabledFor(ctx, &aimode.AiConfig{Enabled: true})) + }) + + t.Run("flag on beats integration gate", func(t *testing.T) { + t.Setenv(aimode.EnvAIMode, "") + t.Setenv(aimode.EnvAgentIntegration, "disabled") + ctx := newCtx(true) + CliAIModeFlag(ctx.Flags()).SetAssigned(true) + assert.True(t, enabledFor(ctx, &aimode.AiConfig{Enabled: false})) + }) +} + +// TestExplicitOptOutKeepsHumanErrorUnderAgentDetection covers the opt-out path +// end to end: a detected agent environment plus --no-cli-ai-mode renders the +// human error, not the JSON envelope. +func TestExplicitOptOutKeepsHumanErrorUnderAgentDetection(t *testing.T) { + t.Setenv(aimode.EnvAIMode, "") + t.Setenv(aimode.EnvAgentIntegration, "") + ctx := cli.NewCommandContext(new(bytes.Buffer), new(bytes.Buffer)) + cmd := &cli.Command{Name: "aliyun", EnableUnknownFlag: true} + config.AddFlags(cmd.Flags()) + AddFlags(cmd.Flags()) + ctx.EnterCommand(cmd) + ctx.SetAgentName("codex") + + commando := &Commando{profile: config.Profile{Language: "en"}} + cause := &engine.UsageError{ + Code: "UNKNOWN_FLAG", + Err: fmt.Errorf("%w (run `aliyun ecs describe-instances --help` for accepted flags)", + &argparser.UnknownFlagError{Flag: "Profile", Known: []string{"instance-type"}}), + } + got := commando.finishCommandRun(ctx, []string{"ecs", "describe-instances", "--no-cli-ai-mode", "--Profile", "default"}, cause) + var agentErr *cli.AgentError + assert.False(t, errors.As(got, &agentErr), "opt-out must not produce a JSON envelope") + assert.Contains(t, got.Error(), "did you mean --profile") +} diff --git a/openapi/agent_error_test.go b/openapi/agent_error_test.go index 47d6a5d1f..a84a8e970 100644 --- a/openapi/agent_error_test.go +++ b/openapi/agent_error_test.go @@ -89,6 +89,7 @@ func TestKebabProfileCaseMismatchSuggestsLowercaseProfile(t *testing.T) { require.NoError(t, err) assert.JSONEq(t, `{ "message":"unknown flag --Profile", + "schema_version":1, "did_you_mean":["--profile"], "recovery":{ "action":"inspect_action_help", @@ -1160,7 +1161,7 @@ func TestAgentErrorEnvelopeEndToEndIsOneCleanJSONDocument(t *testing.T) { assert.NotContains(t, stderr.String(), cli.AIModeEnableTextHint) var decoded map[string]interface{} require.NoError(t, json.Unmarshal(stderr.Bytes(), &decoded)) - assert.ElementsMatch(t, []string{"message", "did_you_mean", "recovery"}, mapKeys(decoded)) + assert.ElementsMatch(t, []string{"message", "schema_version", "did_you_mean", "recovery"}, mapKeys(decoded)) assert.Equal(t, []interface{}{"--instance-type"}, decoded["did_you_mean"]) recovery := decoded["recovery"].(map[string]interface{}) assert.Equal(t, "search_parameter", recovery["action"]) @@ -1208,7 +1209,7 @@ func TestCLIOutputJSONStructuresLocalErrorWhenAIModeIsDisabled(t *testing.T) { assert.NotContains(t, stderr.String(), cli.AIModeEnableTextHint) var decoded map[string]interface{} require.NoError(t, json.Unmarshal(stderr.Bytes(), &decoded)) - assert.ElementsMatch(t, []string{"message", "did_you_mean", "recovery"}, mapKeys(decoded)) + assert.ElementsMatch(t, []string{"message", "schema_version", "did_you_mean", "recovery"}, mapKeys(decoded)) assert.Equal(t, []interface{}{"--instance-type"}, decoded["did_you_mean"]) recovery := decoded["recovery"].(map[string]interface{}) assert.Equal(t, "search_parameter", recovery["action"]) diff --git a/openapi/flags.go b/openapi/flags.go index ba0d470eb..9b5debc40 100644 --- a/openapi/flags.go +++ b/openapi/flags.go @@ -514,8 +514,8 @@ func NewCliAIModeFlag() *cli.Flag { Name: CliAIModeFlagName, AssignedMode: cli.AssignedNone, Short: i18n.T( - "for this command only, append AI-mode User-Agent segment (skills from configure ai-mode) even if global ai-mode is off", - "仅本次命令追加 AI 模式 UA 段（skills 来自 configure ai-mode），即使全局 ai-mode 未开启", + "enable AI mode for this command only (structured JSON errors, compact Help, recovery guidance), even when global ai-mode is off", + "仅本次命令启用 AI 模式（结构化 JSON 错误、精简帮助、恢复指引），即使全局 ai-mode 未开启", ), } } @@ -527,8 +527,8 @@ func NewCliNoAIModeFlag() *cli.Flag { AssignedMode: cli.AssignedNone, Hidden: true, Short: i18n.T( - "for this command only, do not append AI-mode User-Agent segment even if global ai-mode is on", - "仅本次命令不追加 AI 模式 UA 段，即使全局 ai-mode 已开启", + "disable AI mode for this command only, even when global ai-mode is on or an agent environment is detected", + "仅本次命令关闭 AI 模式，即使全局 ai-mode 已开启或检测到 Agent 环境", ), } } From f9b9f99f149838b258950afefe7da0e133301d3e Mon Sep 17 00:00:00 2001 From: ZhiJiaXing Date: Tue, 15 Sep 2026 17:23:25 +0800 Subject: [PATCH 4/5] fix: make style-migration tip tests engine-environment independent The styleMigrationTip test assumed the runtime engine serves no product metadata, which held only in degraded local environments; with full CI metadata the engine serves ecs and the equivalent-command branch fired instead. Extract engineServedCommands as a package-level variable (the established agentErrorNormalizer pattern) so tests pin the gate in both directions, and add the previously untested served-command branch. --- openapi/errors.go | 3 +-- openapi/style_migration.go | 7 +++++++ openapi/style_migration_test.go | 32 ++++++++++++++++++++++++++++++-- 3 files changed, 38 insertions(+), 4 deletions(-) diff --git a/openapi/errors.go b/openapi/errors.go index 78345c6ee..407ea074f 100644 --- a/openapi/errors.go +++ b/openapi/errors.go @@ -22,7 +22,6 @@ import ( "github.com/aliyun/aliyun-cli/v3/cli" "github.com/aliyun/aliyun-cli/v3/cli/plugin" "github.com/aliyun/aliyun-cli/v3/meta" - "github.com/aliyun/aliyun-cli/v3/openapi/runtimehost" ) // LegacyMissingRequiredError marks required-parameter validation failures from @@ -243,7 +242,7 @@ func (e *InvalidParameterError) attachStyleMigration(api *canonicalmeta.API, ctx // The kebab command name is only useful when the engine actually serves // it for this product; otherwise the equivalent would advise a command // that cannot run. - if !containsString(runtimehost.ProductCommands(e.ProductCode), e.kebabCommand) { + if !containsString(engineServedCommands(e.ProductCode), e.kebabCommand) { e.kebabCommand = "" return } diff --git a/openapi/style_migration.go b/openapi/style_migration.go index c26e667db..a56409136 100644 --- a/openapi/style_migration.go +++ b/openapi/style_migration.go @@ -21,10 +21,17 @@ import ( "github.com/aliyun/aliyun-cli/v3/canonicalmeta" "github.com/aliyun/aliyun-cli/v3/cli" "github.com/aliyun/aliyun-cli/v3/config" + "github.com/aliyun/aliyun-cli/v3/openapi/runtimehost" "github.com/aliyun/aliyun-openapi-runtime/argparser" "github.com/aliyun/aliyun-openapi-runtime/runtime" ) +// engineServedCommands reports the kebab command names the runtime engine +// serves for a product. It is a package-level variable so tests can pin the +// gate deterministically (the real engine metadata is not available in unit +// test environments). +var engineServedCommands = runtimehost.ProductCommands + // styleRenameMaps builds the bidirectional rename tables between the kebab // option names the engine serves and the legacy raw parameter names of one // API's top-level parameters. The rename is metadata-driven — a kebab name diff --git a/openapi/style_migration_test.go b/openapi/style_migration_test.go index 1d3f80337..8491273e2 100644 --- a/openapi/style_migration_test.go +++ b/openapi/style_migration_test.go @@ -138,18 +138,46 @@ func TestRebuildStyleEquivalentCommand(t *testing.T) { }) } +// stubEngineServedCommands pins the engine-serving gate for deterministic +// tests; the real engine metadata is not available in unit test environments. +func stubEngineServedCommands(t *testing.T, served []string) { + t.Helper() + original := engineServedCommands + engineServedCommands = func(string) []string { return served } + t.Cleanup(func() { engineServedCommands = original }) +} + func TestInvalidParameterError_StyleMigrationTipWithoutServedKebabCommand(t *testing.T) { - // In the test environment the engine serves no product, so the tip - // degrades to naming the style-correct flag only. + // The engine serves nothing for this product: the tip degrades to naming + // the style-correct flag only, and no unrunnable equivalent is advised. + stubEngineServedCommands(t, nil) err := NewInvalidParameterErrorFromCanonical("biz-region-id", styleMigrationTestAPI(), "ecs", cli.NewFlagSet()) err.attachStyleMigration(styleMigrationTestAPI(), cli.NewCommandContext(new(bytes.Buffer), new(bytes.Buffer))) assert.Equal(t, "--biz-region-id is the kebab-style name of --RegionId; use --RegionId here.", err.styleMigrationTip()) + assert.Equal(t, "", err.equivalentCommand) plain := NewInvalidParameterErrorFromCanonical("InstnaceId", styleMigrationTestAPI(), "ecs", cli.NewFlagSet()) plain.attachStyleMigration(styleMigrationTestAPI(), cli.NewCommandContext(new(bytes.Buffer), new(bytes.Buffer))) assert.Equal(t, "", plain.styleMigrationTip()) } +func TestInvalidParameterError_StyleMigrationTipWithEquivalentCommand(t *testing.T) { + // The engine serves the kebab command: the tip carries the rebuilt + // equivalent command with every assigned flag translated. + stubEngineServedCommands(t, []string{"describe-instances"}) + ctx := cli.NewCommandContext(new(bytes.Buffer), new(bytes.Buffer)) + unknown := cli.NewFlagSet() + unknown.Add(newAssignedFlag("biz-region-id", "cn-hangzhou")) + unknown.Add(newAssignedFlag("InstanceId", "i-123")) + ctx.SetUnknownFlags(unknown) + + err := NewInvalidParameterErrorFromCanonical("biz-region-id", styleMigrationTestAPI(), "ecs", cli.NewFlagSet()) + err.attachStyleMigration(styleMigrationTestAPI(), ctx) + tip := err.styleMigrationTip() + assert.Contains(t, tip, "Equivalent command:") + assert.Contains(t, tip, "aliyun ecs describe-instances --biz-region-id cn-hangzhou --instance-id i-123") +} + func newStyleMixedTestCommando(t *testing.T) *Commando { t.Helper() repo := newFakeCanonicalRepo() From 2b62180813adbd031d3ee6dcf47bf7f8211fd865 Mon Sep 17 00:00:00 2001 From: ZhiJiaXing Date: Tue, 15 Sep 2026 17:45:52 +0800 Subject: [PATCH 5/5] fix: fail closed on -FILE flags in equivalent commands, nil-guard api errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A --param-FILE flag means read-from-file on the PascalCase chain; stripping the suffix would silently turn file content into a literal path value, and the kebab engine has no per-parameter -FILE convention — the equivalent command now declines to rebuild instead of emitting wrong advice. Also align InvalidApiError/InvalidUnifiedApiError Error() with the nil guards their suggestion methods already have. --- openapi/errors.go | 6 ++++++ openapi/errors_additional_test.go | 2 ++ openapi/style_migration.go | 9 ++++++++- openapi/style_migration_test.go | 14 ++++++++++++++ 4 files changed, 30 insertions(+), 1 deletion(-) diff --git a/openapi/errors.go b/openapi/errors.go index 407ea074f..454fbeb94 100644 --- a/openapi/errors.go +++ b/openapi/errors.go @@ -91,6 +91,9 @@ type InvalidApiError struct { } func (e *InvalidApiError) Error() string { + if e.product == nil { + return fmt.Sprintf("%q is not a valid api.", e.Name) + } product := e.product.GetLowerCode() if command := apiRecoveryCommand(e.Name, product, e.product.ApiNames); command != "" { return fmt.Sprintf("%q is not a valid api. Search matching APIs with `%s`.", e.Name, command) @@ -316,6 +319,9 @@ type InvalidUnifiedApiError struct { } func (e *InvalidUnifiedApiError) Error() string { + if e.product == nil { + return fmt.Sprintf("%q is not a valid api.", e.Name) + } product := e.product.GetLowerCode() candidates := append(append([]string(nil), e.product.ApiNames...), e.lPlugin.CmdNames...) if command := apiRecoveryCommand(e.Name, product, candidates); command != "" { diff --git a/openapi/errors_additional_test.go b/openapi/errors_additional_test.go index fed1966c6..20c52070b 100644 --- a/openapi/errors_additional_test.go +++ b/openapi/errors_additional_test.go @@ -64,6 +64,8 @@ func TestProductAPIAndParameterAgentContracts(t *testing.T) { assert.Nil(t, (&InvalidApiError{Name: "missing"}).AgentSuggestions()) assert.Nil(t, (&InvalidApiError{Name: "missing"}).GetSuggestions()) assert.Nil(t, (&InvalidUnifiedApiError{Name: "missing"}).GetSuggestions()) + assert.Equal(t, `"missing" is not a valid api.`, (&InvalidApiError{Name: "missing"}).Error()) + assert.Equal(t, `"missing" is not a valid api.`, (&InvalidUnifiedApiError{Name: "missing"}).Error()) flags := cli.NewFlagSet() flags.Add(&cli.Flag{Name: "region"}) diff --git a/openapi/style_migration.go b/openapi/style_migration.go index a56409136..de70b53ca 100644 --- a/openapi/style_migration.go +++ b/openapi/style_migration.go @@ -105,7 +105,14 @@ func rebuildStyleEquivalentCommand(product, targetCommand string, ctx *cli.Conte if f == nil || !f.IsAssigned() { continue } - name := strings.TrimSuffix(f.Name, "-FILE") + // The -FILE suffix means "read the value from this file" on the + // PascalCase chain. Stripping it would silently turn file content + // into a literal path value, and the kebab engine has no + // per-parameter -FILE convention to preserve it — fail closed. + if strings.HasSuffix(f.Name, "-FILE") { + return "" + } + name := f.Name switch { case validTarget[name]: // already spelled in the target style diff --git a/openapi/style_migration_test.go b/openapi/style_migration_test.go index 8491273e2..b7c9f0a2d 100644 --- a/openapi/style_migration_test.go +++ b/openapi/style_migration_test.go @@ -136,6 +136,20 @@ func TestRebuildStyleEquivalentCommand(t *testing.T) { got := rebuildStyleEquivalentCommand("ecs", "describe-instances", ctx, rawToKebab, keySet(kebabToRaw)) assert.Equal(t, "", got) }) + + t.Run("FILE-suffixed flags fail closed", func(t *testing.T) { + // --body-FILE reads the value from a file on the PascalCase chain; + // the kebab engine has no per-parameter -FILE convention, so the + // equivalent must not be rebuilt at all rather than lose the + // read-from-file semantics. + ctx := newContext() + unknown := cli.NewFlagSet() + unknown.Add(newAssignedFlag("biz-region-id", "cn-hangzhou")) + unknown.Add(newAssignedFlag("body-FILE", "/tmp/x.json")) + ctx.SetUnknownFlags(unknown) + got := rebuildStyleEquivalentCommand("ecs", "describe-instances", ctx, rawToKebab, keySet(kebabToRaw)) + assert.Equal(t, "", got) + }) } // stubEngineServedCommands pins the engine-serving gate for deterministic