From d41214988ad2151017c263dbe9d8ebbfc085435d Mon Sep 17 00:00:00 2001 From: Dennis Date: Wed, 22 Jul 2026 14:07:05 +0800 Subject: [PATCH] fix(shortcut): keep external contacts in name resolution; alias resource-url msg-id MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two independent shortcut correctness fixes surfaced by the audit: - Name→ID resolution (chat +dm / +broadcast / … via the shared resolver) dropped every search_contact_by_key_word row with an empty userId. External / cross-org contacts arrive with only an openDingTalkId, so they were silently discarded — making resolution report a real person as missing, or collapse to the wrong single match when an in-org namesake existed. Keep any row with at least one usable identity (userId or openDingTalkId) and fall the display name back through nick/showName/flowerName/staffName/userName. - chat +messages-resource-url required --message-id with no alias, so an agent copying the message list's openMessageId/msgId output field hit "unknown flag". Accept --msg-id / --open-message-id as aliases (declared via an at-least-one constraint since a shortcut's Required check only sees the primary flag name), mirroring the earlier chat message download-media fix. Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 5 ++ internal/shortcut/chat/chat_message.go | 13 +++- .../chat/compatibility_coverage_test.go | 30 +++++++++ internal/shortcut/smart/broadcast.go | 2 +- .../smart/compatibility_coverage_test.go | 30 ++++++++- internal/shortcut/smart/dm.go | 2 +- internal/shortcut/smart/resolve.go | 53 +++++++++++++-- internal/shortcut/smart/resolve_test.go | 67 +++++++++++++++++++ internal/shortcut/smart/share_doc.go | 2 +- 9 files changed, 191 insertions(+), 13 deletions(-) create mode 100644 internal/shortcut/smart/resolve_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index f40617df..8f51e827 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,11 @@ The format is inspired by [Keep a Changelog](https://keepachangelog.com/) and th ## [Unreleased] +### Fixed + +- **Name→ID resolution kept external contacts** — the shared contact resolver (`chat +dm`, `+broadcast`, …) no longer drops `search_contact_by_key_word` rows that carry only an `openDingTalkId` (external / cross-org contacts have an empty `userId`), so those people are found instead of reported missing or collapsed into a wrong single match; the display name also falls back through `nick`/`showName`/`flowerName`/`staffName`/`userName`. +- **`chat +messages-resource-url` flag aliases** — the media download-URL shortcut now accepts `--msg-id` / `--open-message-id` as aliases for `--message-id` (matching the `openMessageId`/`msgId` output field), so agents chaining from a message list no longer hit "unknown flag". + ## [1.0.55-beta.5] - 2026-07-28 This beta validates expanded personal event consumption, complete Agent-visible diff --git a/internal/shortcut/chat/chat_message.go b/internal/shortcut/chat/chat_message.go index 96dbe1be..af9199dc 100644 --- a/internal/shortcut/chat/chat_message.go +++ b/internal/shortcut/chat/chat_message.go @@ -730,15 +730,24 @@ var MessagesResourceURL = shortcut.Shortcut{ Flags: []shortcut.Flag{ {Name: "type", Type: shortcut.FlagString, Default: "mediaId", Desc: "资源类型", Enum: []string{"mediaId"}}, {Name: "resource-id", Type: shortcut.FlagString, Desc: "资源 ID（消息中的 mediaId）", Required: true}, - {Name: "message-id", Type: shortcut.FlagString, Desc: "消息 openMessageId", Required: true}, + {Name: "message-id", Type: shortcut.FlagString, Desc: "消息 openMessageId"}, + {Name: "msg-id", Type: shortcut.FlagString, Desc: "--message-id 的别名", Hidden: true}, + {Name: "open-message-id", Type: shortcut.FlagString, Desc: "--message-id 的别名", Hidden: true}, {Name: "open-conversation-id", Type: shortcut.FlagString, Desc: "会话 openConversationId", Required: true}, }, + // message-id is required, but accept the natural aliases agents reach for + // (the message-list output field is openMessageId/msgId). Declared via a + // constraint rather than Required because a shortcut's Required check only + // looks at the primary flag name, so a hidden alias could not satisfy it. + Constraints: []shortcut.Constraint{ + {Kind: shortcut.ConstraintAtLeastOne, Flags: []string{"message-id", "msg-id", "open-message-id"}}, + }, Tips: []string{`dws chat +messages-resource-url --type mediaId --resource-id --message-id --open-conversation-id `}, Execute: func(rt *shortcut.RuntimeContext) error { return rt.CallMCP("get_resource_download_url", map[string]any{ "resourceType": rt.Str("type"), "resourceId": rt.Str("resource-id"), - "openMessageId": rt.Str("message-id"), + "openMessageId": rt.StrFirst("message-id", "msg-id", "open-message-id"), "openConversationId": rt.Str("open-conversation-id"), }) }, diff --git a/internal/shortcut/chat/compatibility_coverage_test.go b/internal/shortcut/chat/compatibility_coverage_test.go index c9becedb..3e4f903f 100644 --- a/internal/shortcut/chat/compatibility_coverage_test.go +++ b/internal/shortcut/chat/compatibility_coverage_test.go @@ -103,6 +103,36 @@ func TestCrossPlatformCoverageCompatibilityAliases(t *testing.T) { wantTool: "query_msg_read_status", wantArgs: map[string]any{"openConversationId": "cid-1"}, }, + { + name: "message resource url msg id alias", + argv: []string{ + "chat", "+messages-resource-url", "--resource-id", "resource-1", + "--msg-id", "msg-1", "--open-conversation-id", "cid-1", + }, + wantProduct: "im", + wantTool: "get_resource_download_url", + wantArgs: map[string]any{ + "resourceType": "mediaId", + "resourceId": "resource-1", + "openMessageId": "msg-1", + "openConversationId": "cid-1", + }, + }, + { + name: "message resource url open message id alias", + argv: []string{ + "chat", "+messages-resource-url", "--resource-id", "resource-1", + "--open-message-id", "msg-1", "--open-conversation-id", "cid-1", + }, + wantProduct: "im", + wantTool: "get_resource_download_url", + wantArgs: map[string]any{ + "resourceType": "mediaId", + "resourceId": "resource-1", + "openMessageId": "msg-1", + "openConversationId": "cid-1", + }, + }, } for _, tc := range tests { diff --git a/internal/shortcut/smart/broadcast.go b/internal/shortcut/smart/broadcast.go index 7b7ceca1..e912602a 100644 --- a/internal/shortcut/smart/broadcast.go +++ b/internal/shortcut/smart/broadcast.go @@ -67,7 +67,7 @@ var Broadcast = shortcut.Shortcut{ // Step 1 — resolve this name to a unique userId. On failure // (unknown / ambiguous) record it and keep going. - user, err := resolveUser(rt, name) + user, err := resolveOpenDingTalkUser(rt, name) if err != nil { failed = append(failed, fmt.Sprintf("%s（%s）", name, err.Error())) continue diff --git a/internal/shortcut/smart/compatibility_coverage_test.go b/internal/shortcut/smart/compatibility_coverage_test.go index d0b50ff9..8f2a0d1b 100644 --- a/internal/shortcut/smart/compatibility_coverage_test.go +++ b/internal/shortcut/smart/compatibility_coverage_test.go @@ -16,6 +16,7 @@ package smart import ( "context" "io" + "strings" "testing" "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/helpers" @@ -31,7 +32,8 @@ type platformCoverageCall struct { } type platformCoverageCaller struct { - calls []platformCoverageCall + calls []platformCoverageCall + contactSearchResult string } func (f *platformCoverageCaller) CallTool(_ context.Context, product, tool string, args map[string]any) (*edition.ToolResult, error) { @@ -39,7 +41,10 @@ func (f *platformCoverageCaller) CallTool(_ context.Context, product, tool strin text := `{"result":[]}` switch product + "/" + tool { case "contact/search_contact_by_key_word": - text = `{"result":[{"userId":"u1","name":"张三","openDingTalkId":"open1"}]}` + text = f.contactSearchResult + if text == "" { + text = `{"result":[{"userId":"u1","name":"张三","openDingTalkId":"open1"}]}` + } case "contact/get_current_user_profile": text = `{"result":{"userId":"u1"}}` case "im/search_groups": @@ -48,6 +53,27 @@ func (f *platformCoverageCaller) CallTool(_ context.Context, product, tool strin return &edition.ToolResult{Content: []edition.ContentBlock{{Type: "text", Text: text}}}, nil } +func TestCrossPlatformCoverageExternalContactAmbiguity(t *testing.T) { + fake := &platformCoverageCaller{ + contactSearchResult: `{"result":[ + {"userId":"u1","name":"张三","openDingTalkId":"open1"}, + {"openDingtalkId":"open-external","nick":"外部张三"} + ]}`, + } + helpers.InitDeps(fake) + root := newPlatformCoverageRoot() + root.SetArgs([]string{"chat", "+dm", "--to", "张三", "--text", "你好", "--yes"}) + err := root.Execute() + if err == nil { + t.Fatal("ambiguous internal and external contacts unexpectedly resolved") + } + for _, want := range []string{"张三(u1)", "外部张三(open-external)"} { + if !strings.Contains(err.Error(), want) { + t.Errorf("ambiguity error %q does not contain %q", err, want) + } + } +} + func (f *platformCoverageCaller) Format() string { return "json" } func (f *platformCoverageCaller) DryRun() bool { return false } func (f *platformCoverageCaller) Fields() string { return "" } diff --git a/internal/shortcut/smart/dm.go b/internal/shortcut/smart/dm.go index 7c40493c..f456afdd 100644 --- a/internal/shortcut/smart/dm.go +++ b/internal/shortcut/smart/dm.go @@ -49,7 +49,7 @@ var DM = shortcut.Shortcut{ text := rt.Str("text") // Step 1 — resolve the recipient name to a unique userId. - user, err := resolveUser(rt, rt.Str("to")) + user, err := resolveOpenDingTalkUser(rt, rt.Str("to")) if err != nil { return err } diff --git a/internal/shortcut/smart/resolve.go b/internal/shortcut/smart/resolve.go index 2db48b6c..f688134b 100644 --- a/internal/shortcut/smart/resolve.go +++ b/internal/shortcut/smart/resolve.go @@ -35,6 +35,17 @@ type contactUser struct { // - errors with a clear message if nobody matches; // - errors listing the candidates if the name is ambiguous (never guesses). func resolveUser(rt *shortcut.RuntimeContext, name string) (contactUser, error) { + return resolveUserByName(rt, name, false) +} + +// resolveOpenDingTalkUser also accepts external / cross-org contacts that have +// an openDingTalkId but no organization-scoped userId. Use it only for flows +// whose downstream interface consumes openDingTalkId. +func resolveOpenDingTalkUser(rt *shortcut.RuntimeContext, name string) (contactUser, error) { + return resolveUserByName(rt, name, true) +} + +func resolveUserByName(rt *shortcut.RuntimeContext, name string, includeOpenIDOnly bool) (contactUser, error) { data, err := rt.CallMCPData("contact", "search_contact_by_key_word", map[string]any{ "keyword": name, }) @@ -42,18 +53,31 @@ func resolveUser(rt *shortcut.RuntimeContext, name string) (contactUser, error) return contactUser{}, err } users := extractUsers(data) + if !includeOpenIDOnly { + users = usersWithUserID(users) + } switch { case len(users) == 0: return contactUser{}, apperrors.NewValidation( fmt.Sprintf("通讯录里没找到叫 %q 的人；换个更完整的姓名再试。", name)) case len(users) > 1: return contactUser{}, apperrors.NewValidation(fmt.Sprintf( - "%q 匹配到 %d 个人：%s。请用更精确的姓名，或直接用对应命令传 userId。", + "%q 匹配到 %d 个人：%s。请用更精确的姓名，或改用对应命令直接传用户 ID。", name, len(users), strings.Join(userLabels(users), "、"))) } return users[0], nil } +func usersWithUserID(users []contactUser) []contactUser { + out := make([]contactUser, 0, len(users)) + for _, user := range users { + if user.userID != "" { + out = append(out, user) + } + } + return out +} + // extractUsers pulls {userId, openDingTalkId, name} out of a // search_contact_by_key_word response ({"result": [ {userId, name, ...} ]}). func extractUsers(data map[string]any) []contactUser { @@ -68,14 +92,27 @@ func extractUsers(data map[string]any) []contactUser { continue } id, _ := m["userId"].(string) - if id == "" { - continue - } - nm, _ := m["name"].(string) openID, _ := m["openDingTalkId"].(string) if openID == "" { openID, _ = m["openDingtalkId"].(string) } + // External / cross-org contacts come back with an empty userId and only + // an openDingTalkId (verified live). Dropping them here made name→ID + // resolution miss those people, or collapse to the wrong single match + // when an in-org namesake also existed. Keep any row with at least one + // usable identity; downstream (e.g. +dm) can act on the openDingTalkId. + if id == "" && openID == "" { + continue + } + nm, _ := m["name"].(string) + if nm == "" { + for _, k := range []string{"nick", "showName", "flowerName", "staffName", "userName"} { + if v, _ := m[k].(string); v != "" { + nm = v + break + } + } + } out = append(out, contactUser{userID: id, openDingTalkID: openID, name: nm}) } return out @@ -84,7 +121,11 @@ func extractUsers(data map[string]any) []contactUser { func userLabels(users []contactUser) []string { out := make([]string, 0, len(users)) for _, u := range users { - out = append(out, fmt.Sprintf("%s(%s)", u.name, u.userID)) + id := u.userID + if id == "" { + id = u.openDingTalkID + } + out = append(out, fmt.Sprintf("%s(%s)", u.name, id)) } return out } diff --git a/internal/shortcut/smart/resolve_test.go b/internal/shortcut/smart/resolve_test.go new file mode 100644 index 00000000..df0d0363 --- /dev/null +++ b/internal/shortcut/smart/resolve_test.go @@ -0,0 +1,67 @@ +// Copyright 2026 Alibaba Group +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package smart + +import "testing" + +func TestExtractUsers(t *testing.T) { + data := map[string]any{ + "result": []any{ + // in-org contact: full identity + map[string]any{"userId": "024083", "name": "朱鸿", "openDingTalkId": "DHUGO"}, + // external / cross-org contact: no userId, only openDingTalkId + + // display name under a non-"name" key — must be kept, not dropped. + map[string]any{"openDingtalkId": "DEXT", "nick": "外部小王"}, + // unusable row (no identity at all) is skipped + map[string]any{"name": "无ID的人"}, + // non-map entry is skipped + "garbage", + }, + } + users := extractUsers(data) + if len(users) != 2 { + t.Fatalf("extractUsers kept %d, want 2: %#v", len(users), users) + } + if users[0].userID != "024083" || users[0].openDingTalkID != "DHUGO" || users[0].name != "朱鸿" { + t.Errorf("in-org user = %#v", users[0]) + } + // external contact kept via openDingTalkId, name resolved from "nick" + if users[1].userID != "" || users[1].openDingTalkID != "DEXT" || users[1].name != "外部小王" { + t.Errorf("external user = %#v", users[1]) + } +} + +func TestExtractUsersNoResult(t *testing.T) { + if got := extractUsers(map[string]any{}); got != nil { + t.Errorf("no result should be nil, got %#v", got) + } +} + +func TestUsersWithUserIDExcludesOpenIDOnlyContacts(t *testing.T) { + users := []contactUser{ + {userID: "user-1", openDingTalkID: "open-1", name: "内部用户"}, + {openDingTalkID: "open-external", name: "外部联系人"}, + } + got := usersWithUserID(users) + if len(got) != 1 || got[0].userID != "user-1" { + t.Fatalf("usersWithUserID() = %#v, want only the organization user", got) + } +} + +func TestUserLabelsFallsBackToOpenDingTalkID(t *testing.T) { + got := userLabels([]contactUser{{openDingTalkID: "open-external", name: "外部联系人"}}) + if len(got) != 1 || got[0] != "外部联系人(open-external)" { + t.Fatalf("userLabels() = %#v", got) + } +} diff --git a/internal/shortcut/smart/share_doc.go b/internal/shortcut/smart/share_doc.go index 83a19dd4..6d68a25f 100644 --- a/internal/shortcut/smart/share_doc.go +++ b/internal/shortcut/smart/share_doc.go @@ -51,7 +51,7 @@ var ShareDoc = shortcut.Shortcut{ note := rt.Str("note") // Step 1 — resolve the recipient name to a unique userId. - user, err := resolveUser(rt, rt.Str("to")) + user, err := resolveOpenDingTalkUser(rt, rt.Str("to")) if err != nil { return err }