diff --git a/shortcuts/im/builders_test.go b/shortcuts/im/builders_test.go index 8f15ede21..80a8a8225 100644 --- a/shortcuts/im/builders_test.go +++ b/shortcuts/im/builders_test.go @@ -410,6 +410,9 @@ func TestShortcutValidateBranches(t *testing.T) { if err == nil || !strings.Contains(err.Error(), "--content is not valid JSON") { t.Fatalf("ImMessagesSend.Validate() error = %v", err) } + if !strings.Contains(err.Error(), "--text") { + t.Fatalf("ImMessagesSend.Validate() error = %v, want it to mention --text as a recovery alternative", err) + } }) t.Run("ImMessagesSend media with text", func(t *testing.T) { @@ -651,6 +654,9 @@ func TestShortcutValidateBranches(t *testing.T) { if err == nil || !strings.Contains(err.Error(), "requires user identity") { t.Fatalf("ImChatMessageList.Validate() error = %v, want requires user identity", err) } + if !strings.Contains(err.Error(), "--as user") || !strings.Contains(err.Error(), "--chat-id") { + t.Fatalf("ImChatMessageList.Validate() error = %v, want it to mention both --as user and --chat-id as recovery actions", err) + } }) t.Run("ImMessagesMGet empty ids", func(t *testing.T) { diff --git a/shortcuts/im/helpers.go b/shortcuts/im/helpers.go index 9fcd3c0e0..6f8fefc76 100644 --- a/shortcuts/im/helpers.go +++ b/shortcuts/im/helpers.go @@ -1482,7 +1482,7 @@ type shortcutItem struct { func collectChatIDs(rt *common.RuntimeContext) ([]string, error) { raw := rt.StrSlice("chat-id") if len(raw) == 0 { - return nil, errs.NewValidationError(errs.SubtypeInvalidArgument, "--chat-id is required (oc_xxx); repeat the flag or pass comma-separated values").WithParam("--chat-id") + return nil, errs.NewValidationError(errs.SubtypeInvalidArgument, "--chat-id is required (oc_xxx); repeat the flag or pass comma-separated values").WithParam("--chat-id").WithHint("get the open_chat_id from im +chat-search (by name) or im +chat-list (my chats)") } seen := make(map[string]struct{}, len(raw)) @@ -1494,7 +1494,7 @@ func collectChatIDs(rt *common.RuntimeContext) ([]string, error) { } if !strings.HasPrefix(v, "oc_") { return nil, errs.NewValidationError(errs.SubtypeInvalidArgument, - "invalid --chat-id %q: must be an open_chat_id starting with oc_", v).WithParam("--chat-id") + "invalid --chat-id %q: must be an open_chat_id starting with oc_", v).WithParam("--chat-id").WithHint("get the open_chat_id from im +chat-search (by name) or im +chat-list (my chats)") } if _, ok := seen[v]; ok { continue @@ -1503,7 +1503,7 @@ func collectChatIDs(rt *common.RuntimeContext) ([]string, error) { out = append(out, v) } if len(out) == 0 { - return nil, errs.NewValidationError(errs.SubtypeInvalidArgument, "--chat-id is required (oc_xxx)").WithParam("--chat-id") + return nil, errs.NewValidationError(errs.SubtypeInvalidArgument, "--chat-id is required (oc_xxx)").WithParam("--chat-id").WithHint("get the open_chat_id from im +chat-search (by name) or im +chat-list (my chats)") } if len(out) > feedShortcutBatchLimit { return nil, errs.NewValidationError(errs.SubtypeInvalidArgument, diff --git a/shortcuts/im/im_feed_shortcut_test.go b/shortcuts/im/im_feed_shortcut_test.go index ca903b035..1e5973f4b 100644 --- a/shortcuts/im/im_feed_shortcut_test.go +++ b/shortcuts/im/im_feed_shortcut_test.go @@ -118,6 +118,48 @@ func TestCollectChatIDs(t *testing.T) { } } +// TestCollectChatIDsHint locks that every collectChatIDs validation error +// carries an actionable recovery hint pointing the user at how to discover a +// real open_chat_id (im +chat-search / im +chat-list), not just what shape +// the flag must take. +func TestCollectChatIDsHint(t *testing.T) { + tests := []struct { + name string + input []string + }{ + {name: "missing chat-id", input: nil}, + {name: "bad prefix", input: []string{"om_abc"}}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cmd := newFeedShortcutCreateCmd(t) + for _, v := range tt.input { + if err := cmd.Flags().Set("chat-id", v); err != nil { + t.Fatalf("Set chat-id %q error = %v", v, err) + } + } + runtime := &common.RuntimeContext{Cmd: cmd} + + _, err := collectChatIDs(runtime) + if err == nil { + t.Fatalf("collectChatIDs() expected error, got nil") + } + + problem, ok := errs.ProblemOf(err) + if !ok { + t.Fatalf("collectChatIDs() error is not a typed Problem: %v", err) + } + if problem.Subtype != errs.SubtypeInvalidArgument { + t.Fatalf("collectChatIDs() Subtype = %v, want %v", problem.Subtype, errs.SubtypeInvalidArgument) + } + if !strings.Contains(problem.Hint, "+chat-search") || !strings.Contains(problem.Hint, "+chat-list") { + t.Fatalf("collectChatIDs() Hint = %q, want it to mention both +chat-search and +chat-list", problem.Hint) + } + }) + } +} + func TestBuildShortcutItems(t *testing.T) { got := buildShortcutItems([]string{"oc_a", "oc_b"}) if len(got) != 2 { diff --git a/shortcuts/im/tips_examples_test.go b/shortcuts/im/tips_examples_test.go index fe2aa1ace..67a8eda4c 100644 --- a/shortcuts/im/tips_examples_test.go +++ b/shortcuts/im/tips_examples_test.go @@ -11,9 +11,12 @@ import ( "github.com/larksuite/cli/shortcuts/common" ) -// The 12 high-frequency IM shortcuts covered by the governance closeout. -// Every entry must carry at least one copyable "Example:" tip locked by the -// tests below; other IM shortcuts (feed/flag series) are intentionally exempt. +// 12 high-frequency IM shortcuts covered by the original governance closeout, +// plus 6 feed/flag shortcuts that carry a real guessing surface (see the +// inline comment below). Every entry must carry at least one copyable +// "Example:" tip locked by the tests below. The 3 pagination-only feed/flag +// shortcuts (+feed-shortcut-list, +feed-group-list, +flag-list) are +// intentionally exempt — see the inline comment further down. var tipsExampleTargets = []string{ "+messages-send", "+messages-search", "+chat-messages-list", "+messages-reply", "+chat-search", "+chat-list", "+messages-mget", "+threads-messages-list", diff --git a/tests/cli_e2e/im/failure_inventory.md b/tests/cli_e2e/im/failure_inventory.md index 5362892bc..f34ea958b 100644 --- a/tests/cli_e2e/im/failure_inventory.md +++ b/tests/cli_e2e/im/failure_inventory.md @@ -40,13 +40,15 @@ next action, (4) know how to verify it. All four → PASS. - lock: TestResolveIsHeaderMutualExclusionHint ## feed.chat_id.not_oc_prefix -- source: shortcuts/im/helpers.go parseFeedChatIDs +- source: shortcuts/im/helpers.go collectChatIDs - user_task: pass a message id (om_) or plain id where an open_chat_id is required - command: `lark-cli im +feed-shortcut-create --chat-id om_test000 --dry-run` -- observed (replayed): `{"ok":false,"identity":"user","error":{"type":"validation","subtype":"invalid_argument","message":"invalid --chat-id \"om_test000\": must be an open_chat_id starting with oc_","param":"--chat-id"}}` -- verdict: PASS +- observed (replayed, before fix): `{"ok":false,"identity":"user","error":{"type":"validation","subtype":"invalid_argument","message":"invalid --chat-id \"om_test000\": must be an open_chat_id starting with oc_","param":"--chat-id"}}` — names what is required (an oc_ id) but gives no next action or ID-source hint +- observed (after fix): same envelope plus `"hint":"get the open_chat_id from im +chat-search (by name) or im +chat-list (my chats)"` +- verdict: FIX_HINT (fixed in this PR) +- expected_hint: get the open_chat_id from im +chat-search or im +chat-list - expected_next_action: fetch the oc_ id via +chat-search / +chat-list and retry -- lock: shortcuts/im/im_feed_shortcut_test.go::TestCollectChatIDs/rejects_bad_prefix; tests/cli_e2e/im/feed_shortcut_workflow_test.go::TestIM_FeedShortcutDryRun/create_dry-run_rejects_non-oc_chat_ids +- lock: shortcuts/im/im_feed_shortcut_test.go::TestCollectChatIDsHint ## chat-messages-list.bot_identity.user_id - source: shortcuts/im/im_chat_messages_list.go (Validate), shortcuts/im/helpers.go resolveP2PChatID diff --git a/tests/cli_e2e/im/tips_examples_dryrun_test.go b/tests/cli_e2e/im/tips_examples_dryrun_test.go index 592f03ede..d51a48505 100644 --- a/tests/cli_e2e/im/tips_examples_dryrun_test.go +++ b/tests/cli_e2e/im/tips_examples_dryrun_test.go @@ -17,16 +17,19 @@ import ( // Placeholder substitutions turning copyable help examples into syntactically // valid dry-run invocations. IDs are obvious fakes; --dry-run never hits the API. var tipsPlaceholderValues = map[string]string{ - "": "oc_e2etest000000000000000000", - "": "ou_e2etest000000000000000000", - "": "om_e2etest000000000000000000", - "": "omt_e2etest00000000000000000", - "": "file_v3_e2etest0000000000000", - "": "img_v3_e2etest00000000000000", - "": "ou_e2etest000000000000000001", - "": "ou_e2etest000000000000000002", - "": "om_e2etest000000000000000001", - "": "om_e2etest000000000000000002", + "": "oc_e2etest000000000000000000", + "": "ou_e2etest000000000000000000", + "": "om_e2etest000000000000000000", + "": "omt_e2etest00000000000000000", + "": "file_v3_e2etest0000000000000", + "": "img_v3_e2etest00000000000000", + "": "ou_e2etest000000000000000001", + "": "ou_e2etest000000000000000002", + "": "om_e2etest000000000000000001", + "": "om_e2etest000000000000000002", + "": "ofg_e2etest00000000000000000", + "": "oc_e2etest000000000000000001", + "": "oc_e2etest000000000000000002", } // firstExampleArgs extracts the first "Example:" tip of the shortcut, replaces @@ -115,3 +118,69 @@ func TestIMTipsFirstExampleDryRunChatMessagesList(t *testing.T) { func TestIMTipsFirstExampleDryRunResourcesDownload(t *testing.T) { runFirstExampleDryRun(t, "+messages-resources-download", "/open-apis/im/v1/messages/") } + +// tipsExampleAllTargets mirrors shortcuts/im/tips_examples_test.go's +// tipsExampleTargets: the 12 high-frequency + 6 feed/flag shortcuts whose +// help carries a locked copyable "Example:" tip. Kept as a literal copy here +// because that list lives in an internal _test.go file not visible outside +// the shortcuts/im package. +var tipsExampleAllTargets = []string{ + "+messages-send", "+messages-search", "+chat-messages-list", "+messages-reply", + "+chat-search", "+chat-list", "+messages-mget", "+threads-messages-list", + "+messages-resources-download", "+chat-create", "+chat-update", "+chat-members-list", + "+feed-shortcut-create", "+feed-shortcut-remove", + "+feed-group-list-item", "+feed-group-query-item", + "+flag-create", "+flag-cancel", +} + +// defaultAsForCommand picks the identity to run the dry-run under by reading +// the shortcut's own AuthTypes: "bot" when the shortcut supports bot identity +// (matching the 3 pre-existing path-assertion tests above), otherwise "user" +// for user-only shortcuts (+messages-search and the whole feed/flag series). +func defaultAsForCommand(t *testing.T, command string) string { + t.Helper() + for _, sc := range imshortcuts.Shortcuts() { + if sc.Command != command { + continue + } + for _, a := range sc.AuthTypes { + if a == "bot" { + return "bot" + } + } + return "user" + } + t.Fatalf("shortcut %s not found", command) + return "" +} + +// TestIMTipsFirstExampleDryRunAll extends the executability lock from the 3 +// path-assertion tests above (messages-send, chat-messages-list, +// resources-download) to every one of the 18 shortcuts carrying a locked +// Example tip: the first example, with placeholders substituted and +// --dry-run appended, must exit 0. This only asserts exit code, not the API +// path — the 3 tests above keep that stronger assertion for their targets. +func TestIMTipsFirstExampleDryRunAll(t *testing.T) { + for _, cmd := range tipsExampleAllTargets { + cmd := cmd + t.Run(cmd, func(t *testing.T) { + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) + t.Setenv("LARKSUITE_CLI_APP_ID", "im_tips_dryrun_test") + t.Setenv("LARKSUITE_CLI_APP_SECRET", "im_tips_dryrun_secret") + t.Setenv("LARKSUITE_CLI_BRAND", "feishu") + + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + + as := defaultAsForCommand(t, cmd) + args := append(firstExampleArgs(t, cmd), "--dry-run") + result, err := clie2e.RunCmd(ctx, clie2e.Request{ + Args: args, + DefaultAs: as, + WorkDir: t.TempDir(), + }) + require.NoError(t, err) + result.AssertExitCode(t, 0) + }) + } +}