diff --git a/cmd/build.go b/cmd/build.go index a748544b0..84828ed77 100644 --- a/cmd/build.go +++ b/cmd/build.go @@ -117,6 +117,13 @@ func buildInternal(ctx context.Context, inv cmdutil.InvocationContext, opts ...B installTipsHelpFunc(rootCmd) rootCmd.SilenceErrors = true + // SilenceUsage as a static field (not only in PersistentPreRun) so it also + // covers flag-parse errors, which fail before PreRun runs — otherwise cobra + // dumps usage instead of our structured error. SetFlagErrorFunc on root is + // inherited by every subcommand, turning unknown-flag errors into a + // structured "did you mean" envelope. + rootCmd.SilenceUsage = true + rootCmd.SetFlagErrorFunc(flagDidYouMean) RegisterGlobalFlags(rootCmd.PersistentFlags(), &cfg.globals) rootCmd.PersistentPreRun = func(cmd *cobra.Command, args []string) { diff --git a/cmd/event/suggestions.go b/cmd/event/suggestions.go index f3275fda4..2bdaf5dff 100644 --- a/cmd/event/suggestions.go +++ b/cmd/event/suggestions.go @@ -10,6 +10,7 @@ import ( eventlib "github.com/larksuite/cli/internal/event" "github.com/larksuite/cli/internal/output" + "github.com/larksuite/cli/internal/suggest" ) const maxSuggestions = 3 @@ -28,7 +29,7 @@ func suggestEventKeys(input string) []string { hits = append(hits, match{def.Key, 0}) continue } - if d := levenshtein(input, def.Key); d <= threshold { + if d := suggest.Levenshtein(input, def.Key); d <= threshold { hits = append(hits, match{def.Key, d}) } } @@ -69,34 +70,3 @@ func unknownEventKeyErr(key string) error { "Run 'lark-cli event list' to see available keys.", ) } - -// levenshtein computes classic edit distance (two-row DP). -func levenshtein(a, b string) int { - if a == b { - return 0 - } - ra, rb := []rune(a), []rune(b) - if len(ra) == 0 { - return len(rb) - } - if len(rb) == 0 { - return len(ra) - } - prev := make([]int, len(rb)+1) - curr := make([]int, len(rb)+1) - for j := range prev { - prev[j] = j - } - for i := 1; i <= len(ra); i++ { - curr[0] = i - for j := 1; j <= len(rb); j++ { - cost := 1 - if ra[i-1] == rb[j-1] { - cost = 0 - } - curr[j] = min(prev[j]+1, curr[j-1]+1, prev[j-1]+cost) - } - prev, curr = curr, prev - } - return prev[len(rb)] -} diff --git a/cmd/event/suggestions_test.go b/cmd/event/suggestions_test.go index 0838fb623..fdaaa2c01 100644 --- a/cmd/event/suggestions_test.go +++ b/cmd/event/suggestions_test.go @@ -10,27 +10,6 @@ import ( _ "github.com/larksuite/cli/events" ) -func TestLevenshtein(t *testing.T) { - cases := []struct { - a, b string - want int - }{ - {"", "", 0}, - {"a", "", 1}, - {"", "abc", 3}, - {"kitten", "kitten", 0}, - {"kitten", "sitten", 1}, - {"kitten", "sitting", 3}, - {"飞书", "飞书", 0}, - {"飞书", "飞s", 1}, - } - for _, tc := range cases { - if got := levenshtein(tc.a, tc.b); got != tc.want { - t.Errorf("levenshtein(%q,%q) = %d, want %d", tc.a, tc.b, got, tc.want) - } - } -} - func TestSuggestEventKeys(t *testing.T) { cases := []struct { name string diff --git a/cmd/flag_suggest_test.go b/cmd/flag_suggest_test.go new file mode 100644 index 000000000..7adb35053 --- /dev/null +++ b/cmd/flag_suggest_test.go @@ -0,0 +1,70 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package cmd + +import ( + "errors" + "slices" + "strings" + "testing" + + "github.com/larksuite/cli/internal/output" + "github.com/spf13/cobra" +) + +func TestUnknownFlagName(t *testing.T) { + cases := []struct { + in string + name string + ok bool + }{ + {"unknown flag: --query", "query", true}, + {"unknown flag: --with-styles", "with-styles", true}, + {"unknown shorthand flag: 'z' in -z", "", false}, + {"flag needs an argument: --find", "", false}, + {`invalid argument "x" for "--count"`, "", false}, + } + for _, c := range cases { + name, ok := unknownFlagName(errors.New(c.in)) + if name != c.name || ok != c.ok { + t.Errorf("unknownFlagName(%q) = (%q,%v), want (%q,%v)", c.in, name, ok, c.name, c.ok) + } + } +} + +func TestFlagDidYouMean_UnknownFlagSuggestsAndListsValid(t *testing.T) { + c := &cobra.Command{Use: "demo"} + c.Flags().String("range", "", "") + c.Flags().String("find", "", "") + c.Flags().Bool("dry-run", false, "") + + err := flagDidYouMean(c, errors.New("unknown flag: --rang")) // typo of --range + var exitErr *output.ExitError + if !errors.As(err, &exitErr) { + t.Fatalf("expected *output.ExitError, got %T", err) + } + if exitErr.Detail.Type != "unknown_flag" { + t.Errorf("type = %q, want unknown_flag", exitErr.Detail.Type) + } + if !strings.Contains(exitErr.Detail.Hint, "--range") { + t.Errorf("hint should suggest --range, got %q", exitErr.Detail.Hint) + } + detail, _ := exitErr.Detail.Detail.(map[string]any) + valid, _ := detail["valid_flags"].([]string) + if !slices.Contains(valid, "find") || !slices.Contains(valid, "range") { + t.Errorf("valid_flags should list find & range, got %v", valid) + } +} + +func TestFlagDidYouMean_OtherErrorStaysGeneric(t *testing.T) { + c := &cobra.Command{Use: "demo"} + err := flagDidYouMean(c, errors.New("flag needs an argument: --find")) + var exitErr *output.ExitError + if !errors.As(err, &exitErr) { + t.Fatalf("expected *output.ExitError, got %T", err) + } + if exitErr.Detail.Type != "flag_error" { + t.Errorf("type = %q, want flag_error (non-unknown-flag errors stay generic)", exitErr.Detail.Type) + } +} diff --git a/cmd/root.go b/cmd/root.go index 00d9a24bc..297e1d93b 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -26,8 +26,10 @@ import ( "github.com/larksuite/cli/internal/output" "github.com/larksuite/cli/internal/registry" "github.com/larksuite/cli/internal/skillscheck" + "github.com/larksuite/cli/internal/suggest" "github.com/larksuite/cli/internal/update" "github.com/spf13/cobra" + "github.com/spf13/pflag" ) const rootLong = `lark-cli — Lark/Feishu CLI tool. @@ -297,6 +299,12 @@ func writeSecurityPolicyError(w io.Writer, spErr *internalauth.SecurityPolicyErr func installUnknownSubcommandGuard(cmd *cobra.Command) { if cmd.HasSubCommands() && cmd.Run == nil && cmd.RunE == nil { cmd.RunE = unknownSubcommandRunE + // Route an unknown subcommand to unknownSubcommandRunE even when flags + // are also present (e.g. `sheets +cells-find --url ...`). A pure group + // consumes no flags itself, so unknown flags belong to the (missing) + // subcommand; whitelisting them here prevents cobra from erroring on the + // flag first and printing usage instead of our structured suggestion. + cmd.FParseErrWhitelist.UnknownFlags = true if cmd.Annotations == nil { cmd.Annotations = map[string]string{} } @@ -313,10 +321,12 @@ func unknownSubcommandRunE(cmd *cobra.Command, args []string) error { } unknown := args[0] available := availableSubcommandNames(cmd) + suggestions := suggest.Closest(unknown, available, 6) msg := fmt.Sprintf("unknown subcommand %q for %q", unknown, cmd.CommandPath()) hint := fmt.Sprintf("run `%s --help` to see available subcommands", cmd.CommandPath()) - if len(available) > 0 { - hint = fmt.Sprintf("available subcommands: %s", strings.Join(available, ", ")) + if len(suggestions) > 0 { + hint = fmt.Sprintf("did you mean one of: %s? (run `%s --help` for the full list)", + strings.Join(suggestions, ", "), cmd.CommandPath()) } return &output.ExitError{ Code: output.ExitValidation, @@ -327,6 +337,7 @@ func unknownSubcommandRunE(cmd *cobra.Command, args []string) error { Detail: map[string]any{ "unknown": unknown, "command_path": cmd.CommandPath(), + "suggestions": suggestions, "available": available, }, }, @@ -349,6 +360,81 @@ func availableSubcommandNames(cmd *cobra.Command) []string { return subs } +// flagDidYouMean is the root FlagErrorFunc (inherited by all subcommands). It +// converts cobra's flag-parse errors into the structured ErrorEnvelope: an +// unknown flag gets a focused "did you mean" hint plus the full valid-flag list +// in detail (so agents recover even when the typo is semantic, e.g. --query vs +// --find, where edit distance alone finds nothing). Other flag errors stay +// structured but generic. +func flagDidYouMean(c *cobra.Command, ferr error) error { + name, isUnknown := unknownFlagName(ferr) + if !isUnknown { + return &output.ExitError{ + Code: output.ExitValidation, + Detail: &output.ErrDetail{ + Type: "flag_error", + Message: ferr.Error(), + Hint: fmt.Sprintf("run `%s --help` for valid flags", c.CommandPath()), + }, + } + } + valid := visibleFlagNames(c) + suggestions := suggest.Closest(name, valid, 3) + hint := fmt.Sprintf("run `%s --help` to see valid flags", c.CommandPath()) + if len(suggestions) > 0 { + for i := range suggestions { + suggestions[i] = "--" + suggestions[i] + } + hint = fmt.Sprintf("did you mean %s? (run `%s --help` for all flags)", + strings.Join(suggestions, ", "), c.CommandPath()) + } + return &output.ExitError{ + Code: output.ExitValidation, + Detail: &output.ErrDetail{ + Type: "unknown_flag", + Message: fmt.Sprintf("unknown flag %q for %q", "--"+name, c.CommandPath()), + Hint: hint, + Detail: map[string]any{ + "unknown": "--" + name, + "command_path": c.CommandPath(), + "suggestions": suggestions, + "valid_flags": valid, + }, + }, + } +} + +// unknownFlagName extracts the offending long-flag name from cobra's flag-parse +// error text ("unknown flag: --query" → "query"). Returns ok=false for anything +// else (missing argument, invalid value, unknown shorthand) so the caller keeps +// those structured but generic — hallucinated flags are essentially always long. +func unknownFlagName(err error) (string, bool) { + const p = "unknown flag: --" + msg := err.Error() + i := strings.Index(msg, p) + if i < 0 { + return "", false + } + rest := msg[i+len(p):] + if j := strings.IndexAny(rest, " \t"); j >= 0 { + rest = rest[:j] + } + return rest, true +} + +// visibleFlagNames lists the non-hidden flag names of c (for suggestions and +// the valid_flags detail). +func visibleFlagNames(c *cobra.Command) []string { + var names []string + c.Flags().VisitAll(func(f *pflag.Flag) { + if !f.Hidden { + names = append(names, f.Name) + } + }) + sort.Strings(names) + return names +} + // installTipsHelpFunc wraps the default help function to append a TIPS section // when a command has tips set via cmdutil.SetTips. It also force-shows global // flags that are normally hidden in single-app mode (currently --profile) diff --git a/cmd/unknown_subcommand_test.go b/cmd/unknown_subcommand_test.go index 4bba607d5..2cc6f2d84 100644 --- a/cmd/unknown_subcommand_test.go +++ b/cmd/unknown_subcommand_test.go @@ -113,11 +113,11 @@ func TestUnknownSubcommandRunE_UnknownReturnsStructuredError(t *testing.T) { if !strings.Contains(exitErr.Detail.Message, `"+bogus"`) { t.Errorf("message should echo the unknown token, got %q", exitErr.Detail.Message) } - if !strings.Contains(exitErr.Detail.Hint, "+search") || !strings.Contains(exitErr.Detail.Hint, "+upload") { - t.Errorf("hint should list available shortcuts, got %q", exitErr.Detail.Hint) - } - if strings.Contains(exitErr.Detail.Hint, "+secret") { - t.Error("hidden commands must not appear in the hint") + // "+bogus" has no close neighbor among drive's subcommands, so the hint falls + // back to pointing at --help; the full machine-readable list lives in + // detail.available below (which also excludes hidden commands). + if !strings.Contains(exitErr.Detail.Hint, "--help") { + t.Errorf("hint should guide to --help when there is no suggestion, got %q", exitErr.Detail.Hint) } detail, ok := exitErr.Detail.Detail.(map[string]any) diff --git a/internal/cmdpolicy/suggest.go b/internal/cmdpolicy/suggest.go index 2f7362e31..ea2ae5979 100644 --- a/internal/cmdpolicy/suggest.go +++ b/internal/cmdpolicy/suggest.go @@ -5,6 +5,7 @@ package cmdpolicy import ( "github.com/larksuite/cli/extension/platform" + "github.com/larksuite/cli/internal/suggest" ) // suggestRisk returns the closest valid Risk literal by edit distance @@ -20,9 +21,9 @@ func suggestRisk(bad string) string { platform.RiskRead, platform.RiskWrite, platform.RiskHighRiskWrite, } best := string(candidates[0]) - bestDist := levenshtein(lowered, best) + bestDist := suggest.Levenshtein(lowered, best) for _, c := range candidates[1:] { - if d := levenshtein(lowered, string(c)); d < bestDist { + if d := suggest.Levenshtein(lowered, string(c)); d < bestDist { bestDist, best = d, string(c) } } @@ -40,47 +41,3 @@ func toLower(s string) string { } return string(b) } - -// levenshtein computes the classic edit distance between two strings. -// O(len(a)*len(b)) time, O(min(a,b)) space. Three-element string set -// makes raw performance irrelevant — clarity beats trickiness here. -func levenshtein(a, b string) int { - if len(a) == 0 { - return len(b) - } - if len(b) == 0 { - return len(a) - } - prev := make([]int, len(b)+1) - curr := make([]int, len(b)+1) - for j := 0; j <= len(b); j++ { - prev[j] = j - } - for i := 1; i <= len(a); i++ { - curr[0] = i - for j := 1; j <= len(b); j++ { - cost := 1 - if a[i-1] == b[j-1] { - cost = 0 - } - curr[j] = min3( - prev[j]+1, // deletion - curr[j-1]+1, // insertion - prev[j-1]+cost, // substitution - ) - } - prev, curr = curr, prev - } - return prev[len(b)] -} - -func min3(a, b, c int) int { - m := a - if b < m { - m = b - } - if c < m { - m = c - } - return m -} diff --git a/internal/cmdpolicy/suggest_test.go b/internal/cmdpolicy/suggest_test.go index da91495a2..e8aae8e6c 100644 --- a/internal/cmdpolicy/suggest_test.go +++ b/internal/cmdpolicy/suggest_test.go @@ -29,23 +29,3 @@ func TestSuggestRisk(t *testing.T) { } } } - -func TestLevenshtein(t *testing.T) { - cases := []struct { - a, b string - want int - }{ - {"", "", 0}, - {"", "abc", 3}, - {"abc", "", 3}, - {"abc", "abc", 0}, - {"wrtie", "write", 2}, - {"kitten", "sitting", 3}, - } - for _, c := range cases { - got := levenshtein(c.a, c.b) - if got != c.want { - t.Errorf("levenshtein(%q,%q) = %d, want %d", c.a, c.b, got, c.want) - } - } -} diff --git a/internal/cmdutil/factory_default.go b/internal/cmdutil/factory_default.go index f18a816b3..fa3801092 100644 --- a/internal/cmdutil/factory_default.go +++ b/internal/cmdutil/factory_default.go @@ -102,7 +102,7 @@ func safeRedirectPolicy(req *http.Request, via []*http.Request) error { func cachedHttpClientFunc(f *Factory) func() (*http.Client, error) { return sync.OnceValues(func() (*http.Client, error) { - util.WarnIfProxied(f.IOStreams.ErrOut) + util.WarnIfProxied(f.IOStreams.ErrOut, f.IOStreams.IsTerminal) var transport http.RoundTripper = util.SharedTransport() transport = &RetryTransport{Base: transport} @@ -129,7 +129,7 @@ func cachedLarkClientFunc(f *Factory) func() (*lark.Client, error) { lark.WithLogLevel(larkcore.LogLevelError), lark.WithHeaders(BaseSecurityHeaders()), } - util.WarnIfProxied(f.IOStreams.ErrOut) + util.WarnIfProxied(f.IOStreams.ErrOut, f.IOStreams.IsTerminal) opts = append(opts, lark.WithHttpClient(&http.Client{ Transport: buildSDKTransport(), CheckRedirect: safeRedirectPolicy, diff --git a/internal/suggest/suggest.go b/internal/suggest/suggest.go new file mode 100644 index 000000000..fe4471f6b --- /dev/null +++ b/internal/suggest/suggest.go @@ -0,0 +1,104 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +// Package suggest provides the shared "did you mean" primitives: a rune-aware +// Levenshtein edit distance and a prefix-weighted Closest ranker. It is the +// single home for these so cmd, cmd/event, and internal/cmdpolicy stop each +// carrying their own copy. +package suggest + +import "sort" + +// Levenshtein computes the classic edit distance between two strings. It is +// rune-aware, so it is correct for multi-byte input. +func Levenshtein(a, b string) int { + if a == b { + return 0 + } + ra, rb := []rune(a), []rune(b) + if len(ra) == 0 { + return len(rb) + } + if len(rb) == 0 { + return len(ra) + } + prev := make([]int, len(rb)+1) + curr := make([]int, len(rb)+1) + for j := range prev { + prev[j] = j + } + for i := 1; i <= len(ra); i++ { + curr[0] = i + for j := 1; j <= len(rb); j++ { + cost := 1 + if ra[i-1] == rb[j-1] { + cost = 0 + } + curr[j] = min(prev[j]+1, curr[j-1]+1, prev[j-1]+cost) + } + prev, curr = curr, prev + } + return prev[len(rb)] +} + +// Closest returns up to maxN of candidates that plausibly match typed, ranked +// by shared-prefix length (desc) then edit distance (asc), keeping only +// reasonably-close ones. +// +// Shared prefix is weighted first on purpose: hallucinated names are often +// semantically close but lexically far (e.g. "+cells-find" vs "+cells-search", +// "--with-styles" vs nothing close), where the common prefix is the strongest +// signal of intent that raw edit distance misses. +func Closest(typed string, candidates []string, maxN int) []string { + type scored struct { + name string + prefix int + dist int + } + limit := editLimit(typed) + ranked := make([]scored, 0, len(candidates)) + for _, c := range candidates { + p := sharedPrefixLen(typed, c) + d := Levenshtein(typed, c) + // Keep only plausible matches: a meaningful shared prefix, or an edit + // distance within budget. Drop everything else so the hint stays short. + if p >= 3 || d <= limit { + ranked = append(ranked, scored{name: c, prefix: p, dist: d}) + } + } + sort.Slice(ranked, func(i, j int) bool { + if ranked[i].prefix != ranked[j].prefix { + return ranked[i].prefix > ranked[j].prefix + } + if ranked[i].dist != ranked[j].dist { + return ranked[i].dist < ranked[j].dist + } + return ranked[i].name < ranked[j].name + }) + if maxN <= 0 || maxN > len(ranked) { + maxN = len(ranked) + } + out := make([]string, 0, maxN) + for _, s := range ranked[:maxN] { + out = append(out, s.name) + } + return out +} + +// editLimit allows roughly one third of the typed length in edits (min 2), so +// short names tolerate a couple of typos and longer ones proportionally more. +func editLimit(s string) int { + if l := len([]rune(s)) / 3; l > 2 { + return l + } + return 2 +} + +func sharedPrefixLen(a, b string) int { + ra, rb := []rune(a), []rune(b) + n := 0 + for n < len(ra) && n < len(rb) && ra[n] == rb[n] { + n++ + } + return n +} diff --git a/internal/suggest/suggest_test.go b/internal/suggest/suggest_test.go new file mode 100644 index 000000000..dcf194349 --- /dev/null +++ b/internal/suggest/suggest_test.go @@ -0,0 +1,74 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package suggest + +import ( + "slices" + "testing" +) + +func TestClosest_HallucinatedSharesPrefix(t *testing.T) { + cmds := []string{ + "+cells-get", "+cells-set", "+cells-search", "+cells-replace", + "+cells-clear", "+cells-merge", "+csv-get", "+chart-create", + "+pivot-create", "+sheet-info", + } + // "+cells-find" is semantically +cells-search but lexically far; the shared + // "+cells-" prefix should still surface the right family (incl. +cells-search). + got := Closest("+cells-find", cmds, 6) + if len(got) == 0 || len(got) > 6 { + t.Fatalf("expected 1..6 suggestions, got %v", got) + } + if !slices.Contains(got, "+cells-search") { + t.Errorf("expected +cells-search among suggestions, got %v", got) + } + for _, s := range got { + if len(s) < 7 || s[:7] != "+cells-" { + t.Errorf("suggestion %q does not share the +cells- prefix", s) + } + } +} + +func TestClosest_TypoRanksExactNeighborFirst(t *testing.T) { + got := Closest("+cell-get", []string{"+cells-get", "+cells-set", "+csv-get", "+sheet-info"}, 3) + if len(got) == 0 || got[0] != "+cells-get" { + t.Errorf("expected +cells-get first for typo +cell-get, got %v", got) + } +} + +func TestClosest_NoPlausibleMatch(t *testing.T) { + if got := Closest("+zzzzzz", []string{"+cells-get", "+csv-get"}, 6); len(got) != 0 { + t.Errorf("expected no suggestions for unrelated input, got %v", got) + } +} + +func TestLevenshtein(t *testing.T) { + cases := []struct { + a, b string + want int + }{ + {"", "abc", 3}, + {"abc", "", 3}, + {"abc", "abc", 0}, + {"kitten", "sitting", 3}, + {"cell-get", "cells-get", 1}, + {"--query", "--find", 5}, + {"飞书", "飞书", 0}, // rune-aware: multi-byte equal + {"飞书", "飞s", 1}, // one rune substitution, not byte count + } + for _, c := range cases { + if d := Levenshtein(c.a, c.b); d != c.want { + t.Errorf("Levenshtein(%q,%q) = %d, want %d", c.a, c.b, d, c.want) + } + } +} + +func TestSharedPrefixLen(t *testing.T) { + if got := sharedPrefixLen("+cells-find", "+cells-search"); got != 7 { + t.Errorf("sharedPrefixLen = %d, want 7", got) + } + if got := sharedPrefixLen("abc", "xyz"); got != 0 { + t.Errorf("sharedPrefixLen = %d, want 0", got) + } +} diff --git a/internal/util/proxy.go b/internal/util/proxy.go index d9e251859..d8d6ba577 100644 --- a/internal/util/proxy.go +++ b/internal/util/proxy.go @@ -58,7 +58,18 @@ func redactProxyURL(raw string) string { // WarnIfProxied prints a one-time warning to w when a proxy environment variable // is detected and proxy is not disabled via LARK_CLI_NO_PROXY. Proxy credentials // are redacted. Safe to call multiple times; only the first call prints. -func WarnIfProxied(w io.Writer) { +// +// The warning is suppressed entirely when interactive is false — i.e. stdin is +// not a TTY, which is the case for agent / CI / piped invocations. Those callers +// frequently parse the CLI's stdout as JSON and merge streams with `2>&1`; a +// stray stderr warning then corrupts the parsed payload. Suppressing in the +// non-interactive case keeps machine-consumed output clean, while human +// interactive sessions still get the security notice. Passing interactive=false +// does not consume the once guard, so a later interactive call can still warn. +func WarnIfProxied(w io.Writer, interactive bool) { + if !interactive { + return + } proxyWarningOnce.Do(func() { if os.Getenv(EnvNoProxy) != "" { return diff --git a/internal/util/proxy_test.go b/internal/util/proxy_test.go index f78720963..6d2a6a376 100644 --- a/internal/util/proxy_test.go +++ b/internal/util/proxy_test.go @@ -97,7 +97,7 @@ func TestWarnIfProxied_WithProxy(t *testing.T) { t.Setenv("HTTPS_PROXY", "http://corp-proxy:3128") var buf bytes.Buffer - WarnIfProxied(&buf) + WarnIfProxied(&buf, true) out := buf.String() if out == "" { @@ -119,13 +119,30 @@ func TestWarnIfProxied_WithoutProxy(t *testing.T) { } var buf bytes.Buffer - WarnIfProxied(&buf) + WarnIfProxied(&buf, true) if buf.Len() != 0 { t.Errorf("expected no output when no proxy is set, got: %s", buf.String()) } } +func TestWarnIfProxied_SilentWhenNonInteractive(t *testing.T) { + proxyWarningOnce = sync.Once{} + + // Non-interactive (interactive=false) mirrors agent / CI / piped invocations + // where stdin is not a TTY. The proxy warning must be suppressed so callers + // that parse stdout as JSON — often merging streams with `2>&1` — are not + // corrupted by a stray stderr line. + t.Setenv("HTTPS_PROXY", "http://corp-proxy:3128") + + var buf bytes.Buffer + WarnIfProxied(&buf, false) + + if buf.Len() != 0 { + t.Errorf("expected no warning in non-interactive mode, got: %s", buf.String()) + } +} + func TestWarnIfProxied_SilentWhenDisabled(t *testing.T) { proxyWarningOnce = sync.Once{} @@ -133,7 +150,7 @@ func TestWarnIfProxied_SilentWhenDisabled(t *testing.T) { t.Setenv(EnvNoProxy, "1") var buf bytes.Buffer - WarnIfProxied(&buf) + WarnIfProxied(&buf, true) if buf.Len() != 0 { t.Errorf("expected no warning when proxy is disabled, got: %s", buf.String()) @@ -146,10 +163,10 @@ func TestWarnIfProxied_OnlyOnce(t *testing.T) { t.Setenv("HTTP_PROXY", "http://proxy:1234") var buf bytes.Buffer - WarnIfProxied(&buf) + WarnIfProxied(&buf, true) first := buf.String() - WarnIfProxied(&buf) + WarnIfProxied(&buf, true) second := buf.String() if first == "" { @@ -189,7 +206,7 @@ func TestWarnIfProxied_RedactsCredentials(t *testing.T) { t.Setenv("HTTPS_PROXY", "http://admin:s3cret@proxy:8080") var buf bytes.Buffer - WarnIfProxied(&buf) + WarnIfProxied(&buf, true) out := buf.String() if bytes.Contains([]byte(out), []byte("s3cret")) { diff --git a/shortcuts/sheets/csv_put_range_alias_test.go b/shortcuts/sheets/csv_put_range_alias_test.go new file mode 100644 index 000000000..707eebd1d --- /dev/null +++ b/shortcuts/sheets/csv_put_range_alias_test.go @@ -0,0 +1,76 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package sheets + +import "testing" + +// +csv-put locates with --start-cell, while +csv-get / +cells-set locate with +// --range. Agents routinely carry --range over to +csv-put and hit a guaranteed +// first-try failure. csvPutInput now accepts --range as an alias for +// --start-cell; a range value collapses to its top-left cell. +func TestCsvPutInput_RangeAliasForStartCell(t *testing.T) { + tests := []struct { + name string + raw map[string]interface{} + wantAnchor string + }{ + {"start-cell direct (unchanged)", map[string]interface{}{"csv": "a,b", "start-cell": "B2"}, "B2"}, + {"range alias, single cell", map[string]interface{}{"csv": "a,b", "range": "B2"}, "B2"}, + {"range alias collapses to top-left", map[string]interface{}{"csv": "a,b", "range": "A1:H17"}, "A1"}, + {"start-cell wins when both set", map[string]interface{}{"csv": "a,b", "start-cell": "C3", "range": "A1:H17"}, "C3"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + fv := newMapFlagViewForCommand("+csv-put", tt.raw) + input, err := csvPutInput(fv, "tok", "sid", "") + if err != nil { + t.Fatalf("csvPutInput returned error: %v", err) + } + got, _ := input["start_cell"].(string) + if got != tt.wantAnchor { + t.Errorf("start_cell = %q, want %q", got, tt.wantAnchor) + } + }) + } +} + +// With neither --start-cell nor --range set, +csv-put keeps its existing +// behavior: --start-cell defaults to A1, so the paste anchors at A1. +func TestCsvPutInput_DefaultsToA1(t *testing.T) { + fv := newMapFlagViewForCommand("+csv-put", map[string]interface{}{"csv": "a,b"}) + input, err := csvPutInput(fv, "tok", "sid", "") + if err != nil { + t.Fatalf("csvPutInput returned error: %v", err) + } + if got, _ := input["start_cell"].(string); got != "A1" { + t.Errorf("start_cell = %q, want %q (default)", got, "A1") + } +} + +// csvPutWriteRangeFromInput surfaces the real paste footprint so agents can see +// how far a CSV reaches from its anchor — it auto-expands to the CSV's own size, +// not to any user-set range. +func TestCsvPutWriteRangeFromInput(t *testing.T) { + tests := []struct { + name string + input map[string]interface{} + want string + ok bool + }{ + {"3x3 at B2", map[string]interface{}{"start_cell": "B2", "csv": "a,b,c\n1,2,3\n4,5,6"}, "B2:D4", true}, + {"single cell at A1", map[string]interface{}{"start_cell": "A1", "csv": "x"}, "A1:A1", true}, + {"1 row 3 cols at C3", map[string]interface{}{"start_cell": "C3", "csv": "a,b,c"}, "C3:E3", true}, + {"ragged rows use max width", map[string]interface{}{"start_cell": "A1", "csv": "a,b\nc,d,e"}, "A1:C2", true}, + {"missing csv", map[string]interface{}{"start_cell": "A1"}, "", false}, + {"non-single anchor", map[string]interface{}{"start_cell": "A1:B2", "csv": "x"}, "", false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, ok := csvPutWriteRangeFromInput(tt.input) + if ok != tt.ok || got != tt.want { + t.Errorf("got (%q, %v), want (%q, %v)", got, ok, tt.want, tt.ok) + } + }) + } +} diff --git a/shortcuts/sheets/data/flag-defs.json b/shortcuts/sheets/data/flag-defs.json index 3dfd06bc4..bf11ad83a 100644 --- a/shortcuts/sheets/data/flag-defs.json +++ b/shortcuts/sheets/data/flag-defs.json @@ -1876,6 +1876,14 @@ "desc": "Allow overwriting (default true); set false to error if any target cell is non-empty", "default": "true" }, + { + "name": "range", + "kind": "own", + "type": "string", + "required": "optional", + "desc": "alias for --start-cell (parity with +csv-get / +cells-set, which locate with --range); a range like A1:H17 collapses to its top-left cell", + "hidden": true + }, { "name": "dry-run", "kind": "system", diff --git a/shortcuts/sheets/lark_sheet_write_cells.go b/shortcuts/sheets/lark_sheet_write_cells.go index 6ff0af538..3374d2d91 100644 --- a/shortcuts/sheets/lark_sheet_write_cells.go +++ b/shortcuts/sheets/lark_sheet_write_cells.go @@ -5,6 +5,7 @@ package sheets import ( "context" + "encoding/csv" "fmt" "image" _ "image/gif" @@ -17,6 +18,7 @@ import ( "github.com/larksuite/cli/internal/output" "github.com/larksuite/cli/internal/validate" "github.com/larksuite/cli/shortcuts/common" + "github.com/spf13/cobra" ) // ─── lark_sheet_write_cells ─────────────────────────────────────────── @@ -205,13 +207,28 @@ var CsvPut = common.Shortcut{ Scopes: []string{"sheets:spreadsheet:write_only"}, AuthTypes: []string{"user", "bot"}, HasFormat: true, - Flags: flagsFor("+csv-put"), - Validate: validateViaInput(csvPutInput), + Flags: flagsFor("+csv-put"), // includes the hidden --range alias (defined in the base flags table) + PostMount: func(cmd *cobra.Command) { + // --range is an accepted alias for --start-cell (see csvPutInput). + // Neither is individually required; exactly one must be set. flag-defs + // marks --start-cell required, so clear that annotation and switch to a + // one-required group — otherwise cobra rejects `--range A1` for a + // missing --start-cell before the handler ever runs. + if fl := cmd.Flags().Lookup("start-cell"); fl != nil { + delete(fl.Annotations, cobra.BashCompOneRequiredFlag) + } + cmd.MarkFlagsOneRequired("start-cell", "range") + }, + Validate: validateViaInput(csvPutInput), DryRun: func(ctx context.Context, runtime *common.RuntimeContext) *common.DryRunAPI { token, _ := resolveSpreadsheetToken(runtime) sheetID, sheetName, _ := resolveSheetSelector(runtime) input, _ := csvPutInput(runtime, token, sheetID, sheetName) - return invokeToolDryRun(token, ToolKindWrite, "set_range_from_csv", input) + dr := invokeToolDryRun(token, ToolKindWrite, "set_range_from_csv", input) + if rng, ok := csvPutWriteRangeFromInput(input); ok { + dr = dr.Set("writes_range", rng) + } + return dr }, Execute: func(ctx context.Context, runtime *common.RuntimeContext) error { token, err := resolveSpreadsheetToken(runtime) @@ -230,11 +247,54 @@ var CsvPut = common.Shortcut{ if err != nil { return err } + if rng, ok := csvPutWriteRangeFromInput(input); ok { + if m, isMap := out.(map[string]interface{}); isMap { + m["writes_range"] = rng + } + } runtime.Out(out, nil) return nil }, } +// csvPutWriteRangeFromInput computes the rectangle +csv-put will actually write, +// from the built tool input (start_cell + csv). +csv-put pastes from the anchor +// and auto-expands to the CSV's own row/column count — the footprint is the +// result, not a user-set boundary. Surfacing it (e.g. "B2:D4") in dry-run and in +// the success envelope lets agents see how far a paste reaches before it +// silently overwrites neighbouring cells (use --allow-overwrite=false to block +// that). Returns ok=false when the anchor is not a single cell or the CSV has no +// parseable fields. +func csvPutWriteRangeFromInput(input map[string]interface{}) (string, bool) { + anchor, _ := input["start_cell"].(string) + csvText, _ := input["csv"].(string) + if anchor == "" || csvText == "" { + return "", false + } + col0, row0, ok := splitCellRef(anchor) + if !ok { + return "", false + } + r := csv.NewReader(strings.NewReader(csvText)) + r.FieldsPerRecord = -1 // tolerate ragged rows; we only need the max width + records, err := r.ReadAll() + if err != nil || len(records) == 0 { + return "", false + } + cols := 0 + for _, rec := range records { + if len(rec) > cols { + cols = len(rec) + } + } + if cols == 0 { + return "", false + } + endCol := columnIndexToLetter(col0 + cols - 1) + endRow := row0 + len(records) // row0 is 0-based; +len(records) is the 1-based bottom row + return fmt.Sprintf("%s:%s%d", anchor, endCol, endRow), true +} + func csvPutInput(runtime flagView, token, sheetID, sheetName string) (map[string]interface{}, error) { if err := requireSheetSelector(sheetID, sheetName); err != nil { return nil, err @@ -243,6 +303,18 @@ func csvPutInput(runtime flagView, token, sheetID, sheetName string) (map[string return nil, common.FlagErrorf("--csv is required") } anchor := strings.TrimSpace(runtime.Str("start-cell")) + // --range is accepted as an alias for --start-cell. +csv-get and +cells-set + // locate with --range, so agents routinely carry --range over to +csv-put and + // hit a guaranteed first-try failure. Honor it when --start-cell was not + // explicitly set — guard on Changed, not emptiness, because --start-cell + // defaults to "A1" and is therefore never empty. A range like "A1:H17" + // collapses to its top-left cell; +csv-put pastes from the anchor and + // auto-expands, so the range's lower-right bound is irrelevant. + if !runtime.Changed("start-cell") { + if rng := strings.TrimSpace(runtime.Str("range")); rng != "" { + anchor = strings.TrimSpace(strings.SplitN(rng, ":", 2)[0]) + } + } if anchor == "" { return nil, common.FlagErrorf("--start-cell is required") } diff --git a/skills/lark-sheets/SKILL.md b/skills/lark-sheets/SKILL.md index cd01bf069..adfb4c36d 100644 --- a/skills/lark-sheets/SKILL.md +++ b/skills/lark-sheets/SKILL.md @@ -32,6 +32,29 @@ metadata: | 透视表 pivot | `--pivot-table-id` | 迷你图（按组） | `--group-id` | | 浮动图片 | `--float-image-id` | | | +## 场景 → 命令速查（拿不准命令名先查这里，别按直觉拼） + +把高频意图映射到**真实存在**的 shortcut / flag。agent 常从 Excel / Google Sheets / 飞书 OpenAPI 误迁移命令名或 flag，先对照本表，避免一次必然失败的试错。完整 shortcut 见各工具参考。 + +| 你要做的事 | ✅ 正确写法 | ❌ 不存在（会被 cobra 拒） | +| --- | --- | --- | +| 读数据（纯值 / CSV） | `+csv-get`（范围用 `--range`） | — | +| 读值 + 公式 / 样式 / 批注 | `+cells-get --include value,formula,style,comment,data_validation` | `--value-render-option`、`--with-styles`、`--with-merges`、`--include-merged-cells` | +| 写纯值（整块 CSV 平铺） | `+csv-put`（定位用 `--start-cell`，单个左上角锚点格；也接受 `--range` 别名，区间自动取左上角） | — | +| 写值 / 公式 / 样式 | `+cells-set`（定位用 `--range`） | — | +| 查找单元格 | `+cells-search`（关键字用 `--find`） | `+cells-find`、`+find`、`--query` | +| 查找并替换 | `+cells-replace` | — | +| 看子表结构（合并 / 行高列宽 / 冻结 / 隐藏） | `+sheet-info` | `+sheet-get`、`+structure-get`、`+sheet-structure-get` | +| 看工作簿 / 子表清单 | `+workbook-info` | — | +| 导出 xlsx / 单表 csv | `+workbook-export` | — | +| 清除内容 / 格式 | `+cells-clear`（范围维度用 `--scope`，取值 content / formats / all） | `--type` | +| 批量清除多区域 | `+cells-batch-clear`（`--scope`） | `--target` | +| 调整列宽 / 行高 | `+cols-resize` / `+rows-resize`（行、列是两个独立命令） | `--dimension`（无此 flag） | +| 分组汇总 / 透视 | `+pivot-create`（默认不传落点 flag → 自动新建子表，零覆盖） | 用 SUMIF / 本地脚本拼一张假透视表 | + +> ⚠️ **定位 flag**：`+cells-get` / `+cells-set` / `+csv-get` 用 `--range`；`+csv-put` 规范用 `--start-cell`（单个左上角锚点格），也接受 `--range` 别名（区间自动取左上角），二者择一即可。 +> ⚠️ **读取附加信息**一律走 `+cells-get --include …`，**没有** `--value-render-option` / `--with-styles` 这类 flag；**看合并单元格**用 `+sheet-info` 的 `merged_cells`，不要在 `+cells-get` 里找 merge flag。 + ## References 本 skill 的 reference 分两组：先读**通用方法与规范**（横切所有任务的工作流、铁律、样式、公式规则，不含具体 shortcut），它们规定了"怎么做对"；再按操作对象进入**工具参考**查具体 shortcut 与调用细节。编辑类任务务必先过一遍通用方法与规范，其中的铁律对所有工具参考一律生效。 diff --git a/skills/lark-sheets/references/lark-sheets-core-operations.md b/skills/lark-sheets/references/lark-sheets-core-operations.md index cce1ad08b..46b07fa40 100644 --- a/skills/lark-sheets/references/lark-sheets-core-operations.md +++ b/skills/lark-sheets/references/lark-sheets-core-operations.md @@ -32,7 +32,7 @@ | 用户需求语义 | 路径 | |---|---| - | "完善 / 补齐 / 填空 / 修正所有 XX" / 数据分析 / 清洗 / 大数据集 | **A：分批 `+csv-get` 导出到本地 + pandas 处理 + 分批回写**（默认覆盖所有对应数据行，不以用户选区为准） | + | "完善 / 补齐 / 填空 / 修正所有 XX" / 数据分析 / 清洗 / 大数据集 | **A：先判能否用原生**（公式 / `+pivot` / `+filter`，见第 5 步）；确需代码再**分批 `+csv-get` 导出 + 本地脚本处理 + 分批回写**（默认覆盖所有对应数据行，不以用户选区为准；动代码前先过下方「运行环境前提」） | | "查一下 / 看看 / 统计 / 汇总" 等只读 | B：`+csv-get` 读到上下文 | | 需要公式 / 样式 / 批注 | C：`+cells-get` | | 续写 / 扩展 / 完善已有内容 | D：`+csv-get` 看结构 + `+cells-get` 读源区样式 + `+sheet-info --include row_heights,merges`（见铁律 5） | @@ -59,6 +59,15 @@ 7. **验证**：重新读取受影响区域确认值 / 公式 / 样式 / 批注符合预期；对象类（图表 / 透视表 / 条件格式 / 筛选 / 迷你图 / 浮动图片）重新读对象配置确认；出错先定位错误类型 / 受影响区域 / 根因再修复重验。 +## 运行环境前提（动用本地代码 / 脚本前必读） + +铁律是**原生工具优先、代码兜底**（见上）。一旦确需本地 `python` / `node`（多步清洗、统计建模、公式试错 3 次降级），先过这几条——实测大量失败绕路都源于此： + +- **解释器不一定存在**：目标环境常是 Windows + Git Bash，**`python3` 往往指向 Microsoft Store 占位符（不可用）**。动手前一次性探测 `python --version 2>&1 || python3 --version 2>&1 || py --version 2>&1 || node --version 2>&1`，选可用的那个；同一条命令失败后别原样重发。能不写代码就不写——优先飞书公式 / `+pivot` / `+csv-put`。 +- **临时文件路径**：Windows 上 `/tmp` 不等于系统临时目录，写进去外部 `python` / `node` 读不到。用 `$TEMP`（或脚本内 `tempfile.gettempdir()` / `os.tmpdir()`）取真实临时目录，不要硬编码 `/tmp`；仍放在用户项目目录之外。 +- **解析 CLI 输出别用 `2>&1`**：`[WARN] proxy …` 等提示走 stderr，`2>&1` 会把它混进 stdout 的 JSON，导致 `json.load` / `ConvertFrom-Json` 解析失败。要解析就直接管道 stdout（`lark-cli … | jq …`），或先 `> file`（只重定向 stdout）再读；需要诊断时把 stderr 单独导到另一个文件。 +- **文件编码**：喂给 CLI 的 CSV / JSON 用 **UTF-8 无 BOM**（BOM 会污染首格或触发 `invalid character` 解析错）；读 CLI 输出的脚本显式指定 `encoding='utf-8'`。 + ## 公式策略 - **公式优先于硬编码**（同铁律 4）：能用公式表达的计算一律写公式，源数据变化才能自动重算。 diff --git a/skills/lark-sheets/references/lark-sheets-write-cells.md b/skills/lark-sheets/references/lark-sheets-write-cells.md index 35acaf939..dd772f246 100644 --- a/skills/lark-sheets/references/lark-sheets-write-cells.md +++ b/skills/lark-sheets/references/lark-sheets-write-cells.md @@ -301,6 +301,7 @@ _公共四件套 · 系统：`--dry-run`_ | `--start-cell` | string | required | 目标区域起点 A1（如 `A1`、`B5`，不带 sheet 前缀；用 `--sheet-id` / `--sheet-name` 指定 sheet）；必须是单个单元格，不接受范围写法；终点按 CSV 实际行列数自动推断 | | `--csv` | string + File + Stdin（非 JSON 文本） | required | RFC 4180 CSV 文本；只写纯值，不带公式/样式/批注 | | `--allow-overwrite` | bool | optional | 允许覆盖（默认 true）；设为 false 时若目标非空报错 | +| `--range` | string | optional | --start-cell 的别名（与 +csv-get / +cells-set 一致，用 --range 定位）；传区间（如 A1:H17）时自动取其左上角单元格（隐藏 flag：不在 `--help` 列出，但可正常传入） | ## Schemas @@ -423,6 +424,12 @@ lark-cli sheets +csv-put --spreadsheet-token shtXXX --sheet-id "$SID" \ > # ↑ A2 实际写入字符串 "=SUM(B2:B10)"，**不是公式**。需要写公式请用 +cells-set。 > ``` +> **定位 + 写入边界（关键，避免误覆盖）**： +> - 定位用 `--start-cell`（锚点 = 左上角单元格）；也接受 `--range` 别名（与 `+csv-get` / `+cells-set` 一致，传区间会自动取左上角）。 +> - ⚠️ `--start-cell` / `--range` **只定左上角、不限制写入大小**：CSV 从锚点按自身行列数 auto-expand 铺开。给一个"小 range"**不会**截断数据——超出部分照写，且默认覆盖。这与 `+cells-set --range`（精确矩形、`--cells` 必须与 range 同维）语义相反，别把那套心智搬过来。 +> - dry-run 与成功响应都回显 `writes_range`（实际落区，如 `B2:D4`）：**写前先 `--dry-run` 看一眼落区**，确认不会盖到相邻数据。 +> - 要保护非空 cell：`--allow-overwrite=false`（落区内出现非空 cell 即报错）。 + ### Validate / DryRun / Execute 约束 - `Validate`：XOR 公共四件套；`+cells-set` 的 `--cells` 必须能解析为 JSON 二维矩阵且行列数与 `--range` 完全一致；`+cells-set-style` 的样式 flag 至少一个非空（或带 `--border-styles`）；`+cells-set-image` 的 `--range` 必须是单 cell（起止 cell 相同）；`+csv-put` 的 `--csv` 必须能按 RFC 4180 解析；防爆参数上限校验。