From b4c020c811d9c5601011e76d2ce86f76d0db479f Mon Sep 17 00:00:00 2001 From: huangmengxuan Date: Thu, 2 Apr 2026 19:39:20 +0800 Subject: [PATCH 1/2] fix: reject positional arguments in shortcuts with clear error Shortcuts silently ignored positional arguments (e.g. `lark-cli docs +search "hello"`), causing empty results. Add Args validator to all declarative shortcuts so cobra prints usage and a clear error message telling users to pass values via flags instead. Change-Id: I7579f9c871138cf91dd5f5d8c1d51bda3f77a1db Co-Authored-By: Claude Opus 4.6 --- shortcuts/common/runner.go | 14 +++++++++ shortcuts/common/runner_args_test.go | 47 ++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+) create mode 100644 shortcuts/common/runner_args_test.go diff --git a/shortcuts/common/runner.go b/shortcuts/common/runner.go index 7623c8d4..406a32f3 100644 --- a/shortcuts/common/runner.go +++ b/shortcuts/common/runner.go @@ -540,6 +540,7 @@ func (s Shortcut) mountDeclarative(parent *cobra.Command, f *cmdutil.Factory) { cmd := &cobra.Command{ Use: shortcut.Command, Short: shortcut.Description, + Args: rejectPositionalArgs(&shortcut), RunE: func(cmd *cobra.Command, _ []string) error { return runShortcut(cmd, f, &shortcut, botOnly) }, @@ -681,6 +682,19 @@ func handleShortcutDryRun(f *cmdutil.Factory, rctx *RuntimeContext, s *Shortcut) return nil } +// rejectPositionalArgs returns a cobra.PositionalArgs that rejects any +// positional arguments. The error is intentionally a plain error (not +// ExitError) so that cobra prints usage and the root handler prints a +// simple "Error:" line instead of a JSON envelope. +func rejectPositionalArgs(_ *Shortcut) cobra.PositionalArgs { + return func(cmd *cobra.Command, args []string) error { + if len(args) == 0 { + return nil + } + return fmt.Errorf("positional arguments are not supported (got %q); pass values via flags", args[0]) + } +} + func registerShortcutFlags(cmd *cobra.Command, s *Shortcut) { for _, fl := range s.Flags { desc := fl.Desc diff --git a/shortcuts/common/runner_args_test.go b/shortcuts/common/runner_args_test.go new file mode 100644 index 00000000..af5f96fb --- /dev/null +++ b/shortcuts/common/runner_args_test.go @@ -0,0 +1,47 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package common + +import ( + "strings" + "testing" + + "github.com/spf13/cobra" +) + +func TestRejectPositionalArgs_WithArgs(t *testing.T) { + t.Parallel() + + s := &Shortcut{ + Flags: []Flag{ + {Name: "query", Desc: "search keyword"}, + }, + } + validator := rejectPositionalArgs(s) + + err := validator(&cobra.Command{}, []string{"hello"}) + if err == nil { + t.Fatal("expected error for positional arg, got nil") + } + if !strings.Contains(err.Error(), "positional arguments are not supported") { + t.Errorf("expected positional args rejection message, got: %v", err) + } + if !strings.Contains(err.Error(), `"hello"`) { + t.Errorf("expected the positional arg value in error, got: %v", err) + } +} + +func TestRejectPositionalArgs_NoArgs(t *testing.T) { + t.Parallel() + + s := &Shortcut{} + validator := rejectPositionalArgs(s) + + if err := validator(&cobra.Command{}, nil); err != nil { + t.Fatalf("expected no error for nil args, got: %v", err) + } + if err := validator(&cobra.Command{}, []string{}); err != nil { + t.Fatalf("expected no error for empty args, got: %v", err) + } +} From 1b6d962a43deb5741654ad4ef2c0aa1f6588a282 Mon Sep 17 00:00:00 2001 From: huangmengxuan Date: Thu, 2 Apr 2026 19:55:32 +0800 Subject: [PATCH 2/2] fix: address PR review comments - Remove unused *Shortcut parameter from rejectPositionalArgs - Show all positional args in error message instead of only the first - Add test case for multiple positional arguments Change-Id: Ifea92d09ddabcd35fbf2db98d9888d18af59b894 Co-Authored-By: Claude Opus 4.6 --- shortcuts/common/runner.go | 6 +++--- shortcuts/common/runner_args_test.go | 27 +++++++++++++++++++-------- 2 files changed, 22 insertions(+), 11 deletions(-) diff --git a/shortcuts/common/runner.go b/shortcuts/common/runner.go index 406a32f3..051e0b3a 100644 --- a/shortcuts/common/runner.go +++ b/shortcuts/common/runner.go @@ -540,7 +540,7 @@ func (s Shortcut) mountDeclarative(parent *cobra.Command, f *cmdutil.Factory) { cmd := &cobra.Command{ Use: shortcut.Command, Short: shortcut.Description, - Args: rejectPositionalArgs(&shortcut), + Args: rejectPositionalArgs(), RunE: func(cmd *cobra.Command, _ []string) error { return runShortcut(cmd, f, &shortcut, botOnly) }, @@ -686,12 +686,12 @@ func handleShortcutDryRun(f *cmdutil.Factory, rctx *RuntimeContext, s *Shortcut) // positional arguments. The error is intentionally a plain error (not // ExitError) so that cobra prints usage and the root handler prints a // simple "Error:" line instead of a JSON envelope. -func rejectPositionalArgs(_ *Shortcut) cobra.PositionalArgs { +func rejectPositionalArgs() cobra.PositionalArgs { return func(cmd *cobra.Command, args []string) error { if len(args) == 0 { return nil } - return fmt.Errorf("positional arguments are not supported (got %q); pass values via flags", args[0]) + return fmt.Errorf("positional arguments are not supported (got %q); pass values via flags", args) } } diff --git a/shortcuts/common/runner_args_test.go b/shortcuts/common/runner_args_test.go index af5f96fb..f04999da 100644 --- a/shortcuts/common/runner_args_test.go +++ b/shortcuts/common/runner_args_test.go @@ -13,12 +13,7 @@ import ( func TestRejectPositionalArgs_WithArgs(t *testing.T) { t.Parallel() - s := &Shortcut{ - Flags: []Flag{ - {Name: "query", Desc: "search keyword"}, - }, - } - validator := rejectPositionalArgs(s) + validator := rejectPositionalArgs() err := validator(&cobra.Command{}, []string{"hello"}) if err == nil { @@ -32,11 +27,27 @@ func TestRejectPositionalArgs_WithArgs(t *testing.T) { } } +func TestRejectPositionalArgs_MultipleArgs(t *testing.T) { + t.Parallel() + + validator := rejectPositionalArgs() + + err := validator(&cobra.Command{}, []string{"hello", "world"}) + if err == nil { + t.Fatal("expected error for multiple positional args, got nil") + } + if !strings.Contains(err.Error(), "positional arguments are not supported") { + t.Errorf("unexpected error message: %v", err) + } + if !strings.Contains(err.Error(), "hello") || !strings.Contains(err.Error(), "world") { + t.Errorf("expected all positional args in error, got: %v", err) + } +} + func TestRejectPositionalArgs_NoArgs(t *testing.T) { t.Parallel() - s := &Shortcut{} - validator := rejectPositionalArgs(s) + validator := rejectPositionalArgs() if err := validator(&cobra.Command{}, nil); err != nil { t.Fatalf("expected no error for nil args, got: %v", err)