diff --git a/shortcuts/common/binder.go b/shortcuts/common/binder.go index 433af0791..5c5451452 100644 --- a/shortcuts/common/binder.go +++ b/shortcuts/common/binder.go @@ -232,11 +232,11 @@ func setLeaf(dst reflect.Value, src reflect.Value) { // cobra reports the flag was explicitly provided. nil means "variant not // selected" — the framework's runFrameworkRules and runValidateValue // both honor this nil/non-nil split. -// - Non-pointer leaf in a group (e.g. argstype.MediaInput inside -// VideoContent): always copy the cobra flag value back. Empty string is -// a valid "not provided" sentinel for group completeness checks. -// - Pointer-to-group / pointer-to-bucket (e.g. *VideoContent inside the -// MessageContent bucket): allocate iff at least one inner flag was +// - Non-pointer leaf in a group (e.g. a typed-primitive field inside a +// paired group struct): always copy the cobra flag value back. Empty +// string is a valid "not provided" sentinel for group completeness checks. +// - Pointer-to-group / pointer-to-bucket (a nested group/bucket pointer +// inside an outer OneOf bucket): allocate iff at least one inner flag was // Changed, then recurse to bind the inner fields. func bindBuckets(cmd *cobra.Command, argsVal reflect.Value, specs []fieldSpec) error { for _, s := range specs { @@ -426,8 +426,9 @@ func asString(fv reflect.Value) string { // runValidateValue calls ValidateValue on every Validatable field, recursing // into OneOf bucket / group sub-structs so typed-primitive leaves inside -// nested Args structs (e.g. ChatID inside MessageTarget) still get their -// format check. Returns the first error to keep error envelopes deterministic. +// nested Args structs (e.g. a typed ID primitive inside a OneOf bucket) still +// get their format check. Returns the first error to keep error envelopes +// deterministic. func runValidateValue(rt *RuntimeContext, argsVal reflect.Value, specs []fieldSpec) error { for _, s := range specs { fv := argsVal.FieldByName(s.GoFieldName) @@ -442,10 +443,10 @@ func runValidateValue(rt *RuntimeContext, argsVal reflect.Value, specs []fieldSp } structVal = structVal.Elem() } - // Some buckets/groups implement Validatable themselves (e.g. - // RawContent does JSON validity in its ValidateValue). Call the - // struct-level check BEFORE recursing into inner fields so the - // cross-field rule fires even when none of the inner leaves are + // Some buckets/groups implement Validatable themselves (e.g. a + // raw-JSON variant that checks JSON validity in its ValidateValue). + // Call the struct-level check BEFORE recursing into inner fields so + // the cross-field rule fires even when none of the inner leaves are // individually Validatable. if fv.CanInterface() { if val, ok := fv.Interface().(Validatable); ok { @@ -494,10 +495,10 @@ func runValidateValue(rt *RuntimeContext, argsVal reflect.Value, specs []fieldSp // returns a *errs.ValidationError on the first violation. Each rule's // stderr-facing param is the Args field name (not the inner struct type name), // so OneOf bucket errors mention the user-visible field (e.g. "Target") rather -// than the implementation-detail type name ("MessageTarget"). +// than the implementation-detail type name behind it. // -// Recurses into OneOf bucket sub-structs so a nested group (e.g. VideoContent -// inside MessageContent) still gets its checkGroup fire automatically. +// Recurses into OneOf bucket sub-structs so a nested group inside a bucket +// still gets its checkGroup fire automatically. func runFrameworkRules(cmd *cobra.Command, argsVal reflect.Value, specs []fieldSpec) error { for _, s := range specs { fv := argsVal.FieldByName(s.GoFieldName) @@ -609,7 +610,7 @@ func checkGroup(cmd *cobra.Command, _ reflect.Value, s fieldSpec) error { continue } // Flags with a default value are never "missing" — the default is a - // valid implicit value (e.g. RawContent.MsgType defaults to "text"). + // valid implicit value (e.g. an enum flag that defaults to a value). // Only flags without defaults need explicit user input when the // group is partially populated. if child.DefaultValue != "" { @@ -618,8 +619,8 @@ func checkGroup(cmd *cobra.Command, _ reflect.Value, s fieldSpec) error { missing = append(missing, "--"+child.FlagName) } if anySet && len(missing) > 0 { - // Group errors use the inner struct TYPE name (e.g. "VideoContent"), - // not the Args field name (e.g. "Video"). This matches the spec's + // Group errors use the inner struct TYPE name (the group struct's own + // name), not the Args field name. This matches the spec's // "shortcut_group_incomplete" envelope contract — callers identify // the *kind* of group that is incomplete, which is the type name. // OneOf bucket errors use the field name instead (see checkOneOf). diff --git a/shortcuts/common/protocol.go b/shortcuts/common/protocol.go index 3eafd9aa8..ca66143d4 100644 --- a/shortcuts/common/protocol.go +++ b/shortcuts/common/protocol.go @@ -41,8 +41,9 @@ type OneOfMarker interface { OneOf() } -// Validatable is implemented by typed primitives (and may be by RawContent- -// style sub-structs) that own their format check. The binder calls +// Validatable is implemented by typed primitives (and may be by sub-structs +// that validate a composite value, e.g. a raw JSON body) that own their +// format check. The binder calls // ValidateValue per field after Normalize. Returning an error must produce // a *errs.ValidationError so the stderr envelope carries type/subtype/param. type Validatable interface { diff --git a/shortcuts/common/typed_help.go b/shortcuts/common/typed_help.go index a8dcb60d0..e4c1cf557 100644 --- a/shortcuts/common/typed_help.go +++ b/shortcuts/common/typed_help.go @@ -21,7 +21,7 @@ import ( // we only override per-command via cmd.SetHelpFunc. // // Section titles use the Args struct's field name (e.g. "TARGET", "CONTENT"), -// not the inner Go type name (e.g. "MESSAGETARGET"), so the help mirrors the +// not the inner Go type name behind the field, so the help mirrors the // user-visible variable name rather than an implementation detail. func buildTypedHelp(specs []fieldSpec, examples []HelpExample) func(*cobra.Command, []string) { return func(cmd *cobra.Command, _ []string) { @@ -43,8 +43,8 @@ func buildTypedHelp(specs []fieldSpec, examples []HelpExample) func(*cobra.Comma // renderOneOfSections walks each OneOf bucket and prints "CHOOSE ONE :" // followed by every flag inside the bucket — including flags inside nested -// groups (e.g. VideoContent.Cover under VideoContent.File) and nested raw- -// content variants (RawContent.JSON / RawContent.MsgType). +// groups (a paired group's companion flag under its trigger) and nested +// raw-content variants (a raw-JSON variant's body + msg-type flags). func renderOneOfSections(w io.Writer, specs []fieldSpec, rendered map[string]struct{}) { for _, s := range specs { if !s.IsOneOfBkt { @@ -62,9 +62,10 @@ func renderOneOfSections(w io.Writer, specs []fieldSpec, rendered map[string]str // inside a nested group renders at the parent's indent (it IS the variant // selector, not a companion). Non-trigger leaves with a default value also // render at parent indent (a default makes them "optional companions" rather -// than required follow-ons — see RawContent.MsgType). Only non-trigger leaves -// WITHOUT a default render at parent+2 indent, signaling "you must supply -// this together with the trigger" (see VideoContent.Cover under --video). +// than required follow-ons — e.g. an enum flag with a default). Only +// non-trigger leaves WITHOUT a default render at parent+2 indent, signaling +// "you must supply this together with the trigger" (e.g. a required companion +// flag under its trigger flag). func renderFlagsInBucket(w io.Writer, specs []fieldSpec, indent string, rendered map[string]struct{}) { for _, child := range specs { if child.IsGroup || child.IsOneOfBkt { diff --git a/shortcuts/im/builders_test.go b/shortcuts/im/builders_test.go index 3cde035ea..ca78fc3d1 100644 --- a/shortcuts/im/builders_test.go +++ b/shortcuts/im/builders_test.go @@ -349,61 +349,83 @@ func TestShortcutValidateBranches(t *testing.T) { } }) - // ImMessagesSend was migrated to common.TypedShortcut[*ImMessagesSendArgs]. - // Validate now takes (ctx, *ImMessagesSendArgs, *RuntimeContext) and only - // covers the rules the shared framework binder does not handle yet - // (VideoContent paired-flag group + explicit --msg-type interplay). Rules - // previously hand-rolled here — OneOf "exactly one target/content" mutual - // exclusion, --content JSON validity — moved to the framework binder / - // RawContent.ValidateValue. Those are now asserted at the framework - // layer (shortcuts/common/binder_test.go) and from the CLI end in - // tests_e2e/shortcuts/. See im_messages_send_test.go for the new typed - // unit tests. + t.Run("ImMessagesSend conflicting target", func(t *testing.T) { + runtime := newTestRuntimeContext(t, map[string]string{ + "chat-id": "oc_123", + "user-id": "ou_123", + "text": "hello", + }, nil) + err := ImMessagesSend.Validate(context.Background(), runtime) + if err == nil || !strings.Contains(err.Error(), "--chat-id and --user-id are mutually exclusive") { + t.Fatalf("ImMessagesSend.Validate() error = %v", err) + } + }) - t.Run("ImMessagesSend valid text passes Validate", func(t *testing.T) { + t.Run("ImMessagesSend invalid content json", func(t *testing.T) { + runtime := newTestRuntimeContext(t, map[string]string{ + "chat-id": "oc_123", + "content": "{invalid", + }, nil) + err := ImMessagesSend.Validate(context.Background(), runtime) + if err == nil || !strings.Contains(err.Error(), "--content is not valid JSON") { + t.Fatalf("ImMessagesSend.Validate() error = %v", err) + } + }) + + t.Run("ImMessagesSend media with text", func(t *testing.T) { + runtime := newTestRuntimeContext(t, map[string]string{ + "chat-id": "oc_123", + "text": "hello", + "image": "img_123", + }, nil) + err := ImMessagesSend.Validate(context.Background(), runtime) + if err == nil || !strings.Contains(err.Error(), "--image/--file/--video/--audio cannot be used with --text, --markdown, or --content") { + t.Fatalf("ImMessagesSend.Validate() error = %v", err) + } + }) + + t.Run("ImMessagesSend valid text", func(t *testing.T) { runtime := newTestRuntimeContext(t, map[string]string{ "chat-id": "oc_123", "text": "hello", }, nil) - args := &ImMessagesSendArgs{} - if err := ImMessagesSend.Validate(context.Background(), args, runtime); err != nil { + if err := ImMessagesSend.Validate(context.Background(), runtime); err != nil { t.Fatalf("ImMessagesSend.Validate() unexpected error = %v", err) } }) - t.Run("ImMessagesSend video with video-cover passes Validate", func(t *testing.T) { + t.Run("ImMessagesSend video with video-cover passes validate", func(t *testing.T) { + // Previously broken: the deleted check used imageKey instead of videoCoverKey, + // so --video + --video-cover would incorrectly fail at Validate. runtime := newTestRuntimeContext(t, map[string]string{ "chat-id": "oc_123", "video": "file_456", "video-cover": "img_789", }, nil) - args := &ImMessagesSendArgs{} - if err := ImMessagesSend.Validate(context.Background(), args, runtime); err != nil { + if err := ImMessagesSend.Validate(context.Background(), runtime); err != nil { t.Fatalf("ImMessagesSend.Validate() unexpected error = %v", err) } }) - t.Run("ImMessagesSend video without video-cover → group_incomplete", func(t *testing.T) { + t.Run("ImMessagesSend video without video-cover fails validate", func(t *testing.T) { runtime := newTestRuntimeContext(t, map[string]string{ "chat-id": "oc_123", "video": "file_456", }, nil) - args := &ImMessagesSendArgs{} - err := ImMessagesSend.Validate(context.Background(), args, runtime) - if err == nil || !strings.Contains(err.Error(), "VideoContent requires --video-cover") { - t.Fatalf("ImMessagesSend.Validate() error = %v, want VideoContent requires --video-cover", err) + err := ImMessagesSend.Validate(context.Background(), runtime) + if err == nil || !strings.Contains(err.Error(), "--video-cover is required when using --video") { + t.Fatalf("ImMessagesSend.Validate() error = %v", err) } }) - t.Run("ImMessagesSend video-cover without video → group_incomplete", func(t *testing.T) { + t.Run("ImMessagesSend video-cover without video fails validate", func(t *testing.T) { runtime := newTestRuntimeContext(t, map[string]string{ "chat-id": "oc_123", "video-cover": "img_789", }, nil) - args := &ImMessagesSendArgs{} - err := ImMessagesSend.Validate(context.Background(), args, runtime) - if err == nil || !strings.Contains(err.Error(), "VideoContent requires --video") { - t.Fatalf("ImMessagesSend.Validate() error = %v, want VideoContent requires --video", err) + err := ImMessagesSend.Validate(context.Background(), runtime) + if err == nil || !strings.Contains(err.Error(), "--video-cover can only be used with --video") { + t.Fatalf("ImMessagesSend.Validate() error = %v", err) } }) @@ -413,8 +435,7 @@ func TestShortcutValidateBranches(t *testing.T) { "msg-type": "file", "image": "img_123", }, nil) - args := &ImMessagesSendArgs{} - err := ImMessagesSend.Validate(context.Background(), args, runtime) + err := ImMessagesSend.Validate(context.Background(), runtime) if err == nil || !strings.Contains(err.Error(), "conflicts with the inferred message type") { t.Fatalf("ImMessagesSend.Validate() error = %v", err) } @@ -702,8 +723,7 @@ func TestShortcutDryRunShapes(t *testing.T) { "image": "img_123", "idempotency-key": "uuid-2", }, nil) - args := &ImMessagesSendArgs{IdempotencyKey: "uuid-2"} - got := mustMarshalDryRun(t, ImMessagesSend.DryRun(context.Background(), args, runtime)) + got := mustMarshalDryRun(t, ImMessagesSend.DryRun(context.Background(), runtime)) if !strings.Contains(got, `"receive_id_type":"open_id"`) || !strings.Contains(got, `"msg_type":"image"`) || !strings.Contains(got, `"uuid":"uuid-2"`) || !strings.Contains(got, `\"image_key\":\"img_123\"`) { t.Fatalf("ImMessagesSend.DryRun() = %s", got) } @@ -714,8 +734,7 @@ func TestShortcutDryRunShapes(t *testing.T) { "chat-id": "oc_123", "image": "https://example.com/a.png", }, nil) - args := &ImMessagesSendArgs{} - got := mustMarshalDryRun(t, ImMessagesSend.DryRun(context.Background(), args, runtime)) + got := mustMarshalDryRun(t, ImMessagesSend.DryRun(context.Background(), runtime)) if !strings.Contains(got, `"description":"dry-run uses placeholder media keys for --image URL input; execution uploads it before sending"`) || !strings.Contains(got, `"msg_type":"image"`) || !strings.Contains(got, `\"image_key\":\"img_dryrun_upload\"`) { diff --git a/shortcuts/im/helpers.go b/shortcuts/im/helpers.go index 2f0ebe765..a49e3b950 100644 --- a/shortcuts/im/helpers.go +++ b/shortcuts/im/helpers.go @@ -13,7 +13,6 @@ import ( "math" "net/http" "net/url" - "os" "path" "path/filepath" "regexp" @@ -74,27 +73,6 @@ func validateMessageID(input string) (string, error) { return input, nil } -// isMediaKey returns true if the value looks like an existing API key rather -// than a local file path. -func isMediaKey(value string) bool { - return strings.HasPrefix(value, "img_") || strings.HasPrefix(value, "file_") -} - -// validateMediaFlagPath validates a media flag value as a local file path via -// FileIO. Empty values, URLs, and media keys are skipped (not local files). -// The typed-primitive layer (argstype.MediaInput) handles cwd-relative path -// safety; this loose os.Stat check stays at the shortcut layer because it -// touches the filesystem. -func validateMediaFlagPath(fio fileio.FileIO, flagName, value string) error { - if value == "" || strings.HasPrefix(value, "http://") || strings.HasPrefix(value, "https://") || isMediaKey(value) { - return nil - } - if _, err := fio.Stat(value); err != nil && !os.IsNotExist(err) { - return output.ErrValidation("%s: %v", flagName, err) - } - return nil -} - // buildMediaContentFromKey builds (msgType, contentJSON) for DryRun purposes from flag values. // Local paths and URLs are represented with placeholder keys because DryRun does not upload media. func buildMediaContentFromKey(text, imageKey, fileKey, videoKey, videoCoverKey, audioKey string) (msgType, content, desc string) { diff --git a/shortcuts/im/helpers_test.go b/shortcuts/im/helpers_test.go index 7b2414ef6..114ae524f 100644 --- a/shortcuts/im/helpers_test.go +++ b/shortcuts/im/helpers_test.go @@ -704,9 +704,6 @@ func TestShortcuts(t *testing.T) { for _, shortcut := range Shortcuts() { commands = append(commands, shortcut.Command) } - for _, m := range TypedShortcuts() { - commands = append(commands, m.GetCommand()) - } want := []string{ "+chat-create", @@ -718,16 +715,13 @@ func TestShortcuts(t *testing.T) { "+messages-reply", "+messages-resources-download", "+messages-search", - // +messages-send moved from legacy Shortcuts() to TypedShortcuts() - // as part of the shortcut-protocol pilot; the unified list still - // covers it. + "+messages-send", "+threads-messages-list", "+flag-create", "+flag-cancel", "+flag-list", - "+messages-send", } if !reflect.DeepEqual(commands, want) { - t.Fatalf("Shortcuts()+TypedShortcuts() commands = %#v, want %#v", commands, want) + t.Fatalf("Shortcuts() commands = %#v, want %#v", commands, want) } } diff --git a/shortcuts/im/im_messages_send.go b/shortcuts/im/im_messages_send.go index d5bdaa6f7..4b8a30984 100644 --- a/shortcuts/im/im_messages_send.go +++ b/shortcuts/im/im_messages_send.go @@ -5,44 +5,18 @@ package im import ( "context" + "encoding/json" "net/http" + "os" + "strings" - larkcore "github.com/larksuite/oapi-sdk-go/v3/core" - "github.com/spf13/cobra" - - "github.com/larksuite/cli/errs" + "github.com/larksuite/cli/extension/fileio" + "github.com/larksuite/cli/internal/output" "github.com/larksuite/cli/shortcuts/common" - "github.com/larksuite/cli/shortcuts/common/argstype" + larkcore "github.com/larksuite/oapi-sdk-go/v3/core" ) -// ImMessagesSendArgs is the typed argument struct backing `im +messages-send`. -// -// Target / Content are OneOf buckets (see protocol.go) — the framework enforces -// "exactly one trigger flag set" and emits structured errors with subtype -// shortcut_oneof_missing / shortcut_oneof_multiple if the caller violates that. -// VideoContent is a paired group inside the Content bucket — the framework -// emits shortcut_group_incomplete when --video is set without --video-cover. -type ImMessagesSendArgs struct { - Target MessageTarget - Content MessageContent - IdempotencyKey string `flag:"idempotency-key" desc:"idempotency key (prevents duplicate sends)"` -} - -// imSendExamples are rendered in the --help EXAMPLES section by the typed help -// builder. The text mirrors §"Pilot 改造" of the shortcut-protocol spec -// (lines 442-446). -var imSendExamples = []common.HelpExample{ - {Title: "text to chat", Cmd: `--chat-id oc_x --text "hi"`}, - {Title: "markdown to user", Cmd: `--user-id ou_x --markdown "**hi**"`}, - {Title: "video with cover", Cmd: `--chat-id oc_x --video v.mp4 --video-cover c.png`}, -} - -// ImMessagesSend is the typed pilot for the shortcut-protocol refactor. -// Behavior matches the legacy common.Shortcut version exactly (same flags, -// same body shape, same envelope) — only the wiring changes: OneOf / group / -// typed-primitive format are framework-enforced instead of hand-rolled -// inside a Validate closure. -var ImMessagesSend = common.TypedShortcut[*ImMessagesSendArgs]{ +var ImMessagesSend = common.Shortcut{ Service: "im", Command: "+messages-send", Description: "Send a message to a chat or direct message; user/bot; sends to chat-id or user-id with text/markdown/post/media, supports idempotency key", @@ -51,47 +25,35 @@ var ImMessagesSend = common.TypedShortcut[*ImMessagesSendArgs]{ UserScopes: []string{"im:message.send_as_user", "im:message"}, BotScopes: []string{"im:message:send_as_bot"}, AuthTypes: []string{"bot", "user"}, - Examples: imSendExamples, - - // Validate covers the one cross-field rule the framework cannot infer - // from enum tags alone: an explicit --msg-type that conflicts with the - // inferred type from the selected content flag (e.g. --text "hi" - // --msg-type image). - // - // Other invariants (OneOf missing/multiple, VideoContent group - // completeness, ChatID/UserOpenID/MediaInput format, RawContent JSON - // validity, msg-type enum membership) are enforced upstream by the - // framework's runValidateValue + runFrameworkRules. The bindMessagesSendArgs - // + validateVideoGroup defensive calls below let direct invocations of - // this closure (e.g. unit tests that bypass the framework Validate stack) - // still see populated args and surface the VideoContent group error — - // in the production runShortcut path the framework has already done both - // and short-circuited before this hook ever runs. - Validate: func(ctx context.Context, args *ImMessagesSendArgs, rt *common.RuntimeContext) error { - bindMessagesSendArgs(rt.Cmd, args) - if err := validateVideoGroup(rt.Cmd); err != nil { - return err - } - return validateMsgTypeInterplay(rt.Cmd, args) + Flags: []common.Flag{ + {Name: "chat-id", Desc: "(required, mutually exclusive with --user-id) chat ID (oc_xxx)"}, + {Name: "user-id", Desc: "(required, mutually exclusive with --chat-id) user open_id (ou_xxx)"}, + {Name: "msg-type", Default: "text", Desc: "message type for --content JSON; when using --text/--markdown/--image/--file/--video/--audio, the effective type is inferred automatically", Enum: []string{"text", "post", "image", "file", "audio", "media", "interactive", "share_chat", "share_user"}}, + {Name: "content", Desc: "(one of --content/--text/--markdown/--image/--file/--video/--audio required) message content JSON"}, + {Name: "text", Desc: "plain text message (auto-wrapped as JSON)"}, + {Name: "markdown", Desc: "markdown text (auto-wrapped as post format with style optimization; image URLs auto-resolved)"}, + {Name: "idempotency-key", Desc: "idempotency key (prevents duplicate sends)"}, + {Name: "image", Desc: "image key (img_xxx), URL, or cwd-relative local path (absolute paths and .. are rejected)"}, + {Name: "file", Desc: "file key (file_xxx), URL, or cwd-relative local path (absolute paths and .. are rejected)"}, + {Name: "video", Desc: "video file key (file_xxx), URL, or cwd-relative local path (absolute paths and .. are rejected); must be used together with --video-cover"}, + {Name: "video-cover", Desc: "video cover image key (img_xxx), URL, or cwd-relative local path (absolute paths and .. are rejected); required when using --video"}, + {Name: "audio", Desc: "audio file key (file_xxx), URL, or cwd-relative local path (absolute paths and .. are rejected)"}, }, - - DryRun: func(ctx context.Context, args *ImMessagesSendArgs, rt *common.RuntimeContext) *common.DryRunAPI { - // Defensive: framework's bindBuckets has already populated args when - // reaching this closure via the production runShortcut path. The call - // is retained so direct invocations (e.g. unit tests that pass an - // empty Args struct + populated cobra flags) still bind correctly. - bindMessagesSendArgs(rt.Cmd, args) - - text := strOrEmpty(args.Content.Text) - markdown := strOrEmpty(args.Content.Markdown) - imageKey := mediaOrEmpty(args.Content.Image) - fileKey := mediaOrEmpty(args.Content.File) - videoKey, videoCoverKey := videoOrEmpty(args.Content.Video) - audioKey := mediaOrEmpty(args.Content.Audio) - content, msgType := rawOrDefault(args.Content.Raw) - idempotencyKey := args.IdempotencyKey - + DryRun: func(ctx context.Context, runtime *common.RuntimeContext) *common.DryRunAPI { + chatFlag := runtime.Str("chat-id") + userFlag := runtime.Str("user-id") + msgType := runtime.Str("msg-type") + content := runtime.Str("content") desc := "" + text := runtime.Str("text") + markdown := runtime.Str("markdown") + idempotencyKey := runtime.Str("idempotency-key") + imageKey := runtime.Str("image") + fileKey := runtime.Str("file") + videoKey := runtime.Str("video") + videoCoverKey := runtime.Str("video-cover") + audioKey := runtime.Str("audio") + if markdown != "" { msgType = "post" content, desc = wrapMarkdownAsPostForDryRun(markdown) @@ -100,10 +62,10 @@ var ImMessagesSend = common.TypedShortcut[*ImMessagesSendArgs]{ } receiveIdType := "chat_id" - receiveId := chatOrEmpty(args.Target.Chat) - if userID := userOrEmpty(args.Target.User); userID != "" { + receiveId := chatFlag + if userFlag != "" { receiveIdType = "open_id" - receiveId = userID + receiveId = userFlag } if msgType == "text" || msgType == "post" { @@ -124,25 +86,71 @@ var ImMessagesSend = common.TypedShortcut[*ImMessagesSendArgs]{ Params(map[string]interface{}{"receive_id_type": receiveIdType}). Body(body) }, + Validate: func(ctx context.Context, runtime *common.RuntimeContext) error { + chatFlag := runtime.Str("chat-id") + userFlag := runtime.Str("user-id") + msgType := runtime.Str("msg-type") + content := runtime.Str("content") + text := runtime.Str("text") + markdown := runtime.Str("markdown") + imageKey := runtime.Str("image") + fileKey := runtime.Str("file") + videoKey := runtime.Str("video") + videoCoverKey := runtime.Str("video-cover") + audioKey := runtime.Str("audio") - Execute: func(ctx context.Context, args *ImMessagesSendArgs, rt *common.RuntimeContext) error { - // Defensive: see DryRun's note about double-binding. - bindMessagesSendArgs(rt.Cmd, args) + fio := runtime.FileIO() + for _, mf := range []struct{ flag, val string }{ + {"--image", imageKey}, {"--file", fileKey}, {"--video", videoKey}, + {"--video-cover", videoCoverKey}, {"--audio", audioKey}, + } { + if err := validateMediaFlagPath(fio, mf.flag, mf.val); err != nil { + return err + } + } - text := strOrEmpty(args.Content.Text) - markdown := strOrEmpty(args.Content.Markdown) - imageVal := mediaOrEmpty(args.Content.Image) - fileVal := mediaOrEmpty(args.Content.File) - videoVal, videoCoverVal := videoOrEmpty(args.Content.Video) - audioVal := mediaOrEmpty(args.Content.Audio) - content, msgType := rawOrDefault(args.Content.Raw) - idempotencyKey := args.IdempotencyKey + if err := common.ExactlyOne(runtime, "chat-id", "user-id"); err != nil { + return err + } - // Pre-flight: reject unreadable local paths early. The MediaInput typed - // primitive only enforces cwd-relative safety (absolute path / `..` - // rejection) — the loose os.Stat check stays here because it touches - // the filesystem and is not appropriate at the format-validation layer. - fio := rt.FileIO() + // Validate ID formats + if chatFlag != "" { + if _, err := common.ValidateChatID(chatFlag); err != nil { + return err + } + } + if userFlag != "" { + if _, err := common.ValidateUserID(userFlag); err != nil { + return err + } + } + + if msg := validateContentFlags(text, markdown, content, imageKey, fileKey, videoKey, videoCoverKey, audioKey); msg != "" { + return common.FlagErrorf(msg) + } + if content != "" && !json.Valid([]byte(content)) { + return common.FlagErrorf("--content is not valid JSON: %s\nexample: --content '{\"text\":\"hello\"}' or --text 'hello'", content) + } + if msg := validateExplicitMsgType(runtime.Cmd, msgType, text, markdown, imageKey, fileKey, videoKey, audioKey); msg != "" { + return common.FlagErrorf(msg) + } + + return nil + }, + Execute: func(ctx context.Context, runtime *common.RuntimeContext) error { + chatFlag := runtime.Str("chat-id") + userFlag := runtime.Str("user-id") + msgType := runtime.Str("msg-type") + content := runtime.Str("content") + text := runtime.Str("text") + markdown := runtime.Str("markdown") + idempotencyKey := runtime.Str("idempotency-key") + imageVal := runtime.Str("image") + fileVal := runtime.Str("file") + videoVal := runtime.Str("video") + videoCoverVal := runtime.Str("video-cover") + audioVal := runtime.Str("audio") + fio := runtime.FileIO() for _, mf := range []struct{ flag, val string }{ {"--image", imageVal}, {"--file", fileVal}, {"--video", videoVal}, {"--video-cover", videoCoverVal}, {"--audio", audioVal}, @@ -151,21 +159,20 @@ var ImMessagesSend = common.TypedShortcut[*ImMessagesSendArgs]{ return err } } - - // Resolve content type. + // Resolve content type if markdown != "" { - msgType, content = "post", resolveMarkdownAsPost(ctx, rt, markdown) - } else if mt, c, err := resolveMediaContent(ctx, rt, text, imageVal, fileVal, videoVal, videoCoverVal, audioVal); err != nil { + msgType, content = "post", resolveMarkdownAsPost(ctx, runtime, markdown) + } else if mt, c, err := resolveMediaContent(ctx, runtime, text, imageVal, fileVal, videoVal, videoCoverVal, audioVal); err != nil { return err } else if mt != "" { msgType, content = mt, c } receiveIdType := "chat_id" - receiveId := chatOrEmpty(args.Target.Chat) - if userID := userOrEmpty(args.Target.User); userID != "" { + receiveId := chatFlag + if userFlag != "" { receiveIdType = "open_id" - receiveId = userID + receiveId = userFlag } normalizedContent := content @@ -182,13 +189,13 @@ var ImMessagesSend = common.TypedShortcut[*ImMessagesSendArgs]{ data["uuid"] = idempotencyKey } - resData, err := rt.DoAPIJSON(http.MethodPost, "/open-apis/im/v1/messages", + resData, err := runtime.DoAPIJSON(http.MethodPost, "/open-apis/im/v1/messages", larkcore.QueryParams{"receive_id_type": []string{receiveIdType}}, data) if err != nil { return err } - rt.Out(map[string]interface{}{ + runtime.Out(map[string]interface{}{ "message_id": resData["message_id"], "chat_id": resData["chat_id"], "create_time": common.FormatTimeWithSeconds(resData["create_time"]), @@ -197,195 +204,19 @@ var ImMessagesSend = common.TypedShortcut[*ImMessagesSendArgs]{ }, } -// bindMessagesSendArgs populates the OneOf sub-struct pointer fields from -// cobra flag state. The shared framework binder (binder.go) currently only -// binds top-level fields; sub-struct binding lives here so the typed closures -// can read `args.Target.Chat` / `args.Content.Text` etc. instead of falling -// back to `rt.Cmd.Flags().GetString`. Pointer is set iff cobra reports the -// flag was explicitly provided (matches the "nil = not set" OneOf contract). -func bindMessagesSendArgs(cmd *cobra.Command, args *ImMessagesSendArgs) { - if cmd == nil || args == nil { - return - } - flags := cmd.Flags() - - // Target bucket — exactly one of --chat-id / --user-id. - if flags.Changed("chat-id") { - v, _ := flags.GetString("chat-id") - id := argstype.ChatID(v) - args.Target.Chat = &id - } - if flags.Changed("user-id") { - v, _ := flags.GetString("user-id") - id := argstype.UserOpenID(v) - args.Target.User = &id - } - - // Content bucket — exactly one of --text / --markdown / --image / --file / - // --video (+ --video-cover) / --audio / --content. - if flags.Changed("text") { - v, _ := flags.GetString("text") - args.Content.Text = &v - } - if flags.Changed("markdown") { - v, _ := flags.GetString("markdown") - args.Content.Markdown = &v - } - if flags.Changed("image") { - v, _ := flags.GetString("image") - m := argstype.MediaInput(v) - args.Content.Image = &m - } - if flags.Changed("file") { - v, _ := flags.GetString("file") - m := argstype.MediaInput(v) - args.Content.File = &m - } - if flags.Changed("audio") { - v, _ := flags.GetString("audio") - m := argstype.MediaInput(v) - args.Content.Audio = &m - } - if flags.Changed("video") { - v, _ := flags.GetString("video") - cover, _ := flags.GetString("video-cover") - args.Content.Video = &VideoContent{ - File: argstype.MediaInput(v), - Cover: argstype.MediaInput(cover), - } - } - if flags.Changed("content") { - raw, _ := flags.GetString("content") - msgType, _ := flags.GetString("msg-type") - args.Content.Raw = &RawContent{JSON: raw, MsgType: msgType} - } - - // IdempotencyKey is a top-level string field already bound by the framework - // binder; this read keeps the path uniform for callers using a synthetic - // command (e.g. tests bypassing the framework Validate stage). - if flags.Changed("idempotency-key") && args.IdempotencyKey == "" { - args.IdempotencyKey, _ = flags.GetString("idempotency-key") - } +// isMediaKey returns true if the value looks like an existing API key rather than a local file path. +func isMediaKey(value string) bool { + return strings.HasPrefix(value, "img_") || strings.HasPrefix(value, "file_") } -// strOrEmpty deref-protects a *string field — nil → "". -func strOrEmpty(p *string) string { - if p == nil { - return "" - } - return *p -} - -// chatOrEmpty deref-protects an *argstype.ChatID — nil → "". -func chatOrEmpty(p *argstype.ChatID) string { - if p == nil { - return "" - } - return string(*p) -} - -// userOrEmpty deref-protects an *argstype.UserOpenID — nil → "". -func userOrEmpty(p *argstype.UserOpenID) string { - if p == nil { - return "" - } - return string(*p) -} - -// mediaOrEmpty deref-protects an *argstype.MediaInput — nil → "". -func mediaOrEmpty(p *argstype.MediaInput) string { - if p == nil { - return "" - } - return string(*p) -} - -// videoOrEmpty returns (file, cover) from an optional VideoContent. nil → ("", ""). -func videoOrEmpty(v *VideoContent) (string, string) { - if v == nil { - return "", "" - } - return string(v.File), string(v.Cover) -} - -// rawOrDefault returns (content, msgType) from an optional RawContent. nil → -// ("", "text"). The "text" default matches the RawContent.MsgType tag default -// and ensures the DryRun body still has a sensible msg_type when no raw -// content is supplied (it will be replaced when a media flag is selected). -func rawOrDefault(r *RawContent) (string, string) { - if r == nil { - return "", "text" - } - msgType := r.MsgType - if msgType == "" { - msgType = "text" - } - return r.JSON, msgType -} - -// validateVideoGroup enforces the VideoContent paired-flag rule: --video and -// --video-cover must be supplied together. Produces a structured envelope -// with subtype shortcut_group_incomplete and param "VideoContent". The shared -// binder only runs group-completeness against top-level Args fields, so we -// run the check manually here. -func validateVideoGroup(cmd *cobra.Command) error { - if cmd == nil { +// validateMediaFlagPath validates a media flag value as a local file path via FileIO. +// Empty values, URLs, and media keys are skipped (not local files). +func validateMediaFlagPath(fio fileio.FileIO, flagName, value string) error { + if value == "" || strings.HasPrefix(value, "http://") || strings.HasPrefix(value, "https://") || isMediaKey(value) { return nil } - videoSet := cmd.Flags().Changed("video") - coverSet := cmd.Flags().Changed("video-cover") - if videoSet == coverSet { - return nil - } - missing := "--video-cover" - if !videoSet { - missing = "--video" - } - return &errs.ValidationError{ - Problem: errs.Problem{ - Category: errs.CategoryValidation, - Subtype: errs.SubtypeShortcutGroupIncomplete, - Message: "VideoContent requires " + missing, - Hint: "--video and --video-cover must be supplied together", - }, - Param: "VideoContent", - } -} - -// validateMsgTypeInterplay rejects an explicit --msg-type that conflicts with -// the message type inferred from --text / --markdown / --image / --file / -// --video / --audio. Preserved verbatim from the legacy Validate closure -// because RawContent's enum check accepts any of the listed types — the -// framework can't infer the writer's intent from the other content flags. -func validateMsgTypeInterplay(cmd *cobra.Command, args *ImMessagesSendArgs) error { - if cmd == nil || !cmd.Flags().Changed("msg-type") { - return nil - } - msgType, _ := cmd.Flags().GetString("msg-type") - var inferred string - switch { - case args.Content.Text != nil: - inferred = "text" - case args.Content.Markdown != nil: - inferred = "post" - case args.Content.Image != nil: - inferred = "image" - case args.Content.File != nil: - inferred = "file" - case args.Content.Video != nil: - inferred = "media" - case args.Content.Audio != nil: - inferred = "audio" - } - if inferred == "" || msgType == inferred { - return nil - } - return &errs.ValidationError{ - Problem: errs.Problem{ - Category: errs.CategoryValidation, - Subtype: errs.SubtypeInvalidArgument, - Message: "--msg-type \"" + msgType + "\" conflicts with the inferred message type \"" + inferred + "\" from the selected content flag", - }, - Param: "msg-type", + if _, err := fio.Stat(value); err != nil && !os.IsNotExist(err) { + return output.ErrValidation("%s: %v", flagName, err) } + return nil } diff --git a/shortcuts/im/im_messages_send_test.go b/shortcuts/im/im_messages_send_test.go deleted file mode 100644 index f9dce4683..000000000 --- a/shortcuts/im/im_messages_send_test.go +++ /dev/null @@ -1,382 +0,0 @@ -// Copyright (c) 2026 Lark Technologies Pte. Ltd. -// SPDX-License-Identifier: MIT - -package im - -import ( - "context" - "errors" - "testing" - - "github.com/spf13/cobra" - - "github.com/larksuite/cli/errs" - "github.com/larksuite/cli/internal/cmdutil" - "github.com/larksuite/cli/shortcuts/common" -) - -// TestImMessagesSend_IsMountable is the compile-time gate that locks the pilot -// shortcut onto the TypedShortcut → Mountable contract. A breaking change to -// the framework interface set must be reflected here. -func TestImMessagesSend_IsMountable(t *testing.T) { - var _ common.Mountable = ImMessagesSend - var _ common.ShortcutDescriptor = ImMessagesSend -} - -// TestImMessagesSend_Metadata pins the descriptor surface that auth-login / -// scope-hint / shortcuts.json generators read. Drift here is silent in the -// migration diff because the legacy struct disappears. -func TestImMessagesSend_Metadata(t *testing.T) { - t.Helper() - if got := ImMessagesSend.GetService(); got != "im" { - t.Errorf("GetService = %q, want \"im\"", got) - } - if got := ImMessagesSend.GetCommand(); got != "+messages-send" { - t.Errorf("GetCommand = %q, want \"+messages-send\"", got) - } - if got := ImMessagesSend.GetRisk(); got != "write" { - t.Errorf("GetRisk = %q, want \"write\"", got) - } - wantAuth := map[string]bool{"bot": true, "user": true} - gotAuth := ImMessagesSend.GetAuthTypes() - if len(gotAuth) != len(wantAuth) { - t.Errorf("GetAuthTypes = %v, want set %v", gotAuth, wantAuth) - } - gotSet := make(map[string]bool, len(gotAuth)) - for _, a := range gotAuth { - if gotSet[a] { - t.Errorf("duplicate auth type %q in %v", a, gotAuth) - continue - } - gotSet[a] = true - if !wantAuth[a] { - t.Errorf("unexpected auth type %q in %v", a, gotAuth) - } - } - for a := range wantAuth { - if !gotSet[a] { - t.Errorf("missing auth type %q in %v", a, gotAuth) - } - } -} - -// TestImMessagesSend_ScopesForIdentity verifies user / bot variants resolve -// to the right scope sets. Login flows depend on this routing. -func TestImMessagesSend_ScopesForIdentity(t *testing.T) { - tests := []struct { - identity string - want []string - }{ - {"user", []string{"im:message.send_as_user", "im:message"}}, - {"bot", []string{"im:message:send_as_bot"}}, - {"", []string{"im:message:send_as_bot"}}, // fallback to Scopes - } - for _, tc := range tests { - got := ImMessagesSend.ScopesForIdentity(tc.identity) - if len(got) != len(tc.want) { - t.Errorf("ScopesForIdentity(%q) = %v, want %v", tc.identity, got, tc.want) - continue - } - for i, s := range got { - if s != tc.want[i] { - t.Errorf("ScopesForIdentity(%q)[%d] = %q, want %q", tc.identity, i, s, tc.want[i]) - } - } - } -} - -// TestImMessagesSend_MountRegistersFlags exercises the full Mount path to -// confirm every flag the legacy shortcut accepted is still registered on the -// generated cobra subcommand. Full Execute / Validate integration is in -// tests_e2e/shortcuts/ (runShortcut needs a real Factory). -func TestImMessagesSend_MountRegistersFlags(t *testing.T) { - root := &cobra.Command{Use: "root"} - imParent := &cobra.Command{Use: "im"} - root.AddCommand(imParent) - ImMessagesSend.MountWithContext(context.Background(), imParent, &cmdutil.Factory{}) - - sub, _, err := imParent.Find([]string{"+messages-send"}) - if err != nil { - t.Fatalf("find +messages-send: %v", err) - } - if sub == nil { - t.Fatal("expected +messages-send subcommand registered") - } - - wantFlags := []string{ - "chat-id", "user-id", - "text", "markdown", "image", "file", "video", "video-cover", "audio", - "content", "msg-type", - "idempotency-key", - } - for _, name := range wantFlags { - if sub.Flag(name) == nil { - t.Errorf("expected --%s registered on +messages-send", name) - } - } -} - -// TestImMessagesSend_HelpFuncInstalled verifies the typed help renderer was -// wired by the Mount adapter (covers Examples section + Risk/Tips passthrough -// indirectly — full render content is asserted in typed_help_test.go). -func TestImMessagesSend_HelpFuncInstalled(t *testing.T) { - root := &cobra.Command{Use: "root"} - imParent := &cobra.Command{Use: "im"} - root.AddCommand(imParent) - ImMessagesSend.MountWithContext(context.Background(), imParent, &cmdutil.Factory{}) - - sub, _, _ := imParent.Find([]string{"+messages-send"}) - if sub == nil || sub.HelpFunc() == nil { - t.Fatal("expected typed help func installed on +messages-send") - } -} - -// TestValidateVideoGroup covers the manual paired-flag check that compensates -// for the framework binder not recursing groups inside OneOf buckets. -// --video without --video-cover (and vice versa) must produce a structured -// envelope with subtype shortcut_group_incomplete. -func TestValidateVideoGroup(t *testing.T) { - tests := []struct { - name string - args []string - wantErr bool - wantSub errs.Subtype - wantPara string - }{ - {"both unset → ok", nil, false, "", ""}, - {"both set → ok", []string{"--video=v.mp4", "--video-cover=c.png"}, false, "", ""}, - {"video without cover", []string{"--video=v.mp4"}, true, errs.SubtypeShortcutGroupIncomplete, "VideoContent"}, - {"cover without video", []string{"--video-cover=c.png"}, true, errs.SubtypeShortcutGroupIncomplete, "VideoContent"}, - } - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - cmd := &cobra.Command{Use: "send"} - cmd.Flags().String("video", "", "") - cmd.Flags().String("video-cover", "", "") - if err := cmd.ParseFlags(tc.args); err != nil { - t.Fatalf("ParseFlags: %v", err) - } - err := validateVideoGroup(cmd) - if (err != nil) != tc.wantErr { - t.Fatalf("err = %v, wantErr = %v", err, tc.wantErr) - } - if !tc.wantErr { - return - } - var ve *errs.ValidationError - if !errors.As(err, &ve) { - t.Fatalf("expected *errs.ValidationError, got %T (%v)", err, err) - } - if ve.Subtype != tc.wantSub { - t.Errorf("Subtype = %q, want %q", ve.Subtype, tc.wantSub) - } - if ve.Param != tc.wantPara { - t.Errorf("Param = %q, want %q", ve.Param, tc.wantPara) - } - }) - } -} - -// TestValidateMsgTypeInterplay covers the legacy guard that rejects an -// explicit --msg-type that contradicts the inferred type. The Content bucket -// pointer fields drive the inference (matching the typed-args read path). -func TestValidateMsgTypeInterplay(t *testing.T) { - mkArgs := func() *ImMessagesSendArgs { return &ImMessagesSendArgs{} } - tests := []struct { - name string - setup func(*ImMessagesSendArgs) - msgType string - changed bool - wantErr bool - wantParam string - }{ - {"msg-type unchanged → ok", func(a *ImMessagesSendArgs) { s := "hi"; a.Content.Text = &s }, "text", false, false, ""}, - {"text + text → ok", func(a *ImMessagesSendArgs) { s := "hi"; a.Content.Text = &s }, "text", true, false, ""}, - {"text + image → conflict", func(a *ImMessagesSendArgs) { s := "hi"; a.Content.Text = &s }, "image", true, true, "msg-type"}, - {"markdown + image → conflict", func(a *ImMessagesSendArgs) { s := "**hi**"; a.Content.Markdown = &s }, "image", true, true, "msg-type"}, - {"no content selected → ok", func(a *ImMessagesSendArgs) {}, "image", true, false, ""}, - } - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - cmd := &cobra.Command{Use: "send"} - cmd.Flags().String("msg-type", "text", "") - args := []string{} - if tc.changed { - args = append(args, "--msg-type="+tc.msgType) - } - if err := cmd.ParseFlags(args); err != nil { - t.Fatalf("ParseFlags: %v", err) - } - a := mkArgs() - tc.setup(a) - err := validateMsgTypeInterplay(cmd, a) - if (err != nil) != tc.wantErr { - t.Fatalf("err = %v, wantErr = %v", err, tc.wantErr) - } - if !tc.wantErr { - return - } - var ve *errs.ValidationError - if !errors.As(err, &ve) { - t.Fatalf("expected *errs.ValidationError, got %T", err) - } - if ve.Subtype != errs.SubtypeInvalidArgument { - t.Errorf("Subtype = %q, want invalid_argument", ve.Subtype) - } - if ve.Param != tc.wantParam { - t.Errorf("Param = %q, want %q", ve.Param, tc.wantParam) - } - }) - } -} - -// TestBindMessagesSendArgs covers the local sub-struct binder that fills the -// Content / Target OneOf buckets from cobra flag state. Pointer is non-nil iff -// the flag was explicitly Changed — matches the "nil = unset" OneOf contract. -func TestBindMessagesSendArgs(t *testing.T) { - cmd := &cobra.Command{Use: "send"} - cmd.Flags().String("chat-id", "", "") - cmd.Flags().String("user-id", "", "") - cmd.Flags().String("text", "", "") - cmd.Flags().String("markdown", "", "") - cmd.Flags().String("image", "", "") - cmd.Flags().String("file", "", "") - cmd.Flags().String("video", "", "") - cmd.Flags().String("video-cover", "", "") - cmd.Flags().String("audio", "", "") - cmd.Flags().String("content", "", "") - cmd.Flags().String("msg-type", "text", "") - cmd.Flags().String("idempotency-key", "", "") - - if err := cmd.ParseFlags([]string{ - "--chat-id=oc_abc", - "--text=hi", - }); err != nil { - t.Fatalf("ParseFlags: %v", err) - } - - args := &ImMessagesSendArgs{} - bindMessagesSendArgs(cmd, args) - - if args.Target.Chat == nil || string(*args.Target.Chat) != "oc_abc" { - t.Errorf("Target.Chat = %v, want non-nil oc_abc", args.Target.Chat) - } - if args.Target.User != nil { - t.Errorf("Target.User = %v, want nil (flag unset)", args.Target.User) - } - if args.Content.Text == nil || *args.Content.Text != "hi" { - t.Errorf("Content.Text = %v, want non-nil \"hi\"", args.Content.Text) - } - if args.Content.Markdown != nil { - t.Errorf("Content.Markdown = %v, want nil", args.Content.Markdown) - } - if args.Content.Video != nil { - t.Errorf("Content.Video = %v, want nil", args.Content.Video) - } - if args.Content.Raw != nil { - t.Errorf("Content.Raw = %v, want nil", args.Content.Raw) - } -} - -// TestBindMessagesSendArgs_VideoPair confirms the paired --video / --video-cover -// flags collapse into one VideoContent struct (the framework would otherwise -// see two unrelated triggers). -func TestBindMessagesSendArgs_VideoPair(t *testing.T) { - cmd := &cobra.Command{Use: "send"} - cmd.Flags().String("chat-id", "", "") - cmd.Flags().String("user-id", "", "") - cmd.Flags().String("text", "", "") - cmd.Flags().String("markdown", "", "") - cmd.Flags().String("image", "", "") - cmd.Flags().String("file", "", "") - cmd.Flags().String("video", "", "") - cmd.Flags().String("video-cover", "", "") - cmd.Flags().String("audio", "", "") - cmd.Flags().String("content", "", "") - cmd.Flags().String("msg-type", "text", "") - cmd.Flags().String("idempotency-key", "", "") - - if err := cmd.ParseFlags([]string{ - "--chat-id=oc_x", - "--video=v.mp4", - "--video-cover=c.png", - }); err != nil { - t.Fatalf("ParseFlags: %v", err) - } - args := &ImMessagesSendArgs{} - bindMessagesSendArgs(cmd, args) - if args.Content.Video == nil { - t.Fatal("Content.Video = nil, want non-nil") - } - if string(args.Content.Video.File) != "v.mp4" { - t.Errorf("Video.File = %q, want \"v.mp4\"", args.Content.Video.File) - } - if string(args.Content.Video.Cover) != "c.png" { - t.Errorf("Video.Cover = %q, want \"c.png\"", args.Content.Video.Cover) - } -} - -// TestBindMessagesSendArgs_Raw covers the --content + --msg-type pair that -// rolls into a RawContent variant. -func TestBindMessagesSendArgs_Raw(t *testing.T) { - cmd := &cobra.Command{Use: "send"} - cmd.Flags().String("chat-id", "", "") - cmd.Flags().String("user-id", "", "") - cmd.Flags().String("text", "", "") - cmd.Flags().String("markdown", "", "") - cmd.Flags().String("image", "", "") - cmd.Flags().String("file", "", "") - cmd.Flags().String("video", "", "") - cmd.Flags().String("video-cover", "", "") - cmd.Flags().String("audio", "", "") - cmd.Flags().String("content", "", "") - cmd.Flags().String("msg-type", "text", "") - cmd.Flags().String("idempotency-key", "", "") - - if err := cmd.ParseFlags([]string{ - "--chat-id=oc_x", - "--content={\"text\":\"hello\"}", - "--msg-type=text", - }); err != nil { - t.Fatalf("ParseFlags: %v", err) - } - args := &ImMessagesSendArgs{} - bindMessagesSendArgs(cmd, args) - if args.Content.Raw == nil { - t.Fatal("Content.Raw = nil, want non-nil") - } - if args.Content.Raw.JSON != `{"text":"hello"}` { - t.Errorf("Raw.JSON = %q, want \"{...}\"", args.Content.Raw.JSON) - } - if args.Content.Raw.MsgType != "text" { - t.Errorf("Raw.MsgType = %q, want \"text\"", args.Content.Raw.MsgType) - } -} - -// TestImMessagesSend_TypedShortcutsRegistered confirms the package-level -// TypedShortcuts() slice exposes the pilot so register.go can mount it. A -// regression here would silently strip --messages-send from the CLI. -func TestImMessagesSend_TypedShortcutsRegistered(t *testing.T) { - list := TypedShortcuts() - found := false - for _, m := range list { - if m.GetService() == "im" && m.GetCommand() == "+messages-send" { - found = true - break - } - } - if !found { - t.Error("im.TypedShortcuts() must contain ImMessagesSend (service=im, command=+messages-send)") - } -} - -// TestImMessagesSend_NotInLegacyShortcuts is the dual of the above: a shortcut -// MUST live in exactly one of Shortcuts() / TypedShortcuts(), otherwise -// register.go will mount the same cobra subcommand twice. -func TestImMessagesSend_NotInLegacyShortcuts(t *testing.T) { - for _, sc := range Shortcuts() { - if sc.Service == "im" && sc.Command == "+messages-send" { - t.Fatalf("ImMessagesSend appears in legacy Shortcuts() — must only be in TypedShortcuts() after migration") - } - } -} diff --git a/shortcuts/im/protocol.go b/shortcuts/im/protocol.go deleted file mode 100644 index b317625dc..000000000 --- a/shortcuts/im/protocol.go +++ /dev/null @@ -1,63 +0,0 @@ -// Copyright (c) 2026 Lark Technologies Pte. Ltd. -// SPDX-License-Identifier: MIT - -package im - -import ( - "encoding/json" - - "github.com/larksuite/cli/errs" - "github.com/larksuite/cli/shortcuts/common" - "github.com/larksuite/cli/shortcuts/common/argstype" -) - -// MessageTarget — exactly one of Chat or User identifies the recipient. -type MessageTarget struct { - Chat *argstype.ChatID `flag:"chat-id" desc:"chat ID (oc_xxx)"` - User *argstype.UserOpenID `flag:"user-id" desc:"user open_id (ou_xxx)"` -} - -func (MessageTarget) OneOf() {} - -// MessageContent — exactly one of the seven content variants is sent. -type MessageContent struct { - Text *string `flag:"text" desc:"plain text message"` - Markdown *string `flag:"markdown" desc:"markdown text"` - Image *argstype.MediaInput `flag:"image" desc:"image local file path / URL / img_xxx key"` - File *argstype.MediaInput `flag:"file" desc:"file local file path / URL / file_xxx key"` - Video *VideoContent - Audio *argstype.MediaInput `flag:"audio" desc:"audio file"` - Raw *RawContent -} - -func (MessageContent) OneOf() {} - -// VideoContent — video file requires an accompanying cover image. -type VideoContent struct { - File argstype.MediaInput `flag:"video" oneof_trigger:"true" desc:"video file path / URL / file_xxx key"` - Cover argstype.MediaInput `flag:"video-cover" desc:"video cover image; required with --video"` -} - -// RawContent — raw JSON body with explicit msg-type, selected by --content. -type RawContent struct { - JSON string `flag:"content" oneof_trigger:"true" desc:"raw message content JSON"` - MsgType string `flag:"msg-type" default:"text" enum:"text,post,image,file,audio,media,interactive,share_chat,share_user" desc:"message type for --content JSON (default: text)"` -} - -func (r *RawContent) ValidateValue(_ *common.RuntimeContext, _ string) error { - if r == nil || r.JSON == "" { - return nil - } - if !json.Valid([]byte(r.JSON)) { - return &errs.ValidationError{ - Problem: errs.Problem{ - Category: errs.CategoryValidation, - Subtype: errs.SubtypeInvalidArgument, - Message: "--content is not valid JSON", - Hint: `pass a JSON string such as {"text":"hello"}`, - }, - Param: "--content", - } - } - return nil -} diff --git a/shortcuts/im/protocol_test.go b/shortcuts/im/protocol_test.go deleted file mode 100644 index 0c175d41e..000000000 --- a/shortcuts/im/protocol_test.go +++ /dev/null @@ -1,33 +0,0 @@ -// Copyright (c) 2026 Lark Technologies Pte. Ltd. -// SPDX-License-Identifier: MIT - -package im - -import "testing" - -func TestMessageTarget_SatisfiesOneOfMarker(t *testing.T) { - var v interface{} = MessageTarget{} - if _, ok := v.(interface{ OneOf() }); !ok { - t.Fatal("MessageTarget must satisfy OneOfMarker") - } -} - -func TestMessageContent_SatisfiesOneOfMarker(t *testing.T) { - var v interface{} = MessageContent{} - if _, ok := v.(interface{ OneOf() }); !ok { - t.Fatal("MessageContent must satisfy OneOfMarker") - } -} - -func TestVideoContent_FieldsExist(t *testing.T) { - v := VideoContent{} - _ = v.File - _ = v.Cover -} - -func TestRawContent_InvalidJSONFails(t *testing.T) { - r := &RawContent{JSON: "{ broken"} - if err := r.ValidateValue(nil, "content"); err == nil { - t.Fatal("expected validation error for malformed JSON") - } -} diff --git a/shortcuts/im/shortcuts.go b/shortcuts/im/shortcuts.go index 625f94340..3c8aadfbe 100644 --- a/shortcuts/im/shortcuts.go +++ b/shortcuts/im/shortcuts.go @@ -5,8 +5,7 @@ package im import "github.com/larksuite/cli/shortcuts/common" -// Shortcuts returns all legacy im shortcuts. ImMessagesSend has been migrated -// to the typed framework — see TypedShortcuts below. +// Shortcuts returns all im shortcuts. func Shortcuts() []common.Shortcut { return []common.Shortcut{ ImChatCreate, @@ -18,23 +17,10 @@ func Shortcuts() []common.Shortcut { ImMessagesReply, ImMessagesResourcesDownload, ImMessagesSearch, + ImMessagesSend, ImThreadsMessagesList, ImFlagCreate, ImFlagCancel, ImFlagList, } } - -// TypedShortcuts returns the im shortcuts that have migrated to the new -// common.TypedShortcut[T] framework. Returned as []common.Mountable so the -// top-level shortcuts/register.go can mount them through the same pipeline -// it uses for legacy shortcuts. -// -// IMPORTANT: a shortcut MUST appear in exactly one of Shortcuts() / -// TypedShortcuts() — duplicating it across both slices would double-mount -// the cobra subcommand. -func TypedShortcuts() []common.Mountable { - return []common.Mountable{ - ImMessagesSend, - } -} diff --git a/shortcuts/im/validate_media_test.go b/shortcuts/im/validate_media_test.go index 7e06efbbf..94ec17f9b 100644 --- a/shortcuts/im/validate_media_test.go +++ b/shortcuts/im/validate_media_test.go @@ -52,18 +52,12 @@ func TestValidateMediaFlagPath(t *testing.T) { } } -// TestIMMediaFlagDescriptionsDocumentPathRestrictions asserts the legacy -// shortcuts (still on common.Shortcut) keep the path-restriction language in -// their --image/--file/--video/--video-cover/--audio descriptions. The -// migrated ImMessagesSend now sources its flag help from argstype.MediaInput -// tags in shortcuts/im/protocol.go, and the absolute-path / `..` rejection is -// enforced by argstype.MediaInput.ValidateValue (covered by -// shortcuts/common/argstype/media_input_test.go and safe_path_test.go). func TestIMMediaFlagDescriptionsDocumentPathRestrictions(t *testing.T) { shortcuts := []struct { name string flags []common.Flag }{ + {name: "messages-send", flags: ImMessagesSend.Flags}, {name: "messages-reply", flags: ImMessagesReply.Flags}, } mediaFlags := []string{"image", "file", "video", "video-cover", "audio"} diff --git a/shortcuts/register.go b/shortcuts/register.go index 9ac64bd55..47fe0fd5a 100644 --- a/shortcuts/register.go +++ b/shortcuts/register.go @@ -59,6 +59,10 @@ func IsShortcutServiceAvailable(service string, brand core.LarkBrand) bool { // it. The slice element type is ShortcutDescriptor so read-only consumers // (auth login, scope hint, shortcuts.json generator) can read metadata without // caring about which concrete implementation backs each entry. +// +// Only legacy shortcuts are registered today; the typed track stays wired +// (ShortcutDescriptor / Mountable dispatch + buildTypedHelp) but currently has +// no domain caller. var allShortcuts []common.ShortcutDescriptor // addLegacy boxes a legacy []common.Shortcut into the descriptor slice. We use @@ -71,21 +75,12 @@ func addLegacy(list []common.Shortcut) { } } -// addTyped boxes a []common.Mountable produced by domain TypedShortcuts() into -// the descriptor slice. TypedShortcut[T] satisfies Mountable directly. -func addTyped(list []common.Mountable) { - for _, m := range list { - allShortcuts = append(allShortcuts, m) - } -} - func init() { addLegacy(apps.Shortcuts()) addLegacy(calendar.Shortcuts()) addLegacy(doc.Shortcuts()) addLegacy(drive.Shortcuts()) addLegacy(im.Shortcuts()) - addTyped(im.TypedShortcuts()) addLegacy(contact_shortcuts.Shortcuts()) addLegacy(sheets.Shortcuts()) addLegacy(base.Shortcuts())