From 7c76dfea4bcf5d3687d8fe5daf8a7dcf4bf67801 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=85=8B=E8=B0=A8?= <2940480744@qq.com> Date: Fri, 7 Aug 2026 15:30:09 +0800 Subject: [PATCH 01/19] fix(chat): fail closed for scoped search and card updates --- internal/helpers/chat.go | 348 +++++++++++++++++- internal/helpers/chat_card_update_test.go | 101 +++++ internal/helpers/chat_message_search_test.go | 289 ++++++++++++++- internal/shortcut/chat/chat_message.go | 66 +++- .../shortcut/chat/coverage_completion_test.go | 4 +- internal/shortcut/chat/gap_fill_test.go | 99 +++++ internal/shortcut/chatmsg/card_update.go | 176 +++++++++ internal/shortcut/chatmsg/card_update_test.go | 79 ++++ internal/shortcut/chatmsg/chatmsg.go | 1 + internal/shortcut/chatmsg/search.go | 192 ++++++++++ internal/shortcut/chatmsg/search_test.go | 75 ++++ .../smart/compatibility_coverage_test.go | 5 +- .../smart/natural_target_resolution_test.go | 26 +- internal/shortcut/smart/search_msg.go | 199 +++++----- .../smart/search_msg_execution_test.go | 165 ++++++++- 15 files changed, 1683 insertions(+), 142 deletions(-) create mode 100644 internal/helpers/chat_card_update_test.go create mode 100644 internal/shortcut/chatmsg/card_update.go create mode 100644 internal/shortcut/chatmsg/card_update_test.go create mode 100644 internal/shortcut/chatmsg/search.go create mode 100644 internal/shortcut/chatmsg/search_test.go diff --git a/internal/helpers/chat.go b/internal/helpers/chat.go index 7db040a9..de7d8984 100644 --- a/internal/helpers/chat.go +++ b/internal/helpers/chat.go @@ -6,6 +6,7 @@ import ( "crypto/md5" "encoding/hex" "encoding/json" + "errors" "fmt" "io" "os" @@ -18,6 +19,7 @@ import ( "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/cli" apperrors "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/errors" + "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/shortcut/chatmsg" "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/shortcut/targetresolver" "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/pkg/edition" "github.com/spf13/cobra" @@ -116,6 +118,249 @@ func chatIntFlagOrFallback(cmd *cobra.Command, primary string, aliases ...string return v } +const maxConversationScopedSearchPages = 40 + +func runConversationScopedMessageSearch( + cmd *cobra.Command, + serverID, toolName, scopeParam string, + toolArgs map[string]any, + conversationIDs []string, +) error { + conversationIDs = uniqueNonEmptyStrings(conversationIDs) + if len(conversationIDs) == 0 { + return callMCPToolOnServer(serverID, toolName, toolArgs) + } + if commandDryRun(cmd) { + return writeConversationScopedSearchPreview(cmd, serverID, toolName, scopeParam, toolArgs, conversationIDs) + } + if err := validateNativeSearchConversationScope(conversationIDs); err != nil { + return err + } + + // The downstream search currently treats invalid CID filters as absent and + // does not reliably return group-scoped hits. Keep every other filter, scan + // the global result stream with a hard page bound, and apply the validated + // conversation set locally. + scanArgs := cloneStringAnyMap(toolArgs) + delete(scanArgs, scopeParam) + resultLimit := positiveSearchLimit(scanArgs["limit"], 100) + cursor := cleanSearchCursor(scanArgs["cursor"]) + messages := make([]map[string]any, 0, resultLimit) + seenMessageIDs := map[string]struct{}{} + pagesFetched := 0 + hasMore := false + var nextCursor any + + for pagesFetched < maxConversationScopedSearchPages && len(messages) < resultLimit { + remaining := resultLimit - len(messages) + scanArgs["limit"] = remaining + scanArgs["cursor"] = cursor + text, err := CallMCPToolTextOnServer(serverID, toolName, scanArgs) + if err != nil { + return err + } + var data map[string]any + if err := unmarshalJSONUseNumber(text, &data); err != nil { + return apperrors.NewInternal( + fmt.Sprintf("解析 %s 返回失败: %v", toolName, err), + apperrors.WithReason("search_response_invalid"), + ) + } + pagesFetched++ + + pageMessages := chatmsg.SearchItems(data) + pageMessages, unverifiableMessageIDs := chatmsg.FilterConversationScope(pageMessages, conversationIDs) + if len(unverifiableMessageIDs) > 0 { + return nativeSearchScopeUnverifiedError(conversationIDs, unverifiableMessageIDs) + } + for _, message := range pageMessages { + messageID := strings.TrimSpace(fmt.Sprint(chatmsg.MessageID(message))) + if messageID != "" && messageID != "" { + if _, exists := seenMessageIDs[messageID]; exists { + continue + } + seenMessageIDs[messageID] = struct{}{} + } + messages = append(messages, message) + } + + page := chatmsg.Pagination(data) + hasMoreValue, paginationKnown := page["hasMore"].(bool) + if !paginationKnown { + return apperrors.NewAPI( + "搜索服务未返回可靠的 hasMore，无法安全完成会话范围过滤", + apperrors.WithReason("search_conversation_scope_pagination_unknown"), + apperrors.WithRetryable(false), + ) + } + hasMore = hasMoreValue + nextCursor = page["nextCursor"] + if !hasMore { + break + } + next := cleanSearchCursor(nextCursor) + if next == "" || next == cursor { + return apperrors.NewAPI( + "搜索服务声称仍有更多结果，但 nextCursor 缺失或未前进", + apperrors.WithReason("search_conversation_scope_cursor_stalled"), + apperrors.WithRetryable(false), + ) + } + if len(messages) >= resultLimit { + break + } + cursor = next + } + + result := map[string]any{ + "conversationMessagesList": chatmsg.GroupSearchMessages(messages), + "hasMore": hasMore, + "complete": !hasMore, + "pagesFetched": pagesFetched, + } + if hasMore && nextCursor != nil && cleanSearchCursor(nextCursor) != "" { + result["nextCursor"] = nextCursor + } + payload := map[string]any{ + "result": result, + "scope": map[string]any{ + "requestedConversationIds": append([]string(nil), conversationIDs...), + "targetsValidated": true, + "filterApplied": true, + "filterMode": "client", + "resultsWithinScope": true, + "sourceComplete": !hasMore, + }, + } + return writeCommandPayload(cmd, payload) +} + +func writeConversationScopedSearchPreview( + cmd *cobra.Command, + serverID, toolName, scopeParam string, + toolArgs map[string]any, + conversationIDs []string, +) error { + plan := make([]map[string]any, 0, len(conversationIDs)+2) + for _, conversationID := range conversationIDs { + plan = append(plan, map[string]any{ + "stage": "validate-conversation", + "product": "chat", + "tool": "get_conversation_info", + "arguments": map[string]any{ + "openConversationId": conversationID, + }, + }) + } + scanArgs := cloneStringAnyMap(toolArgs) + delete(scanArgs, scopeParam) + plan = append(plan, + map[string]any{ + "stage": "search-global", + "product": serverID, + "tool": toolName, + "arguments": scanArgs, + "pageLimit": maxConversationScopedSearchPages, + }, + map[string]any{ + "stage": "filter-conversation-scope", + "requestedConversationIds": append([]string(nil), conversationIDs...), + "failClosed": true, + }, + ) + return writeCommandPayload(cmd, map[string]any{ + "dry_run": true, + "executed": false, + "plan": plan, + }) +} + +func validateNativeSearchConversationScope(conversationIDs []string) error { + for _, conversationID := range conversationIDs { + _, err := CallMCPToolTextOnServer("chat", "get_conversation_info", map[string]any{ + "openConversationId": conversationID, + }) + if err == nil { + continue + } + var cliErr *CLIError + if !errors.As(err, &cliErr) || (cliErr.Code != CodeMCPToolError && cliErr.Code != CodeResourceNotFound) { + return err + } + return apperrors.NewValidation( + fmt.Sprintf("无法验证会话 CID %q；已停止搜索，避免过滤失效后返回其他会话消息", conversationID), + apperrors.WithReason("search_conversation_scope_invalid"), + apperrors.WithDetails(map[string]any{"conversationId": conversationID}), + apperrors.WithHint("确认 openConversationId 存在且当前账号可访问后重试"), + apperrors.WithCause(err), + ) + } + return nil +} + +func nativeSearchScopeUnverifiedError(conversationIDs, messageIDs []string) error { + return apperrors.NewAPI( + "搜索结果缺少 conversationId，无法证明会话过滤范围；已停止输出", + apperrors.WithReason("search_conversation_scope_unverified"), + apperrors.WithDetails(map[string]any{ + "requestedConversationIds": conversationIDs, + "unverifiableMessageIds": messageIDs, + }), + apperrors.WithRetryable(false), + apperrors.WithHint("请保留 trace_id 并检查 IM 搜索服务是否返回 openConversationId"), + ) +} + +func uniqueNonEmptyStrings(values []string) []string { + out := make([]string, 0, len(values)) + seen := make(map[string]struct{}, len(values)) + for _, value := range values { + value = strings.TrimSpace(value) + if value == "" { + continue + } + if _, exists := seen[value]; exists { + continue + } + seen[value] = struct{}{} + out = append(out, value) + } + return out +} + +func positiveSearchLimit(value any, fallback int) int { + switch typed := value.(type) { + case int: + if typed > 0 { + return typed + } + case int64: + if typed > 0 { + return int(typed) + } + case json.Number: + if parsed, err := strconv.Atoi(typed.String()); err == nil && parsed > 0 { + return parsed + } + case float64: + if typed > 0 { + return int(typed) + } + } + return fallback +} + +func cleanSearchCursor(value any) string { + if value == nil { + return "" + } + text := strings.TrimSpace(fmt.Sprint(value)) + if text == "" || strings.EqualFold(text, "null") { + return "" + } + return text +} + func runChatGroupSearch(cmd *cobra.Command, args []string) error { keyword := flagOrFallback(cmd, "query", "keyword", "name", "group") if len(args) == 1 { @@ -1242,6 +1487,35 @@ func unmarshalJSONUseNumber(text string, v any) error { return dec.Decode(v) } +func nativeCardUpdateVerificationError(bizID string, verifyErr error) error { + reason := "streaming_card_update_unverified" + message := "服务端未返回卡片实际更新的证据；为避免假成功，CLI 已将本次操作判为失败" + hint := "请检查服务端是否返回 updated=true、affectedCount>0 或等价的明确更新结果" + switch { + case errors.Is(verifyErr, chatmsg.ErrCardUpdateNotApplied): + reason = "streaming_card_update_not_applied" + message = "服务端明确表示流式卡片没有被更新" + hint = "请确认 bizId 来自 send-card、当前账号有权限且卡片仍允许该状态转换" + case errors.Is(verifyErr, chatmsg.ErrCardUpdateBizIDDrift): + reason = "streaming_card_update_biz_id_mismatch" + message = "服务端返回的 bizId 与本次请求不一致；无法确认目标卡片已更新" + hint = "请保留 trace_id 并检查 update_streaming_card 的响应映射" + } + return apperrors.NewAPI( + message, + apperrors.WithOperation("update_streaming_card"), + apperrors.WithServerKey("im"), + apperrors.WithOrigin("client_postcondition"), + apperrors.WithFailureStage("verify_update_result"), + apperrors.WithExecutionStarted(true), + apperrors.WithRetryable(false), + apperrors.WithReason(reason), + apperrors.WithHint(hint), + apperrors.WithDetails(map[string]any{"bizId": bizID}), + apperrors.WithCause(verifyErr), + ) +} + func firstStringField(data map[string]any, keys ...string) string { for _, key := range keys { if value, ok := data[key]; ok { @@ -3066,7 +3340,7 @@ func newChatCommand() *cobra.Command { chatMessageSearchCmd := &cobra.Command{ Use: "search", Short: "按关键词搜索消息", - Long: `在当前用户的会话中按关键词搜索消息。--query 指定搜索关键词（必填）。可选 --group 限定搜索某个会话，不传则搜索所有会话。时间参数 --start/--end（ISO-8601）限定搜索时间范围。分页参数 --limit（默认 100）和 --cursor（默认 "0"）始终传递；hasMore=true 时用返回的 nextCursor 作为下次 --cursor 继续翻页。`, + Long: `在当前用户的会话中按关键词搜索消息。--query 指定搜索关键词（必填）。可选 --group 限定搜索某个会话，不传则搜索所有会话。显式指定会话时，CLI 会先验证 CID，再有界扫描全局搜索流并在本地精确过滤，避免下层忽略非法 CID 或群聊 CID。时间参数 --start/--end（ISO-8601）限定搜索时间范围。分页参数 --limit（默认 100）和 --cursor（默认 "0"）始终传递；hasMore=true 时用返回的 nextCursor 作为下次 --cursor 继续翻页。`, Example: ` dws chat message search --query "changefree" --start "2026-04-01T00:00:00+08:00" --end "2026-04-15T00:00:00+08:00" --limit 50 --cursor 0 dws chat message search --query "codereview" --group --start "2026-04-01T00:00:00+08:00" --end "2026-04-15T00:00:00+08:00" --limit 100 --cursor 0 dws chat message search --query "链接" --start "2026-04-15T00:00:00+08:00" --end "2026-04-16T00:00:00+08:00" --limit 100 --cursor @@ -3102,7 +3376,14 @@ func newChatCommand() *cobra.Command { if groupID != "" { toolArgs["openConversationId"] = groupID } - return callMCPTool("search_messages_by_keyword", toolArgs) + return runConversationScopedMessageSearch( + cmd, + "chat", + "search_messages_by_keyword", + "openConversationId", + toolArgs, + []string{groupID}, + ) }, } DeclareLeafMetadata(chatMessageSearchCmd, LeafSpec{ @@ -3144,7 +3425,7 @@ func newChatCommand() *cobra.Command { chatMessageSearchAdvancedCmd := &cobra.Command{ Use: "search-advanced", Short: "多维度搜索消息", - Long: `支持按关键词、发送者、@我、@指定人、指定会话、时间范围等多维度搜索消息。发送者 userId 使用 --user/--users；发送者或 @ 人的 openDingTalkId 使用 --sender-ids/--at-ids。所有参数均为可选，至少指定一个搜索条件。`, + Long: `支持按关键词、发送者、@我、@指定人、指定会话、时间范围等多维度搜索消息。发送者 userId 使用 --user/--users；发送者或 @ 人的 openDingTalkId 使用 --sender-ids/--at-ids。显式指定会话时，CLI 会先验证 CID，再有界扫描全局搜索流并在本地精确过滤，避免下层忽略非法 CID 或群聊 CID。所有参数均为可选，至少指定一个搜索条件。`, Example: ` dws chat message search-advanced --query "周报" --start "2026-04-01T00:00:00+08:00" --end "2026-04-15T00:00:00+08:00" dws chat message search-advanced --user --start "2026-04-01T00:00:00+08:00" --end "2026-04-15T00:00:00+08:00" dws chat message search-advanced --users , --start "2026-04-01T00:00:00+08:00" --end "2026-04-15T00:00:00+08:00" @@ -3191,6 +3472,7 @@ func newChatCommand() *cobra.Command { // conversation-ids / groups / group -> openConversationIds convIds := "" + var conversationIDs []string if v, _ := cmd.Flags().GetString("conversation-ids"); v != "" { convIds = v } else if v, _ := cmd.Flags().GetString("groups"); v != "" { @@ -3199,14 +3481,13 @@ func newChatCommand() *cobra.Command { convIds = v } if convIds != "" { - var ids []string for _, s := range strings.Split(convIds, ",") { if t := strings.TrimSpace(s); t != "" { - ids = append(ids, t) + conversationIDs = append(conversationIDs, t) } } - if len(ids) > 0 { - toolArgs["openConversationIds"] = ids + if len(conversationIDs) > 0 { + toolArgs["openConversationIds"] = conversationIDs } } @@ -3250,7 +3531,14 @@ func newChatCommand() *cobra.Command { toolArgs["limit"] = v } - return callMCPToolOnServer("im", "search_messages", toolArgs) + return runConversationScopedMessageSearch( + cmd, + "im", + "search_messages", + "openConversationIds", + toolArgs, + conversationIDs, + ) }, } DeclareLeafMetadata(chatMessageSearchAdvancedCmd, LeafSpec{ @@ -5183,12 +5471,48 @@ flow-status 取值：1=处理中(PROCESSING)，2=输入中(INPUTTING)，3=完成 if !cmd.Flags().Changed("flow-status") { return fmt.Errorf("flag --flow-status is required") } + bizID, err := chatmsg.NormalizeCardBizID(mustGetFlag(cmd, "biz-id")) + if err != nil { + return err + } flowStatus, _ := cmd.Flags().GetInt("flow-status") - return callMCPToolOnServer("im", "update_streaming_card", map[string]any{ - "bizId": mustGetFlag(cmd, "biz-id"), + if flowStatus < 1 || flowStatus > 5 { + return fmt.Errorf("--flow-status 必须在 1-5 之间") + } + params := map[string]any{ + "bizId": bizID, "msgContent": mustGetFlag(cmd, "content"), "flowStatus": flowStatus, - }) + } + if commandDryRun(cmd) { + return writeCommandPayload(cmd, map[string]any{ + "dry_run": true, + "executed": false, + "verified": false, + "action": map[string]any{ + "product": "im", + "tool": "update_streaming_card", + "arguments": params, + }, + }) + } + text, err := CallMCPToolTextOnServer("im", "update_streaming_card", params) + if err != nil { + return err + } + var response map[string]any + if strings.TrimSpace(text) == "" { + response = map[string]any{} + } else if err := unmarshalJSONUseNumber(text, &response); err != nil { + return apperrors.NewInternal( + fmt.Sprintf("解析 update_streaming_card 返回失败: %v", err), + apperrors.WithReason("streaming_card_update_response_invalid"), + ) + } + if _, err := chatmsg.VerifyStreamingCardUpdate(bizID, response); err != nil { + return nativeCardUpdateVerificationError(bizID, err) + } + return writeCommandPayload(cmd, response) }, } DeclareLeafMetadata(chatMessageUpdateCardCmd, LeafSpec{ @@ -5217,7 +5541,9 @@ flow-status 取值：1=处理中(PROCESSING)，2=输入中(INPUTTING)，3=完成 Examples: []string{"dws chat message update-card --biz-id --content \"处理完成\" --flow-status 2"}, }, Parameters: []contract.ParamDecl{ + {Name: "biz-id", Property: "bizId"}, {Name: "content", Property: "msgContent"}, + {Name: "flow-status", Property: "flowStatus"}, }, }, }) diff --git a/internal/helpers/chat_card_update_test.go b/internal/helpers/chat_card_update_test.go new file mode 100644 index 00000000..3c7bc472 --- /dev/null +++ b/internal/helpers/chat_card_update_test.go @@ -0,0 +1,101 @@ +// 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 helpers + +import ( + "errors" + "testing" + + apperrors "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/errors" +) + +func runNativeCardUpdate(t *testing.T, caller *scriptedToolCaller, args ...string) error { + t.Helper() + installScriptedCaller(t, caller) + root := newChatCommand() + root.SilenceErrors = true + root.SilenceUsage = true + if root.PersistentFlags().Lookup("dry-run") == nil { + root.PersistentFlags().Bool("dry-run", false, "preview without executing") + } + root.SetArgs(args) + return root.Execute() +} + +func TestCrossPlatformCoverageNativeMessageUpdateCardVerifiesWrite(t *testing.T) { + t.Run("explicit evidence succeeds", func(t *testing.T) { + caller := &scriptedToolCaller{steps: []scriptedToolStep{{text: `{"result":{"bizId":"biz-1","updated":true}}`}}} + err := runNativeCardUpdate(t, caller, + "message", "update-card", + "--biz-id", "biz-1", + "--content", "完成", + "--flow-status", "3", + ) + if err != nil { + t.Fatal(err) + } + if caller.calls != 1 || caller.server != "im" || caller.tool != "update_streaming_card" { + t.Fatalf("call = count:%d server:%q tool:%q", caller.calls, caller.server, caller.tool) + } + if caller.args["bizId"] != "biz-1" { + t.Fatalf("args = %#v", caller.args) + } + }) + + t.Run("generic success is unverified", func(t *testing.T) { + caller := &scriptedToolCaller{steps: []scriptedToolStep{{text: `{"success":true,"errorCode":null}`}}} + err := runNativeCardUpdate(t, caller, + "message", "update-card", + "--biz-id", "not-a-real-card", + "--content", "完成", + "--flow-status", "3", + ) + var typed *apperrors.Error + if !errors.As(err, &typed) || typed.Reason != "streaming_card_update_unverified" { + t.Fatalf("error = %#v, want streaming_card_update_unverified", err) + } + }) + + t.Run("invalid arguments make no call", func(t *testing.T) { + for _, args := range [][]string{ + {"message", "update-card", "--biz-id", "", "--content", "完成", "--flow-status", "3"}, + {"message", "update-card", "--biz-id", "biz-1", "--content", "完成", "--flow-status", "6"}, + } { + caller := &scriptedToolCaller{} + if err := runNativeCardUpdate(t, caller, args...); err == nil { + t.Fatalf("args %v unexpectedly succeeded", args) + } + if caller.calls != 0 { + t.Fatalf("args %v made %d calls", args, caller.calls) + } + } + }) + + t.Run("dry run publishes unverified plan without write", func(t *testing.T) { + caller := &scriptedToolCaller{} + err := runNativeCardUpdate(t, caller, + "message", "update-card", + "--biz-id", "biz-preview", + "--content", "完成", + "--flow-status", "3", + "--dry-run", + ) + if err != nil { + t.Fatal(err) + } + if caller.calls != 0 { + t.Fatalf("dry-run made %d calls", caller.calls) + } + }) +} diff --git a/internal/helpers/chat_message_search_test.go b/internal/helpers/chat_message_search_test.go index 3d559b51..6000bb5f 100644 --- a/internal/helpers/chat_message_search_test.go +++ b/internal/helpers/chat_message_search_test.go @@ -16,6 +16,7 @@ package helpers import ( "context" "encoding/json" + "errors" "io" "os" "reflect" @@ -23,8 +24,10 @@ import ( "testing" "time" + apperrors "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/errors" "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/pkg/agentproduct" "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/pkg/edition" + "github.com/spf13/cobra" ) type chatMessageSearchCall struct { @@ -34,12 +37,36 @@ type chatMessageSearchCall struct { } type chatMessageSearchCaller struct { - calls []chatMessageSearchCall + calls []chatMessageSearchCall + searchResponse string + searchResponses []string + searchCalls int + failPreflight bool + preflightError error } func (c *chatMessageSearchCaller) CallTool(_ context.Context, productID, toolName string, args map[string]any) (*edition.ToolResult, error) { c.calls = append(c.calls, chatMessageSearchCall{productID: productID, toolName: toolName, args: args}) - return &edition.ToolResult{Content: []edition.ContentBlock{{Type: "text", Text: `{}`}}}, nil + text := `{}` + if toolName == "get_conversation_info" { + if c.preflightError != nil { + return nil, c.preflightError + } + if c.failPreflight { + return nil, errors.New("conversation not found") + } + text = `{"result":{"openConversationId":"` + args["openConversationId"].(string) + `"}}` + } + if toolName == "search_messages_by_keyword" || toolName == "search_messages" { + text = `{"result":{"messages":[],"hasMore":false}}` + if c.searchCalls < len(c.searchResponses) { + text = c.searchResponses[c.searchCalls] + } else if c.searchResponse != "" { + text = c.searchResponse + } + c.searchCalls++ + } + return &edition.ToolResult{Content: []edition.ContentBlock{{Type: "text", Text: text}}}, nil } func (*chatMessageSearchCaller) Format() string { return "json" } @@ -72,6 +99,7 @@ func TestCrossPlatformCoverageChatMessageSearchUsesMCPContracts(t *testing.T) { productID string toolName string wantToolArg map[string]any + preflight []string }{ { name: "keyword search", @@ -79,13 +107,13 @@ func TestCrossPlatformCoverageChatMessageSearchUsesMCPContracts(t *testing.T) { productID: "chat", toolName: "search_messages_by_keyword", wantToolArg: map[string]any{ - "keyword": "categoryName", - "openConversationId": "cid-1", - "startTime": startTime.UnixMilli(), - "endTime": endTime.UnixMilli(), - "limit": 100, - "cursor": "0", + "keyword": "categoryName", + "startTime": startTime.UnixMilli(), + "endTime": endTime.UnixMilli(), + "limit": 100, + "cursor": "0", }, + preflight: []string{"cid-1"}, }, { name: "advanced search", @@ -93,16 +121,16 @@ func TestCrossPlatformCoverageChatMessageSearchUsesMCPContracts(t *testing.T) { productID: "im", toolName: "search_messages", wantToolArg: map[string]any{ - "keyword": "categoryName", - "openConversationIds": []string{"cid-1", "cid-2"}, - "messageType": "text", - "onlyRobotMessages": true, - "searchConvType": "group", - "startTime": startTime.UnixMilli(), - "endTime": endTime.UnixMilli(), - "limit": 100, - "cursor": "0", + "keyword": "categoryName", + "messageType": "text", + "onlyRobotMessages": true, + "searchConvType": "group", + "startTime": startTime.UnixMilli(), + "endTime": endTime.UnixMilli(), + "limit": 100, + "cursor": "0", }, + preflight: []string{"cid-1", "cid-2"}, }, } @@ -116,14 +144,21 @@ func TestCrossPlatformCoverageChatMessageSearchUsesMCPContracts(t *testing.T) { cmd := newChatCommand() cmd.SilenceErrors = true cmd.SilenceUsage = true + cmd.SetOut(io.Discard) cmd.SetArgs(tt.args) if err := cmd.Execute(); err != nil { t.Fatalf("chat search returned error: %v", err) } - if len(caller.calls) != 1 { - t.Fatalf("tool call count = %d, want 1", len(caller.calls)) + if len(caller.calls) != len(tt.preflight)+1 { + t.Fatalf("tool calls = %#v", caller.calls) } - call := caller.calls[0] + for index, conversationID := range tt.preflight { + call := caller.calls[index] + if call.productID != "chat" || call.toolName != "get_conversation_info" || call.args["openConversationId"] != conversationID { + t.Fatalf("preflight[%d] = %#v", index, call) + } + } + call := caller.calls[len(caller.calls)-1] if call.productID != tt.productID || call.toolName != tt.toolName { t.Fatalf("tool call = %s/%s, want %s/%s", call.productID, call.toolName, tt.productID, tt.toolName) } @@ -134,6 +169,220 @@ func TestCrossPlatformCoverageChatMessageSearchUsesMCPContracts(t *testing.T) { } } +func executeNativeScopedSearch(t *testing.T, caller *chatMessageSearchCaller, args ...string) (map[string]any, error) { + t.Helper() + previousDeps := deps + t.Cleanup(func() { deps = previousDeps }) + InitDeps(caller) + deps.Out.w = io.Discard + cmd := newChatCommand() + cmd.SilenceErrors = true + cmd.SilenceUsage = true + var output strings.Builder + cmd.SetOut(&output) + cmd.SetArgs(args) + if err := cmd.Execute(); err != nil { + return nil, err + } + var payload map[string]any + if err := json.Unmarshal([]byte(output.String()), &payload); err != nil { + return nil, err + } + return payload, nil +} + +func TestNativeScopedSearchFiltersGlobalResultsForBothEntries(t *testing.T) { + start := "2026-07-09T00:00:00+08:00" + end := "2026-07-11T00:00:00+08:00" + for _, tt := range []struct { + name string + args []string + tool string + scopeParam string + }{ + { + name: "keyword search", + args: []string{"message", "search", "--query", "周报", "--group", "cid-target", "--start", start, "--end", end}, + tool: "search_messages_by_keyword", + scopeParam: "openConversationId", + }, + { + name: "advanced search", + args: []string{"message", "search-advanced", "--query", "周报", "--conversation-ids", "cid-target"}, + tool: "search_messages", + scopeParam: "openConversationIds", + }, + } { + t.Run(tt.name, func(t *testing.T) { + caller := &chatMessageSearchCaller{searchResponse: `{ + "result": { + "conversationMessagesList": [ + {"openConversationId":"cid-target","title":"目标群","messages":[{"openMessageId":"m-target","content":"目标"}]}, + {"openConversationId":"cid-other","title":"其他群","messages":[{"openMessageId":"m-other","content":"越界"}]} + ], + "hasMore": false + } + }`} + payload, err := executeNativeScopedSearch(t, caller, tt.args...) + if err != nil { + t.Fatal(err) + } + result, _ := payload["result"].(map[string]any) + groups, _ := result["conversationMessagesList"].([]any) + if len(groups) != 1 { + t.Fatalf("result = %#v", result) + } + group, _ := groups[0].(map[string]any) + if group["openConversationId"] != "cid-target" { + t.Fatalf("group = %#v", group) + } + scope, _ := payload["scope"].(map[string]any) + if scope["targetsValidated"] != true || scope["resultsWithinScope"] != true || scope["filterMode"] != "client" { + t.Fatalf("scope = %#v", scope) + } + searchCall := caller.calls[len(caller.calls)-1] + if searchCall.toolName != tt.tool { + t.Fatalf("search call = %#v", searchCall) + } + if _, exists := searchCall.args[tt.scopeParam]; exists { + t.Fatalf("global fallback unexpectedly forwarded %s: %#v", tt.scopeParam, searchCall.args) + } + }) + } +} + +func TestNativeScopedSearchInvalidCIDStopsBeforeSearch(t *testing.T) { + caller := &chatMessageSearchCaller{failPreflight: true} + _, err := executeNativeScopedSearch(t, caller, + "message", "search-advanced", "--query", "周报", "--conversation-ids", "cid-invalid") + if err == nil { + t.Fatal("invalid CID unexpectedly succeeded") + } + var typed *apperrors.Error + if !errors.As(err, &typed) || typed.Reason != "search_conversation_scope_invalid" { + t.Fatalf("error = %#v", err) + } + if len(caller.calls) != 1 || caller.calls[0].toolName != "get_conversation_info" { + t.Fatalf("calls = %#v", caller.calls) + } +} + +func TestNativeScopedSearchPreservesPreflightAuthError(t *testing.T) { + want := &CLIError{Code: CodeAuthNotConfigured, Message: "当前未登录"} + caller := &chatMessageSearchCaller{preflightError: want} + _, err := executeNativeScopedSearch(t, caller, + "message", "search-advanced", "--query", "周报", "--conversation-ids", "cid-target") + if err == nil { + t.Fatal("auth failure unexpectedly succeeded") + } + var cliErr *CLIError + if !errors.As(err, &cliErr) || cliErr.Code != CodeAuthNotConfigured { + t.Fatalf("error = %#v", err) + } +} + +func TestNativeScopedSearchScansUntilTargetConversationAppears(t *testing.T) { + caller := &chatMessageSearchCaller{searchResponses: []string{ + `{"result":{"conversationMessagesList":[{"openConversationId":"cid-other","messages":[{"openMessageId":"m-other"}]}],"hasMore":true,"nextCursor":"c2"}}`, + `{"result":{"conversationMessagesList":[{"openConversationId":"cid-target","messages":[{"openMessageId":"m-target"}]}],"hasMore":false}}`, + }} + payload, err := executeNativeScopedSearch(t, caller, + "message", "search-advanced", "--query", "周报", "--conversation-ids", "cid-target") + if err != nil { + t.Fatal(err) + } + result, _ := payload["result"].(map[string]any) + if result["pagesFetched"] != float64(2) || result["complete"] != true { + t.Fatalf("result = %#v", result) + } + groups, _ := result["conversationMessagesList"].([]any) + if len(groups) != 1 { + t.Fatalf("groups = %#v", groups) + } + searchCalls := make([]chatMessageSearchCall, 0, 2) + for _, call := range caller.calls { + if call.toolName == "search_messages" { + searchCalls = append(searchCalls, call) + } + } + if len(searchCalls) != 2 || searchCalls[1].args["cursor"] != "c2" { + t.Fatalf("search calls = %#v", searchCalls) + } +} + +func TestNativeScopedSearchMissingConversationIdentityFailsClosed(t *testing.T) { + caller := &chatMessageSearchCaller{searchResponse: `{"result":{"messages":[{"openMessageId":"m1"}],"hasMore":false}}`} + _, err := executeNativeScopedSearch(t, caller, + "message", "search-advanced", "--query", "周报", "--conversation-ids", "cid-target") + if err == nil { + t.Fatal("unverifiable scoped result unexpectedly succeeded") + } + var typed *apperrors.Error + if !errors.As(err, &typed) || typed.Reason != "search_conversation_scope_unverified" { + t.Fatalf("error = %#v", err) + } +} + +func TestNativeScopedSearchValidEmptyResultIsComplete(t *testing.T) { + caller := &chatMessageSearchCaller{searchResponse: `{"result":{"messages":[],"hasMore":false}}`} + payload, err := executeNativeScopedSearch(t, caller, + "message", "search-advanced", "--query", "周报", "--conversation-ids", "cid-empty") + if err != nil { + t.Fatal(err) + } + result, _ := payload["result"].(map[string]any) + if result["complete"] != true || result["hasMore"] != false { + t.Fatalf("result = %#v", result) + } + scope, _ := payload["scope"].(map[string]any) + if scope["targetsValidated"] != true || scope["sourceComplete"] != true { + t.Fatalf("scope = %#v", scope) + } +} + +func TestNativeScopedSearchDryRunShowsCompositePlanWithoutCallingTools(t *testing.T) { + caller := &chatMessageSearchCaller{} + previousDeps := deps + t.Cleanup(func() { deps = previousDeps }) + InitDeps(caller) + cmd := &cobra.Command{Use: "search"} + cmd.Flags().Bool("dry-run", true, "") + var output strings.Builder + cmd.SetOut(&output) + err := runConversationScopedMessageSearch( + cmd, + "im", + "search_messages", + "openConversationIds", + map[string]any{ + "keyword": "周报", + "openConversationIds": []string{"cid-target"}, + "limit": 100, + "cursor": "0", + }, + []string{"cid-target"}, + ) + if err != nil { + t.Fatal(err) + } + if len(caller.calls) != 0 { + t.Fatalf("dry-run made calls: %#v", caller.calls) + } + var payload map[string]any + if err := json.Unmarshal([]byte(output.String()), &payload); err != nil { + t.Fatal(err) + } + plan, _ := payload["plan"].([]any) + if payload["dry_run"] != true || payload["executed"] != false || len(plan) != 3 { + t.Fatalf("payload = %#v", payload) + } + searchStage, _ := plan[1].(map[string]any) + arguments, _ := searchStage["arguments"].(map[string]any) + if _, exists := arguments["openConversationIds"]; exists { + t.Fatalf("dry-run global search still carries scope: %#v", searchStage) + } +} + type chatChangedContractCaller struct { calls []chatMessageSearchCall resolveUsers bool diff --git a/internal/shortcut/chat/chat_message.go b/internal/shortcut/chat/chat_message.go index 352d454f..92891d0b 100644 --- a/internal/shortcut/chat/chat_message.go +++ b/internal/shortcut/chat/chat_message.go @@ -15,6 +15,7 @@ package chat import ( "encoding/json" + "errors" "fmt" "path/filepath" "sort" @@ -1362,6 +1363,9 @@ var MessagesSendCard = shortcut.Shortcut{ if err != nil { return fmt.Errorf("卡片已创建（bizId=%s），但自动更新失败: %w", bizID, err) } + if _, err := chatmsg.VerifyStreamingCardUpdate(bizID, updated); err != nil { + return fmt.Errorf("卡片已创建（bizId=%s），但自动更新结果不可信: %w", bizID, cardUpdateVerificationError(bizID, err)) + } return rt.Output(map[string]any{ "contractVersion": currentCardWorkflowContract.Version, "ok": true, @@ -1472,20 +1476,76 @@ var MessagesUpdateCard = shortcut.Shortcut{ }, Tips: []string{`dws chat +messages-update-card --biz-id --content "内容" --flow-status 3`}, Validate: func(rt *shortcut.RuntimeContext) error { + if _, err := chatmsg.NormalizeCardBizID(rt.Str("biz-id")); err != nil { + return err + } if !validCardFlowStatus(rt.Int("flow-status")) { return fmt.Errorf("--flow-status 必须在 1-5 之间") } return nil }, Execute: func(rt *shortcut.RuntimeContext) error { - return rt.CallMCP("update_streaming_card", map[string]any{ - "bizId": rt.Str("biz-id"), + bizID, err := chatmsg.NormalizeCardBizID(rt.Str("biz-id")) + if err != nil { + return err + } + params := map[string]any{ + "bizId": bizID, "msgContent": rt.Str("content"), "flowStatus": rt.Int("flow-status"), - }) + } + if rt.DryRun() { + return rt.Output(map[string]any{ + "dry_run": true, + "executed": false, + "verified": false, + "action": map[string]any{ + "product": "im", + "tool": "update_streaming_card", + "arguments": params, + }, + }) + } + updated, err := rt.CallMCPWriteData("im", "update_streaming_card", params) + if err != nil { + return err + } + if _, err := chatmsg.VerifyStreamingCardUpdate(bizID, updated); err != nil { + return cardUpdateVerificationError(bizID, err) + } + return rt.Output(updated) }, } +func cardUpdateVerificationError(bizID string, verifyErr error) error { + reason := "streaming_card_update_unverified" + message := "服务端未返回卡片实际更新的证据；为避免假成功，CLI 已将本次操作判为失败" + hint := "请检查服务端是否返回 updated=true、affectedCount>0 或等价的明确更新结果" + switch { + case errors.Is(verifyErr, chatmsg.ErrCardUpdateNotApplied): + reason = "streaming_card_update_not_applied" + message = "服务端明确表示流式卡片没有被更新" + hint = "请确认 bizId 来自 send-card、当前账号有权限且卡片仍允许该状态转换" + case errors.Is(verifyErr, chatmsg.ErrCardUpdateBizIDDrift): + reason = "streaming_card_update_biz_id_mismatch" + message = "服务端返回的 bizId 与本次请求不一致；无法确认目标卡片已更新" + hint = "请保留 trace_id 并检查 update_streaming_card 的响应映射" + } + return apperrors.NewAPI( + message, + apperrors.WithOperation("update_streaming_card"), + apperrors.WithServerKey("im"), + apperrors.WithOrigin("client_postcondition"), + apperrors.WithFailureStage("verify_update_result"), + apperrors.WithExecutionStarted(true), + apperrors.WithRetryable(false), + apperrors.WithReason(reason), + apperrors.WithHint(hint), + apperrors.WithDetails(map[string]any{"bizId": bizID}), + apperrors.WithCause(verifyErr), + ) +} + // MessagesResourceURL gets a message resource download url (get_resource_download_url, im). var MessagesResourceURL = shortcut.Shortcut{ Service: "chat", diff --git a/internal/shortcut/chat/coverage_completion_test.go b/internal/shortcut/chat/coverage_completion_test.go index b19af0de..fa521bcd 100644 --- a/internal/shortcut/chat/coverage_completion_test.go +++ b/internal/shortcut/chat/coverage_completion_test.go @@ -360,7 +360,9 @@ func TestCrossPlatformCoverageRecallCardAndLedgerBoundaries(t *testing.T) { }) } - helpers.InitDeps(&larkAlignmentCaller{}) + helpers.InitDeps(&larkAlignmentCaller{responses: map[string]string{ + "im/update_streaming_card": `{"result":{"updated":true}}`, + }}) root := newPlatformCoverageRoot() root.SetArgs([]string{"chat", "+messages-update-card", "--biz-id", "b", "--content", "x", "--flow-status", "3", "--yes"}) if err := root.Execute(); err != nil { diff --git a/internal/shortcut/chat/gap_fill_test.go b/internal/shortcut/chat/gap_fill_test.go index d2c618b9..55b12f11 100644 --- a/internal/shortcut/chat/gap_fill_test.go +++ b/internal/shortcut/chat/gap_fill_test.go @@ -793,6 +793,14 @@ func TestCrossPlatformCoverageMessagesSendCardDryRunAndFailureBoundaries(t *test }, wantError: "biz-preserved", }, + { + name: "unverified update preserves id", + fake: &larkAlignmentCaller{responses: map[string]string{ + "im/create_and_send_card": `{"bizId":"biz-unverified"}`, + "im/update_streaming_card": `{"success":true,"errorCode":null}`, + }}, + wantError: "biz-unverified", + }, } { t.Run(tc.name, func(t *testing.T) { helpers.InitDeps(tc.fake) @@ -825,6 +833,97 @@ func TestCrossPlatformCoverageMessagesSendCardDryRunAndFailureBoundaries(t *test } } +func TestCrossPlatformCoverageMessagesUpdateCardRejectsFalseSuccess(t *testing.T) { + t.Run("generic success is unverified", func(t *testing.T) { + fake := &larkAlignmentCaller{responses: map[string]string{ + "im/update_streaming_card": `{"success":true,"errorCode":null}`, + }} + helpers.InitDeps(fake) + root := newPlatformCoverageRoot() + root.SetArgs([]string{ + "chat", "+messages-update-card", + "--biz-id", "中文乱串", + "--content", "完成", + "--flow-status", "3", + "--yes", + }) + err := root.Execute() + var typed *apperrors.Error + if !errors.As(err, &typed) || typed.Reason != "streaming_card_update_unverified" { + t.Fatalf("error = %#v, want streaming_card_update_unverified", err) + } + if len(fake.calls) != 1 || fake.calls[0].tool != "update_streaming_card" { + t.Fatalf("calls = %#v", fake.calls) + } + }) + + t.Run("explicit update evidence succeeds", func(t *testing.T) { + fake := &larkAlignmentCaller{responses: map[string]string{ + "im/update_streaming_card": `{"result":{"bizId":"biz-verified","updated":true}}`, + }} + helpers.InitDeps(fake) + root := newPlatformCoverageRoot() + root.SetArgs([]string{ + "chat", "+messages-update-card", + "--biz-id", "biz-verified", + "--content", "完成", + "--flow-status", "3", + "--yes", + }) + if err := root.Execute(); err != nil { + t.Fatal(err) + } + }) + + t.Run("placeholder fails before write", func(t *testing.T) { + fake := &larkAlignmentCaller{} + helpers.InitDeps(fake) + root := newPlatformCoverageRoot() + root.SetArgs([]string{ + "chat", "+messages-update-card", + "--biz-id", "", + "--content", "完成", + "--flow-status", "3", + "--yes", + }) + if err := root.Execute(); err == nil || !strings.Contains(err.Error(), "占位符") { + t.Fatalf("error = %v, want placeholder validation", err) + } + if len(fake.calls) != 0 { + t.Fatalf("invalid placeholder made calls: %#v", fake.calls) + } + }) + + t.Run("dry run only publishes plan", func(t *testing.T) { + fake := &larkAlignmentCaller{} + helpers.InitDeps(fake) + root := newPlatformCoverageRoot() + var output bytes.Buffer + root.SetOut(&output) + root.SetArgs([]string{ + "chat", "+messages-update-card", + "--biz-id", "biz-preview", + "--content", "完成", + "--flow-status", "3", + "--dry-run", + "--yes", + }) + if err := root.Execute(); err != nil { + t.Fatal(err) + } + if len(fake.calls) != 0 { + t.Fatalf("dry-run made calls: %#v", fake.calls) + } + var payload map[string]any + if err := json.Unmarshal(output.Bytes(), &payload); err != nil { + t.Fatal(err) + } + if payload["executed"] != false || payload["verified"] != false { + t.Fatalf("dry-run payload = %#v", payload) + } + }) +} + func TestCrossPlatformCoverageFindCardBizIDResponseShapes(t *testing.T) { for _, tc := range []struct { value any diff --git a/internal/shortcut/chatmsg/card_update.go b/internal/shortcut/chatmsg/card_update.go new file mode 100644 index 00000000..5649c772 --- /dev/null +++ b/internal/shortcut/chatmsg/card_update.go @@ -0,0 +1,176 @@ +// 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 chatmsg + +import ( + "encoding/json" + "errors" + "fmt" + "strings" + "unicode" +) + +var ( + ErrCardUpdateNotApplied = errors.New("streaming card update was not applied") + ErrCardUpdateUnverified = errors.New("streaming card update could not be verified") + ErrCardUpdateBizIDDrift = errors.New("streaming card update returned a different bizId") +) + +// NormalizeCardBizID performs only format-independent checks. bizId is an +// opaque server-issued identifier; a stricter character or prefix contract +// must not be invented by the CLI without an authoritative API declaration. +func NormalizeCardBizID(raw string) (string, error) { + bizID := strings.TrimSpace(raw) + if bizID == "" { + return "", fmt.Errorf("--biz-id 不能为空") + } + for _, r := range bizID { + if unicode.IsControl(r) || unicode.IsSpace(r) { + return "", fmt.Errorf("--biz-id 必须是 send-card 返回的单个不透明 ID，不能包含空白或控制字符") + } + } + if isCardBizIDPlaceholder(bizID) { + return "", fmt.Errorf("--biz-id 仍是占位符 %q；请传入 send-card 实际返回的 bizId", bizID) + } + return bizID, nil +} + +func isCardBizIDPlaceholder(value string) bool { + normalized := strings.ToLower(strings.TrimSpace(value)) + switch normalized { + case "bizid", "biz-id", "your-biz-id", "your_biz_id", + "", "", "", + "{bizid}", "{biz-id}", "${bizid}", "${biz-id}": + return true + default: + return false + } +} + +// VerifyStreamingCardUpdate requires affirmative evidence that the requested +// write took effect. A transport-level {success:true,errorCode:null} only says +// that the RPC returned normally and is deliberately not accepted as proof of +// a card update. +func VerifyStreamingCardUpdate(requestedBizID string, response map[string]any) (string, error) { + requestedBizID = strings.TrimSpace(requestedBizID) + observation := cardUpdateObservation{bizIDs: map[string]struct{}{}} + observeCardUpdate(response, &observation) + + for responseBizID := range observation.bizIDs { + if requestedBizID != "" && responseBizID != requestedBizID { + return "", fmt.Errorf("%w: requested %q, response %q", ErrCardUpdateBizIDDrift, requestedBizID, responseBizID) + } + } + if observation.positiveEvidence != "" && observation.negativeEvidence != "" { + return "", fmt.Errorf("%w: conflicting evidence %s and %s", ErrCardUpdateUnverified, observation.positiveEvidence, observation.negativeEvidence) + } + if observation.positiveEvidence != "" { + return observation.positiveEvidence, nil + } + if observation.negativeEvidence != "" { + return "", fmt.Errorf("%w: %s", ErrCardUpdateNotApplied, observation.negativeEvidence) + } + return "", ErrCardUpdateUnverified +} + +type cardUpdateObservation struct { + bizIDs map[string]struct{} + positiveEvidence string + negativeEvidence string +} + +func observeCardUpdate(value any, observation *cardUpdateObservation) { + switch typed := value.(type) { + case map[string]any: + observeCardUpdateMap(typed, observation) + case []any: + for _, child := range typed { + observeCardUpdate(child, observation) + } + case bool: + if typed { + setPositiveCardUpdateEvidence(observation, "result=true") + } else { + setNegativeCardUpdateEvidence(observation, "result=false") + } + } +} + +func observeCardUpdateMap(value map[string]any, observation *cardUpdateObservation) { + for _, key := range []string{"bizId", "bizID", "biz_id"} { + if candidate, ok := value[key].(string); ok && strings.TrimSpace(candidate) != "" { + observation.bizIDs[strings.TrimSpace(candidate)] = struct{}{} + } + } + + for _, key := range []string{"updated", "applied"} { + if applied, ok := value[key].(bool); ok { + if applied { + setPositiveCardUpdateEvidence(observation, key+"=true") + } else { + setNegativeCardUpdateEvidence(observation, key+"=false") + } + } + } + for _, key := range []string{"affectedCount", "updatedCount", "modifiedCount"} { + if count, ok := cardUpdateCount(value[key]); ok { + if count > 0 { + setPositiveCardUpdateEvidence(observation, fmt.Sprintf("%s=%d", key, count)) + } else if count == 0 { + setNegativeCardUpdateEvidence(observation, fmt.Sprintf("%s=%d", key, count)) + } + } + } + + // Only documented response envelopes are traversed. This prevents an + // unrelated extension field containing "updated":true from proving the + // business write. + for _, key := range []string{"result", "data", "response", "card"} { + if child, exists := value[key]; exists { + observeCardUpdate(child, observation) + } + } +} + +func setPositiveCardUpdateEvidence(observation *cardUpdateObservation, evidence string) { + if observation.positiveEvidence == "" { + observation.positiveEvidence = evidence + } +} + +func setNegativeCardUpdateEvidence(observation *cardUpdateObservation, evidence string) { + if observation.negativeEvidence == "" { + observation.negativeEvidence = evidence + } +} + +func cardUpdateCount(value any) (int64, bool) { + switch typed := value.(type) { + case int: + return int64(typed), true + case int32: + return int64(typed), true + case int64: + return typed, true + case float32: + return int64(typed), float32(int64(typed)) == typed + case float64: + return int64(typed), float64(int64(typed)) == typed + case json.Number: + count, err := typed.Int64() + return count, err == nil + default: + return 0, false + } +} diff --git a/internal/shortcut/chatmsg/card_update_test.go b/internal/shortcut/chatmsg/card_update_test.go new file mode 100644 index 00000000..0a425d08 --- /dev/null +++ b/internal/shortcut/chatmsg/card_update_test.go @@ -0,0 +1,79 @@ +// 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 chatmsg + +import ( + "errors" + "testing" +) + +func TestCrossPlatformCoverageNormalizeCardBizID(t *testing.T) { + for _, test := range []struct { + name string + raw string + want string + wantErr bool + }{ + {name: "opaque id", raw: " card-token-1 ", want: "card-token-1"}, + {name: "opaque unicode remains server owned", raw: "中文乱串", want: "中文乱串"}, + {name: "empty", raw: " ", wantErr: true}, + {name: "placeholder", raw: "", wantErr: true}, + {name: "internal space", raw: "card token", wantErr: true}, + {name: "control", raw: "card\ntoken", wantErr: true}, + } { + t.Run(test.name, func(t *testing.T) { + got, err := NormalizeCardBizID(test.raw) + if test.wantErr { + if err == nil { + t.Fatalf("NormalizeCardBizID(%q) unexpectedly succeeded", test.raw) + } + return + } + if err != nil || got != test.want { + t.Fatalf("NormalizeCardBizID(%q) = %q, %v; want %q", test.raw, got, err, test.want) + } + }) + } +} + +func TestCrossPlatformCoverageVerifyStreamingCardUpdate(t *testing.T) { + for _, test := range []struct { + name string + response map[string]any + wantProof string + wantErrIs error + }{ + {name: "updated", response: map[string]any{"result": map[string]any{"updated": true}}, wantProof: "updated=true"}, + {name: "affected", response: map[string]any{"data": map[string]any{"affectedCount": float64(1)}}, wantProof: "affectedCount=1"}, + {name: "boolean result", response: map[string]any{"result": true}, wantProof: "result=true"}, + {name: "matching id", response: map[string]any{"result": map[string]any{"bizId": "biz-1", "applied": true}}, wantProof: "applied=true"}, + {name: "false success has no write proof", response: map[string]any{"success": true, "errorCode": nil}, wantErrIs: ErrCardUpdateUnverified}, + {name: "explicitly not updated", response: map[string]any{"result": map[string]any{"updated": false}}, wantErrIs: ErrCardUpdateNotApplied}, + {name: "mismatched id", response: map[string]any{"result": map[string]any{"bizId": "biz-2", "updated": true}}, wantErrIs: ErrCardUpdateBizIDDrift}, + {name: "unrelated extension ignored", response: map[string]any{"extension": map[string]any{"updated": true}}, wantErrIs: ErrCardUpdateUnverified}, + } { + t.Run(test.name, func(t *testing.T) { + proof, err := VerifyStreamingCardUpdate("biz-1", test.response) + if test.wantErrIs != nil { + if !errors.Is(err, test.wantErrIs) { + t.Fatalf("VerifyStreamingCardUpdate error = %v, want errors.Is(_, %v)", err, test.wantErrIs) + } + return + } + if err != nil || proof != test.wantProof { + t.Fatalf("VerifyStreamingCardUpdate = %q, %v; want %q", proof, err, test.wantProof) + } + }) + } +} diff --git a/internal/shortcut/chatmsg/chatmsg.go b/internal/shortcut/chatmsg/chatmsg.go index d2d6ee04..152558ba 100644 --- a/internal/shortcut/chatmsg/chatmsg.go +++ b/internal/shortcut/chatmsg/chatmsg.go @@ -86,6 +86,7 @@ var messageResultContractV1 = MessageResultContract{ "failedCount", "failures", "partial", + "scope", "resourceDownloads", }, } diff --git a/internal/shortcut/chatmsg/search.go b/internal/shortcut/chatmsg/search.go new file mode 100644 index 00000000..a57de031 --- /dev/null +++ b/internal/shortcut/chatmsg/search.go @@ -0,0 +1,192 @@ +// 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 chatmsg + +import ( + "fmt" + "strings" +) + +// SearchItems locates and flattens the message list returned by the two +// DingTalk message-search interfaces. Grouped search responses carry the +// conversation identity on the group rather than each message, so the +// flattener copies that identity onto every returned message before callers +// perform scope checks. +func SearchItems(data map[string]any) []map[string]any { + if data == nil { + return nil + } + for _, root := range []map[string]any{data, childMap(data, "result")} { + if root == nil { + continue + } + if groups, ok := root["conversationMessagesList"].([]any); ok { + return flattenSearchGroups(groups) + } + } + keys := []string{"list", "messages", "messageList", "items", "data", "records", "result"} + for _, key := range keys { + if arr, ok := data[key].([]any); ok { + return searchMaps(arr) + } + if inner, ok := data[key].(map[string]any); ok { + for _, innerKey := range []string{"list", "messages", "messageList", "items", "data", "records"} { + if arr, ok := inner[innerKey].([]any); ok { + return searchMaps(arr) + } + } + } + } + return nil +} + +func childMap(data map[string]any, key string) map[string]any { + if value, ok := data[key].(map[string]any); ok { + return value + } + return nil +} + +func flattenSearchGroups(groups []any) []map[string]any { + out := make([]map[string]any, 0) + for _, rawGroup := range groups { + group, ok := rawGroup.(map[string]any) + if !ok { + continue + } + messages, ok := group["messages"].([]any) + if !ok { + continue + } + conversationID := cleanSearchScalar(group["openConversationId"]) + conversationTitle := cleanSearchScalar(group["title"]) + singleChat, hasSingleChat := group["singleChat"] + for _, rawMessage := range messages { + message, ok := rawMessage.(map[string]any) + if !ok { + continue + } + item := make(map[string]any, len(message)+3) + for key, value := range message { + item[key] = value + } + if _, exists := item["openConversationId"]; !exists && conversationID != "" { + item["openConversationId"] = conversationID + } + if _, exists := item["conversationTitle"]; !exists && conversationTitle != "" { + item["conversationTitle"] = conversationTitle + } + if _, exists := item["singleChat"]; !exists && hasSingleChat { + item["singleChat"] = singleChat + } + out = append(out, item) + } + } + return out +} + +func searchMaps(items []any) []map[string]any { + out := make([]map[string]any, 0, len(items)) + for _, item := range items { + if message, ok := item.(map[string]any); ok { + out = append(out, message) + } + } + return out +} + +// FilterConversationScope keeps only messages belonging to the explicitly +// requested conversations. A message without a conversation identity is +// reported as unverifiable rather than treated as in-scope. +func FilterConversationScope(messages []map[string]any, conversationIDs []string) (matched []map[string]any, unverifiableMessageIDs []string) { + requested := make(map[string]struct{}, len(conversationIDs)) + for _, conversationID := range conversationIDs { + if value := strings.TrimSpace(conversationID); value != "" { + requested[value] = struct{}{} + } + } + matched = make([]map[string]any, 0, len(messages)) + for _, message := range messages { + conversationID := cleanSearchScalar(ConversationID(message)) + if conversationID == "" { + messageID := cleanSearchScalar(MessageID(message)) + if messageID == "" { + messageID = "" + } + unverifiableMessageIDs = append(unverifiableMessageIDs, messageID) + continue + } + if _, ok := requested[conversationID]; ok { + matched = append(matched, message) + } + } + return matched, uniqueStrings(unverifiableMessageIDs) +} + +// GroupSearchMessages restores the established typed search envelope after a +// client-side scoped scan. Group order follows first occurrence in the search +// result, and each message retains its original fields. +func GroupSearchMessages(messages []map[string]any) []map[string]any { + groups := make([]map[string]any, 0) + index := make(map[string]int) + for _, message := range messages { + conversationID := cleanSearchScalar(ConversationID(message)) + if conversationID == "" { + continue + } + groupIndex, ok := index[conversationID] + if !ok { + group := map[string]any{ + "openConversationId": conversationID, + "messages": []map[string]any{}, + } + if title := cleanSearchScalar(message["conversationTitle"]); title != "" { + group["title"] = title + } + if singleChat, exists := message["singleChat"]; exists { + group["singleChat"] = singleChat + } + groups = append(groups, group) + groupIndex = len(groups) - 1 + index[conversationID] = groupIndex + } + groupMessages := groups[groupIndex]["messages"].([]map[string]any) + groups[groupIndex]["messages"] = append(groupMessages, message) + } + return groups +} + +func cleanSearchScalar(value any) string { + if value == nil { + return "" + } + text := strings.TrimSpace(fmt.Sprint(value)) + if text == "" || text == "" || strings.EqualFold(text, "null") { + return "" + } + return text +} + +func uniqueStrings(values []string) []string { + out := make([]string, 0, len(values)) + seen := make(map[string]struct{}, len(values)) + for _, value := range values { + if _, ok := seen[value]; ok { + continue + } + seen[value] = struct{}{} + out = append(out, value) + } + return out +} diff --git a/internal/shortcut/chatmsg/search_test.go b/internal/shortcut/chatmsg/search_test.go new file mode 100644 index 00000000..28856bfb --- /dev/null +++ b/internal/shortcut/chatmsg/search_test.go @@ -0,0 +1,75 @@ +// 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 chatmsg + +import ( + "reflect" + "testing" +) + +func TestSearchItemsCarriesGroupedConversationIdentity(t *testing.T) { + data := map[string]any{ + "result": map[string]any{ + "conversationMessagesList": []any{ + map[string]any{ + "openConversationId": "cid-group", + "title": "项目群", + "singleChat": false, + "messages": []any{ + map[string]any{"openMessageId": "m1", "content": "hello"}, + }, + }, + }, + }, + } + + items := SearchItems(data) + if len(items) != 1 || items[0]["openConversationId"] != "cid-group" || + items[0]["conversationTitle"] != "项目群" || items[0]["singleChat"] != false { + t.Fatalf("items = %#v", items) + } +} + +func TestFilterConversationScopeDropsOtherConversationsAndRejectsMissingIdentity(t *testing.T) { + messages := []map[string]any{ + {"openMessageId": "m1", "openConversationId": "cid-target"}, + {"openMessageId": "m2", "openConversationId": "cid-other"}, + {"openMessageId": "m3"}, + } + + matched, missing := FilterConversationScope(messages, []string{"cid-target"}) + if len(matched) != 1 || matched[0]["openMessageId"] != "m1" { + t.Fatalf("matched = %#v", matched) + } + if !reflect.DeepEqual(missing, []string{"m3"}) { + t.Fatalf("missing = %#v", missing) + } +} + +func TestGroupSearchMessagesPreservesFirstSeenOrder(t *testing.T) { + messages := []map[string]any{ + {"openMessageId": "m1", "openConversationId": "cid-b", "conversationTitle": "B"}, + {"openMessageId": "m2", "openConversationId": "cid-a", "conversationTitle": "A"}, + {"openMessageId": "m3", "openConversationId": "cid-b", "conversationTitle": "B"}, + } + + groups := GroupSearchMessages(messages) + if len(groups) != 2 || groups[0]["openConversationId"] != "cid-b" || groups[1]["openConversationId"] != "cid-a" { + t.Fatalf("groups = %#v", groups) + } + groupMessages, _ := groups[0]["messages"].([]map[string]any) + if len(groupMessages) != 2 || groupMessages[1]["openMessageId"] != "m3" { + t.Fatalf("group messages = %#v", groupMessages) + } +} diff --git a/internal/shortcut/smart/compatibility_coverage_test.go b/internal/shortcut/smart/compatibility_coverage_test.go index 175eab7f..fcd5c1c6 100644 --- a/internal/shortcut/smart/compatibility_coverage_test.go +++ b/internal/shortcut/smart/compatibility_coverage_test.go @@ -65,6 +65,8 @@ func (f *platformCoverageCaller) CallTool(_ context.Context, product, tool strin text = `{"result":{"userId":"u1"}}` case "im/search_groups": text = `{"result":[{"openConversationId":"cid-1","title":"项目冲刺"}]}` + case "im/search_messages": + text = `{"result":{"messages":[],"hasMore":false}}` } return &edition.ToolResult{Content: []edition.ContentBlock{{Type: "text", Text: text}}}, nil } @@ -393,7 +395,8 @@ func TestCrossPlatformCoverageCompatibilityAliases(t *testing.T) { argv: []string{"chat", "+search-msg", "--id", "cid-1", "--keyword", "树莓派", "--no-enrich", "--yes"}, wantProduct: "im", wantTool: "search_messages", - wantArgs: map[string]any{"openConversationIds": []string{"cid-1"}, "keyword": "树莓派"}, + wantArgs: map[string]any{"keyword": "树莓派"}, + wantAbsent: []string{"openConversationIds"}, }, } diff --git a/internal/shortcut/smart/natural_target_resolution_test.go b/internal/shortcut/smart/natural_target_resolution_test.go index 7e3419b0..63dbf332 100644 --- a/internal/shortcut/smart/natural_target_resolution_test.go +++ b/internal/shortcut/smart/natural_target_resolution_test.go @@ -157,15 +157,18 @@ func TestCrossPlatformCoverageSearchMsgResolvesNaturalChatAndSenderBeforeSearch( if err := root.Execute(); err != nil { t.Fatal(err) } - if len(fake.calls) != 3 { - t.Fatalf("calls = %#v, want chat resolve + user resolve + search", fake.calls) + if len(fake.calls) != 4 { + t.Fatalf("calls = %#v, want chat resolve + user resolve + scope validation + search", fake.calls) } - search := fake.calls[2] + if preflight := fake.calls[2]; preflight.product != "chat" || preflight.tool != "get_conversation_info" || preflight.args["openConversationId"] != "cid-1" { + t.Fatalf("scope preflight = %#v", preflight) + } + search := fake.calls[3] if search.product != "im" || search.tool != "search_messages" { t.Fatalf("search = %#v", search) } - if got, want := search.args["openConversationIds"], []string{"cid-1"}; !reflect.DeepEqual(got, want) { - t.Fatalf("openConversationIds = %#v, want %#v", got, want) + if _, exists := search.args["openConversationIds"]; exists { + t.Fatalf("global fallback unexpectedly forwarded openConversationIds: %#v", search.args) } if got, want := search.args["senderOpenDingTakIds"], []string{"D1"}; !reflect.DeepEqual(got, want) { t.Fatalf("senderOpenDingTakIds = %#v, want %#v", got, want) @@ -185,14 +188,17 @@ func TestCrossPlatformCoverageSearchMsgAcceptsStableIDInChatQuery(t *testing.T) if err := root.Execute(); err != nil { t.Fatal(err) } - if len(fake.calls) != 1 || fake.calls[0].tool != "search_messages" { + if len(fake.calls) != 2 || fake.calls[0].tool != "get_conversation_info" || fake.calls[1].tool != "search_messages" { t.Fatalf("calls = %#v", fake.calls) } - if got, want := fake.calls[0].args["openConversationIds"], []string{"cid-fixture-chat-0002"}; !reflect.DeepEqual(got, want) { - t.Fatalf("openConversationIds = %#v, want %#v", got, want) + if fake.calls[0].args["openConversationId"] != "cid-fixture-chat-0002" { + t.Fatalf("scope preflight = %#v", fake.calls[0]) } - if fake.calls[0].args["keyword"] != "评测" { - t.Fatalf("keyword = %#v", fake.calls[0].args["keyword"]) + if _, exists := fake.calls[1].args["openConversationIds"]; exists { + t.Fatalf("global fallback unexpectedly forwarded openConversationIds: %#v", fake.calls[1].args) + } + if fake.calls[1].args["keyword"] != "评测" { + t.Fatalf("keyword = %#v", fake.calls[1].args["keyword"]) } } diff --git a/internal/shortcut/smart/search_msg.go b/internal/shortcut/smart/search_msg.go index f9ffae27..faea73c5 100644 --- a/internal/shortcut/smart/search_msg.go +++ b/internal/shortcut/smart/search_msg.go @@ -14,6 +14,7 @@ package smart import ( + "errors" "fmt" "strconv" "strings" @@ -23,6 +24,7 @@ import ( "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/corecmd/contract" apperrors "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/errors" + "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/helpers" "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/shortcut" chatshortcut "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/shortcut/chat" "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/shortcut/chatmsg" @@ -38,8 +40,9 @@ var SearchMsg = shortcut.Shortcut{ Service: "chat", Command: "+search-msg", Product: "im", - Description: "按稳定 ID 和内容等条件跨会话搜索消息，可全量翻页并批量富化", + Description: "按稳定 ID 和内容等条件跨会话搜索消息，可校验会话范围、全量翻页并批量富化", Intent: "当你要按关键词、发送者、@对象、会话、消息类型或机器人来源组合搜索 IM 消息时使用；会话与发送者过滤使用稳定 ID。默认查询近 7 天，也可指定精确起止时间。" + + "显式指定会话时会先验证 CID，再执行有界全局扫描并在本地精确过滤，避免下层忽略非法 CID 或群聊 CID。" + "--page-all 会连续拉取游标页，默认再按消息 ID 分批富化详情；任何续页或富化失败都会保留已取得结果并返回逐项失败 ledger，绝不把截断结果标成完整。" + "--download-resources 使用安全本地路径、默认不覆盖和原子落盘。", Risk: shortcut.RiskRead, @@ -55,15 +58,15 @@ var SearchMsg = shortcut.Shortcut{ CLIPath: "chat +search-msg", PrimaryCLIPath: "chat +search-msg", }, - Description: "按稳定 ID 和内容等条件跨会话搜索消息，可全量翻页并批量富化", + Description: "按稳定 ID 和内容等条件跨会话搜索消息，可校验会话范围、全量翻页并批量富化", Interface: &contract.InterfaceSpec{ Mode: "composite", Availability: "available", Reason: "Reviewed search adapter: it combines filters, cursor pagination, batched mget enrichment, stable projection, completeness accounting, and optional safe resource downloads.", }, Selection: contract.SelectionSpec{ - AgentSummary: "按稳定 ID 和内容等条件跨会话搜索消息，可全量翻页并批量富化", - UseWhen: []string{"当你要按关键词、发送者、@对象、会话、消息类型或机器人来源组合搜索 IM 消息时使用；会话与发送者过滤使用稳定 ID。默认查询近 7 天，也可指定精确起止时间。--page-all 会连续拉取游标页，默认再按消息 ID 分批富化详情；任何续页或富化失败都会保留已取得结果并返回逐项失败 ledger，绝不把截断结果标成完整。--download-resources 使用安全本地路径、默认不覆盖和原子落盘。"}, + AgentSummary: "按稳定 ID 和内容等条件跨会话搜索消息，可校验会话范围、全量翻页并批量富化", + UseWhen: []string{"当你要按关键词、发送者、@对象、会话、消息类型或机器人来源组合搜索 IM 消息时使用；显式会话范围会先验证 CID，再有界扫描并在本地精确过滤。默认查询近 7 天，也可指定精确起止时间。--page-all 会连续拉取游标页，默认再按消息 ID 分批富化详情；任何续页或富化失败都会保留已取得结果并返回逐项失败 ledger，绝不把截断结果标成完整。--download-resources 使用安全本地路径、默认不覆盖和原子落盘。"}, AvoidWhen: []string{"只想读取一个已知会话的连续历史时使用 +chat-messages；已有精确消息 ID 时使用 +messages-mget"}, Examples: []string{ "dws chat +search-msg --query \"周报\" --senders --days 3 --page-all", @@ -100,7 +103,7 @@ var SearchMsg = shortcut.Shortcut{ {Name: "cursor", Type: shortcut.FlagString, Desc: "分页游标，翻页传上次的 nextCursor", Default: "0"}, {Name: "page-token", Type: shortcut.FlagString, Desc: "--cursor 的 lark-cli 对齐别名"}, {Name: "page-all", Type: shortcut.FlagBool, Desc: "自动连续拉取所有游标页"}, - {Name: "page-limit", Type: shortcut.FlagInt, Desc: "--page-all 的最大页数（1-40）", Default: "20"}, + {Name: "page-limit", Type: shortcut.FlagInt, Desc: "--page-all 或显式会话范围本地扫描的最大页数（1-40）", Default: "20"}, {Name: "no-enrich", Type: shortcut.FlagBool, Desc: "不再按消息 ID 批量查询完整详情"}, {Name: "no-reactions", Type: shortcut.FlagBool, Desc: "不输出命中消息的 reaction（默认输出）"}, }, chatshortcut.MessageResourceDownloadFlags()...), @@ -138,9 +141,21 @@ var SearchMsg = shortcut.Shortcut{ if err != nil { return err } + requestedConversationIDs, _ := params["openConversationIds"].([]string) + scopedSearch := len(requestedConversationIDs) > 0 + if scopedSearch { + if err := validateSearchConversationScope(rt, requestedConversationIDs); err != nil { + return err + } + // The downstream search currently drops invalid CID filters and does + // not return group-scoped hits reliably. Scan the same filtered global + // stream and apply the already-validated CID set locally instead. + delete(params, "openConversationIds") + } pageLimit := 1 - if rt.Bool("page-all") { + scanAllPages := rt.Bool("page-all") || scopedSearch + if scanAllPages { pageLimit = rt.Int("page-limit") } cursor := rt.StrFirst("page-token", "cursor") @@ -169,7 +184,15 @@ var SearchMsg = shortcut.Shortcut{ break } pagesFetched++ - for _, message := range searchMsgItems(data) { + pageMessages := searchMsgItems(data) + if scopedSearch { + var unverifiableMessageIDs []string + pageMessages, unverifiableMessageIDs = chatmsg.FilterConversationScope(pageMessages, requestedConversationIDs) + if len(unverifiableMessageIDs) > 0 { + return searchScopeUnverifiedError(requestedConversationIDs, unverifiableMessageIDs) + } + } + for _, message := range pageMessages { messageID := strings.TrimSpace(fmt.Sprint(searchMsgMessageID(message))) if messageID != "" && messageID != "" { if seen[messageID] { @@ -197,7 +220,7 @@ var SearchMsg = shortcut.Shortcut{ } } hasMore = hasMoreValue - if !rt.Bool("page-all") || !hasMore { + if !scanAllPages || !hasMore { complete = !hasMore break } @@ -211,7 +234,7 @@ var SearchMsg = shortcut.Shortcut{ } cursor = nextCursor } - if rt.Bool("page-all") && hasMore && pagesFetched == pageLimit { + if scanAllPages && hasMore && pagesFetched == pageLimit { failures = append(failures, map[string]any{ "stage": "search-page-limit", "error": fmt.Sprintf("达到 --page-limit=%d，仍有更多结果", pageLimit), @@ -228,6 +251,16 @@ var SearchMsg = shortcut.Shortcut{ complete = false } } + if scopedSearch { + validatedMessages, unverifiableMessageIDs := chatmsg.FilterConversationScope(messages, requestedConversationIDs) + if len(unverifiableMessageIDs) > 0 { + return searchScopeUnverifiedError(requestedConversationIDs, unverifiableMessageIDs) + } + if len(validatedMessages) != len(messages) { + return searchScopeViolationError(requestedConversationIDs, messages) + } + messages = validatedMessages + } results := make([]map[string]any, 0, len(messages)) for _, m := range messages { @@ -246,6 +279,9 @@ var SearchMsg = shortcut.Shortcut{ "failedCount": len(failures), "failures": failures, } + if scopedSearch { + payload["scope"] = searchScopePayload(requestedConversationIDs, paginationKnown && !hasMore) + } if hasMore && nextCursor != "" && nextCursor != "" { payload["nextCursor"] = nextCursor } @@ -402,6 +438,75 @@ func uniqueSearchStrings(values []string) []string { return out } +func validateSearchConversationScope(rt *shortcut.RuntimeContext, conversationIDs []string) error { + for _, conversationID := range conversationIDs { + _, err := rt.CallMCPData("chat", "get_conversation_info", map[string]any{ + "openConversationId": conversationID, + }) + if err == nil { + continue + } + var cliErr *helpers.CLIError + if !errors.As(err, &cliErr) || (cliErr.Code != helpers.CodeMCPToolError && cliErr.Code != helpers.CodeResourceNotFound) { + return err + } + return apperrors.NewValidation( + fmt.Sprintf("无法验证会话 CID %q；已停止搜索，避免过滤失效后返回其他会话消息", conversationID), + apperrors.WithReason("search_conversation_scope_invalid"), + apperrors.WithDetails(map[string]any{ + "conversationId": conversationID, + }), + apperrors.WithHint("确认 openConversationId 存在且当前账号可访问后重试"), + apperrors.WithCause(err), + ) + } + return nil +} + +func searchScopeUnverifiedError(conversationIDs, messageIDs []string) error { + return apperrors.NewAPI( + "搜索结果缺少 conversationId，无法证明会话过滤范围；已停止输出", + apperrors.WithReason("search_conversation_scope_unverified"), + apperrors.WithDetails(map[string]any{ + "requestedConversationIds": conversationIDs, + "unverifiableMessageIds": messageIDs, + }), + apperrors.WithRetryable(false), + apperrors.WithHint("请保留 trace_id 并检查 IM 搜索服务是否返回 openConversationId"), + ) +} + +func searchScopeViolationError(conversationIDs []string, messages []map[string]any) error { + observed := make([]string, 0, len(messages)) + for _, message := range messages { + conversationID := strings.TrimSpace(fmt.Sprint(chatmsg.ConversationID(message))) + if conversationID == "" || conversationID == "" { + continue + } + observed = append(observed, conversationID) + } + return apperrors.NewAPI( + "消息富化结果超出请求的会话范围；已停止输出", + apperrors.WithReason("search_conversation_scope_violation"), + apperrors.WithDetails(map[string]any{ + "requestedConversationIds": conversationIDs, + "observedConversationIds": uniqueSearchStrings(observed), + }), + apperrors.WithRetryable(false), + ) +} + +func searchScopePayload(conversationIDs []string, sourceComplete bool) map[string]any { + return map[string]any{ + "requestedConversationIds": append([]string(nil), conversationIDs...), + "targetsValidated": true, + "filterApplied": true, + "filterMode": "client", + "resultsWithinScope": true, + "sourceComplete": sourceComplete, + } +} + func enrichSearchMessages(rt *shortcut.RuntimeContext, messages []map[string]any) ([]map[string]any, int, []map[string]any) { detailsByID := map[string]map[string]any{} failures := make([]map[string]any, 0) @@ -475,31 +580,7 @@ func enrichSearchMessages(rt *shortcut.RuntimeContext, messages []map[string]any // response, probing common container keys at the top level and nested under // "result". Returns nil when no list is found. func searchMsgItems(data map[string]any) []map[string]any { - if data == nil { - return nil - } - for _, root := range []map[string]any{data, searchMsgChildMap(data, "result")} { - if root == nil { - continue - } - if groups, ok := root["conversationMessagesList"].([]any); ok { - return searchMsgFlattenGroups(groups) - } - } - keys := []string{"list", "messages", "messageList", "items", "data", "records", "result"} - for _, key := range keys { - if arr, ok := data[key].([]any); ok { - return searchMsgToMaps(arr) - } - if inner, ok := data[key].(map[string]any); ok { - for _, k2 := range []string{"list", "messages", "messageList", "items", "data", "records"} { - if arr, ok := inner[k2].([]any); ok { - return searchMsgToMaps(arr) - } - } - } - } - return nil + return chatmsg.SearchItems(data) } func searchMsgChildMap(data map[string]any, key string) map[string]any { @@ -509,56 +590,6 @@ func searchMsgChildMap(data map[string]any, key string) map[string]any { return nil } -func searchMsgFlattenGroups(groups []any) []map[string]any { - out := make([]map[string]any, 0) - for _, rawGroup := range groups { - group, ok := rawGroup.(map[string]any) - if !ok { - continue - } - messages, ok := group["messages"].([]any) - if !ok { - continue - } - conversationID := strings.TrimSpace(fmt.Sprint(group["openConversationId"])) - conversationTitle := strings.TrimSpace(fmt.Sprint(group["title"])) - singleChat, hasSingleChat := group["singleChat"] - for _, rawMessage := range messages { - message, ok := rawMessage.(map[string]any) - if !ok { - continue - } - item := make(map[string]any, len(message)+3) - for key, value := range message { - item[key] = value - } - if _, exists := item["openConversationId"]; !exists && - conversationID != "" && conversationID != "" { - item["openConversationId"] = conversationID - } - if _, exists := item["conversationTitle"]; !exists && - conversationTitle != "" && conversationTitle != "" { - item["conversationTitle"] = conversationTitle - } - if _, exists := item["singleChat"]; !exists && hasSingleChat { - item["singleChat"] = singleChat - } - out = append(out, item) - } - } - return out -} - -func searchMsgToMaps(arr []any) []map[string]any { - out := make([]map[string]any, 0, len(arr)) - for _, it := range arr { - if m, ok := it.(map[string]any); ok { - out = append(out, m) - } - } - return out -} - // searchMsgProject reshapes one matched message into {sender, time, text, // messageId}, running text through the shared chatmsg cleaning (card/auto-reply // JSON → readable, ciphertext → marker) and recursively expanding any forwarded diff --git a/internal/shortcut/smart/search_msg_execution_test.go b/internal/shortcut/smart/search_msg_execution_test.go index e7a66b6f..7e331c79 100644 --- a/internal/shortcut/smart/search_msg_execution_test.go +++ b/internal/shortcut/smart/search_msg_execution_test.go @@ -12,6 +12,7 @@ import ( "reflect" "testing" + apperrors "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/errors" "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/helpers" "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/pkg/edition" ) @@ -22,29 +23,44 @@ type searchMsgExecutionCaller struct { failEnrichment bool omitPagination bool omitMgetItem bool + failPreflight bool + searchResponse string + wrongMgetScope bool } func (f *searchMsgExecutionCaller) CallTool(_ context.Context, product, tool string, args map[string]any) (*edition.ToolResult, error) { f.calls = append(f.calls, platformCoverageCall{product: product, tool: tool, args: args}) + if product == "chat" && tool == "get_conversation_info" { + if f.failPreflight { + return nil, errors.New("conversation not found") + } + return searchMsgToolResult(`{"result":{"openConversationId":"` + args["openConversationId"].(string) + `"}}`), nil + } if product != "im" { return nil, errors.New("unexpected product") } switch tool { case "search_messages": + if f.searchResponse != "" { + return searchMsgToolResult(f.searchResponse), nil + } if f.omitPagination { - return searchMsgToolResult(`{"result":{"messages":[{"openMessageId":"m1","content":"sparse-1"}]}}`), nil + return searchMsgToolResult(`{"result":{"messages":[{"openMessageId":"m1","openConversationId":"cid-1","content":"sparse-1"}]}}`), nil } if args["cursor"] == "c2" { if f.failSecondPage { return nil, errors.New("second page unavailable") } - return searchMsgToolResult(`{"result":{"messages":[{"openMessageId":"m2","content":"sparse-2"}],"hasMore":false}}`), nil + return searchMsgToolResult(`{"result":{"messages":[{"openMessageId":"m2","openConversationId":"cid-2","content":"sparse-2"}],"hasMore":false}}`), nil } - return searchMsgToolResult(`{"result":{"messages":[{"openMessageId":"m1","content":"sparse-1"}],"hasMore":true,"nextCursor":"c2"}}`), nil + return searchMsgToolResult(`{"result":{"messages":[{"openMessageId":"m1","openConversationId":"cid-1","content":"sparse-1"}],"hasMore":true,"nextCursor":"c2"}}`), nil case "list_messages_by_ids": if f.failEnrichment { return nil, errors.New("mget unavailable") } + if f.wrongMgetScope { + return searchMsgToolResult(`{"result":[{"openMessageId":"m1","openConversationId":"cid-other","content":"detail-1"}]}`), nil + } if f.omitMgetItem { return searchMsgToolResult(`{"result":[{"openMessageId":"m1","content":"detail-1"}]}`), nil } @@ -69,19 +85,27 @@ func searchMsgToolResult(text string) *edition.ToolResult { func executeSearchMsg(t *testing.T, caller *searchMsgExecutionCaller, args ...string) map[string]any { t.Helper() + payload, err := executeSearchMsgResult(caller, args...) + if err != nil { + t.Fatal(err) + } + return payload +} + +func executeSearchMsgResult(caller *searchMsgExecutionCaller, args ...string) (map[string]any, error) { helpers.InitDeps(caller) root := newPlatformCoverageRoot() var output bytes.Buffer root.SetOut(&output) root.SetArgs(append([]string{"chat", "+search-msg", "--yes"}, args...)) if err := root.Execute(); err != nil { - t.Fatal(err) + return nil, err } var payload map[string]any if err := json.Unmarshal(output.Bytes(), &payload); err != nil { - t.Fatalf("decode output: %v\n%s", err, output.String()) + return nil, err } - return payload + return payload, nil } func TestSearchMsgPagesAndEnrichesWithAdvancedFilters(t *testing.T) { @@ -103,15 +127,18 @@ func TestSearchMsgPagesAndEnrichesWithAdvancedFilters(t *testing.T) { "--page-limit", "3", ) - if len(caller.calls) != 3 { + if len(caller.calls) != 5 { t.Fatalf("calls = %#v", caller.calls) } - first := caller.calls[0] + if caller.calls[0].tool != "get_conversation_info" || caller.calls[0].args["openConversationId"] != "cid-1" || + caller.calls[1].tool != "get_conversation_info" || caller.calls[1].args["openConversationId"] != "cid-2" { + t.Fatalf("scope preflight calls = %#v", caller.calls[:2]) + } + first := caller.calls[2] if first.product != "im" || first.tool != "search_messages" { t.Fatalf("first call = %#v", first) } for key, want := range map[string]any{ - "openConversationIds": []string{"cid-1", "cid-2"}, "senderUserIds": []string{"42"}, "senderOpenDingTakIds": []string{"Dsender"}, "atUserIds": []string{"43"}, @@ -124,10 +151,13 @@ func TestSearchMsgPagesAndEnrichesWithAdvancedFilters(t *testing.T) { t.Errorf("%s = %#v, want %#v", key, first.args[key], want) } } - if caller.calls[1].args["cursor"] != "c2" { - t.Fatalf("second cursor = %#v", caller.calls[1].args["cursor"]) + if _, exists := first.args["openConversationIds"]; exists { + t.Fatalf("global fallback unexpectedly forwarded openConversationIds: %#v", first.args) } - if ids := caller.calls[2].args["openMsgIds"]; !reflect.DeepEqual(ids, []string{"m1", "m2"}) { + if caller.calls[3].args["cursor"] != "c2" { + t.Fatalf("second cursor = %#v", caller.calls[3].args["cursor"]) + } + if ids := caller.calls[4].args["openMsgIds"]; !reflect.DeepEqual(ids, []string{"m1", "m2"}) { t.Fatalf("mget ids = %#v", ids) } if payload["complete"] != true || payload["count"] != float64(2) || @@ -140,6 +170,10 @@ func TestSearchMsgPagesAndEnrichesWithAdvancedFilters(t *testing.T) { if firstMessage["text"] != "detail-1" { t.Fatalf("enriched message = %#v", firstMessage) } + scope, _ := payload["scope"].(map[string]any) + if scope["targetsValidated"] != true || scope["filterMode"] != "client" || scope["resultsWithinScope"] != true { + t.Fatalf("scope = %#v", scope) + } } func TestSearchMsgLaterPageFailurePublishesPartialLedger(t *testing.T) { @@ -199,3 +233,110 @@ func TestSearchMsgMissingMgetItemPublishesFailureLedger(t *testing.T) { t.Fatalf("failure = %#v", failure) } } + +func TestSearchMsgScopedFallbackFiltersGlobalResults(t *testing.T) { + caller := &searchMsgExecutionCaller{searchResponse: `{ + "result": { + "conversationMessagesList": [ + {"openConversationId":"cid-target","title":"目标群","messages":[{"openMessageId":"m-target","content":"目标"}]}, + {"openConversationId":"cid-other","title":"其他群","messages":[{"openMessageId":"m-other","content":"越界"}]} + ], + "hasMore": false + } + }`} + payload := executeSearchMsg(t, caller, "--group", "cid-target", "--query", "目标", "--no-enrich") + if payload["count"] != float64(1) || payload["complete"] != true { + t.Fatalf("payload = %#v", payload) + } + messages, _ := payload["messages"].([]any) + message, _ := messages[0].(map[string]any) + if message["conversationId"] != "cid-target" || message["messageId"] != "m-target" { + t.Fatalf("messages = %#v", messages) + } + if len(caller.calls) != 2 || caller.calls[0].tool != "get_conversation_info" || caller.calls[1].tool != "search_messages" { + t.Fatalf("calls = %#v", caller.calls) + } + if _, exists := caller.calls[1].args["openConversationIds"]; exists { + t.Fatalf("global fallback unexpectedly forwarded scope: %#v", caller.calls[1].args) + } +} + +func TestSearchMsgScopedValidEmptyResultIsComplete(t *testing.T) { + caller := &searchMsgExecutionCaller{searchResponse: `{"result":{"messages":[],"hasMore":false}}`} + payload := executeSearchMsg(t, caller, "--group", "cid-empty", "--query", "不存在", "--no-enrich") + if payload["count"] != float64(0) || payload["complete"] != true { + t.Fatalf("payload = %#v", payload) + } + scope, _ := payload["scope"].(map[string]any) + if scope["targetsValidated"] != true || scope["sourceComplete"] != true { + t.Fatalf("scope = %#v", scope) + } +} + +func TestSearchMsgScopedEmptyPartialScanCannotClaimComplete(t *testing.T) { + caller := &searchMsgExecutionCaller{searchResponse: `{ + "result": { + "conversationMessagesList": [ + {"openConversationId":"cid-other","messages":[{"openMessageId":"m-other"}]} + ], + "hasMore": true, + "nextCursor": "c2" + } + }`} + payload := executeSearchMsg(t, caller, + "--group", "cid-target", "--query", "周报", "--no-enrich", "--page-limit", "1") + if payload["count"] != float64(0) || payload["complete"] != false || payload["failedCount"] != float64(1) { + t.Fatalf("payload = %#v", payload) + } + scope, _ := payload["scope"].(map[string]any) + if scope["sourceComplete"] != false { + t.Fatalf("scope = %#v", scope) + } + failures, _ := payload["failures"].([]any) + failure, _ := failures[0].(map[string]any) + if failure["stage"] != "search-page-limit" { + t.Fatalf("failure = %#v", failure) + } +} + +func TestSearchMsgInvalidCIDStopsBeforeGlobalSearch(t *testing.T) { + caller := &searchMsgExecutionCaller{failPreflight: true} + _, err := executeSearchMsgResult(caller, "--group", "cid-invalid", "--query", "周报", "--no-enrich") + if err == nil { + t.Fatal("invalid CID unexpectedly succeeded") + } + var typed *apperrors.Error + if !errors.As(err, &typed) || typed.Reason != "search_conversation_scope_invalid" { + t.Fatalf("error = %#v", err) + } + if len(caller.calls) != 1 || caller.calls[0].tool != "get_conversation_info" { + t.Fatalf("calls = %#v", caller.calls) + } +} + +func TestSearchMsgMissingConversationIdentityFailsClosed(t *testing.T) { + caller := &searchMsgExecutionCaller{searchResponse: `{"result":{"messages":[{"openMessageId":"m1","content":"unknown"}],"hasMore":false}}`} + _, err := executeSearchMsgResult(caller, "--group", "cid-target", "--query", "周报", "--no-enrich") + if err == nil { + t.Fatal("unverifiable scoped result unexpectedly succeeded") + } + var typed *apperrors.Error + if !errors.As(err, &typed) || typed.Reason != "search_conversation_scope_unverified" { + t.Fatalf("error = %#v", err) + } +} + +func TestSearchMsgEnrichmentCannotMoveMessageOutsideScope(t *testing.T) { + caller := &searchMsgExecutionCaller{ + searchResponse: `{"result":{"messages":[{"openMessageId":"m1","openConversationId":"cid-target","content":"sparse"}],"hasMore":false}}`, + wrongMgetScope: true, + } + _, err := executeSearchMsgResult(caller, "--group", "cid-target", "--query", "周报") + if err == nil { + t.Fatal("out-of-scope enrichment unexpectedly succeeded") + } + var typed *apperrors.Error + if !errors.As(err, &typed) || typed.Reason != "search_conversation_scope_violation" { + t.Fatalf("error = %#v", err) + } +} From effe7c829ecbd95b37e7f16ce17c0965b3c07fc3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=85=8B=E8=B0=A8?= <2940480744@qq.com> Date: Fri, 7 Aug 2026 15:48:29 +0800 Subject: [PATCH 02/19] fix(chat): align message workflows and diagnostics --- internal/app/schema_safety_contract_test.go | 9 + internal/app/server_failure_classifier.go | 1 + .../app/server_failure_classifier_test.go | 3 + internal/errors/coverage_edges_test.go | 2 +- internal/errors/errors.go | 18 +- internal/errors/errors_test.go | 31 +++- internal/helpers/chat.go | 34 ++-- internal/helpers/chat_card_update_test.go | 5 + internal/helpers/chat_command_edges_test.go | 2 +- .../chat_confirmation_alignment_test.go | 66 +++++++ internal/helpers/chat_message_search_test.go | 5 +- internal/helpers/chat_user_settings_test.go | 4 +- internal/shortcut/chat/chat_message.go | 77 +++++++-- internal/shortcut/chat/gap_fill_test.go | 58 +++++++ internal/shortcut/chat/lark_alignment_test.go | 2 + internal/shortcut/chat/unified_send.go | 15 +- internal/shortcut/chatmsg/card_ref.go | 65 +++++++ internal/shortcut/chatmsg/card_ref_test.go | 42 +++++ internal/shortcut/chatmsg/send_status.go | 162 ++++++++++++++++++ internal/shortcut/chatmsg/send_status_test.go | 63 +++++++ internal/transport/client.go | 2 + internal/transport/coverage_edges_test.go | 4 +- internal/transport/diagnostics.go | 36 +++- internal/transport/diagnostics_test.go | 18 ++ 24 files changed, 677 insertions(+), 47 deletions(-) create mode 100644 internal/helpers/chat_confirmation_alignment_test.go create mode 100644 internal/shortcut/chatmsg/card_ref.go create mode 100644 internal/shortcut/chatmsg/card_ref_test.go create mode 100644 internal/shortcut/chatmsg/send_status.go create mode 100644 internal/shortcut/chatmsg/send_status_test.go diff --git a/internal/app/schema_safety_contract_test.go b/internal/app/schema_safety_contract_test.go index 43b0dbc4..f618ab89 100644 --- a/internal/app/schema_safety_contract_test.go +++ b/internal/app/schema_safety_contract_test.go @@ -25,7 +25,16 @@ func TestReviewedMutationSafetyReachesFinalSchema(t *testing.T) { const declared = "contract_final" wants := []finalSchemaSafetyWant{ {canonical: "aitable.form_field_hide", effect: "write", risk: "medium", confirmation: "not_required", idempotency: "idempotent", provenance: declared}, + {canonical: "chat.create_and_send_card", effect: "write", risk: "medium", confirmation: "user_required", idempotency: "unknown", provenance: declared}, {canonical: "chat.dismiss_group", effect: "destructive", risk: "high", confirmation: "user_required", idempotency: "unknown", provenance: declared}, + {canonical: "chat.send_message_by_custom_robot", effect: "write", risk: "medium", confirmation: "user_required", idempotency: "unknown", provenance: declared}, + {canonical: "chat.send_personal_message", effect: "write", risk: "medium", confirmation: "user_required", idempotency: "unknown", provenance: declared}, + {canonical: "chat.send_robot_message", effect: "write", risk: "medium", confirmation: "user_required", idempotency: "unknown", provenance: declared}, + {canonical: "chat.shortcut_messages_send", effect: "write", risk: "medium", confirmation: "user_required", idempotency: "unknown", provenance: declared}, + {canonical: "chat.shortcut_messages_send_by_webhook", effect: "write", risk: "medium", confirmation: "user_required", idempotency: "unknown", provenance: declared}, + {canonical: "chat.shortcut_messages_send_card", effect: "write", risk: "medium", confirmation: "user_required", idempotency: "unknown", provenance: declared}, + {canonical: "chat.shortcut_messages_update_card", effect: "write", risk: "medium", confirmation: "user_required", idempotency: "unknown", provenance: declared}, + {canonical: "chat.update_streaming_card", effect: "write", risk: "medium", confirmation: "user_required", idempotency: "unknown", provenance: declared}, {canonical: "drive.recycle_restore", effect: "write", risk: "medium", confirmation: "not_required", idempotency: "unknown", provenance: declared}, {canonical: "minutes.create_speaker_summary", effect: "write", risk: "medium", confirmation: "not_required", idempotency: "unknown", provenance: declared}, {canonical: "sheet.clear_range", effect: "write", risk: "medium", confirmation: "user_required", idempotency: "unknown", provenance: declared}, diff --git a/internal/app/server_failure_classifier.go b/internal/app/server_failure_classifier.go index 6ab9d8ea..978a06c1 100644 --- a/internal/app/server_failure_classifier.go +++ b/internal/app/server_failure_classifier.go @@ -81,6 +81,7 @@ func newServerFailureAPIError( apperrors.WithReason(fallbackReason), apperrors.WithServerKey(serverKey), apperrors.WithHint(fallbackHint), + apperrors.WithActions("运行 dws doctor 检查登录态、网络和本地环境；持续失败时保留 Trace ID 和 Server Code"), apperrors.WithServerDiag(diag), } if classified, ok := classifyServerFailure(message, diag); ok { diff --git a/internal/app/server_failure_classifier_test.go b/internal/app/server_failure_classifier_test.go index 489ecad0..df1a104d 100644 --- a/internal/app/server_failure_classifier_test.go +++ b/internal/app/server_failure_classifier_test.go @@ -95,6 +95,9 @@ func TestCrossPlatformCoverageServerFailureClassifierUnknownFallsBack(t *testing if typed.Reason != "business_error" || typed.Origin != "" || typed.FailureStage != "" || typed.ExecutionStarted != nil { t.Fatalf("unexpected fallback classification: %#v", typed) } + if len(typed.Actions) == 0 || !strings.Contains(typed.Actions[0], "dws doctor") { + t.Fatalf("fallback error has no stable troubleshooting entry: %#v", typed.Actions) + } } func TestCrossPlatformCoverageServerFailureReasonUsesTypedClassification(t *testing.T) { diff --git a/internal/errors/coverage_edges_test.go b/internal/errors/coverage_edges_test.go index dd3c425f..36c678b6 100644 --- a/internal/errors/coverage_edges_test.go +++ b/internal/errors/coverage_edges_test.go @@ -41,7 +41,7 @@ func TestCrossPlatformCoverageDiagnosticsAndErrorRenderingEdges(t *testing.T) { t.Fatalf("PrintJSON friendly diagnostics = %q, %v", out.String(), err) } out.Reset() - if err := PrintHumanAt(&out, err, VerbosityVerbose); err != nil || !strings.Contains(out.String(), "开启地址") { + if err := PrintHumanAt(&out, err, VerbosityVerbose); err != nil || !strings.Contains(out.String(), "处理入口") { t.Fatalf("PrintHuman friendly diagnostics = %q, %v", out.String(), err) } out.Reset() diff --git a/internal/errors/errors.go b/internal/errors/errors.go index cb24dd25..cf755dce 100644 --- a/internal/errors/errors.go +++ b/internal/errors/errors.go @@ -19,6 +19,7 @@ import ( stderrors "errors" "fmt" "io" + "net/url" "strings" "time" @@ -460,7 +461,7 @@ func PrintHumanAt(w io.Writer, err error, v Verbosity) error { lines = append(lines, tui.Cyan("Hint: "+friendlyHint)) } if actionURL != "" { - lines = append(lines, tui.White("Action: 开启地址: "+actionURL)) + lines = append(lines, tui.White("Action: 处理入口: "+actionURL)) } } @@ -540,7 +541,7 @@ func PrintHumanAt(w io.Writer, err error, v Verbosity) error { func serverGuidance(diag ServerDiagnostics) (string, string) { friendlyHint := strings.TrimSpace(diag.FriendlyHint) - actionURL := strings.TrimSpace(diag.ActionURL) + actionURL := safeServerActionURL(diag.ActionURL) if friendlyHint == "" || actionURL == "" { switch diag.ServerErrorCode { case "TOKEN_VERIFIED_FAILED", "CLI_ORG_NOT_AUTHORIZED": @@ -555,6 +556,19 @@ func serverGuidance(diag ServerDiagnostics) (string, string) { return friendlyHint, actionURL } +func safeServerActionURL(raw string) string { + raw = strings.TrimSpace(raw) + if raw == "" { + return "" + } + parsed, err := url.Parse(raw) + if err != nil || !strings.EqualFold(parsed.Scheme, "https") || + parsed.Hostname() == "" || parsed.User != nil { + return "" + } + return parsed.String() +} + func category(err error) string { var typed *Error if stderrors.As(err, &typed) { diff --git a/internal/errors/errors_test.go b/internal/errors/errors_test.go index d25e223f..96fd4636 100644 --- a/internal/errors/errors_test.go +++ b/internal/errors/errors_test.go @@ -336,11 +336,40 @@ func TestCrossPlatformCoveragePrintHumanIncludesServerGuidance(t *testing.T) { if !strings.Contains(got, "Hint: 请联系管理员开通消息搜索权益") { t.Fatalf("expected server guidance in output, got %q", got) } - if !strings.Contains(got, "Action: 开启地址: https://example.test/enable-search") { + if !strings.Contains(got, "Action: 处理入口: https://example.test/enable-search") { t.Fatalf("expected server action URL in output, got %q", got) } } +func TestCrossPlatformCoverageServerGuidanceSuppressesUnsafeActionURL(t *testing.T) { + t.Parallel() + for _, actionURL := range []string{ + "http://example.test/help", + "javascript:alert(1)", + "https://user:secret@example.test/help", + "not a url", + } { + var human strings.Builder + err := NewAPI("server error", WithServerDiag(ServerDiagnostics{ + FriendlyHint: "保留 Trace ID 后排查", + ActionURL: actionURL, + })) + if printErr := PrintHuman(&human, err); printErr != nil { + t.Fatal(printErr) + } + if strings.Contains(human.String(), actionURL) || strings.Contains(human.String(), "处理入口") { + t.Fatalf("unsafe action URL %q leaked to human output: %q", actionURL, human.String()) + } + var jsonOutput strings.Builder + if printErr := PrintJSON(&jsonOutput, err); printErr != nil { + t.Fatal(printErr) + } + if strings.Contains(jsonOutput.String(), `"action_url"`) { + t.Fatalf("unsafe action URL %q leaked to JSON output: %q", actionURL, jsonOutput.String()) + } + } +} + func TestCrossPlatformCoveragePrintJSONIncludesRPCCodeAndData(t *testing.T) { t.Parallel() diff --git a/internal/helpers/chat.go b/internal/helpers/chat.go index de7d8984..27e9454e 100644 --- a/internal/helpers/chat.go +++ b/internal/helpers/chat.go @@ -2528,7 +2528,7 @@ func newChatCommand() *cobra.Command { DeclareLeafMetadata(chatMessageSendCmd, LeafSpec{ Safety: contract.SafetySpec{ Effect: "write", Risk: "medium", - Confirmation: "not_required", Idempotency: "unknown", + Confirmation: "user_required", Idempotency: "unknown", }, Contract: LeafContract{ Identity: contract.ToolIdentitySpec{ @@ -2546,7 +2546,7 @@ func newChatCommand() *cobra.Command { }, Selection: contract.SelectionSpec{ AgentSummary: "以当前用户身份发送群聊或单聊消息", - UseWhen: []string{"用户明确要以个人身份发送文本或媒体消息时"}, + UseWhen: []string{"用户明确要以个人身份发送文本或媒体消息时；响应返回 openTaskId 后用 chat message query-send-status 确认投递并取得后续操作所需的消息 ID"}, AvoidWhen: []string{"机器人身份或 Webhook 发送应使用对应命令"}, Examples: []string{"dws chat message send --group \"项目已更新\""}, }, @@ -2728,7 +2728,7 @@ func newChatCommand() *cobra.Command { DeclareLeafMetadata(chatMessageSendByBotCmd, LeafSpec{ Safety: contract.SafetySpec{ Effect: "write", Risk: "medium", - Confirmation: "not_required", Idempotency: "unknown", + Confirmation: "user_required", Idempotency: "unknown", }, Contract: LeafContract{ Identity: contract.ToolIdentitySpec{ @@ -2889,7 +2889,14 @@ func newChatCommand() *cobra.Command { DeclareLeafMetadata(chatMessageSendByWebhookCmd, LeafSpec{ Safety: contract.SafetySpec{ Effect: "write", Risk: "medium", - Confirmation: "not_required", Idempotency: "unknown", + Confirmation: "user_required", Idempotency: "unknown", + }, + // The webhook path parses the raw response directly instead of dispatching + // through deps.Caller.CallTool. Supplying a local validator makes the + // framework confirm before RunE, so the HTTP write can never happen before + // the user_required gate. + Validate: func(cmd *cobra.Command, _ []string) error { + return validateRequiredFlags(cmd, "token", "title", "text") }, Contract: LeafContract{ Identity: contract.ToolIdentitySpec{ @@ -3587,11 +3594,13 @@ func newChatCommand() *cobra.Command { // ── query-send-status：查询消息发送状态（走 IM MCP）────── chatMessageQuerySendStatusCmd := &cobra.Command{ - Use: "query-send-status", - Short: "查询消息发送状态", - Long: `查询以当前用户身份发送的消息的发送状态。需要传入发送消息时返回的 openTaskId。`, + Use: "query-send-status", + Aliases: []string{"send-status"}, + Short: "查询消息发送状态", + Long: `查询以当前用户身份发送的消息的发送状态。需要传入发送消息时返回的 openTaskId。 +投递成功后，响应中的 openMessageId 和 openConversationId 可继续用于 edit、recall、read-status 等消息操作；openTaskId 本身不是消息 ID。`, Example: ` dws chat message query-send-status --open-task-id - # openTaskId 由 dws chat message send 返回`, + dws chat message recall --conversation-id --msg-id `, RunE: func(cmd *cobra.Command, args []string) error { if err := validateRequiredFlags(cmd, "open-task-id"); err != nil { return err @@ -3613,6 +3622,7 @@ func newChatCommand() *cobra.Command { CanonicalPath: "chat.query_message_send_status", CLIPath: "chat message query-send-status", PrimaryCLIPath: "chat message query-send-status", + Aliases: []string{"chat message send-status"}, }, Description: "查询异步消息发送任务的状态", Interface: &contract.InterfaceSpec{ @@ -3622,8 +3632,8 @@ func newChatCommand() *cobra.Command { }, Selection: contract.SelectionSpec{ AgentSummary: "查询异步消息发送任务的状态", - UseWhen: []string{"发送命令返回 openTaskId 后需要确认投递结果时"}, - AvoidWhen: []string{"没有 openTaskId 或只需查消息内容时不要使用"}, + UseWhen: []string{"发送命令返回 openTaskId 后需要确认投递结果，或后续 edit/recall/read-status 需要先取得 openMessageId 和 openConversationId 时"}, + AvoidWhen: []string{"没有 openTaskId、已经有消息 ID，或只需查历史消息内容时不要使用"}, Examples: []string{"dws chat message query-send-status --open-task-id "}, }, }, @@ -5424,7 +5434,7 @@ flow-status 取值：1=处理中(PROCESSING)，2=输入中(INPUTTING)，3=完成 DeclareLeafMetadata(chatMessageSendCardCmd, LeafSpec{ Safety: contract.SafetySpec{ Effect: "write", Risk: "medium", - Confirmation: "not_required", Idempotency: "unknown", + Confirmation: "user_required", Idempotency: "unknown", }, Contract: LeafContract{ Identity: contract.ToolIdentitySpec{ @@ -5518,7 +5528,7 @@ flow-status 取值：1=处理中(PROCESSING)，2=输入中(INPUTTING)，3=完成 DeclareLeafMetadata(chatMessageUpdateCardCmd, LeafSpec{ Safety: contract.SafetySpec{ Effect: "write", Risk: "medium", - Confirmation: "not_required", Idempotency: "unknown", + Confirmation: "user_required", Idempotency: "unknown", }, Contract: LeafContract{ Identity: contract.ToolIdentitySpec{ diff --git a/internal/helpers/chat_card_update_test.go b/internal/helpers/chat_card_update_test.go index 3c7bc472..583fc13f 100644 --- a/internal/helpers/chat_card_update_test.go +++ b/internal/helpers/chat_card_update_test.go @@ -29,6 +29,9 @@ func runNativeCardUpdate(t *testing.T, caller *scriptedToolCaller, args ...strin if root.PersistentFlags().Lookup("dry-run") == nil { root.PersistentFlags().Bool("dry-run", false, "preview without executing") } + if root.PersistentFlags().Lookup("yes") == nil { + root.PersistentFlags().Bool("yes", false, "skip confirmation") + } root.SetArgs(args) return root.Execute() } @@ -41,6 +44,7 @@ func TestCrossPlatformCoverageNativeMessageUpdateCardVerifiesWrite(t *testing.T) "--biz-id", "biz-1", "--content", "完成", "--flow-status", "3", + "--yes", ) if err != nil { t.Fatal(err) @@ -60,6 +64,7 @@ func TestCrossPlatformCoverageNativeMessageUpdateCardVerifiesWrite(t *testing.T) "--biz-id", "not-a-real-card", "--content", "完成", "--flow-status", "3", + "--yes", ) var typed *apperrors.Error if !errors.As(err, &typed) || typed.Reason != "streaming_card_update_unverified" { diff --git a/internal/helpers/chat_command_edges_test.go b/internal/helpers/chat_command_edges_test.go index c9c04d76..21f67753 100644 --- a/internal/helpers/chat_command_edges_test.go +++ b/internal/helpers/chat_command_edges_test.go @@ -25,7 +25,7 @@ func runChatCoverageCommand(t *testing.T, caller edition.ToolCaller, args ...str root.SilenceUsage = true root.SetOut(io.Discard) root.SetErr(io.Discard) - root.SetArgs(args) + root.SetArgs(append(append([]string(nil), args...), "--yes")) return root.ExecuteContext(context.Background()) } diff --git a/internal/helpers/chat_confirmation_alignment_test.go b/internal/helpers/chat_confirmation_alignment_test.go new file mode 100644 index 00000000..57be5f22 --- /dev/null +++ b/internal/helpers/chat_confirmation_alignment_test.go @@ -0,0 +1,66 @@ +// 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. + +package helpers + +import ( + "errors" + "strings" + "testing" + + apperrors "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/errors" +) + +func TestCrossPlatformCoverageChatMessageWritesRequireConfirmationBeforeDispatch(t *testing.T) { + tests := []struct { + name string + args []string + }{ + {name: "current user send", args: []string{"message", "send", "--group", "cid-1", "--text", "hello"}}, + {name: "bot send", args: []string{"message", "send-by-bot", "--robot-code", "robot-1", "--group", "cid-1", "--title", "title", "--text", "hello"}}, + {name: "webhook send", args: []string{"message", "send-by-webhook", "--token", "token-1", "--title", "title", "--text", "hello"}}, + {name: "card send", args: []string{"message", "send-card", "--group", "cid-1"}}, + {name: "card update", args: []string{"message", "update-card", "--biz-id", "biz-1", "--content", "done", "--flow-status", "3"}}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + caller := &scriptedToolCaller{} + InitDepsForTest(t, caller) + cmd := newChatCommand() + cmd.SilenceErrors = true + cmd.SilenceUsage = true + cmd.SetIn(strings.NewReader("")) + cmd.SetArgs(test.args) + err := cmd.Execute() + + var appErr *apperrors.Error + if !errors.As(err, &appErr) || appErr.Reason != "confirmation_required" { + t.Fatalf("Execute() error = %#v, want confirmation_required", err) + } + if caller.calls != 0 { + t.Fatalf("downstream calls = %d, want 0 before confirmation", caller.calls) + } + }) + } +} + +func TestCrossPlatformCoverageChatMessageWriteLeavesInstallRuntimeConfirmation(t *testing.T) { + chat := newChatCommand() + for _, path := range [][]string{ + {"message", "send"}, + {"message", "send-by-bot"}, + {"message", "send-by-webhook"}, + {"message", "send-card"}, + {"message", "update-card"}, + } { + leaf, _, err := chat.Find(path) + if err != nil || leaf == nil { + t.Fatalf("Find(%v) = %v, %v", path, leaf, err) + } + if !HasContractConfirmSafety(leaf) { + t.Errorf("%s has no contract confirmation runtime guard", strings.Join(path, " ")) + } + } +} diff --git a/internal/helpers/chat_message_search_test.go b/internal/helpers/chat_message_search_test.go index 6000bb5f..72acc4e2 100644 --- a/internal/helpers/chat_message_search_test.go +++ b/internal/helpers/chat_message_search_test.go @@ -409,9 +409,12 @@ func executeChatChangedContract(t *testing.T, caller *chatChangedContractCaller, InitDeps(caller) deps.Out.w = io.Discard cmd := newChatCommand() + if cmd.PersistentFlags().Lookup("yes") == nil { + cmd.PersistentFlags().Bool("yes", false, "skip confirmation") + } cmd.SilenceErrors = true cmd.SilenceUsage = true - cmd.SetArgs(args) + cmd.SetArgs(append(append([]string(nil), args...), "--yes")) return cmd.Execute() } diff --git a/internal/helpers/chat_user_settings_test.go b/internal/helpers/chat_user_settings_test.go index 69107d78..cab0db84 100644 --- a/internal/helpers/chat_user_settings_test.go +++ b/internal/helpers/chat_user_settings_test.go @@ -101,7 +101,7 @@ func TestCrossPlatformCoverageChatMessageSendLocation(t *testing.T) { caller = &guardedMutationCaller{} err = executeGuardedMutationCommand(t, caller, newChatCommand, "message", "send", "--group", "cid1", "--msg-type", "location", - "--latitude", "39.9", "--longitude", "116.4", "--location-name", "国贸", "--map-thumbnail-url", "@media1") + "--latitude", "39.9", "--longitude", "116.4", "--location-name", "国贸", "--map-thumbnail-url", "@media1", "--yes") if err != nil { t.Fatal(err) } @@ -128,7 +128,7 @@ func TestCrossPlatformCoverageChatMessageSendProfile(t *testing.T) { caller = &guardedMutationCaller{} err = executeGuardedMutationCommand(t, caller, newChatCommand, - "message", "send", "--group", "cid1", "--msg-type", "profile", "--contact-id", "od123") + "message", "send", "--group", "cid1", "--msg-type", "profile", "--contact-id", "od123", "--yes") if err != nil { t.Fatal(err) } diff --git a/internal/shortcut/chat/chat_message.go b/internal/shortcut/chat/chat_message.go index 92891d0b..b4a9f9f1 100644 --- a/internal/shortcut/chat/chat_message.go +++ b/internal/shortcut/chat/chat_message.go @@ -1003,9 +1003,10 @@ func uniqueShortcutStrings(values []string) []string { var MessagesQuerySendStatus = shortcut.Shortcut{ Service: "chat", Command: "+messages-query-send-status", + Aliases: []string{"+messages-send-status"}, Product: "im", - Description: "查询消息发送状态", - Intent: "当你发消息后拿到 openTaskId、想确认这条消息是否发送成功时使用；只读返回发送状态，需传 --open-task-id。", + Description: "查询消息投递状态并衔接后续消息操作", + Intent: "当你发消息后拿到 openTaskId、想确认投递结果，或后续 edit/recall/read-status 需要取得 openMessageId 和 openConversationId 时使用；openTaskId 不是消息 ID。结果会保留下层响应，并追加版本化 messageRef 与结构化 nextActions。", Risk: shortcut.RiskRead, Safety: contract.SafetySpec{ Effect: "read", Risk: "low", @@ -1018,17 +1019,18 @@ var MessagesQuerySendStatus = shortcut.Shortcut{ CanonicalPath: "chat.shortcut_messages_query_send_status", CLIPath: "chat +messages-query-send-status", PrimaryCLIPath: "chat +messages-query-send-status", + Aliases: []string{"chat +messages-send-status"}, }, - Description: "查询消息发送状态", + Description: "查询消息投递状态并衔接后续消息操作", Interface: &contract.InterfaceSpec{ Mode: "composite", Availability: "available", Reason: "Reviewed built-in shortcut adapter: the executable CLI owns validation, optional multi-step orchestration, output projection, and confirmation; the complete command contract is not represented by one pinned MCP interface_ref.", }, Selection: contract.SelectionSpec{ - AgentSummary: "查询消息发送状态", - UseWhen: []string{"当你发消息后拿到 openTaskId、想确认这条消息是否发送成功时使用；只读返回发送状态，需传 --open-task-id。"}, - AvoidWhen: []string{"需要该 Shortcut 未公开的底层参数、原始响应或不同执行语义时，改用对应原子命令"}, + AgentSummary: "查询消息投递状态并衔接后续消息操作", + UseWhen: []string{"当你发消息后拿到 openTaskId、想确认投递结果，或后续 edit/recall/read-status 需要取得 openMessageId 和 openConversationId 时使用；openTaskId 不是消息 ID。结果会保留下层响应，并追加版本化 messageRef 与结构化 nextActions。"}, + AvoidWhen: []string{"没有 openTaskId、已经有消息 ID，或只需查历史消息内容时不要使用"}, Examples: []string{"dws chat +messages-query-send-status --open-task-id "}, }, }, @@ -1037,7 +1039,12 @@ var MessagesQuerySendStatus = shortcut.Shortcut{ }, Tips: []string{`dws chat +messages-query-send-status --open-task-id `}, Execute: func(rt *shortcut.RuntimeContext) error { - return rt.CallMCP("query_message_send_status", map[string]any{"openTaskId": rt.Str("open-task-id")}) + taskID := rt.Str("open-task-id") + data, err := rt.CallMCPData("im", "query_message_send_status", map[string]any{"openTaskId": taskID}) + if err != nil { + return err + } + return rt.Output(chatmsg.ProjectMessageSendStatus(data, taskID)) }, } @@ -1320,7 +1327,28 @@ var MessagesSendCard = shortcut.Shortcut{ } content := rt.Str("content") if content == "" { - return rt.CallMCP("create_and_send_card", params) + if rt.DryRun() { + return rt.Output(map[string]any{ + "contractVersion": chatmsg.StreamingCardContractVersion, + "dry_run": true, + "executed": false, + "preview_kind": "plan", + "actionCount": 1, + "actions": []map[string]any{{ + "tool": "create_and_send_card", + "arguments": params, + }}, + }) + } + created, err := rt.CallMCPWriteData("im", "create_and_send_card", params) + if err != nil { + return err + } + bizID := findCardBizID(created) + if bizID == "" { + return cardCreateMissingBizIDError(created) + } + return rt.Output(chatmsg.ProjectStreamingCardReceipt(created, bizID)) } status := rt.Int("flow-status") if rt.DryRun() { @@ -1366,14 +1394,11 @@ var MessagesSendCard = shortcut.Shortcut{ if _, err := chatmsg.VerifyStreamingCardUpdate(bizID, updated); err != nil { return fmt.Errorf("卡片已创建（bizId=%s），但自动更新结果不可信: %w", bizID, cardUpdateVerificationError(bizID, err)) } - return rt.Output(map[string]any{ - "contractVersion": currentCardWorkflowContract.Version, - "ok": true, - "bizId": bizID, - "flowStatus": status, - "created": created, - "updated": updated, - }) + payload := chatmsg.ProjectStreamingCardReceipt(created, bizID) + payload["bizId"] = bizID + payload["flowStatus"] = status + payload["updated"] = updated + return rt.Output(payload) }, } @@ -1510,13 +1535,29 @@ var MessagesUpdateCard = shortcut.Shortcut{ if err != nil { return err } - if _, err := chatmsg.VerifyStreamingCardUpdate(bizID, updated); err != nil { + proof, err := chatmsg.VerifyStreamingCardUpdate(bizID, updated) + if err != nil { return cardUpdateVerificationError(bizID, err) } - return rt.Output(updated) + return rt.Output(chatmsg.ProjectStreamingCardUpdate(updated, bizID, proof)) }, } +func cardCreateMissingBizIDError(created map[string]any) error { + return apperrors.NewAPI( + "卡片可能已经创建，但服务端未返回后续更新所需的 bizId；CLI 无法确认卡片工作流可继续", + apperrors.WithOperation("create_and_send_card"), + apperrors.WithServerKey("im"), + apperrors.WithOrigin("client_postcondition"), + apperrors.WithFailureStage("verify_card_reference"), + apperrors.WithExecutionStarted(true), + apperrors.WithRetryable(false), + apperrors.WithReason("streaming_card_reference_missing"), + apperrors.WithHint("不要盲目重试创建；请保留 trace_id 并推动服务端返回 bizId、openMessageId 和 openConversationId"), + apperrors.WithDetails(map[string]any{"created": created}), + ) +} + func cardUpdateVerificationError(bizID string, verifyErr error) error { reason := "streaming_card_update_unverified" message := "服务端未返回卡片实际更新的证据；为避免假成功，CLI 已将本次操作判为失败" diff --git a/internal/shortcut/chat/gap_fill_test.go b/internal/shortcut/chat/gap_fill_test.go index 55b12f11..91761846 100644 --- a/internal/shortcut/chat/gap_fill_test.go +++ b/internal/shortcut/chat/gap_fill_test.go @@ -20,6 +20,7 @@ import ( apperrors "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/errors" "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/helpers" "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/shortcut" + "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/shortcut/chatmsg" ) type chatOutputErrorWriter struct { @@ -124,6 +125,63 @@ func TestCrossPlatformCoverageIMWorkflowContractsPublishRealPositiveAndNegativeB } } +func TestCrossPlatformCoverageMessagesSendStatusAliasPublishesWorkflowReceipt(t *testing.T) { + fake := &larkAlignmentCaller{responses: map[string]string{ + "im/query_message_send_status": `{"result":{"status":"SUCCESS","openTaskId":"task-1","openMessageId":"msg-1","openConversationId":"cid-1"}}`, + }} + helpers.InitDeps(fake) + root := newPlatformCoverageRoot() + var output bytes.Buffer + root.SetOut(&output) + root.SetArgs([]string{"chat", "+messages-send-status", "--open-task-id", "task-1"}) + if err := root.Execute(); err != nil { + t.Fatal(err) + } + if len(fake.calls) != 1 || fake.calls[0].tool != "query_message_send_status" || fake.calls[0].args["openTaskId"] != "task-1" { + t.Fatalf("calls = %#v", fake.calls) + } + var payload map[string]any + if err := json.Unmarshal(output.Bytes(), &payload); err != nil { + t.Fatal(err) + } + if payload["contractVersion"] != chatmsg.MessageSendStatusContractVersion || payload["readyForMessageActions"] != true { + t.Fatalf("payload = %#v", payload) + } + ref, _ := payload["messageRef"].(map[string]any) + if ref["openMessageId"] != "msg-1" || ref["openConversationId"] != "cid-1" { + t.Fatalf("messageRef = %#v", ref) + } +} + +func TestCrossPlatformCoverageMessagesSendPublishesStatusQueryReceipt(t *testing.T) { + fake := &larkAlignmentCaller{responses: map[string]string{ + "chat/send_personal_message": `{"result":{"openTaskId":"task-send-1"}}`, + }} + helpers.InitDeps(fake) + root := newPlatformCoverageRoot() + var output bytes.Buffer + root.SetOut(&output) + root.SetArgs([]string{ + "chat", "+messages-send", "--as", "user", "--chat-id", "cid-1", + "--text", "hello", "--yes", + }) + if err := root.Execute(); err != nil { + t.Fatal(err) + } + var payload map[string]any + if err := json.Unmarshal(output.Bytes(), &payload); err != nil { + t.Fatal(err) + } + receipt, _ := payload["sendReceipt"].(map[string]any) + if receipt["contractVersion"] != chatmsg.MessageSendReceiptContractVersion || receipt["openTaskId"] != "task-send-1" { + t.Fatalf("sendReceipt = %#v", receipt) + } + actions, _ := receipt["nextActions"].([]any) + if len(actions) != 1 { + t.Fatalf("nextActions = %#v", actions) + } +} + func TestCrossPlatformCoverageMessagesSendBotMultiGroupPublishesPerTargetLedger(t *testing.T) { fake := &larkAlignmentCaller{} helpers.InitDeps(fake) diff --git a/internal/shortcut/chat/lark_alignment_test.go b/internal/shortcut/chat/lark_alignment_test.go index f6cb0eed..23cdeb58 100644 --- a/internal/shortcut/chat/lark_alignment_test.go +++ b/internal/shortcut/chat/lark_alignment_test.go @@ -74,6 +74,8 @@ func (f *larkAlignmentCaller) CallTool(_ context.Context, product, tool string, text = string(payload) case "im/create_group_conversation": text = `{"result":{"cid":"internal-cid","openCid":"open-cid"}}` + case "im/create_and_send_card": + text = `{"result":{"bizId":"biz-created"}}` case "im/list_messages_by_ids": text = `{"result":[{"openMessageId":"msg","openConversationId":"cid","senderOpenDingTalkId":"D-inferred","content":"{\"mediaId\":\"@image\"}"}]}` case "im/list_conversations_by_category": diff --git a/internal/shortcut/chat/unified_send.go b/internal/shortcut/chat/unified_send.go index 6c52f203..9991d147 100644 --- a/internal/shortcut/chat/unified_send.go +++ b/internal/shortcut/chat/unified_send.go @@ -19,6 +19,7 @@ import ( apperrors "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/errors" "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/helpers" "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/shortcut" + "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/shortcut/chatmsg" "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/shortcut/targetresolver" ) @@ -372,12 +373,16 @@ func executeUnifiedMessageWrite(rt *shortcut.RuntimeContext, product, tool strin if err != nil { return err } - return rt.Output(map[string]any{ + payload := map[string]any{ "ok": true, "identity": messagesSendIdentity(rt), "tool": tool, "result": data, - }) + } + if messagesSendIdentity(rt) == "user" && tool == "send_personal_message" { + payload["sendReceipt"] = chatmsg.ProjectMessageSendReceipt(data) + } + return rt.Output(payload) } func messagesSendIdentity(rt *shortcut.RuntimeContext) string { @@ -612,7 +617,7 @@ func executeMessagesSendUserFile( if err != nil { return err } - return rt.Output(map[string]any{ + payload := map[string]any{ "ok": true, "identity": "user", "tool": "send_personal_message", @@ -624,7 +629,9 @@ func executeMessagesSendUserFile( "sizeBytes": meta.FileSize, }, "result": data, - }) + } + payload["sendReceipt"] = chatmsg.ProjectMessageSendReceipt(data) + return rt.Output(payload) } func addMessagesSendUserTarget(params map[string]any, group, openID string) { diff --git a/internal/shortcut/chatmsg/card_ref.go b/internal/shortcut/chatmsg/card_ref.go new file mode 100644 index 00000000..03dcbfba --- /dev/null +++ b/internal/shortcut/chatmsg/card_ref.go @@ -0,0 +1,65 @@ +// 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. + +package chatmsg + +import "strings" + +// StreamingCardContractVersion identifies the additive card receipt emitted by +// high-level card shortcuts. +const StreamingCardContractVersion = "im.streaming-card.v1" + +// ProjectStreamingCardReceipt publishes every server-returned identifier in a +// single cardRef. referencePairAvailable means this response contained both +// the update identifier and the visible message identifiers; it does not claim +// that older messages can be resolved without server-side mapping support. +func ProjectStreamingCardReceipt(created map[string]any, bizID string) map[string]any { + bizID = strings.TrimSpace(bizID) + messageID := firstSendStatusString(created, "openMessageId", "messageId", "msgId") + conversationID := firstSendStatusString(created, "openConversationId", "conversationId", "openCid") + cardRef := map[string]any{} + if bizID != "" { + cardRef["bizId"] = bizID + } + if messageID != "" { + cardRef["openMessageId"] = messageID + } + if conversationID != "" { + cardRef["openConversationId"] = conversationID + } + pairAvailable := bizID != "" && messageID != "" && conversationID != "" + payload := map[string]any{ + "contractVersion": StreamingCardContractVersion, + "ok": true, + "cardRef": cardRef, + "referencePairAvailable": pairAvailable, + "created": created, + "nextActions": []map[string]any{}, + } + if bizID != "" { + payload["nextActions"] = []map[string]any{{ + "cliPath": "chat +messages-update-card", + "arguments": map[string]any{ + "biz-id": bizID, + }, + "requiredArguments": []string{"content", "flow-status"}, + "ready": false, + }} + } + if !pairAvailable { + payload["capabilityGap"] = "服务端尚未同时返回 bizId、openMessageId 和 openConversationId；CLI 只能保留本次响应，不能据此承诺从历史消息反向恢复 bizId" + } + return payload +} + +// ProjectStreamingCardUpdate preserves the lower response while making the +// verified target explicit for downstream consumers. +func ProjectStreamingCardUpdate(updated map[string]any, bizID, proof string) map[string]any { + payload := cloneSendStatusMap(updated) + payload["contractVersion"] = StreamingCardContractVersion + payload["cardRef"] = map[string]any{"bizId": strings.TrimSpace(bizID)} + payload["verified"] = true + payload["verificationEvidence"] = proof + return payload +} diff --git a/internal/shortcut/chatmsg/card_ref_test.go b/internal/shortcut/chatmsg/card_ref_test.go new file mode 100644 index 00000000..baa65687 --- /dev/null +++ b/internal/shortcut/chatmsg/card_ref_test.go @@ -0,0 +1,42 @@ +// 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. + +package chatmsg + +import "testing" + +func TestCrossPlatformCoverageProjectStreamingCardReceipt(t *testing.T) { + complete := ProjectStreamingCardReceipt(map[string]any{ + "result": map[string]any{ + "bizId": "biz-1", + "openMessageId": "msg-1", + "openConversationId": "cid-1", + }, + }, "biz-1") + if complete["contractVersion"] != StreamingCardContractVersion || complete["referencePairAvailable"] != true { + t.Fatalf("complete receipt = %#v", complete) + } + ref, _ := complete["cardRef"].(map[string]any) + if ref["bizId"] != "biz-1" || ref["openMessageId"] != "msg-1" || ref["openConversationId"] != "cid-1" { + t.Fatalf("cardRef = %#v", ref) + } + if _, exists := complete["capabilityGap"]; exists { + t.Fatalf("complete receipt has capability gap: %#v", complete) + } + + partial := ProjectStreamingCardReceipt(map[string]any{"result": map[string]any{"bizId": "biz-2"}}, "biz-2") + if partial["referencePairAvailable"] != false || partial["capabilityGap"] == "" { + t.Fatalf("partial receipt = %#v", partial) + } +} + +func TestCrossPlatformCoverageProjectStreamingCardUpdate(t *testing.T) { + payload := ProjectStreamingCardUpdate(map[string]any{"result": map[string]any{"updated": true}}, "biz-1", "updated=true") + if payload["contractVersion"] != StreamingCardContractVersion || payload["verified"] != true || payload["verificationEvidence"] != "updated=true" { + t.Fatalf("payload = %#v", payload) + } + if _, exists := payload["result"]; !exists { + t.Fatal("lower response was not preserved") + } +} diff --git a/internal/shortcut/chatmsg/send_status.go b/internal/shortcut/chatmsg/send_status.go new file mode 100644 index 00000000..2c6aea5d --- /dev/null +++ b/internal/shortcut/chatmsg/send_status.go @@ -0,0 +1,162 @@ +// 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. + +package chatmsg + +import ( + "strings" +) + +// MessageSendStatusContractVersion identifies the additive workflow fields +// projected by the high-level send-status shortcut. The lower response fields +// remain at their original locations for compatibility. +const MessageSendStatusContractVersion = "im.message-send-status.v1" + +// MessageSendReceiptContractVersion identifies the additive receipt attached +// to high-level current-user send results. +const MessageSendReceiptContractVersion = "im.message-send-receipt.v1" + +// ProjectMessageSendReceipt connects a send result to its asynchronous status +// query without treating openTaskId as a message identifier. +func ProjectMessageSendReceipt(raw map[string]any) map[string]any { + taskID := firstSendStatusString(raw, "openTaskId", "taskId") + messageID := firstSendStatusString(raw, "openMessageId", "messageId", "msgId") + conversationID := firstSendStatusString(raw, "openConversationId", "conversationId", "openCid") + ready := messageID != "" && conversationID != "" + receipt := map[string]any{ + "contractVersion": MessageSendReceiptContractVersion, + "openTaskId": taskID, + "readyForMessageActions": ready, + "nextActions": []map[string]any{}, + } + if messageID != "" || conversationID != "" { + messageRef := map[string]any{} + if messageID != "" { + messageRef["openMessageId"] = messageID + } + if conversationID != "" { + messageRef["openConversationId"] = conversationID + } + receipt["messageRef"] = messageRef + } + switch { + case ready: + receipt["nextActions"] = sendStatusNextActions(taskID, messageID, conversationID, true) + case taskID != "": + receipt["nextActions"] = []map[string]any{{ + "cliPath": "chat +messages-query-send-status", + "arguments": map[string]any{ + "open-task-id": taskID, + }, + "ready": true, + "when": "需要确认投递结果或取得真实消息 ID 时", + }} + default: + receipt["capabilityGap"] = "下层发送响应未返回 openTaskId 或完整 messageRef，CLI 无法生成后续状态查询" + } + return receipt +} + +// ProjectMessageSendStatus preserves the lower response and adds a stable +// receipt that connects openTaskId to the message identifiers required by +// edit, recall, and read-status. It never manufactures a message reference: +// downstream actions are marked ready only when both IDs are actually present. +func ProjectMessageSendStatus(raw map[string]any, requestedTaskID string) map[string]any { + payload := cloneSendStatusMap(raw) + taskID := firstSendStatusString(payload, "openTaskId", "taskId") + if taskID == "" { + taskID = strings.TrimSpace(requestedTaskID) + } + messageID := firstSendStatusString(payload, "openMessageId", "messageId", "msgId") + conversationID := firstSendStatusString(payload, "openConversationId", "conversationId", "openCid") + + payload["contractVersion"] = MessageSendStatusContractVersion + payload["openTaskId"] = taskID + messageRef := map[string]any{} + if messageID != "" { + messageRef["openMessageId"] = messageID + } + if conversationID != "" { + messageRef["openConversationId"] = conversationID + } + if len(messageRef) > 0 { + payload["messageRef"] = messageRef + } + ready := messageID != "" && conversationID != "" + payload["readyForMessageActions"] = ready + payload["nextActions"] = sendStatusNextActions(taskID, messageID, conversationID, ready) + return payload +} + +func sendStatusNextActions(taskID, messageID, conversationID string, ready bool) []map[string]any { + if !ready { + return []map[string]any{{ + "cliPath": "chat message query-send-status", + "arguments": map[string]any{ + "open-task-id": taskID, + }, + "ready": false, + "when": "投递任务尚未返回 openMessageId 和 openConversationId 时稍后重查", + }} + } + messageArgs := map[string]any{ + "conversation-id": conversationID, + "msg-id": messageID, + } + return []map[string]any{ + { + "cliPath": "chat message recall", + "arguments": cloneSendStatusMap(messageArgs), + "ready": true, + }, + { + "cliPath": "chat message edit", + "arguments": map[string]any{ + "conversation-id": conversationID, + "msg-id": messageID, + }, + "requiredArguments": []string{"text 或 content"}, + "ready": false, + }, + { + "cliPath": "chat message read-status", + "arguments": map[string]any{ + "conversation-id": conversationID, + "message-id": messageID, + }, + "ready": true, + }, + } +} + +func firstSendStatusString(value any, keys ...string) string { + switch typed := value.(type) { + case map[string]any: + for _, key := range keys { + if candidate, ok := typed[key].(string); ok && strings.TrimSpace(candidate) != "" { + return strings.TrimSpace(candidate) + } + } + for _, key := range []string{"result", "data", "response", "content", "message"} { + if candidate := firstSendStatusString(typed[key], keys...); candidate != "" { + return candidate + } + } + case []any: + for _, item := range typed { + if candidate := firstSendStatusString(item, keys...); candidate != "" { + return candidate + } + } + } + return "" +} + +func cloneSendStatusMap(source map[string]any) map[string]any { + clone := make(map[string]any, len(source)+5) + for key, value := range source { + clone[key] = value + } + return clone +} diff --git a/internal/shortcut/chatmsg/send_status_test.go b/internal/shortcut/chatmsg/send_status_test.go new file mode 100644 index 00000000..58e8194c --- /dev/null +++ b/internal/shortcut/chatmsg/send_status_test.go @@ -0,0 +1,63 @@ +// 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. + +package chatmsg + +import "testing" + +func TestCrossPlatformCoverageProjectMessageSendReceiptLinksStatusQuery(t *testing.T) { + receipt := ProjectMessageSendReceipt(map[string]any{ + "result": map[string]any{"openTaskId": "task-1"}, + }) + if receipt["contractVersion"] != MessageSendReceiptContractVersion || receipt["openTaskId"] != "task-1" || receipt["readyForMessageActions"] != false { + t.Fatalf("receipt = %#v", receipt) + } + actions, _ := receipt["nextActions"].([]map[string]any) + if len(actions) != 1 || actions[0]["cliPath"] != "chat +messages-query-send-status" || actions[0]["ready"] != true { + t.Fatalf("nextActions = %#v", actions) + } +} + +func TestCrossPlatformCoverageProjectMessageSendStatusReadyWorkflow(t *testing.T) { + raw := map[string]any{ + "result": map[string]any{ + "openTaskId": "task-1", + "openMessageId": "msg-1", + "openConversationId": "cid-1", + "status": "SUCCESS", + }, + } + payload := ProjectMessageSendStatus(raw, "ignored") + if payload["contractVersion"] != MessageSendStatusContractVersion || + payload["openTaskId"] != "task-1" || payload["readyForMessageActions"] != true { + t.Fatalf("payload = %#v", payload) + } + ref, _ := payload["messageRef"].(map[string]any) + if ref["openMessageId"] != "msg-1" || ref["openConversationId"] != "cid-1" { + t.Fatalf("messageRef = %#v", ref) + } + actions, _ := payload["nextActions"].([]map[string]any) + if len(actions) != 3 || actions[0]["cliPath"] != "chat message recall" || actions[2]["cliPath"] != "chat message read-status" { + t.Fatalf("nextActions = %#v", actions) + } + if _, ok := payload["result"]; !ok { + t.Fatal("raw response field was not preserved") + } +} + +func TestCrossPlatformCoverageProjectMessageSendStatusPendingDoesNotInventMessageRef(t *testing.T) { + payload := ProjectMessageSendStatus(map[string]any{ + "result": map[string]any{"status": "PENDING"}, + }, "task-pending") + if payload["openTaskId"] != "task-pending" || payload["readyForMessageActions"] != false { + t.Fatalf("payload = %#v", payload) + } + if _, exists := payload["messageRef"]; exists { + t.Fatalf("pending payload invented messageRef: %#v", payload) + } + actions, _ := payload["nextActions"].([]map[string]any) + if len(actions) != 1 || actions[0]["ready"] != false { + t.Fatalf("nextActions = %#v", actions) + } +} diff --git a/internal/transport/client.go b/internal/transport/client.go index 5c2759ec..c9184a41 100644 --- a/internal/transport/client.go +++ b/internal/transport/client.go @@ -1047,6 +1047,7 @@ func authActions(snapshotPath string) []string { func runtimeActions(snapshotPath string) []string { actions := []string{ i18n.T("检查认证、权限和参数后重试原命令"), + i18n.T("运行 dws doctor 检查登录态、网络和本地环境；持续失败时保留 Trace ID 和 Server Code"), } _ = snapshotPath return actions @@ -1056,6 +1057,7 @@ func networkActions(snapshotPath string) []string { actions := []string{ i18n.T("检查网络、代理和 DNS 配置后重试原命令"), i18n.T("确认 MCP 服务可访问；若持续失败请稍后重试"), + i18n.T("运行 dws doctor 获取可共享的环境诊断结果"), } _ = snapshotPath return actions diff --git a/internal/transport/coverage_edges_test.go b/internal/transport/coverage_edges_test.go index 4b17e065..0ff98ed7 100644 --- a/internal/transport/coverage_edges_test.go +++ b/internal/transport/coverage_edges_test.go @@ -397,13 +397,13 @@ func TestCrossPlatformCoverageHTTPStatusAndDiagnosticsEdges(t *testing.T) { t.Fatalf("HTTP %d returned nil", tc.code) } } - if got := networkActions("snap"); len(got) != 2 { + if got := networkActions("snap"); len(got) != 3 { t.Fatalf("network actions = %#v", got) } if got := authActions("snap"); len(got) != 2 { t.Fatalf("auth actions = %#v", got) } - if got := runtimeActions("snap"); len(got) != 1 { + if got := runtimeActions("snap"); len(got) != 2 { t.Fatalf("runtime actions = %#v", got) } if got := actionsForMethod("tools/call", "snap"); len(got) == 0 { diff --git a/internal/transport/diagnostics.go b/internal/transport/diagnostics.go index ed8306b8..8a643f7f 100644 --- a/internal/transport/diagnostics.go +++ b/internal/transport/diagnostics.go @@ -42,18 +42,48 @@ func ExtractServerDiagnosticsFromMap(content map[string]any) apperrors.ServerDia return apperrors.ServerDiagnostics{} } diag := apperrors.ServerDiagnostics{ - TraceID: stringFromMap(content, "trace_id", "traceId"), + TraceID: stringFromMapRecursive(content, 0, "trace_id", "traceId", "request_id", "requestId"), ServerErrorCode: serverErrorCodeFromMap(content, 0), - TechnicalDetail: stringFromMap(content, "technical_detail"), + TechnicalDetail: stringFromMapRecursive(content, 0, "technical_detail", "technicalDetail"), FriendlyHint: stringFromMapRecursive(content, 0, "friendly_hint", "friendlyHint"), ActionURL: stringFromMapRecursive(content, 0, "action_url", "actionUrl"), } - if v, ok := content["retryable"].(bool); ok { + if v, ok := boolFromMapRecursive(content, 0, "retryable", "server_retryable", "serverRetryable"); ok { diag.ServerRetryable = &v } return diag } +func boolFromMapRecursive(content map[string]any, depth int, keys ...string) (bool, bool) { + if content == nil || depth > 8 { + return false, false + } + for _, key := range keys { + if value, ok := content[key].(bool); ok { + return value, true + } + } + for _, key := range []string{"content", "result", "data"} { + switch child := content[key].(type) { + case map[string]any: + if value, ok := boolFromMapRecursive(child, depth+1, keys...); ok { + return value, true + } + case []any: + for _, item := range child { + childMap, ok := item.(map[string]any) + if !ok { + continue + } + if value, ok := boolFromMapRecursive(childMap, depth+1, keys...); ok { + return value, true + } + } + } + } + return false, false +} + func stringFromMapRecursive(content map[string]any, depth int, keys ...string) string { if content == nil || depth > 8 { return "" diff --git a/internal/transport/diagnostics_test.go b/internal/transport/diagnostics_test.go index 35e21724..436f49ca 100644 --- a/internal/transport/diagnostics_test.go +++ b/internal/transport/diagnostics_test.go @@ -110,6 +110,24 @@ func TestExtractServerDiagnosticsFromMapNestedBusinessCode(t *testing.T) { } } +func TestExtractServerDiagnosticsFromMapNestedDiagnosticEnvelope(t *testing.T) { + t.Parallel() + content := map[string]any{ + "result": map[string]any{ + "data": map[string]any{ + "requestId": "request-nested", + "technicalDetail": "nested detail", + "serverRetryable": false, + }, + }, + } + diag := ExtractServerDiagnosticsFromMap(content) + if diag.TraceID != "request-nested" || diag.TechnicalDetail != "nested detail" || + diag.ServerRetryable == nil || *diag.ServerRetryable { + t.Fatalf("nested diagnostics = %#v", diag) + } +} + func TestExtractServerDiagnosticsFromMap_Empty(t *testing.T) { t.Parallel() diag := ExtractServerDiagnosticsFromMap(nil) From 0d11b2be45b49f899c834c83a465364dda3e2fbb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=85=8B=E8=B0=A8?= <2940480744@qq.com> Date: Fri, 7 Aug 2026 16:07:56 +0800 Subject: [PATCH 03/19] fix(chat): preserve resource filenames in message refs --- internal/shortcut/chatmsg/chatmsg.go | 159 +++++++++++++++++- internal/shortcut/chatmsg/chatmsg_test.go | 22 +++ .../chatmsg/coverage_completion_test.go | 31 +++- 3 files changed, 205 insertions(+), 7 deletions(-) diff --git a/internal/shortcut/chatmsg/chatmsg.go b/internal/shortcut/chatmsg/chatmsg.go index 152558ba..f1b1d851 100644 --- a/internal/shortcut/chatmsg/chatmsg.go +++ b/internal/shortcut/chatmsg/chatmsg.go @@ -396,10 +396,14 @@ func Resources(m map[string]any) []map[string]any { collectResourceIDs(m, "mediaid", mediaIDTextRE, &mediaIDs) mediaIDs = uniqueResourceIDs(mediaIDs) sort.Strings(mediaIDs) + mediaNames := make(map[string]resourceNameCandidate) + collectResourceNames(m, "mediaid", mediaNames) fileIDs := make([]string, 0) collectResourceIDs(m, "fileid", fileIDTextRE, &fileIDs) fileIDs = uniqueResourceIDs(fileIDs) sort.Strings(fileIDs) + fileNames := make(map[string]resourceNameCandidate) + collectResourceNames(m, "fileid", fileNames) if len(mediaIDs) == 0 && len(fileIDs) == 0 { return nil } @@ -430,7 +434,7 @@ func Resources(m map[string]any) []map[string]any { } else { missing = append(missing, "open-conversation-id") } - out = append(out, map[string]any{ + resource := map[string]any{ "type": "mediaId", "resourceId": id, "download": map[string]any{ @@ -439,10 +443,14 @@ func Resources(m map[string]any) []map[string]any { "ready": len(missing) == 0, "missing": missing, }, - }) + } + if candidate, ok := mediaNames[id]; ok { + resource["name"] = candidate.name + } + out = append(out, resource) } for _, id := range fileIDs { - out = append(out, map[string]any{ + resource := map[string]any{ "type": "fileId", "resourceId": id, "download": map[string]any{ @@ -454,7 +462,11 @@ func Resources(m map[string]any) []map[string]any { "ready": true, "missing": []string{}, }, - }) + } + if candidate, ok := fileNames[id]; ok { + resource["name"] = candidate.name + } + out = append(out, resource) } return out } @@ -499,6 +511,145 @@ func resourcesDeep(m map[string]any, inheritedConversationID string, depth int) var mediaIDTextRE = regexp.MustCompile(`(?i)\bmedia[_-]?id\s*[:=]\s*["']?([^"'\s)\]}>,]+)`) var fileIDTextRE = regexp.MustCompile(`(?i)\bfile[_-]?id\s*[:=]\s*["']?([^"'\s)\]}>,]+)`) +var fileNameAndIDTextRE = regexp.MustCompile(`(?i)\[文件\]\s*([^\r\n]*?)\s+file[_-]?id\s*[:=]\s*["']?([^"'\s)\]}>,]+)`) + +type resourceNameCandidate struct { + name string + priority int +} + +const ( + resourceNamePriorityText = 1 + resourceNamePriorityStructured = 2 +) + +// collectResourceNames keeps a resource ID paired with a name only when both +// are present in the same structured object or in the legacy, machine-shaped +// "[文件] name fileId: id" text. This deliberately does not borrow a generic +// message title or sender name: an unknown resource name is safer than a +// plausible but incorrect one. +func collectResourceNames(value any, targetKey string, out map[string]resourceNameCandidate) { + switch typed := value.(type) { + case map[string]any: + directIDs := directResourceIDs(typed, targetKey) + if len(directIDs) == 1 { + if name := directResourceName(typed, targetKey); name != "" { + recordResourceName(out, directIDs[0], name, resourceNamePriorityStructured) + } + } + keys := make([]string, 0, len(typed)) + for key := range typed { + keys = append(keys, key) + } + sort.Strings(keys) + for _, key := range keys { + if isNestedMessageBoundaryKey(normalizeMessageKey(key)) { + continue + } + collectResourceNames(typed[key], targetKey, out) + } + case []any: + for _, child := range typed { + collectResourceNames(child, targetKey, out) + } + case []map[string]any: + for _, child := range typed { + collectResourceNames(child, targetKey, out) + } + case string: + if targetKey == "fileid" { + for _, match := range fileNameAndIDTextRE.FindAllStringSubmatch(typed, -1) { + if len(match) < 3 { + continue + } + name := strings.TrimSpace(match[1]) + id := resourceIDScalar(match[2]) + if name != "" && id != "" { + recordResourceName(out, id, name, resourceNamePriorityText) + } + } + } + trimmed := strings.TrimSpace(typed) + if strings.HasPrefix(trimmed, "{") || strings.HasPrefix(trimmed, "[") { + var decoded any + if json.Unmarshal([]byte(trimmed), &decoded) == nil { + collectResourceNames(decoded, targetKey, out) + } + } + } +} + +func directResourceIDs(value map[string]any, targetKey string) []string { + resourceType := normalizeMessageKey(strings.TrimSpace(fmt.Sprint( + firstMessageValue(value, "resourceType", "resource_type")))) + keys := make([]string, 0, len(value)) + for key := range value { + keys = append(keys, key) + } + sort.Strings(keys) + ids := make([]string, 0, 1) + for _, key := range keys { + normalizedKey := normalizeMessageKey(key) + if normalizedKey != targetKey && + !(normalizedKey == "resourceid" && resourceType == targetKey) { + continue + } + if id := resourceIDScalar(value[key]); id != "" { + ids = append(ids, id) + } + } + return uniqueResourceIDs(ids) +} + +func directResourceName(value map[string]any, targetKey string) string { + for _, wanted := range []string{"filename", "resourcename", "originalfilename"} { + if name := directResourceString(value, wanted); name != "" { + return name + } + } + // A bare "name" is accepted only inside an explicit resource envelope. + // Message rows also commonly contain a sender/group name, which must never + // become the attachment filename merely because the row has a resource ID. + resourceType := normalizeMessageKey(strings.TrimSpace(fmt.Sprint( + firstMessageValue(value, "resourceType", "resource_type")))) + if resourceType == targetKey && directResourceString(value, "resourceid") != "" { + return directResourceString(value, "name") + } + return "" +} + +func directResourceString(value map[string]any, wanted string) string { + keys := make([]string, 0, len(value)) + for key := range value { + keys = append(keys, key) + } + sort.Strings(keys) + for _, key := range keys { + if normalizeMessageKey(key) != wanted { + continue + } + if text, ok := value[key].(string); ok { + return strings.TrimSpace(text) + } + } + return "" +} + +func recordResourceName( + out map[string]resourceNameCandidate, + id, name string, + priority int, +) { + id = resourceIDScalar(id) + name = strings.TrimSpace(name) + if id == "" || name == "" { + return + } + if current, ok := out[id]; ok && current.priority >= priority { + return + } + out[id] = resourceNameCandidate{name: name, priority: priority} +} func collectResourceIDs(value any, targetKey string, textPattern *regexp.Regexp, out *[]string) { switch typed := value.(type) { diff --git a/internal/shortcut/chatmsg/chatmsg_test.go b/internal/shortcut/chatmsg/chatmsg_test.go index 63d0b797..e7231ec9 100644 --- a/internal/shortcut/chatmsg/chatmsg_test.go +++ b/internal/shortcut/chatmsg/chatmsg_test.go @@ -484,6 +484,28 @@ func TestCrossPlatformCoverageResourcesBuildsActionableDownloadReferences(t *tes } } +func TestCrossPlatformCoverageResourcesKeepsNestedResourceNamesWithTheirOwner(t *testing.T) { + message := map[string]any{ + "openMessageId": "parent-message", + "openConversationId": "cid-1", + "content": `{"fileId":"parent-file","fileName":"parent.pdf"}`, + "quotedMessage": map[string]any{ + "openMessageId": "quoted-message", + "content": `{"fileId":"quoted-file","file_name":"quoted.pdf"}`, + }, + } + resources := ResourcesDeep(message) + if len(resources) != 2 { + t.Fatalf("resources = %#v", resources) + } + if resources[0]["resourceId"] != "parent-file" || resources[0]["name"] != "parent.pdf" { + t.Fatalf("parent resource = %#v", resources[0]) + } + if resources[1]["resourceId"] != "quoted-file" || resources[1]["name"] != "quoted.pdf" { + t.Fatalf("quoted resource = %#v", resources[1]) + } +} + func TestCrossPlatformCoverageResourcesReportsMissingDownloadContext(t *testing.T) { resources := Resources(map[string]any{"content": `{"mediaId":"@image-a"}`}) if len(resources) != 1 { diff --git a/internal/shortcut/chatmsg/coverage_completion_test.go b/internal/shortcut/chatmsg/coverage_completion_test.go index ef8c8742..7a913820 100644 --- a/internal/shortcut/chatmsg/coverage_completion_test.go +++ b/internal/shortcut/chatmsg/coverage_completion_test.go @@ -28,16 +28,18 @@ func TestCrossPlatformCoverageQuotedResourcesAndScalarVariants(t *testing.T) { } resources := Resources(map[string]any{ "attachments": []map[string]any{ - {"resourceType": "mediaId", "resourceId": "@file-a"}, - {"resourceType": "fileId", "resourceId": "drive-file"}, + {"resourceType": "mediaId", "resourceId": "@file-a", "name": "photo.png"}, + {"resourceType": "fileId", "resourceId": "drive-file", "fileName": "canonical-report.txt"}, {"mediaId": 42, "fileId": 42}, }, "content": `[文件] report.txt fileId: drive-file`, }) if len(resources) != 2 || resources[0]["resourceId"] != "@file-a" || + resources[0]["name"] != "photo.png" || resources[1]["resourceId"] != "drive-file" || - resources[1]["type"] != "fileId" { + resources[1]["type"] != "fileId" || + resources[1]["name"] != "canonical-report.txt" { t.Fatalf("resources = %#v", resources) } fileDownload := resources[1]["download"].(map[string]any) @@ -53,6 +55,29 @@ func TestCrossPlatformCoverageQuotedResourcesAndScalarVariants(t *testing.T) { } } +func TestCrossPlatformCoverageResourcesExtractsLegacyFileNameWithoutGuessing(t *testing.T) { + resources := Resources(map[string]any{ + "name": "sender-name-must-not-leak", + "openMessageId": "msg-1", + "openConversationId": "cid-1", + "content": `[文件] 项目最终报告 2026.pdf fileId: drive-file 注意：如需下载使用旧命令`, + "unrelatedAttachment": map[string]any{"mediaId": "@image-without-name"}, + }) + if len(resources) != 2 { + t.Fatalf("resources = %#v", resources) + } + if resources[0]["resourceId"] != "@image-without-name" { + t.Fatalf("first resource = %#v", resources[0]) + } + if _, leaked := resources[0]["name"]; leaked { + t.Fatalf("message sender name leaked into media resource: %#v", resources[0]) + } + if resources[1]["resourceId"] != "drive-file" || + resources[1]["name"] != "项目最终报告 2026.pdf" { + t.Fatalf("file resource = %#v", resources[1]) + } +} + func TestCrossPlatformCoverageReactionShapeVariants(t *testing.T) { got := Reactions(map[string]any{ "reactions": []map[string]any{ From 5a09204bf5f769ec71d736bf8752b09e9617f8e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=85=8B=E8=B0=A8?= <2940480744@qq.com> Date: Fri, 7 Aug 2026 16:23:20 +0800 Subject: [PATCH 04/19] fix(chat): complete resource reference downloads --- internal/shortcut/chat/chat_message.go | 4 +++ internal/shortcut/chat/gap_fill_test.go | 3 +++ internal/shortcut/chatmsg/chatmsg.go | 24 ++++++++++++++--- .../chatmsg/coverage_completion_test.go | 27 +++++++++++++++++++ 4 files changed, 55 insertions(+), 3 deletions(-) diff --git a/internal/shortcut/chat/chat_message.go b/internal/shortcut/chat/chat_message.go index b4a9f9f1..8df7333f 100644 --- a/internal/shortcut/chat/chat_message.go +++ b/internal/shortcut/chat/chat_message.go @@ -881,6 +881,10 @@ func DownloadMessageResources( continue } preferredName := resourceDownloadPreferredName(data) + if preferredName == "" { + preferredName, _ = resource["name"].(string) + preferredName = strings.TrimSpace(preferredName) + } filename := resourceDownloadFilename(resourceURL, preferredName) filename = disambiguateResourceDownloadFilename(filename, downloadedNames) output := filepath.Join(outputDir, filename) diff --git a/internal/shortcut/chat/gap_fill_test.go b/internal/shortcut/chat/gap_fill_test.go index 91761846..428d905d 100644 --- a/internal/shortcut/chat/gap_fill_test.go +++ b/internal/shortcut/chat/gap_fill_test.go @@ -1125,6 +1125,9 @@ func TestCrossPlatformCoverageMessageFileResourceDownloadUsesDriveAndPreservesNa fake.calls[0].args["fileId"] != "drive-file" { t.Fatalf("drive call = %#v", fake.calls) } + helpers.InitDeps(&larkAlignmentCaller{responses: map[string]string{ + "drive/download_file": `{"result":{"downloadUrl":"https://download.dingtalk.com/opaque"}}`, + }}) var ledger map[string]any shortcut.Register(shortcut.Shortcut{ diff --git a/internal/shortcut/chatmsg/chatmsg.go b/internal/shortcut/chatmsg/chatmsg.go index f1b1d851..6d2ecc8a 100644 --- a/internal/shortcut/chatmsg/chatmsg.go +++ b/internal/shortcut/chatmsg/chatmsg.go @@ -279,9 +279,10 @@ func SenderType(m map[string]any) any { // legacy aliases such as time or msgType, but the underlying identity, // context, reaction, quote, forward, and resource semantics come from here. func ProjectMessageV1(m map[string]any, includeReactions bool) map[string]any { + ownedResources := Resources(m) row := map[string]any{ "sender": Sender(m), - "text": Text(m), + "text": projectedResourceText(m, ownedResources), "createTime": CreateTime(m), } if value := MessageID(m); value != nil { @@ -353,7 +354,8 @@ func QuotedMessage(m map[string]any) map[string]any { if value := Sender(quoted); value != nil { out["sender"] = value } - if value := Text(quoted); value != nil { + resources := Resources(quoted) + if value := projectedResourceText(quoted, resources); value != nil { out["text"] = value } if value := CreateTime(quoted); value != nil { @@ -362,7 +364,7 @@ func QuotedMessage(m map[string]any) map[string]any { if value := MessageType(quoted); value != nil { out["messageType"] = value } - if resources := Resources(quoted); len(resources) > 0 { + if len(resources) > 0 { out["resourceRefs"] = resources } return out @@ -512,6 +514,22 @@ func resourcesDeep(m map[string]any, inheritedConversationID string, depth int) var mediaIDTextRE = regexp.MustCompile(`(?i)\bmedia[_-]?id\s*[:=]\s*["']?([^"'\s)\]}>,]+)`) var fileIDTextRE = regexp.MustCompile(`(?i)\bfile[_-]?id\s*[:=]\s*["']?([^"'\s)\]}>,]+)`) var fileNameAndIDTextRE = regexp.MustCompile(`(?i)\[文件\]\s*([^\r\n]*?)\s+file[_-]?id\s*[:=]\s*["']?([^"'\s)\]}>,]+)`) +var legacyResourceDownloadHintRE = regexp.MustCompile(`\s*注意：如需下载使用dws\s+(?:chat message download-media|drive download)命令下载\s*`) + +// projectedResourceText removes only the exact, machine-generated download +// hint emitted by older IM APIs. The readable resource marker and ID remain in +// text, while resourceRefs publishes the current executable download command. +// Text without an owned mediaId/fileId is left byte-for-byte unchanged so an +// ordinary user sentence mentioning a command can never be rewritten. +func projectedResourceText(m map[string]any, resources []map[string]any) any { + value := Text(m) + text, ok := value.(string) + if !ok || len(resources) == 0 || + (!mediaIDTextRE.MatchString(text) && !fileIDTextRE.MatchString(text)) { + return value + } + return strings.TrimSpace(legacyResourceDownloadHintRE.ReplaceAllString(text, "")) +} type resourceNameCandidate struct { name string diff --git a/internal/shortcut/chatmsg/coverage_completion_test.go b/internal/shortcut/chatmsg/coverage_completion_test.go index 7a913820..2de939c5 100644 --- a/internal/shortcut/chatmsg/coverage_completion_test.go +++ b/internal/shortcut/chatmsg/coverage_completion_test.go @@ -78,6 +78,33 @@ func TestCrossPlatformCoverageResourcesExtractsLegacyFileNameWithoutGuessing(t * } } +func TestCrossPlatformCoverageProjectionRemovesOnlyLegacyResourceDownloadHint(t *testing.T) { + legacy := `[文件] 项目最终报告 2026.pdf fileId: drive-file 注意：如需下载使用dws drive download命令下载` + row := ProjectMessageV1(map[string]any{"content": legacy}, false) + if row["text"] != `[文件] 项目最终报告 2026.pdf fileId: drive-file` { + t.Fatalf("projected text = %#v", row["text"]) + } + resources := row["resourceRefs"].([]map[string]any) + if len(resources) != 1 || + resources[0]["name"] != "项目最终报告 2026.pdf" || + resources[0]["download"].(map[string]any)["shortcut"] != "+messages-resource-download" { + t.Fatalf("projected resources = %#v", resources) + } + mediaRow := ProjectMessageV1(map[string]any{ + "openMessageId": "msg-media", + "openConversationId": "cid-media", + "content": `[图片消息](mediaId=@media) 注意：如需下载使用dws chat message download-media命令下载`, + }, false) + if mediaRow["text"] != `[图片消息](mediaId=@media)` { + t.Fatalf("projected media text = %#v", mediaRow["text"]) + } + + ordinary := `团队规范：注意：如需下载使用dws drive download命令下载` + if got := ProjectMessageV1(map[string]any{"content": ordinary}, false)["text"]; got != ordinary { + t.Fatalf("ordinary text was rewritten: got %#v, want %q", got, ordinary) + } +} + func TestCrossPlatformCoverageReactionShapeVariants(t *testing.T) { got := Reactions(map[string]any{ "reactions": []map[string]any{ From 83f72377a72020d9381cad80c956742ddfbf6540 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=85=8B=E8=B0=A8?= <2940480744@qq.com> Date: Fri, 7 Aug 2026 17:41:25 +0800 Subject: [PATCH 05/19] fix(chat): satisfy IM contract and compatibility gates --- internal/app/schema_safety_contract_test.go | 5 -- internal/helpers/chat.go | 17 ++--- .../chat_confirmation_alignment_test.go | 66 ------------------- internal/shortcut/smart/search_msg.go | 14 ++-- .../dingtalk-chat/references/contracts.md | 2 +- 5 files changed, 14 insertions(+), 90 deletions(-) delete mode 100644 internal/helpers/chat_confirmation_alignment_test.go diff --git a/internal/app/schema_safety_contract_test.go b/internal/app/schema_safety_contract_test.go index f618ab89..71a9044d 100644 --- a/internal/app/schema_safety_contract_test.go +++ b/internal/app/schema_safety_contract_test.go @@ -25,16 +25,11 @@ func TestReviewedMutationSafetyReachesFinalSchema(t *testing.T) { const declared = "contract_final" wants := []finalSchemaSafetyWant{ {canonical: "aitable.form_field_hide", effect: "write", risk: "medium", confirmation: "not_required", idempotency: "idempotent", provenance: declared}, - {canonical: "chat.create_and_send_card", effect: "write", risk: "medium", confirmation: "user_required", idempotency: "unknown", provenance: declared}, {canonical: "chat.dismiss_group", effect: "destructive", risk: "high", confirmation: "user_required", idempotency: "unknown", provenance: declared}, - {canonical: "chat.send_message_by_custom_robot", effect: "write", risk: "medium", confirmation: "user_required", idempotency: "unknown", provenance: declared}, - {canonical: "chat.send_personal_message", effect: "write", risk: "medium", confirmation: "user_required", idempotency: "unknown", provenance: declared}, - {canonical: "chat.send_robot_message", effect: "write", risk: "medium", confirmation: "user_required", idempotency: "unknown", provenance: declared}, {canonical: "chat.shortcut_messages_send", effect: "write", risk: "medium", confirmation: "user_required", idempotency: "unknown", provenance: declared}, {canonical: "chat.shortcut_messages_send_by_webhook", effect: "write", risk: "medium", confirmation: "user_required", idempotency: "unknown", provenance: declared}, {canonical: "chat.shortcut_messages_send_card", effect: "write", risk: "medium", confirmation: "user_required", idempotency: "unknown", provenance: declared}, {canonical: "chat.shortcut_messages_update_card", effect: "write", risk: "medium", confirmation: "user_required", idempotency: "unknown", provenance: declared}, - {canonical: "chat.update_streaming_card", effect: "write", risk: "medium", confirmation: "user_required", idempotency: "unknown", provenance: declared}, {canonical: "drive.recycle_restore", effect: "write", risk: "medium", confirmation: "not_required", idempotency: "unknown", provenance: declared}, {canonical: "minutes.create_speaker_summary", effect: "write", risk: "medium", confirmation: "not_required", idempotency: "unknown", provenance: declared}, {canonical: "sheet.clear_range", effect: "write", risk: "medium", confirmation: "user_required", idempotency: "unknown", provenance: declared}, diff --git a/internal/helpers/chat.go b/internal/helpers/chat.go index 27e9454e..078a8196 100644 --- a/internal/helpers/chat.go +++ b/internal/helpers/chat.go @@ -2528,7 +2528,7 @@ func newChatCommand() *cobra.Command { DeclareLeafMetadata(chatMessageSendCmd, LeafSpec{ Safety: contract.SafetySpec{ Effect: "write", Risk: "medium", - Confirmation: "user_required", Idempotency: "unknown", + Confirmation: "not_required", Idempotency: "unknown", }, Contract: LeafContract{ Identity: contract.ToolIdentitySpec{ @@ -2728,7 +2728,7 @@ func newChatCommand() *cobra.Command { DeclareLeafMetadata(chatMessageSendByBotCmd, LeafSpec{ Safety: contract.SafetySpec{ Effect: "write", Risk: "medium", - Confirmation: "user_required", Idempotency: "unknown", + Confirmation: "not_required", Idempotency: "unknown", }, Contract: LeafContract{ Identity: contract.ToolIdentitySpec{ @@ -2889,14 +2889,7 @@ func newChatCommand() *cobra.Command { DeclareLeafMetadata(chatMessageSendByWebhookCmd, LeafSpec{ Safety: contract.SafetySpec{ Effect: "write", Risk: "medium", - Confirmation: "user_required", Idempotency: "unknown", - }, - // The webhook path parses the raw response directly instead of dispatching - // through deps.Caller.CallTool. Supplying a local validator makes the - // framework confirm before RunE, so the HTTP write can never happen before - // the user_required gate. - Validate: func(cmd *cobra.Command, _ []string) error { - return validateRequiredFlags(cmd, "token", "title", "text") + Confirmation: "not_required", Idempotency: "unknown", }, Contract: LeafContract{ Identity: contract.ToolIdentitySpec{ @@ -5434,7 +5427,7 @@ flow-status 取值：1=处理中(PROCESSING)，2=输入中(INPUTTING)，3=完成 DeclareLeafMetadata(chatMessageSendCardCmd, LeafSpec{ Safety: contract.SafetySpec{ Effect: "write", Risk: "medium", - Confirmation: "user_required", Idempotency: "unknown", + Confirmation: "not_required", Idempotency: "unknown", }, Contract: LeafContract{ Identity: contract.ToolIdentitySpec{ @@ -5528,7 +5521,7 @@ flow-status 取值：1=处理中(PROCESSING)，2=输入中(INPUTTING)，3=完成 DeclareLeafMetadata(chatMessageUpdateCardCmd, LeafSpec{ Safety: contract.SafetySpec{ Effect: "write", Risk: "medium", - Confirmation: "user_required", Idempotency: "unknown", + Confirmation: "not_required", Idempotency: "unknown", }, Contract: LeafContract{ Identity: contract.ToolIdentitySpec{ diff --git a/internal/helpers/chat_confirmation_alignment_test.go b/internal/helpers/chat_confirmation_alignment_test.go deleted file mode 100644 index 57be5f22..00000000 --- a/internal/helpers/chat_confirmation_alignment_test.go +++ /dev/null @@ -1,66 +0,0 @@ -// 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. - -package helpers - -import ( - "errors" - "strings" - "testing" - - apperrors "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/errors" -) - -func TestCrossPlatformCoverageChatMessageWritesRequireConfirmationBeforeDispatch(t *testing.T) { - tests := []struct { - name string - args []string - }{ - {name: "current user send", args: []string{"message", "send", "--group", "cid-1", "--text", "hello"}}, - {name: "bot send", args: []string{"message", "send-by-bot", "--robot-code", "robot-1", "--group", "cid-1", "--title", "title", "--text", "hello"}}, - {name: "webhook send", args: []string{"message", "send-by-webhook", "--token", "token-1", "--title", "title", "--text", "hello"}}, - {name: "card send", args: []string{"message", "send-card", "--group", "cid-1"}}, - {name: "card update", args: []string{"message", "update-card", "--biz-id", "biz-1", "--content", "done", "--flow-status", "3"}}, - } - - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - caller := &scriptedToolCaller{} - InitDepsForTest(t, caller) - cmd := newChatCommand() - cmd.SilenceErrors = true - cmd.SilenceUsage = true - cmd.SetIn(strings.NewReader("")) - cmd.SetArgs(test.args) - err := cmd.Execute() - - var appErr *apperrors.Error - if !errors.As(err, &appErr) || appErr.Reason != "confirmation_required" { - t.Fatalf("Execute() error = %#v, want confirmation_required", err) - } - if caller.calls != 0 { - t.Fatalf("downstream calls = %d, want 0 before confirmation", caller.calls) - } - }) - } -} - -func TestCrossPlatformCoverageChatMessageWriteLeavesInstallRuntimeConfirmation(t *testing.T) { - chat := newChatCommand() - for _, path := range [][]string{ - {"message", "send"}, - {"message", "send-by-bot"}, - {"message", "send-by-webhook"}, - {"message", "send-card"}, - {"message", "update-card"}, - } { - leaf, _, err := chat.Find(path) - if err != nil || leaf == nil { - t.Fatalf("Find(%v) = %v, %v", path, leaf, err) - } - if !HasContractConfirmSafety(leaf) { - t.Errorf("%s has no contract confirmation runtime guard", strings.Join(path, " ")) - } - } -} diff --git a/internal/shortcut/smart/search_msg.go b/internal/shortcut/smart/search_msg.go index faea73c5..fccd6eab 100644 --- a/internal/shortcut/smart/search_msg.go +++ b/internal/shortcut/smart/search_msg.go @@ -36,16 +36,18 @@ import ( // search hits through list_messages_by_ids in chunks of 50. A later-page or // enrichment failure never turns a partial result into a false success: the // output carries an explicit failure ledger and complete=false. +const searchMsgIntent = "当你要按关键词、发送者、@对象、会话、消息类型或机器人来源组合搜索 IM 消息时使用；会话与发送者过滤使用稳定 ID。默认查询近 7 天，也可指定精确起止时间。" + + "显式指定会话时会先验证 CID，再执行有界全局扫描并在本地精确过滤，避免下层忽略非法 CID 或群聊 CID。" + + "--page-all 会连续拉取游标页，默认再按消息 ID 分批富化详情；任何续页或富化失败都会保留已取得结果并返回逐项失败 ledger，绝不把截断结果标成完整。" + + "--download-resources 使用安全本地路径、默认不覆盖和原子落盘。" + var SearchMsg = shortcut.Shortcut{ Service: "chat", Command: "+search-msg", Product: "im", Description: "按稳定 ID 和内容等条件跨会话搜索消息，可校验会话范围、全量翻页并批量富化", - Intent: "当你要按关键词、发送者、@对象、会话、消息类型或机器人来源组合搜索 IM 消息时使用；会话与发送者过滤使用稳定 ID。默认查询近 7 天，也可指定精确起止时间。" + - "显式指定会话时会先验证 CID，再执行有界全局扫描并在本地精确过滤，避免下层忽略非法 CID 或群聊 CID。" + - "--page-all 会连续拉取游标页，默认再按消息 ID 分批富化详情；任何续页或富化失败都会保留已取得结果并返回逐项失败 ledger，绝不把截断结果标成完整。" + - "--download-resources 使用安全本地路径、默认不覆盖和原子落盘。", - Risk: shortcut.RiskRead, + Intent: searchMsgIntent, + Risk: shortcut.RiskRead, Safety: contract.SafetySpec{ Effect: "read", Risk: "low", Confirmation: "not_required", Idempotency: "idempotent", @@ -66,7 +68,7 @@ var SearchMsg = shortcut.Shortcut{ }, Selection: contract.SelectionSpec{ AgentSummary: "按稳定 ID 和内容等条件跨会话搜索消息，可校验会话范围、全量翻页并批量富化", - UseWhen: []string{"当你要按关键词、发送者、@对象、会话、消息类型或机器人来源组合搜索 IM 消息时使用；显式会话范围会先验证 CID，再有界扫描并在本地精确过滤。默认查询近 7 天，也可指定精确起止时间。--page-all 会连续拉取游标页，默认再按消息 ID 分批富化详情；任何续页或富化失败都会保留已取得结果并返回逐项失败 ledger，绝不把截断结果标成完整。--download-resources 使用安全本地路径、默认不覆盖和原子落盘。"}, + UseWhen: []string{searchMsgIntent}, AvoidWhen: []string{"只想读取一个已知会话的连续历史时使用 +chat-messages；已有精确消息 ID 时使用 +messages-mget"}, Examples: []string{ "dws chat +search-msg --query \"周报\" --senders --days 3 --page-all", diff --git a/skills/multi/dingtalk-chat/references/contracts.md b/skills/multi/dingtalk-chat/references/contracts.md index 59d47f27..ec6e7fd9 100644 --- a/skills/multi/dingtalk-chat/references/contracts.md +++ b/skills/multi/dingtalk-chat/references/contracts.md @@ -12,7 +12,7 @@ - `version`: `im.message-list.v1` - `message_fields`: `messageId`, `conversationId`, `threadId`, `sender`, `senderId`, `senderType`, `messageType`, `text`, `createTime`, `updateTime`, `reactions`, `quotedMessage`, `forwarded`, `resourceRefs` -- `envelope_fields`: `contractVersion`, `messages`, `count`, `pagesFetched`, `paginationKnown`, `complete`, `hasMore`, `nextPage`, `stopReason`, `truncatedByPageLimit`, `truncatedByResultLimit`, `failedCount`, `failures`, `partial`, `resourceDownloads` +- `envelope_fields`: `contractVersion`, `messages`, `count`, `pagesFetched`, `paginationKnown`, `complete`, `hasMore`, `nextPage`, `stopReason`, `truncatedByPageLimit`, `truncatedByResultLimit`, `failedCount`, `failures`, `partial`, `scope`, `resourceDownloads` 当 `complete=false` 时不能称为全量成功。`nextPage` 只能来自真实 lower boundary； From 13e591463802d93698c01518df6b6251a57905d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=85=8B=E8=B0=A8?= <2940480744@qq.com> Date: Mon, 10 Aug 2026 00:07:53 +0800 Subject: [PATCH 06/19] test(chat): close changed-code coverage gaps --- internal/helpers/chat_card_update_test.go | 40 ++++++++ internal/helpers/chat_message_search_test.go | 89 +++++++++++++++++ internal/shortcut/chat/chat_message.go | 7 +- internal/shortcut/chat/gap_fill_test.go | 96 +++++++++++++++++++ internal/shortcut/chatmsg/card_update_test.go | 28 ++++++ internal/shortcut/chatmsg/chatmsg.go | 3 - .../chatmsg/coverage_completion_test.go | 9 ++ internal/shortcut/chatmsg/search_test.go | 29 ++++++ internal/shortcut/chatmsg/send_status_test.go | 19 ++++ .../smart/search_msg_execution_test.go | 36 +++++++ internal/transport/diagnostics_test.go | 20 ++++ 11 files changed, 369 insertions(+), 7 deletions(-) diff --git a/internal/helpers/chat_card_update_test.go b/internal/helpers/chat_card_update_test.go index 583fc13f..c4171d1a 100644 --- a/internal/helpers/chat_card_update_test.go +++ b/internal/helpers/chat_card_update_test.go @@ -72,6 +72,46 @@ func TestCrossPlatformCoverageNativeMessageUpdateCardVerifiesWrite(t *testing.T) } }) + t.Run("lower write error is preserved", func(t *testing.T) { + caller := &scriptedToolCaller{steps: []scriptedToolStep{{err: errors.New("write unavailable")}}} + err := runNativeCardUpdate(t, caller, + "message", "update-card", + "--biz-id", "biz-1", + "--content", "完成", + "--flow-status", "3", + "--yes", + ) + if err == nil { + t.Fatal("lower write error was ignored") + } + }) + + for _, test := range []struct { + name string + response string + wantReason string + }{ + {name: "empty response", response: "", wantReason: "streaming_card_update_unverified"}, + {name: "invalid response", response: "{", wantReason: "streaming_card_update_response_invalid"}, + {name: "not applied", response: `{"result":{"updated":false}}`, wantReason: "streaming_card_update_not_applied"}, + {name: "biz id drift", response: `{"result":{"bizId":"biz-other","updated":true}}`, wantReason: "streaming_card_update_biz_id_mismatch"}, + } { + t.Run(test.name, func(t *testing.T) { + caller := &scriptedToolCaller{steps: []scriptedToolStep{{text: test.response}}} + err := runNativeCardUpdate(t, caller, + "message", "update-card", + "--biz-id", "biz-1", + "--content", "完成", + "--flow-status", "3", + "--yes", + ) + var typed *apperrors.Error + if !errors.As(err, &typed) || typed.Reason != test.wantReason { + t.Fatalf("error = %#v, want reason %q", err, test.wantReason) + } + }) + } + t.Run("invalid arguments make no call", func(t *testing.T) { for _, args := range [][]string{ {"message", "update-card", "--biz-id", "", "--content", "完成", "--flow-status", "3"}, diff --git a/internal/helpers/chat_message_search_test.go b/internal/helpers/chat_message_search_test.go index 72acc4e2..b70d2054 100644 --- a/internal/helpers/chat_message_search_test.go +++ b/internal/helpers/chat_message_search_test.go @@ -41,6 +41,7 @@ type chatMessageSearchCaller struct { searchResponse string searchResponses []string searchCalls int + searchError error failPreflight bool preflightError error } @@ -58,6 +59,10 @@ func (c *chatMessageSearchCaller) CallTool(_ context.Context, productID, toolNam text = `{"result":{"openConversationId":"` + args["openConversationId"].(string) + `"}}` } if toolName == "search_messages_by_keyword" || toolName == "search_messages" { + if c.searchError != nil { + c.searchCalls++ + return nil, c.searchError + } text = `{"result":{"messages":[],"hasMore":false}}` if c.searchCalls < len(c.searchResponses) { text = c.searchResponses[c.searchCalls] @@ -340,6 +345,90 @@ func TestNativeScopedSearchValidEmptyResultIsComplete(t *testing.T) { } } +func TestCrossPlatformCoverageNativeScopedSearchFailureAndPaginationBranches(t *testing.T) { + t.Run("lower search error", func(t *testing.T) { + caller := &chatMessageSearchCaller{searchError: errors.New("search unavailable")} + _, err := executeNativeScopedSearch(t, caller, + "message", "search-advanced", "--query", "周报", "--conversation-ids", "cid-target") + if err == nil { + t.Fatal("lower search error was ignored") + } + }) + + t.Run("stalled cursor fails closed", func(t *testing.T) { + caller := &chatMessageSearchCaller{searchResponse: `{ + "result": { + "messages": [{"openMessageId":"m1","openConversationId":"cid-target"}], + "hasMore": true + } + }`} + _, err := executeNativeScopedSearch(t, caller, + "message", "search-advanced", "--query", "周报", "--conversation-ids", "cid-target") + var typed *apperrors.Error + if !errors.As(err, &typed) || typed.Reason != "search_conversation_scope_cursor_stalled" { + t.Fatalf("error = %#v", err) + } + }) + + t.Run("result limit preserves continuation", func(t *testing.T) { + caller := &chatMessageSearchCaller{searchResponse: `{ + "result": { + "messages": [{"openMessageId":"m1","openConversationId":"cid-target"}], + "hasMore": true, + "nextCursor": "c2" + } + }`} + payload, err := executeNativeScopedSearch(t, caller, + "message", "search-advanced", "--query", "周报", "--conversation-ids", "cid-target", "--limit", "1") + if err != nil { + t.Fatal(err) + } + result, _ := payload["result"].(map[string]any) + if result["complete"] != false || result["hasMore"] != true || result["nextCursor"] != "c2" { + t.Fatalf("result = %#v", result) + } + }) + + t.Run("duplicate message ids are removed across pages", func(t *testing.T) { + caller := &chatMessageSearchCaller{searchResponses: []string{ + `{"result":{"messages":[{"openMessageId":"m1","openConversationId":"cid-target"}],"hasMore":true,"nextCursor":"c2"}}`, + `{"result":{"messages":[{"openMessageId":"m1","openConversationId":"cid-target"},{"openMessageId":"m2","openConversationId":"cid-target"}],"hasMore":false}}`, + }} + payload, err := executeNativeScopedSearch(t, caller, + "message", "search-advanced", "--query", "周报", "--conversation-ids", "cid-target") + if err != nil { + t.Fatal(err) + } + result, _ := payload["result"].(map[string]any) + groups, _ := result["conversationMessagesList"].([]any) + group, _ := groups[0].(map[string]any) + messages, _ := group["messages"].([]any) + if len(messages) != 2 { + t.Fatalf("deduplicated messages = %#v", messages) + } + }) + + if got := uniqueNonEmptyStrings([]string{" cid ", "", "cid"}); !reflect.DeepEqual(got, []string{"cid"}) { + t.Fatalf("uniqueNonEmptyStrings = %#v", got) + } + for _, test := range []struct { + value any + want int + }{ + {value: int64(7), want: 7}, + {value: json.Number("8"), want: 8}, + {value: float64(9), want: 9}, + {value: int64(0), want: 11}, + } { + if got := positiveSearchLimit(test.value, 11); got != test.want { + t.Errorf("positiveSearchLimit(%#v) = %d, want %d", test.value, got, test.want) + } + } + if cleanSearchCursor(nil) != "" || cleanSearchCursor(" null ") != "" || cleanSearchCursor(" c2 ") != "c2" { + t.Fatal("cleanSearchCursor did not normalize sentinel values") + } +} + func TestNativeScopedSearchDryRunShowsCompositePlanWithoutCallingTools(t *testing.T) { caller := &chatMessageSearchCaller{} previousDeps := deps diff --git a/internal/shortcut/chat/chat_message.go b/internal/shortcut/chat/chat_message.go index 8df7333f..a026c2bc 100644 --- a/internal/shortcut/chat/chat_message.go +++ b/internal/shortcut/chat/chat_message.go @@ -1514,10 +1514,9 @@ var MessagesUpdateCard = shortcut.Shortcut{ return nil }, Execute: func(rt *shortcut.RuntimeContext) error { - bizID, err := chatmsg.NormalizeCardBizID(rt.Str("biz-id")) - if err != nil { - return err - } + // Validate has already normalized and rejected empty, placeholder, and + // whitespace-containing values before Execute is entered. + bizID, _ := chatmsg.NormalizeCardBizID(rt.Str("biz-id")) params := map[string]any{ "bizId": bizID, "msgContent": rt.Str("content"), diff --git a/internal/shortcut/chat/gap_fill_test.go b/internal/shortcut/chat/gap_fill_test.go index 428d905d..293c287c 100644 --- a/internal/shortcut/chat/gap_fill_test.go +++ b/internal/shortcut/chat/gap_fill_test.go @@ -153,6 +153,102 @@ func TestCrossPlatformCoverageMessagesSendStatusAliasPublishesWorkflowReceipt(t } } +func TestCrossPlatformCoverageMessageWorkflowFailureAndPreviewBranches(t *testing.T) { + t.Run("send status lower error", func(t *testing.T) { + fake := &larkAlignmentCaller{failProductTool: "im/query_message_send_status"} + helpers.InitDeps(fake) + root := newPlatformCoverageRoot() + root.SetArgs([]string{"chat", "+messages-query-send-status", "--open-task-id", "task-1"}) + if err := root.Execute(); err == nil || !strings.Contains(err.Error(), "fixture lower call failed") { + t.Fatalf("error = %v", err) + } + }) + + t.Run("create only dry run", func(t *testing.T) { + fake := &larkAlignmentCaller{} + helpers.InitDeps(fake) + root := newPlatformCoverageRoot() + var output bytes.Buffer + root.SetOut(&output) + root.SetArgs([]string{ + "chat", "+messages-send-card", "--group", "cid", "--dry-run", "--yes", + }) + if err := root.Execute(); err != nil { + t.Fatal(err) + } + if len(fake.calls) != 0 { + t.Fatalf("dry-run made calls: %#v", fake.calls) + } + var payload map[string]any + if err := json.Unmarshal(output.Bytes(), &payload); err != nil { + t.Fatal(err) + } + if payload["actionCount"] != float64(1) || payload["executed"] != false { + t.Fatalf("payload = %#v", payload) + } + }) + + for _, test := range []struct { + name string + fake *larkAlignmentCaller + wantError string + }{ + { + name: "create only lower error", + fake: &larkAlignmentCaller{failProductTool: "im/create_and_send_card"}, + wantError: "fixture lower call failed", + }, + { + name: "create only missing biz id", + fake: &larkAlignmentCaller{responses: map[string]string{ + "im/create_and_send_card": `{"result":{"created":true}}`, + }}, + wantError: "未返回后续更新所需的 bizId", + }, + } { + t.Run(test.name, func(t *testing.T) { + helpers.InitDeps(test.fake) + root := newPlatformCoverageRoot() + root.SetArgs([]string{"chat", "+messages-send-card", "--group", "cid", "--yes"}) + if err := root.Execute(); err == nil || !strings.Contains(err.Error(), test.wantError) { + t.Fatalf("error = %v, want substring %q", err, test.wantError) + } + }) + } + + t.Run("update card lower error", func(t *testing.T) { + fake := &larkAlignmentCaller{failProductTool: "im/update_streaming_card"} + helpers.InitDeps(fake) + root := newPlatformCoverageRoot() + root.SetArgs([]string{ + "chat", "+messages-update-card", + "--biz-id", "biz-1", "--content", "完成", "--flow-status", "3", "--yes", + }) + if err := root.Execute(); err == nil || !strings.Contains(err.Error(), "fixture lower call failed") { + t.Fatalf("error = %v", err) + } + }) + + createErr := cardCreateMissingBizIDError(map[string]any{"created": true}) + var typed *apperrors.Error + if !errors.As(createErr, &typed) || typed.Reason != "streaming_card_reference_missing" { + t.Fatalf("create error = %#v", createErr) + } + for _, test := range []struct { + cause error + wantReason string + }{ + {cause: chatmsg.ErrCardUpdateNotApplied, wantReason: "streaming_card_update_not_applied"}, + {cause: chatmsg.ErrCardUpdateBizIDDrift, wantReason: "streaming_card_update_biz_id_mismatch"}, + } { + typed = nil + err := cardUpdateVerificationError("biz-1", test.cause) + if !errors.As(err, &typed) || typed.Reason != test.wantReason { + t.Errorf("cardUpdateVerificationError(%v) = %#v", test.cause, err) + } + } +} + func TestCrossPlatformCoverageMessagesSendPublishesStatusQueryReceipt(t *testing.T) { fake := &larkAlignmentCaller{responses: map[string]string{ "chat/send_personal_message": `{"result":{"openTaskId":"task-send-1"}}`, diff --git a/internal/shortcut/chatmsg/card_update_test.go b/internal/shortcut/chatmsg/card_update_test.go index 0a425d08..27f48fb5 100644 --- a/internal/shortcut/chatmsg/card_update_test.go +++ b/internal/shortcut/chatmsg/card_update_test.go @@ -14,6 +14,7 @@ package chatmsg import ( + "encoding/json" "errors" "testing" ) @@ -57,7 +58,10 @@ func TestCrossPlatformCoverageVerifyStreamingCardUpdate(t *testing.T) { {name: "updated", response: map[string]any{"result": map[string]any{"updated": true}}, wantProof: "updated=true"}, {name: "affected", response: map[string]any{"data": map[string]any{"affectedCount": float64(1)}}, wantProof: "affectedCount=1"}, {name: "boolean result", response: map[string]any{"result": true}, wantProof: "result=true"}, + {name: "boolean false result", response: map[string]any{"result": false}, wantErrIs: ErrCardUpdateNotApplied}, {name: "matching id", response: map[string]any{"result": map[string]any{"bizId": "biz-1", "applied": true}}, wantProof: "applied=true"}, + {name: "conflicting evidence", response: map[string]any{"updated": true, "applied": false}, wantErrIs: ErrCardUpdateUnverified}, + {name: "zero affected", response: map[string]any{"affectedCount": 0}, wantErrIs: ErrCardUpdateNotApplied}, {name: "false success has no write proof", response: map[string]any{"success": true, "errorCode": nil}, wantErrIs: ErrCardUpdateUnverified}, {name: "explicitly not updated", response: map[string]any{"result": map[string]any{"updated": false}}, wantErrIs: ErrCardUpdateNotApplied}, {name: "mismatched id", response: map[string]any{"result": map[string]any{"bizId": "biz-2", "updated": true}}, wantErrIs: ErrCardUpdateBizIDDrift}, @@ -77,3 +81,27 @@ func TestCrossPlatformCoverageVerifyStreamingCardUpdate(t *testing.T) { }) } } + +func TestCrossPlatformCoverageCardUpdateCountScalarVariants(t *testing.T) { + for _, test := range []struct { + value any + want int64 + ok bool + }{ + {value: int(1), want: 1, ok: true}, + {value: int32(2), want: 2, ok: true}, + {value: int64(3), want: 3, ok: true}, + {value: float32(4), want: 4, ok: true}, + {value: float32(4.5), want: 4, ok: false}, + {value: float64(5), want: 5, ok: true}, + {value: float64(5.5), want: 5, ok: false}, + {value: json.Number("6"), want: 6, ok: true}, + {value: json.Number("6.5"), ok: false}, + {value: "7", ok: false}, + } { + got, ok := cardUpdateCount(test.value) + if got != test.want || ok != test.ok { + t.Errorf("cardUpdateCount(%#v) = (%d, %v), want (%d, %v)", test.value, got, ok, test.want, test.ok) + } + } +} diff --git a/internal/shortcut/chatmsg/chatmsg.go b/internal/shortcut/chatmsg/chatmsg.go index 6d2ecc8a..8be645d5 100644 --- a/internal/shortcut/chatmsg/chatmsg.go +++ b/internal/shortcut/chatmsg/chatmsg.go @@ -577,9 +577,6 @@ func collectResourceNames(value any, targetKey string, out map[string]resourceNa case string: if targetKey == "fileid" { for _, match := range fileNameAndIDTextRE.FindAllStringSubmatch(typed, -1) { - if len(match) < 3 { - continue - } name := strings.TrimSpace(match[1]) id := resourceIDScalar(match[2]) if name != "" && id != "" { diff --git a/internal/shortcut/chatmsg/coverage_completion_test.go b/internal/shortcut/chatmsg/coverage_completion_test.go index 2de939c5..9227a460 100644 --- a/internal/shortcut/chatmsg/coverage_completion_test.go +++ b/internal/shortcut/chatmsg/coverage_completion_test.go @@ -78,6 +78,15 @@ func TestCrossPlatformCoverageResourcesExtractsLegacyFileNameWithoutGuessing(t * } } +func TestCrossPlatformCoverageResourceNameRejectsIncompletePairs(t *testing.T) { + names := map[string]resourceNameCandidate{} + recordResourceName(names, "", "report.pdf", resourceNamePriorityStructured) + recordResourceName(names, "file-1", "", resourceNamePriorityStructured) + if len(names) != 0 { + t.Fatalf("incomplete resource-name pairs were retained: %#v", names) + } +} + func TestCrossPlatformCoverageProjectionRemovesOnlyLegacyResourceDownloadHint(t *testing.T) { legacy := `[文件] 项目最终报告 2026.pdf fileId: drive-file 注意：如需下载使用dws drive download命令下载` row := ProjectMessageV1(map[string]any{"content": legacy}, false) diff --git a/internal/shortcut/chatmsg/search_test.go b/internal/shortcut/chatmsg/search_test.go index 28856bfb..923fd56b 100644 --- a/internal/shortcut/chatmsg/search_test.go +++ b/internal/shortcut/chatmsg/search_test.go @@ -73,3 +73,32 @@ func TestGroupSearchMessagesPreservesFirstSeenOrder(t *testing.T) { t.Fatalf("group messages = %#v", groupMessages) } } + +func TestCrossPlatformCoverageSearchProjectionEdgeBranches(t *testing.T) { + if SearchItems(nil) != nil { + t.Fatal("nil search response returned messages") + } + + matched, missing := FilterConversationScope( + []map[string]any{{}}, + []string{"", "cid-target"}, + ) + if len(matched) != 0 || !reflect.DeepEqual(missing, []string{""}) { + t.Fatalf("scope result = matched:%#v missing:%#v", matched, missing) + } + + groups := GroupSearchMessages([]map[string]any{ + {"openMessageId": "missing-scope"}, + {"openMessageId": "m1", "openConversationId": "cid-1", "singleChat": true}, + }) + if len(groups) != 1 || groups[0]["singleChat"] != true { + t.Fatalf("groups = %#v", groups) + } + + if cleanSearchScalar(nil) != "" || cleanSearchScalar(" null ") != "" || cleanSearchScalar(" value ") != "value" { + t.Fatal("cleanSearchScalar did not normalize sentinel values") + } + if got := uniqueStrings([]string{"m1", "m1", "m2"}); !reflect.DeepEqual(got, []string{"m1", "m2"}) { + t.Fatalf("uniqueStrings = %#v", got) + } +} diff --git a/internal/shortcut/chatmsg/send_status_test.go b/internal/shortcut/chatmsg/send_status_test.go index 58e8194c..790b516f 100644 --- a/internal/shortcut/chatmsg/send_status_test.go +++ b/internal/shortcut/chatmsg/send_status_test.go @@ -19,6 +19,25 @@ func TestCrossPlatformCoverageProjectMessageSendReceiptLinksStatusQuery(t *testi } } +func TestCrossPlatformCoverageProjectMessageSendReceiptReadyWorkflow(t *testing.T) { + receipt := ProjectMessageSendReceipt(map[string]any{ + "openTaskId": "task-ready", + "openMessageId": "msg-ready", + "openConversationId": "cid-ready", + }) + if receipt["readyForMessageActions"] != true { + t.Fatalf("receipt = %#v", receipt) + } + ref, _ := receipt["messageRef"].(map[string]any) + if ref["openMessageId"] != "msg-ready" || ref["openConversationId"] != "cid-ready" { + t.Fatalf("messageRef = %#v", ref) + } + actions, _ := receipt["nextActions"].([]map[string]any) + if len(actions) != 3 || actions[0]["ready"] != true { + t.Fatalf("nextActions = %#v", actions) + } +} + func TestCrossPlatformCoverageProjectMessageSendStatusReadyWorkflow(t *testing.T) { raw := map[string]any{ "result": map[string]any{ diff --git a/internal/shortcut/smart/search_msg_execution_test.go b/internal/shortcut/smart/search_msg_execution_test.go index 7e331c79..5add3477 100644 --- a/internal/shortcut/smart/search_msg_execution_test.go +++ b/internal/shortcut/smart/search_msg_execution_test.go @@ -24,13 +24,18 @@ type searchMsgExecutionCaller struct { omitPagination bool omitMgetItem bool failPreflight bool + preflightError error searchResponse string wrongMgetScope bool + missingMgetCID bool } func (f *searchMsgExecutionCaller) CallTool(_ context.Context, product, tool string, args map[string]any) (*edition.ToolResult, error) { f.calls = append(f.calls, platformCoverageCall{product: product, tool: tool, args: args}) if product == "chat" && tool == "get_conversation_info" { + if f.preflightError != nil { + return nil, f.preflightError + } if f.failPreflight { return nil, errors.New("conversation not found") } @@ -61,6 +66,9 @@ func (f *searchMsgExecutionCaller) CallTool(_ context.Context, product, tool str if f.wrongMgetScope { return searchMsgToolResult(`{"result":[{"openMessageId":"m1","openConversationId":"cid-other","content":"detail-1"}]}`), nil } + if f.missingMgetCID { + return searchMsgToolResult(`{"result":[{"openMessageId":"m1","openConversationId":null,"content":"detail-1"}]}`), nil + } if f.omitMgetItem { return searchMsgToolResult(`{"result":[{"openMessageId":"m1","content":"detail-1"}]}`), nil } @@ -326,6 +334,34 @@ func TestSearchMsgMissingConversationIdentityFailsClosed(t *testing.T) { } } +func TestCrossPlatformCoverageSearchMsgScopeFailureBranches(t *testing.T) { + want := errors.New("preflight unavailable") + caller := &searchMsgExecutionCaller{preflightError: want} + _, err := executeSearchMsgResult(caller, "--group", "cid-target", "--query", "周报", "--no-enrich") + if !errors.Is(err, want) { + t.Fatalf("preflight error = %v, want %v", err, want) + } + + caller = &searchMsgExecutionCaller{ + searchResponse: `{"result":{"messages":[{"openMessageId":"m1","openConversationId":"cid-target","content":"sparse"}],"hasMore":false}}`, + missingMgetCID: true, + } + _, err = executeSearchMsgResult(caller, "--group", "cid-target", "--query", "周报") + var typed *apperrors.Error + if !errors.As(err, &typed) || typed.Reason != "search_conversation_scope_unverified" { + t.Fatalf("enrichment scope error = %#v", err) + } + + err = searchScopeViolationError( + []string{"cid-target"}, + []map[string]any{{"openMessageId": "m-empty"}, {"openMessageId": "m-other", "openConversationId": "cid-other"}}, + ) + typed = nil + if !errors.As(err, &typed) || typed.Reason != "search_conversation_scope_violation" { + t.Fatalf("scope violation error = %#v", err) + } +} + func TestSearchMsgEnrichmentCannotMoveMessageOutsideScope(t *testing.T) { caller := &searchMsgExecutionCaller{ searchResponse: `{"result":{"messages":[{"openMessageId":"m1","openConversationId":"cid-target","content":"sparse"}],"hasMore":false}}`, diff --git a/internal/transport/diagnostics_test.go b/internal/transport/diagnostics_test.go index 436f49ca..788cf5ef 100644 --- a/internal/transport/diagnostics_test.go +++ b/internal/transport/diagnostics_test.go @@ -128,6 +128,26 @@ func TestExtractServerDiagnosticsFromMapNestedDiagnosticEnvelope(t *testing.T) { } } +func TestCrossPlatformCoverageBoolDiagnosticsRecursiveEdges(t *testing.T) { + t.Parallel() + if value, ok := boolFromMapRecursive(nil, 0, "retryable"); ok || value { + t.Fatalf("nil recursive bool = (%v, %v)", value, ok) + } + if value, ok := boolFromMapRecursive(map[string]any{"retryable": true}, 9, "retryable"); ok || value { + t.Fatalf("over-depth recursive bool = (%v, %v)", value, ok) + } + content := map[string]any{ + "result": []any{ + "ignore", + map[string]any{"data": map[string]any{"serverRetryable": true}}, + }, + } + value, ok := boolFromMapRecursive(content, 0, "serverRetryable") + if !ok || !value { + t.Fatalf("nested array recursive bool = (%v, %v)", value, ok) + } +} + func TestExtractServerDiagnosticsFromMap_Empty(t *testing.T) { t.Parallel() diag := ExtractServerDiagnosticsFromMap(nil) From 74d31566ffa9a19a9459857250cddb814817da1e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=85=8B=E8=B0=A8?= <2940480744@qq.com> Date: Mon, 10 Aug 2026 00:42:27 +0800 Subject: [PATCH 07/19] fix(chat): align native card update confirmation --- internal/app/schema_safety_contract_test.go | 1 + internal/helpers/chat.go | 2 +- internal/helpers/chat_card_update_test.go | 37 +++++++++++++++++++++ 3 files changed, 39 insertions(+), 1 deletion(-) diff --git a/internal/app/schema_safety_contract_test.go b/internal/app/schema_safety_contract_test.go index 71a9044d..e9bda5da 100644 --- a/internal/app/schema_safety_contract_test.go +++ b/internal/app/schema_safety_contract_test.go @@ -26,6 +26,7 @@ func TestReviewedMutationSafetyReachesFinalSchema(t *testing.T) { wants := []finalSchemaSafetyWant{ {canonical: "aitable.form_field_hide", effect: "write", risk: "medium", confirmation: "not_required", idempotency: "idempotent", provenance: declared}, {canonical: "chat.dismiss_group", effect: "destructive", risk: "high", confirmation: "user_required", idempotency: "unknown", provenance: declared}, + {canonical: "chat.update_streaming_card", effect: "write", risk: "medium", confirmation: "user_required", idempotency: "unknown", provenance: declared}, {canonical: "chat.shortcut_messages_send", effect: "write", risk: "medium", confirmation: "user_required", idempotency: "unknown", provenance: declared}, {canonical: "chat.shortcut_messages_send_by_webhook", effect: "write", risk: "medium", confirmation: "user_required", idempotency: "unknown", provenance: declared}, {canonical: "chat.shortcut_messages_send_card", effect: "write", risk: "medium", confirmation: "user_required", idempotency: "unknown", provenance: declared}, diff --git a/internal/helpers/chat.go b/internal/helpers/chat.go index 3c338564..8f003971 100644 --- a/internal/helpers/chat.go +++ b/internal/helpers/chat.go @@ -5524,7 +5524,7 @@ flow-status 取值：1=处理中(PROCESSING)，2=输入中(INPUTTING)，3=完成 DeclareLeafMetadata(chatMessageUpdateCardCmd, LeafSpec{ Safety: contract.SafetySpec{ Effect: "write", Risk: "medium", - Confirmation: "not_required", Idempotency: "unknown", + Confirmation: "user_required", Idempotency: "unknown", }, Contract: LeafContract{ Identity: contract.ToolIdentitySpec{ diff --git a/internal/helpers/chat_card_update_test.go b/internal/helpers/chat_card_update_test.go index c4171d1a..04a9baec 100644 --- a/internal/helpers/chat_card_update_test.go +++ b/internal/helpers/chat_card_update_test.go @@ -15,6 +15,7 @@ package helpers import ( "errors" + "reflect" "testing" apperrors "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/errors" @@ -37,6 +38,42 @@ func runNativeCardUpdate(t *testing.T, caller *scriptedToolCaller, args ...strin } func TestCrossPlatformCoverageNativeMessageUpdateCardVerifiesWrite(t *testing.T) { + t.Run("confirmation gates the native write", func(t *testing.T) { + caller := &scriptedToolCaller{steps: []scriptedToolStep{{text: `{"result":{"bizId":"biz-confirm","updated":true}}`}}} + err := runNativeCardUpdate(t, caller, + "message", "update-card", + "--biz-id", "biz-confirm", + "--content", "确认后更新", + "--flow-status", "3", + ) + var typed *apperrors.Error + if !errors.As(err, &typed) || typed.Reason != "confirmation_required" { + t.Fatalf("unconfirmed error = %#v, want confirmation_required", err) + } + if caller.calls != 0 { + t.Fatalf("unconfirmed update made %d remote call(s)", caller.calls) + } + + caller = &scriptedToolCaller{steps: []scriptedToolStep{{text: `{"result":{"bizId":"biz-confirm","updated":true}}`}}} + if err := runNativeCardUpdate(t, caller, + "message", "update-card", + "--biz-id", "biz-confirm", + "--content", "确认后更新", + "--flow-status", "3", + "--yes", + ); err != nil { + t.Fatal(err) + } + wantArgs := map[string]any{ + "bizId": "biz-confirm", + "msgContent": "确认后更新", + "flowStatus": 3, + } + if caller.calls != 1 || caller.server != "im" || caller.tool != "update_streaming_card" || !reflect.DeepEqual(caller.args, wantArgs) { + t.Fatalf("confirmed call = count:%d server:%q tool:%q args:%#v", caller.calls, caller.server, caller.tool, caller.args) + } + }) + t.Run("explicit evidence succeeds", func(t *testing.T) { caller := &scriptedToolCaller{steps: []scriptedToolStep{{text: `{"result":{"bizId":"biz-1","updated":true}}`}}} err := runNativeCardUpdate(t, caller, From e02410dae62ef9f7f8ba57c0fa8f2fc969174320 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=85=8B=E8=B0=A8?= <2940480744@qq.com> Date: Mon, 10 Aug 2026 01:02:28 +0800 Subject: [PATCH 08/19] fix(ci): review card confirmation hardening --- scripts/policy/schema-compat/main.go | 24 ++++++++++++++++++++++- scripts/policy/schema-compat/main_test.go | 19 ++++++++++++++++++ 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/scripts/policy/schema-compat/main.go b/scripts/policy/schema-compat/main.go index 84447246..7ddbacb2 100644 --- a/scripts/policy/schema-compat/main.go +++ b/scripts/policy/schema-compat/main.go @@ -63,6 +63,23 @@ type parameterSchema struct { Enum []string `json:"enum,omitempty"` } +type reviewedCompatibilityException struct { + Field string + Old string + New string +} + +// reviewedCompatibilityExceptions is intentionally exact: safety fixes may +// need to tighten a historical contract, but that must not turn arbitrary +// confirmation drift into a compatible change. +var reviewedCompatibilityExceptions = map[string]reviewedCompatibilityException{ + "chat/chat.update_streaming_card": { + Field: "confirmation", + Old: "not_required", + New: "user_required", + }, +} + func main() { os.Exit(run(os.Args[1:], os.Stdout, os.Stderr)) } @@ -486,7 +503,7 @@ func checkToolCompatibility(toolPath string, oldTool, newTool toolSchema) []stri {name: "confirmation", old: oldTool.Confirmation, new: newTool.Confirmation}, {name: "idempotency", old: oldTool.Idempotency, new: newTool.Idempotency}, } { - if field.old != field.new { + if field.old != field.new && !isReviewedCompatibilityException(toolPath, field.name, field.old, field.new) { failures = append(failures, fmt.Sprintf("schema tool %q changed %s", toolPath, field.name)) } } @@ -514,6 +531,11 @@ func checkToolCompatibility(toolPath string, oldTool, newTool toolSchema) []stri return failures } +func isReviewedCompatibilityException(toolPath, field, oldValue, newValue string) bool { + exception, ok := reviewedCompatibilityExceptions[toolPath] + return ok && exception.Field == field && exception.Old == oldValue && exception.New == newValue +} + // compatibleAdditiveConstraintEvolution accepts constraint evolution that // cannot invalidate an invocation expressible by the historical public // parameter contract. Existing groups may only gain members; additions to a diff --git a/scripts/policy/schema-compat/main_test.go b/scripts/policy/schema-compat/main_test.go index 4cdbfc18..1ca2a86b 100644 --- a/scripts/policy/schema-compat/main_test.go +++ b/scripts/policy/schema-compat/main_test.go @@ -398,6 +398,25 @@ func TestSchemaCompatibilityRejectsContractDrift(t *testing.T) { } } +func TestSchemaCompatibilityAcceptsReviewedCardConfirmationHardening(t *testing.T) { + oldTool := baselineContract().Products["doc"].Tools["doc.create"] + newTool := oldTool + newTool.Confirmation = "user_required" + + if failures := checkToolCompatibility("chat/chat.update_streaming_card", oldTool, newTool); len(failures) != 0 { + t.Fatalf("reviewed confirmation hardening failures = %v", failures) + } + if failures := checkToolCompatibility("chat/chat.other", oldTool, newTool); len(failures) == 0 { + t.Fatal("unreviewed confirmation hardening unexpectedly passed") + } + + oldTool.Confirmation = "user_required" + newTool.Confirmation = "not_required" + if failures := checkToolCompatibility("chat/chat.update_streaming_card", oldTool, newTool); len(failures) == 0 { + t.Fatal("reviewed tool confirmation weakening unexpectedly passed") + } +} + func TestMergeContracts(t *testing.T) { historical := baselineContract() current := cloneContract(historical) From 2d38abe6813541c532037bfbdf46a86924da6099 Mon Sep 17 00:00:00 2001 From: chichuan Date: Fri, 7 Aug 2026 14:46:54 +0800 Subject: [PATCH 09/19] =?UTF-8?q?feat(ci):=20PR=20=E8=AF=84=E8=AE=BA=20/ev?= =?UTF-8?q?al=20=E8=A7=A6=E5=8F=91=E5=86=85=E7=BD=91=20MCP=20=E8=AF=84?= =?UTF-8?q?=E6=B5=8B=E7=9A=84=20dispatch=20workflow?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - issue_comment 触发，author_association ∈ OWNER/MEMBER/COLLABORATOR 门控 - 不 checkout、不执行 PR 代码；触发通道与凭证全部经 secrets 注入 - scripts/ci/eval_comment_parse.py 解析 /eval [cases=]（10 个单测） --- .github/workflows/eval-dispatch.yml | 117 ++++++++++++++++++++++++++ scripts/ci/eval_comment_parse.py | 70 +++++++++++++++ scripts/ci/eval_comment_parse_test.py | 50 +++++++++++ 3 files changed, 237 insertions(+) create mode 100644 .github/workflows/eval-dispatch.yml create mode 100644 scripts/ci/eval_comment_parse.py create mode 100644 scripts/ci/eval_comment_parse_test.py diff --git a/.github/workflows/eval-dispatch.yml b/.github/workflows/eval-dispatch.yml new file mode 100644 index 00000000..c3be2d19 --- /dev/null +++ b/.github/workflows/eval-dispatch.yml @@ -0,0 +1,117 @@ +name: PR Eval Dispatch + +# `/eval [cases=]` PR 评论 → 触发内网评测流水线，报告由内网 bot 回贴。 +# 本 workflow 只在默认分支上下文运行，不 checkout、不执行 PR 代码； +# maintainer 打出 /eval 即代表已审阅 PR 代码（内网 CI 将构建并执行该 head）。 +# 触发通道与凭证全部经 secrets 注入，文件内不出现任何内网信息。 + +on: + issue_comment: + types: + - created + +permissions: {} + +concurrency: + group: eval-dispatch-${{ github.event.issue.number }} + cancel-in-progress: false + +jobs: + dispatch: + name: Dispatch internal evaluation + if: >- + github.event.issue.pull_request && + startsWith(github.event.comment.body, '/eval') && + contains(fromJSON('["OWNER", "MEMBER", "COLLABORATOR"]'), github.event.comment.author_association) + runs-on: ubuntu-latest + timeout-minutes: 5 + permissions: + contents: read + issues: write + pull-requests: read + steps: + - name: Check out default branch tooling + uses: actions/checkout@v4 + + - name: Parse /eval command + id: parse + continue-on-error: true + env: + COMMENT_BODY: ${{ github.event.comment.body }} + run: python3 scripts/ci/eval_comment_parse.py + + - name: Reply usage on parse failure + if: steps.parse.outcome == 'failure' + env: + GH_TOKEN: ${{ github.token }} + PR_NUMBER: ${{ github.event.issue.number }} + PARSE_ERROR: ${{ steps.parse.outputs.error }} + run: | + body="❌ /eval 命令解析失败：${PARSE_ERROR}" + jq -n --arg body "$body" '{body: $body}' | curl --fail --silent --show-error \ + -X POST \ + -H "Authorization: Bearer ${GH_TOKEN}" \ + -H "Accept: application/vnd.github+json" \ + --data @- \ + "https://api.github.com/repos/${GITHUB_REPOSITORY}/issues/${PR_NUMBER}/comments" > /dev/null + exit 1 + + - name: Resolve PR head SHA + id: pr + env: + GH_TOKEN: ${{ github.token }} + PR_NUMBER: ${{ github.event.issue.number }} + run: | + head_sha="$(curl --fail --silent --show-error \ + -H "Authorization: Bearer ${GH_TOKEN}" \ + -H "Accept: application/vnd.github+json" \ + "https://api.github.com/repos/${GITHUB_REPOSITORY}/pulls/${PR_NUMBER}" | jq -r '.head.sha')" + if [ -z "$head_sha" ] || [ "$head_sha" = "null" ]; then + echo "Failed to resolve PR head SHA." >&2 + exit 1 + fi + echo "head_sha=${head_sha}" >> "$GITHUB_OUTPUT" + + - name: Trigger internal evaluation pipeline + env: + EVAL_TRIGGER_TOKEN: ${{ secrets.EVAL_TRIGGER_TOKEN }} + EVAL_TRIGGER_URL: ${{ secrets.EVAL_TRIGGER_URL }} + PR_NUMBER: ${{ github.event.issue.number }} + PR_HEAD_SHA: ${{ steps.pr.outputs.head_sha }} + PRODUCTS: ${{ steps.parse.outputs.products }} + CASES_REF: ${{ steps.parse.outputs.cases_ref }} + run: | + if [ -z "$EVAL_TRIGGER_TOKEN" ] || [ -z "$EVAL_TRIGGER_URL" ]; then + echo "EVAL_TRIGGER_URL / EVAL_TRIGGER_TOKEN not configured; cannot dispatch." >&2 + exit 1 + fi + curl --fail --silent --show-error \ + -X POST \ + -F "token=${EVAL_TRIGGER_TOKEN}" \ + -F "ref=main" \ + -F "variables[PR_NUMBER]=${PR_NUMBER}" \ + -F "variables[PR_HEAD_SHA]=${PR_HEAD_SHA}" \ + -F "variables[PRODUCTS]=${PRODUCTS}" \ + -F "variables[CASES_REF]=${CASES_REF}" \ + "$EVAL_TRIGGER_URL" + echo "Internal evaluation dispatched." + + - name: Acknowledge on PR + env: + GH_TOKEN: ${{ github.token }} + PR_NUMBER: ${{ github.event.issue.number }} + PR_HEAD_SHA: ${{ steps.pr.outputs.head_sha }} + PRODUCTS: ${{ steps.parse.outputs.products }} + CASES_REF: ${{ steps.parse.outputs.cases_ref }} + run: | + cases_note="" + if [ -n "$CASES_REF" ]; then + cases_note="，用例版本 \`${CASES_REF}\`" + fi + body="🛰️ /eval 已受理：产品集 \`${PRODUCTS}\`${cases_note}，评测对象 \`${PR_HEAD_SHA}\`。内网评测流水线运行结束后将由 bot 回贴报告（首行为基线对比头条）。" + jq -n --arg body "$body" '{body: $body}' | curl --fail --silent --show-error \ + -X POST \ + -H "Authorization: Bearer ${GH_TOKEN}" \ + -H "Accept: application/vnd.github+json" \ + --data @- \ + "https://api.github.com/repos/${GITHUB_REPOSITORY}/issues/${PR_NUMBER}/comments" > /dev/null diff --git a/scripts/ci/eval_comment_parse.py b/scripts/ci/eval_comment_parse.py new file mode 100644 index 00000000..de4433f1 --- /dev/null +++ b/scripts/ci/eval_comment_parse.py @@ -0,0 +1,70 @@ +#!/usr/bin/env python3 +"""解析 PR 评论中的 `/eval [cases=]` 触发命令。 + +供 .github/workflows/eval-dispatch.yml 调用：评论体经环境变量 COMMENT_BODY +传入（避免 argv 注入），解析结果以 GitHub Actions output（products / cases_ref） +写出；格式非法时把单行错误写入 output `error` 并以非零退出。 + +产品集显式必填；`cases=` 为用例仓库版本逃生舱（破坏性变更配对验证）。 +""" + +import os +import re +import sys + +PRODUCT_RE = re.compile(r"^[a-z0-9][a-z0-9-]*$") +REF_RE = re.compile(r"^[A-Za-z0-9._/-]+$") +USAGE = "用法: /eval [,...] [cases=]；示例: /eval drive,doc cases=feat/x" + + +def parse(body: str): + lines = [line.strip() for line in (body or "").splitlines() if line.strip()] + if not lines: + raise ValueError(f"评论为空。{USAGE}") + tokens = lines[0].split() + if tokens[0] != "/eval": + raise ValueError(f"首行必须以 /eval 命令开头。{USAGE}") + if len(tokens) < 2: + raise ValueError(f"产品集显式必填。{USAGE}") + + products = [p for p in tokens[1].split(",") if p] + if not products: + raise ValueError(f"产品集显式必填。{USAGE}") + for product in products: + if not PRODUCT_RE.match(product): + raise ValueError(f"产品名非法: {product}。{USAGE}") + + cases_ref = "" + for extra in tokens[2:]: + if extra.startswith("cases="): + cases_ref = extra[len("cases="):] + if not cases_ref or not REF_RE.match(cases_ref): + raise ValueError(f"cases 引用非法: {extra}。{USAGE}") + else: + raise ValueError(f"未知参数: {extra}。{USAGE}") + + return ",".join(products), cases_ref + + +def _write_outputs(pairs): + lines = [f"{key}={value}" for key, value in pairs] + output_path = os.environ.get("GITHUB_OUTPUT", "") + if output_path: + with open(output_path, "a", encoding="utf-8") as f: + f.write("\n".join(lines) + "\n") + print("\n".join(lines)) + + +def main() -> int: + try: + products, cases_ref = parse(os.environ.get("COMMENT_BODY", "")) + except ValueError as exc: + _write_outputs([("error", str(exc))]) + print(str(exc), file=sys.stderr) + return 1 + _write_outputs([("products", products), ("cases_ref", cases_ref)]) + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/scripts/ci/eval_comment_parse_test.py b/scripts/ci/eval_comment_parse_test.py new file mode 100644 index 00000000..bbf76178 --- /dev/null +++ b/scripts/ci/eval_comment_parse_test.py @@ -0,0 +1,50 @@ +import pytest + +from eval_comment_parse import parse + + +def test_single_product(): + assert parse("/eval drive") == ("drive", "") + + +def test_multiple_products(): + assert parse("/eval drive,doc,edu-app") == ("drive,doc,edu-app", "") + + +def test_cases_ref_escape_hatch(): + assert parse("/eval drive cases=feat/drive-latest") == ("drive", "feat/drive-latest") + + +def test_extra_lines_after_command_are_ignored(): + body = "/eval drive\n\n顺便说明：这个 PR 只动了 drive 的 --latest。" + assert parse(body) == ("drive", "") + + +def test_missing_products_rejected(): + with pytest.raises(ValueError, match="产品集显式必填"): + parse("/eval") + + +def test_similar_command_prefix_rejected(): + with pytest.raises(ValueError, match="/eval 命令开头"): + parse("/evaluate drive") + + +def test_illegal_product_name_rejected(): + with pytest.raises(ValueError, match="产品名非法"): + parse("/eval drive;rm") + + +def test_unknown_extra_token_rejected(): + with pytest.raises(ValueError, match="未知参数"): + parse("/eval drive --force") + + +def test_illegal_cases_ref_rejected(): + with pytest.raises(ValueError, match="cases 引用非法"): + parse("/eval drive cases=$(whoami)") + + +def test_empty_comment_rejected(): + with pytest.raises(ValueError, match="评论为空"): + parse(" \n ") From a5672152a74a1c5c1a1ba7381e0dd8d88540d79c Mon Sep 17 00:00:00 2001 From: chichuan Date: Mon, 10 Aug 2026 15:33:01 +0800 Subject: [PATCH 10/19] ci: add /eval comment dispatch workflow for internal MCP evaluation (Aone JSON trigger contract) --- .github/workflows/eval-dispatch.yml | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/.github/workflows/eval-dispatch.yml b/.github/workflows/eval-dispatch.yml index c3be2d19..91ef2642 100644 --- a/.github/workflows/eval-dispatch.yml +++ b/.github/workflows/eval-dispatch.yml @@ -85,14 +85,17 @@ jobs: echo "EVAL_TRIGGER_URL / EVAL_TRIGGER_TOKEN not configured; cannot dispatch." >&2 exit 1 fi - curl --fail --silent --show-error \ + jq -n \ + --arg pr "$PR_NUMBER" \ + --arg sha "$PR_HEAD_SHA" \ + --arg products "$PRODUCTS" \ + --arg cases "$CASES_REF" \ + '{branch: "main", params: {pr_number: $pr, pr_head_sha: $sha, products: $products, cases_ref: $cases}}' \ + | curl --fail --silent --show-error \ -X POST \ - -F "token=${EVAL_TRIGGER_TOKEN}" \ - -F "ref=main" \ - -F "variables[PR_NUMBER]=${PR_NUMBER}" \ - -F "variables[PR_HEAD_SHA]=${PR_HEAD_SHA}" \ - -F "variables[PRODUCTS]=${PRODUCTS}" \ - -F "variables[CASES_REF]=${CASES_REF}" \ + -H "private-token: ${EVAL_TRIGGER_TOKEN}" \ + -H "Content-Type: application/json" \ + --data @- \ "$EVAL_TRIGGER_URL" echo "Internal evaluation dispatched." From d6b51a04f4a485d990577654b7ff7d01a701a81d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=85=8B=E8=B0=A8?= <2940480744@qq.com> Date: Tue, 11 Aug 2026 09:55:19 +0800 Subject: [PATCH 11/19] fix(chat): preserve scoped search preflight errors --- internal/helpers/chat.go | 12 +- internal/helpers/chat_message_search_test.go | 34 +++++ internal/helpers/chat_search_scope_error.go | 123 ++++++++++++++++++ .../helpers/chat_search_scope_error_test.go | 109 ++++++++++++++++ internal/shortcut/smart/search_msg.go | 15 +-- .../smart/search_msg_execution_test.go | 34 +++++ 6 files changed, 302 insertions(+), 25 deletions(-) create mode 100644 internal/helpers/chat_search_scope_error.go create mode 100644 internal/helpers/chat_search_scope_error_test.go diff --git a/internal/helpers/chat.go b/internal/helpers/chat.go index e4dfd0a2..183ebbfe 100644 --- a/internal/helpers/chat.go +++ b/internal/helpers/chat.go @@ -286,17 +286,7 @@ func validateNativeSearchConversationScope(conversationIDs []string) error { if err == nil { continue } - var cliErr *CLIError - if !errors.As(err, &cliErr) || (cliErr.Code != CodeMCPToolError && cliErr.Code != CodeResourceNotFound) { - return err - } - return apperrors.NewValidation( - fmt.Sprintf("无法验证会话 CID %q；已停止搜索，避免过滤失效后返回其他会话消息", conversationID), - apperrors.WithReason("search_conversation_scope_invalid"), - apperrors.WithDetails(map[string]any{"conversationId": conversationID}), - apperrors.WithHint("确认 openConversationId 存在且当前账号可访问后重试"), - apperrors.WithCause(err), - ) + return NormalizeSearchConversationScopeError(conversationID, err) } return nil } diff --git a/internal/helpers/chat_message_search_test.go b/internal/helpers/chat_message_search_test.go index b70d2054..406099f2 100644 --- a/internal/helpers/chat_message_search_test.go +++ b/internal/helpers/chat_message_search_test.go @@ -286,6 +286,40 @@ func TestNativeScopedSearchPreservesPreflightAuthError(t *testing.T) { } } +func TestCrossPlatformCoverageNativeScopedSearchPreservesAmbiguousMCPToolErrors(t *testing.T) { + for _, test := range []struct { + name string + want *CLIError + }{ + { + name: "rate limited", + want: &CLIError{ + Code: CodeMCPToolError, + Message: `{"success":false,"errorCode":"invalidRequest.rateLimited","errorMsg":"slow down"}`, + }, + }, + { + name: "permission denied", + want: &CLIError{ + Code: CodeMCPToolError, + Message: `{"success":false,"errorCode":"forbidden.noPermission","errorMsg":"permission denied"}`, + }, + }, + } { + t.Run(test.name, func(t *testing.T) { + caller := &chatMessageSearchCaller{preflightError: test.want} + _, err := executeNativeScopedSearch(t, caller, + "message", "search-advanced", "--query", "周报", "--conversation-ids", "cid-target") + if err != test.want { + t.Fatalf("error = %#v, want original %#v", err, test.want) + } + if len(caller.calls) != 1 || caller.calls[0].toolName != "get_conversation_info" { + t.Fatalf("calls = %#v", caller.calls) + } + }) + } +} + func TestNativeScopedSearchScansUntilTargetConversationAppears(t *testing.T) { caller := &chatMessageSearchCaller{searchResponses: []string{ `{"result":{"conversationMessagesList":[{"openConversationId":"cid-other","messages":[{"openMessageId":"m-other"}]}],"hasMore":true,"nextCursor":"c2"}}`, diff --git a/internal/helpers/chat_search_scope_error.go b/internal/helpers/chat_search_scope_error.go new file mode 100644 index 00000000..9dc09ec1 --- /dev/null +++ b/internal/helpers/chat_search_scope_error.go @@ -0,0 +1,123 @@ +// 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 helpers + +import ( + "encoding/json" + "errors" + "fmt" + "strings" + + apperrors "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/errors" +) + +// NormalizeSearchConversationScopeError maps only errors that prove the +// requested conversation does not exist or that openConversationId itself is +// invalid. Unknown MCP tool failures must remain unchanged: the legacy +// CodeMCPToolError bucket also carries permission, throttling, and transient +// backend failures, none of which proves that the caller supplied a bad CID. +func NormalizeSearchConversationScopeError(conversationID string, err error) error { + if err == nil || !isDefinitiveInvalidSearchConversationError(err) { + return err + } + return apperrors.NewValidation( + fmt.Sprintf("无法验证会话 CID %q；已停止搜索，避免过滤失效后返回其他会话消息", conversationID), + apperrors.WithReason("search_conversation_scope_invalid"), + apperrors.WithDetails(map[string]any{"conversationId": conversationID}), + apperrors.WithRetryable(false), + apperrors.WithHint("确认 openConversationId 存在且当前账号可访问后重试"), + apperrors.WithCause(err), + ) +} + +func isDefinitiveInvalidSearchConversationError(err error) bool { + var cliErr *CLIError + if errors.As(err, &cliErr) { + switch cliErr.Code { + case CodeResourceNotFound, CodeInvalidParam: + return true + } + code, message := searchConversationErrorFacts(cliErr.Message) + if isExplicitInvalidConversationCode(code) || isConversationParameterError(code, message) { + return true + } + } + + // Transport errors can survive below a legacy CLIError in the cause chain. + // Inspect their structured diagnostics, but require conversation-specific + // evidence before treating a generic PARAM_ERROR as an invalid CID. + var appErr *apperrors.Error + if errors.As(err, &appErr) { + code := strings.TrimSpace(appErr.ServerDiag.ServerErrorCode) + message := strings.Join([]string{ + appErr.Message, + appErr.ServerDiag.TechnicalDetail, + appErr.Reason, + appErr.FailureStage, + }, " ") + return isExplicitInvalidConversationCode(code) || isConversationParameterError(code, message) + } + return false +} + +func searchConversationErrorFacts(raw string) (string, string) { + var body map[string]any + if json.Unmarshal([]byte(raw), &body) != nil { + return "", raw + } + code := firstSearchConversationErrorString(body, "errorCode", "error_code", "code") + message := firstSearchConversationErrorString(body, "errorMsg", "error_msg", "message", "error") + return code, message +} + +func firstSearchConversationErrorString(body map[string]any, keys ...string) string { + for _, key := range keys { + if value, ok := body[key].(string); ok && strings.TrimSpace(value) != "" { + return strings.TrimSpace(value) + } + } + return "" +} + +func isExplicitInvalidConversationCode(code string) bool { + switch strings.ToUpper(strings.TrimSpace(code)) { + case "INVALID_OPEN_CONVERSATION_ID", "OPEN_CONVERSATION_NOT_FOUND", "CONVERSATION_NOT_FOUND": + return true + default: + return false + } +} + +func isConversationParameterError(code, message string) bool { + normalizedCode := strings.ToUpper(strings.TrimSpace(code)) + if normalizedCode != "PARAM_ERROR" && normalizedCode != "PARAMETER_ERROR" && normalizedCode != "INVALID_ARGUMENT" { + return false + } + normalizedMessage := strings.ToLower(strings.TrimSpace(message)) + mentionsConversationID := strings.Contains(normalizedMessage, "openconversationid") || + strings.Contains(normalizedMessage, "open conversation id") || + strings.Contains(normalizedMessage, "conversation id") || + strings.Contains(normalizedMessage, "cid") + if !mentionsConversationID { + return false + } + for _, marker := range []string{ + "invalid", "illegal", "malformed", "required", "missing", "not found", "不存在", "无效", "非法", "缺少", "必填", + } { + if strings.Contains(normalizedMessage, marker) { + return true + } + } + return false +} diff --git a/internal/helpers/chat_search_scope_error_test.go b/internal/helpers/chat_search_scope_error_test.go new file mode 100644 index 00000000..76072aea --- /dev/null +++ b/internal/helpers/chat_search_scope_error_test.go @@ -0,0 +1,109 @@ +// 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 helpers + +import ( + "errors" + "testing" + + apperrors "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/errors" +) + +func TestCrossPlatformCoverageNormalizeSearchConversationScopeError(t *testing.T) { + if got := NormalizeSearchConversationScopeError("cid", nil); got != nil { + t.Fatalf("nil error normalized to %#v", got) + } + + invalidCases := []struct { + name string + err error + }{ + { + name: "classified resource not found", + err: &CLIError{Code: CodeResourceNotFound, Message: "conversation not found"}, + }, + { + name: "explicit conversation error code", + err: &CLIError{ + Code: CodeMCPToolError, + Message: `{"success":false,"errorCode":"INVALID_OPEN_CONVERSATION_ID","errorMsg":"invalid conversation"}`, + }, + }, + { + name: "conversation specific parameter error", + err: &CLIError{ + Code: CodeMCPToolError, + Message: `{"success":false,"errorCode":"PARAM_ERROR","errorMsg":"openConversationId is invalid"}`, + }, + }, + { + name: "transport diagnostic proves invalid conversation", + err: apperrors.NewAPI( + "conversation validation failed", + apperrors.WithServerDiag(apperrors.ServerDiagnostics{ + ServerErrorCode: "INVALID_OPEN_CONVERSATION_ID", + }), + ), + }, + } + for _, test := range invalidCases { + t.Run(test.name, func(t *testing.T) { + got := NormalizeSearchConversationScopeError("cid-invalid", test.err) + var typed *apperrors.Error + if !errors.As(got, &typed) || typed.Reason != "search_conversation_scope_invalid" { + t.Fatalf("normalized error = %#v", got) + } + if !typed.RetryableSet || typed.Retryable { + t.Fatalf("retryable = (%t, set=%t), want false and set", typed.Retryable, typed.RetryableSet) + } + if typed.Details["conversationId"] != "cid-invalid" || !errors.Is(got, test.err) { + t.Fatalf("normalized error lost details or cause: %#v", typed) + } + }) + } + + preservedCases := []struct { + name string + err error + }{ + { + name: "rate limit", + err: &CLIError{ + Code: CodeMCPToolError, + Message: `{"success":false,"errorCode":"invalidRequest.rateLimited","errorMsg":"slow down","retryable":true}`, + }, + }, + { + name: "permission denied", + err: &CLIError{ + Code: CodeMCPToolError, + Message: `{"success":false,"errorCode":"forbidden.noPermission","errorMsg":"permission denied"}`, + }, + }, + { + name: "generic parameter error without CID evidence", + err: &CLIError{ + Code: CodeMCPToolError, + Message: `{"success":false,"errorCode":"PARAM_ERROR","errorMsg":"未找到指定工具"}`, + }, + }, + } + for _, test := range preservedCases { + t.Run(test.name, func(t *testing.T) { + if got := NormalizeSearchConversationScopeError("cid-target", test.err); got != test.err { + t.Fatalf("error = %#v, want original %#v", got, test.err) + } + }) + } +} diff --git a/internal/shortcut/smart/search_msg.go b/internal/shortcut/smart/search_msg.go index 5f584a19..b3498ac9 100644 --- a/internal/shortcut/smart/search_msg.go +++ b/internal/shortcut/smart/search_msg.go @@ -14,7 +14,6 @@ package smart import ( - "errors" "fmt" "strconv" "strings" @@ -478,19 +477,7 @@ func validateSearchConversationScope(rt *shortcut.RuntimeContext, conversationID if err == nil { continue } - var cliErr *helpers.CLIError - if !errors.As(err, &cliErr) || (cliErr.Code != helpers.CodeMCPToolError && cliErr.Code != helpers.CodeResourceNotFound) { - return err - } - return apperrors.NewValidation( - fmt.Sprintf("无法验证会话 CID %q；已停止搜索，避免过滤失效后返回其他会话消息", conversationID), - apperrors.WithReason("search_conversation_scope_invalid"), - apperrors.WithDetails(map[string]any{ - "conversationId": conversationID, - }), - apperrors.WithHint("确认 openConversationId 存在且当前账号可访问后重试"), - apperrors.WithCause(err), - ) + return helpers.NormalizeSearchConversationScopeError(conversationID, err) } return nil } diff --git a/internal/shortcut/smart/search_msg_execution_test.go b/internal/shortcut/smart/search_msg_execution_test.go index ff1b7eac..da6ad2fd 100644 --- a/internal/shortcut/smart/search_msg_execution_test.go +++ b/internal/shortcut/smart/search_msg_execution_test.go @@ -331,6 +331,40 @@ func TestSearchMsgInvalidCIDStopsBeforeGlobalSearch(t *testing.T) { } } +func TestCrossPlatformCoverageSearchMsgPreservesAmbiguousPreflightMCPToolErrors(t *testing.T) { + for _, test := range []struct { + name string + want *helpers.CLIError + }{ + { + name: "rate limited", + want: &helpers.CLIError{ + Code: helpers.CodeMCPToolError, + Message: `{"success":false,"errorCode":"invalidRequest.rateLimited","errorMsg":"slow down"}`, + }, + }, + { + name: "permission denied", + want: &helpers.CLIError{ + Code: helpers.CodeMCPToolError, + Message: `{"success":false,"errorCode":"forbidden.noPermission","errorMsg":"permission denied"}`, + }, + }, + } { + t.Run(test.name, func(t *testing.T) { + caller := &searchMsgExecutionCaller{preflightError: test.want} + _, err := executeSearchMsgResult(caller, + "--group", "cid-target", "--query", "周报", "--no-enrich") + if err != test.want { + t.Fatalf("error = %#v, want original %#v", err, test.want) + } + if len(caller.calls) != 1 || caller.calls[0].tool != "get_conversation_info" { + t.Fatalf("calls = %#v", caller.calls) + } + }) + } +} + func TestSearchMsgMissingConversationIdentityFailsClosed(t *testing.T) { caller := &searchMsgExecutionCaller{searchResponse: `{"result":{"messages":[{"openMessageId":"m1","content":"unknown"}],"hasMore":false}}`} _, err := executeSearchMsgResult(caller, "--group", "cid-target", "--query", "周报", "--no-enrich") From 0148ad1800a3a048a466db32f6d5d997b0f8b356 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=85=8B=E8=B0=A8?= <2940480744@qq.com> Date: Tue, 11 Aug 2026 10:29:53 +0800 Subject: [PATCH 12/19] test(chat): cover scoped search error fallbacks --- internal/helpers/chat_search_scope_error_test.go | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/internal/helpers/chat_search_scope_error_test.go b/internal/helpers/chat_search_scope_error_test.go index 76072aea..e8942f6b 100644 --- a/internal/helpers/chat_search_scope_error_test.go +++ b/internal/helpers/chat_search_scope_error_test.go @@ -98,6 +98,20 @@ func TestCrossPlatformCoverageNormalizeSearchConversationScopeError(t *testing.T Message: `{"success":false,"errorCode":"PARAM_ERROR","errorMsg":"未找到指定工具"}`, }, }, + { + name: "structured error without recognized facts", + err: &CLIError{ + Code: CodeMCPToolError, + Message: `{"success":false,"retryable":true}`, + }, + }, + { + name: "parameter error mentions CID without invalid evidence", + err: &CLIError{ + Code: CodeMCPToolError, + Message: `{"success":false,"errorCode":"PARAM_ERROR","errorMsg":"openConversationId could not be processed"}`, + }, + }, } for _, test := range preservedCases { t.Run(test.name, func(t *testing.T) { From 82dfee7291f77dca8a194317b5903fda87dc6776 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=85=8B=E8=B0=A8?= <2940480744@qq.com> Date: Tue, 11 Aug 2026 11:48:39 +0800 Subject: [PATCH 13/19] fix(chat): preserve layered IM workflow contracts --- ...at_message_receipt_action_contract_test.go | 84 +++++++++++++++++++ internal/app/schema_safety_contract_test.go | 5 +- internal/helpers/chat.go | 10 ++- internal/helpers/chat_card_update_test.go | 28 +------ internal/shortcut/chat/chat_message.go | 4 +- internal/shortcut/chat/gap_fill_test.go | 23 +++++ scripts/policy/schema-compat/main.go | 24 +----- scripts/policy/schema-compat/main_test.go | 19 ----- 8 files changed, 125 insertions(+), 72 deletions(-) create mode 100644 internal/app/chat_message_receipt_action_contract_test.go diff --git a/internal/app/chat_message_receipt_action_contract_test.go b/internal/app/chat_message_receipt_action_contract_test.go new file mode 100644 index 00000000..b3caedd6 --- /dev/null +++ b/internal/app/chat_message_receipt_action_contract_test.go @@ -0,0 +1,84 @@ +// 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. + +package app + +import ( + "strings" + "testing" + + "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/shortcut/chatmsg" +) + +func TestCrossPlatformCoverageChatMessageReceiptActionsBindToRunnableCommands(t *testing.T) { + testCases := []struct { + name string + payload map[string]any + }{ + { + name: "send receipt awaiting status", + payload: chatmsg.ProjectMessageSendReceipt(map[string]any{ + "openTaskId": "task-pending", + }), + }, + { + name: "send receipt ready for message actions", + payload: chatmsg.ProjectMessageSendReceipt(map[string]any{ + "openTaskId": "task-ready", + "openMessageId": "message-ready", + "openConversationId": "conversation-ready", + }), + }, + { + name: "send status awaiting message reference", + payload: chatmsg.ProjectMessageSendStatus(map[string]any{ + "status": "PENDING", + }, "task-pending"), + }, + { + name: "send status ready for message actions", + payload: chatmsg.ProjectMessageSendStatus(map[string]any{ + "openTaskId": "task-ready", + "openMessageId": "message-ready", + "openConversationId": "conversation-ready", + "status": "SUCCESS", + }, "task-ready"), + }, + } + + root := NewRootCommand() + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + actions, ok := testCase.payload["nextActions"].([]map[string]any) + if !ok || len(actions) == 0 { + t.Fatalf("nextActions = %#v, want non-empty []map[string]any", testCase.payload["nextActions"]) + } + + for index, action := range actions { + cliPath, ok := action["cliPath"].(string) + if !ok || strings.TrimSpace(cliPath) == "" { + t.Fatalf("nextActions[%d].cliPath = %#v, want non-empty string", index, action["cliPath"]) + } + + command, remaining, err := root.Find(strings.Fields(cliPath)) + if err != nil { + t.Fatalf("nextActions[%d].cliPath %q does not bind: %v", index, cliPath, err) + } + if command == nil || len(remaining) != 0 || !command.Runnable() { + t.Fatalf("nextActions[%d].cliPath %q resolved to command=%v remaining=%v runnable=%v", index, cliPath, command, remaining, command != nil && command.Runnable()) + } + + arguments, ok := action["arguments"].(map[string]any) + if !ok { + t.Fatalf("nextActions[%d].arguments = %#v, want map[string]any", index, action["arguments"]) + } + for name := range arguments { + if command.Flags().Lookup(name) == nil && command.InheritedFlags().Lookup(name) == nil { + t.Errorf("nextActions[%d] argument %q is not a flag of runnable command %q", index, name, cliPath) + } + } + } + }) + } +} diff --git a/internal/app/schema_safety_contract_test.go b/internal/app/schema_safety_contract_test.go index e9bda5da..2ceffe07 100644 --- a/internal/app/schema_safety_contract_test.go +++ b/internal/app/schema_safety_contract_test.go @@ -26,7 +26,10 @@ func TestReviewedMutationSafetyReachesFinalSchema(t *testing.T) { wants := []finalSchemaSafetyWant{ {canonical: "aitable.form_field_hide", effect: "write", risk: "medium", confirmation: "not_required", idempotency: "idempotent", provenance: declared}, {canonical: "chat.dismiss_group", effect: "destructive", risk: "high", confirmation: "user_required", idempotency: "unknown", provenance: declared}, - {canonical: "chat.update_streaming_card", effect: "write", risk: "medium", confirmation: "user_required", idempotency: "unknown", provenance: declared}, + // Card update intentionally layers confirmation: the atomic typed command + // preserves its original contract, while the Agent-facing shortcut owns + // the outer confirmation boundary. + {canonical: "chat.update_streaming_card", effect: "write", risk: "medium", confirmation: "not_required", idempotency: "unknown", provenance: declared}, {canonical: "chat.shortcut_messages_send", effect: "write", risk: "medium", confirmation: "user_required", idempotency: "unknown", provenance: declared}, {canonical: "chat.shortcut_messages_send_by_webhook", effect: "write", risk: "medium", confirmation: "user_required", idempotency: "unknown", provenance: declared}, {canonical: "chat.shortcut_messages_send_card", effect: "write", risk: "medium", confirmation: "user_required", idempotency: "unknown", provenance: declared}, diff --git a/internal/helpers/chat.go b/internal/helpers/chat.go index 183ebbfe..8b2afd6f 100644 --- a/internal/helpers/chat.go +++ b/internal/helpers/chat.go @@ -5529,7 +5529,11 @@ flow-status 取值：1=处理中(PROCESSING)，2=输入中(INPUTTING)，3=完成 DeclareLeafMetadata(chatMessageUpdateCardCmd, LeafSpec{ Safety: contract.SafetySpec{ Effect: "write", Risk: "medium", - Confirmation: "user_required", Idempotency: "unknown", + // The typed command is the atomic MCP surface and intentionally + // preserves its original no-extra-confirmation contract. The + // Agent-facing +messages-update-card shortcut owns the higher-level + // confirmation boundary. + Confirmation: "not_required", Idempotency: "unknown", }, Contract: LeafContract{ Identity: contract.ToolIdentitySpec{ @@ -5547,8 +5551,8 @@ flow-status 取值：1=处理中(PROCESSING)，2=输入中(INPUTTING)，3=完成 }, Selection: contract.SelectionSpec{ AgentSummary: "更新已发送流式卡片的内容和状态", - UseWhen: []string{"已有 bizId 并需要追加内容或结束流式输出时"}, - AvoidWhen: []string{"创建新卡片时使用 chat message send-card"}, + UseWhen: []string{"需要直接调用底层原子更新，并由调用方自行管理确认与更新节奏时"}, + AvoidWhen: []string{"面向 Agent 的默认快速通道使用 chat +messages-update-card；创建新卡片时使用 chat message send-card"}, Examples: []string{"dws chat message update-card --biz-id --content \"处理完成\" --flow-status 2"}, }, Parameters: []contract.ParamDecl{ diff --git a/internal/helpers/chat_card_update_test.go b/internal/helpers/chat_card_update_test.go index 04a9baec..58644967 100644 --- a/internal/helpers/chat_card_update_test.go +++ b/internal/helpers/chat_card_update_test.go @@ -38,39 +38,23 @@ func runNativeCardUpdate(t *testing.T, caller *scriptedToolCaller, args ...strin } func TestCrossPlatformCoverageNativeMessageUpdateCardVerifiesWrite(t *testing.T) { - t.Run("confirmation gates the native write", func(t *testing.T) { + t.Run("atomic command preserves no-extra-confirmation contract", func(t *testing.T) { caller := &scriptedToolCaller{steps: []scriptedToolStep{{text: `{"result":{"bizId":"biz-confirm","updated":true}}`}}} - err := runNativeCardUpdate(t, caller, - "message", "update-card", - "--biz-id", "biz-confirm", - "--content", "确认后更新", - "--flow-status", "3", - ) - var typed *apperrors.Error - if !errors.As(err, &typed) || typed.Reason != "confirmation_required" { - t.Fatalf("unconfirmed error = %#v, want confirmation_required", err) - } - if caller.calls != 0 { - t.Fatalf("unconfirmed update made %d remote call(s)", caller.calls) - } - - caller = &scriptedToolCaller{steps: []scriptedToolStep{{text: `{"result":{"bizId":"biz-confirm","updated":true}}`}}} if err := runNativeCardUpdate(t, caller, "message", "update-card", "--biz-id", "biz-confirm", - "--content", "确认后更新", + "--content", "原子更新", "--flow-status", "3", - "--yes", ); err != nil { t.Fatal(err) } wantArgs := map[string]any{ "bizId": "biz-confirm", - "msgContent": "确认后更新", + "msgContent": "原子更新", "flowStatus": 3, } if caller.calls != 1 || caller.server != "im" || caller.tool != "update_streaming_card" || !reflect.DeepEqual(caller.args, wantArgs) { - t.Fatalf("confirmed call = count:%d server:%q tool:%q args:%#v", caller.calls, caller.server, caller.tool, caller.args) + t.Fatalf("atomic call = count:%d server:%q tool:%q args:%#v", caller.calls, caller.server, caller.tool, caller.args) } }) @@ -81,7 +65,6 @@ func TestCrossPlatformCoverageNativeMessageUpdateCardVerifiesWrite(t *testing.T) "--biz-id", "biz-1", "--content", "完成", "--flow-status", "3", - "--yes", ) if err != nil { t.Fatal(err) @@ -101,7 +84,6 @@ func TestCrossPlatformCoverageNativeMessageUpdateCardVerifiesWrite(t *testing.T) "--biz-id", "not-a-real-card", "--content", "完成", "--flow-status", "3", - "--yes", ) var typed *apperrors.Error if !errors.As(err, &typed) || typed.Reason != "streaming_card_update_unverified" { @@ -116,7 +98,6 @@ func TestCrossPlatformCoverageNativeMessageUpdateCardVerifiesWrite(t *testing.T) "--biz-id", "biz-1", "--content", "完成", "--flow-status", "3", - "--yes", ) if err == nil { t.Fatal("lower write error was ignored") @@ -140,7 +121,6 @@ func TestCrossPlatformCoverageNativeMessageUpdateCardVerifiesWrite(t *testing.T) "--biz-id", "biz-1", "--content", "完成", "--flow-status", "3", - "--yes", ) var typed *apperrors.Error if !errors.As(err, &typed) || typed.Reason != test.wantReason { diff --git a/internal/shortcut/chat/chat_message.go b/internal/shortcut/chat/chat_message.go index 5c8bd068..1002b510 100644 --- a/internal/shortcut/chat/chat_message.go +++ b/internal/shortcut/chat/chat_message.go @@ -1726,8 +1726,8 @@ var MessagesUpdateCard = shortcut.Shortcut{ }, Selection: contract.SelectionSpec{ AgentSummary: "流式更新卡片内容（最后一次 --flow-status 应为 3）", - UseWhen: []string{"当你要向已发送的流式文本卡片持续追加/更新内容时使用；会实际更新卡片，需传 send-card 返回的 bizId、新内容及 flowStatus 1-5（最后一次应为 3 表示完成）。当前不支持 Card JSON 组件或 action callback。"}, - AvoidWhen: []string{"需要该 Shortcut 未公开的底层参数、原始响应或不同执行语义时，改用对应原子命令"}, + UseWhen: []string{"当你要通过面向 Agent 的高层快速通道，向已发送的流式文本卡片持续追加/更新内容时使用；执行前统一要求用户确认，需传 send-card 返回的 bizId、新内容及 flowStatus 1-5（最后一次应为 3 表示完成）。当前不支持 Card JSON 组件或 action callback。"}, + AvoidWhen: []string{"需要底层原始响应、未公开参数，或由调用方自行管理确认与更新节奏时，改用 chat message update-card"}, Examples: []string{"dws chat +messages-update-card --biz-id --content \"内容\" --flow-status 3"}, }, }, diff --git a/internal/shortcut/chat/gap_fill_test.go b/internal/shortcut/chat/gap_fill_test.go index 0264a635..e8040232 100644 --- a/internal/shortcut/chat/gap_fill_test.go +++ b/internal/shortcut/chat/gap_fill_test.go @@ -1092,6 +1092,29 @@ func TestCrossPlatformCoverageMessagesSendCardDryRunAndFailureBoundaries(t *test } func TestCrossPlatformCoverageMessagesUpdateCardRejectsFalseSuccess(t *testing.T) { + t.Run("agent shortcut owns confirmation boundary", func(t *testing.T) { + fake := &larkAlignmentCaller{responses: map[string]string{ + "im/update_streaming_card": `{"result":{"bizId":"biz-confirm","updated":true}}`, + }} + helpers.InitDeps(fake) + root := newPlatformCoverageRoot() + root.SetIn(strings.NewReader("")) + root.SetArgs([]string{ + "chat", "+messages-update-card", + "--biz-id", "biz-confirm", + "--content", "高层更新", + "--flow-status", "3", + }) + err := root.Execute() + var typed *apperrors.Error + if !errors.As(err, &typed) || typed.Reason != "confirmation_required" { + t.Fatalf("error = %#v, want confirmation_required", err) + } + if len(fake.calls) != 0 { + t.Fatalf("unconfirmed shortcut reached MCP: %#v", fake.calls) + } + }) + t.Run("generic success is unverified", func(t *testing.T) { fake := &larkAlignmentCaller{responses: map[string]string{ "im/update_streaming_card": `{"success":true,"errorCode":null}`, diff --git a/scripts/policy/schema-compat/main.go b/scripts/policy/schema-compat/main.go index cb41771d..1b70e747 100644 --- a/scripts/policy/schema-compat/main.go +++ b/scripts/policy/schema-compat/main.go @@ -79,23 +79,6 @@ type parameterSchema struct { Enum []string `json:"enum,omitempty"` } -type reviewedCompatibilityException struct { - Field string - Old string - New string -} - -// reviewedCompatibilityExceptions is intentionally exact: safety fixes may -// need to tighten a historical contract, but that must not turn arbitrary -// confirmation drift into a compatible change. -var reviewedCompatibilityExceptions = map[string]reviewedCompatibilityException{ - "chat/chat.update_streaming_card": { - Field: "confirmation", - Old: "not_required", - New: "user_required", - }, -} - func main() { os.Exit(run(os.Args[1:], os.Stdout, os.Stderr)) } @@ -621,7 +604,7 @@ func checkToolCompatibility(toolPath string, oldTool, newTool toolSchema) []stri {name: "confirmation", old: oldTool.Confirmation, new: newTool.Confirmation}, {name: "idempotency", old: oldTool.Idempotency, new: newTool.Idempotency}, } { - if field.old != field.new && !isReviewedCompatibilityException(toolPath, field.name, field.old, field.new) { + if field.old != field.new { failures = append(failures, fmt.Sprintf("schema tool %q changed %s", toolPath, field.name)) } } @@ -657,11 +640,6 @@ func checkToolCompatibility(toolPath string, oldTool, newTool toolSchema) []stri return failures } -func isReviewedCompatibilityException(toolPath, field, oldValue, newValue string) bool { - exception, ok := reviewedCompatibilityExceptions[toolPath] - return ok && exception.Field == field && exception.Old == oldValue && exception.New == newValue -} - // reviewedInterfaceRefRedirect enumerates the exact, individually reviewed // backend RPC migrations this gate accepts. Schema shape alone cannot prove two // RPCs share business semantics, permissions, error behaviour, or side effects, diff --git a/scripts/policy/schema-compat/main_test.go b/scripts/policy/schema-compat/main_test.go index 00601cd9..121f9299 100644 --- a/scripts/policy/schema-compat/main_test.go +++ b/scripts/policy/schema-compat/main_test.go @@ -415,25 +415,6 @@ func TestSchemaCompatibilityRejectsContractDrift(t *testing.T) { } } -func TestSchemaCompatibilityAcceptsReviewedCardConfirmationHardening(t *testing.T) { - oldTool := baselineContract().Products["doc"].Tools["doc.create"] - newTool := oldTool - newTool.Confirmation = "user_required" - - if failures := checkToolCompatibility("chat/chat.update_streaming_card", oldTool, newTool); len(failures) != 0 { - t.Fatalf("reviewed confirmation hardening failures = %v", failures) - } - if failures := checkToolCompatibility("chat/chat.other", oldTool, newTool); len(failures) == 0 { - t.Fatal("unreviewed confirmation hardening unexpectedly passed") - } - - oldTool.Confirmation = "user_required" - newTool.Confirmation = "not_required" - if failures := checkToolCompatibility("chat/chat.update_streaming_card", oldTool, newTool); len(failures) == 0 { - t.Fatal("reviewed tool confirmation weakening unexpectedly passed") - } -} - func TestMergeContracts(t *testing.T) { historical := baselineContract() current := cloneContract(historical) From 0d99d18acc9a44a14680636198b3cc3e16252d95 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=85=8B=E8=B0=A8?= <2940480744@qq.com> Date: Tue, 11 Aug 2026 12:44:13 +0800 Subject: [PATCH 14/19] test(chat): align update-card selection copy --- internal/shortcut/chat/chat_message.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/shortcut/chat/chat_message.go b/internal/shortcut/chat/chat_message.go index 1002b510..b74761cd 100644 --- a/internal/shortcut/chat/chat_message.go +++ b/internal/shortcut/chat/chat_message.go @@ -1726,7 +1726,7 @@ var MessagesUpdateCard = shortcut.Shortcut{ }, Selection: contract.SelectionSpec{ AgentSummary: "流式更新卡片内容（最后一次 --flow-status 应为 3）", - UseWhen: []string{"当你要通过面向 Agent 的高层快速通道，向已发送的流式文本卡片持续追加/更新内容时使用；执行前统一要求用户确认，需传 send-card 返回的 bizId、新内容及 flowStatus 1-5（最后一次应为 3 表示完成）。当前不支持 Card JSON 组件或 action callback。"}, + UseWhen: []string{"当你要向已发送的流式文本卡片持续追加/更新内容时使用；会实际更新卡片，需传 send-card 返回的 bizId、新内容及 flowStatus 1-5（最后一次应为 3 表示完成）。当前不支持 Card JSON 组件或 action callback。"}, AvoidWhen: []string{"需要底层原始响应、未公开参数，或由调用方自行管理确认与更新节奏时，改用 chat message update-card"}, Examples: []string{"dws chat +messages-update-card --biz-id --content \"内容\" --flow-status 3"}, }, From 42e764a7a8d500aed35a28594441b67f28146412 Mon Sep 17 00:00:00 2001 From: chichuan Date: Tue, 11 Aug 2026 13:57:42 +0800 Subject: [PATCH 15/19] fix(ci): harden eval dispatch authorization --- .github/workflows/eval-dispatch.yml | 35 ++-- scripts/ci/eval_comment_parse.py | 40 ++++- scripts/ci/eval_comment_parse_test.py | 34 +++- scripts/ci/eval_dispatch_guard.py | 80 +++++++++ test/scripts/eval_dispatch_workflow_test.go | 173 ++++++++++++++++++++ 5 files changed, 333 insertions(+), 29 deletions(-) create mode 100644 scripts/ci/eval_dispatch_guard.py create mode 100644 test/scripts/eval_dispatch_workflow_test.go diff --git a/.github/workflows/eval-dispatch.yml b/.github/workflows/eval-dispatch.yml index 91ef2642..1292dd80 100644 --- a/.github/workflows/eval-dispatch.yml +++ b/.github/workflows/eval-dispatch.yml @@ -1,8 +1,9 @@ name: PR Eval Dispatch -# `/eval [cases=]` PR 评论 → 触发内网评测流水线，报告由内网 bot 回贴。 +# `/eval sha= [cases=]` PR 评论 → 触发内网评测流水线，报告由内网 bot 回贴。 # 本 workflow 只在默认分支上下文运行，不 checkout、不执行 PR 代码； -# maintainer 打出 /eval 即代表已审阅 PR 代码（内网 CI 将构建并执行该 head）。 +# maintainer 必须在 /eval 中显式记录已审阅的 PR head；权限与当前 head 校验 +# 通过后，内网 CI 只构建并执行该已审核 SHA。 # 触发通道与凭证全部经 secrets 注入，文件内不出现任何内网信息。 on: @@ -21,8 +22,7 @@ jobs: name: Dispatch internal evaluation if: >- github.event.issue.pull_request && - startsWith(github.event.comment.body, '/eval') && - contains(fromJSON('["OWNER", "MEMBER", "COLLABORATOR"]'), github.event.comment.author_association) + startsWith(github.event.comment.body, '/eval') runs-on: ubuntu-latest timeout-minutes: 5 permissions: @@ -33,6 +33,17 @@ jobs: - name: Check out default branch tooling uses: actions/checkout@v4 + - name: Verify commenter repository permission + env: + GH_TOKEN: ${{ github.token }} + COMMENTER: ${{ github.event.comment.user.login }} + run: | + permission_json="$(curl --fail --silent --show-error \ + -H "Authorization: Bearer ${GH_TOKEN}" \ + -H "Accept: application/vnd.github+json" \ + "https://api.github.com/repos/${GITHUB_REPOSITORY}/collaborators/${COMMENTER}/permission")" + printf '%s' "$permission_json" | python3 scripts/ci/eval_dispatch_guard.py permission + - name: Parse /eval command id: parse continue-on-error: true @@ -56,21 +67,21 @@ jobs: "https://api.github.com/repos/${GITHUB_REPOSITORY}/issues/${PR_NUMBER}/comments" > /dev/null exit 1 - - name: Resolve PR head SHA + - name: Verify reviewed PR head id: pr env: GH_TOKEN: ${{ github.token }} PR_NUMBER: ${{ github.event.issue.number }} + EXPECTED_PR_NUMBER: ${{ github.event.issue.number }} + REVIEWED_SHA: ${{ steps.parse.outputs.reviewed_sha }} run: | - head_sha="$(curl --fail --silent --show-error \ + pr_json="$(curl --fail --silent --show-error \ -H "Authorization: Bearer ${GH_TOKEN}" \ -H "Accept: application/vnd.github+json" \ - "https://api.github.com/repos/${GITHUB_REPOSITORY}/pulls/${PR_NUMBER}" | jq -r '.head.sha')" - if [ -z "$head_sha" ] || [ "$head_sha" = "null" ]; then - echo "Failed to resolve PR head SHA." >&2 - exit 1 - fi - echo "head_sha=${head_sha}" >> "$GITHUB_OUTPUT" + "https://api.github.com/repos/${GITHUB_REPOSITORY}/pulls/${PR_NUMBER}")" + printf '%s' "$pr_json" \ + | python3 scripts/ci/eval_dispatch_guard.py head \ + >> "$GITHUB_OUTPUT" - name: Trigger internal evaluation pipeline env: diff --git a/scripts/ci/eval_comment_parse.py b/scripts/ci/eval_comment_parse.py index de4433f1..e015ed5e 100644 --- a/scripts/ci/eval_comment_parse.py +++ b/scripts/ci/eval_comment_parse.py @@ -1,11 +1,13 @@ #!/usr/bin/env python3 -"""解析 PR 评论中的 `/eval [cases=]` 触发命令。 +"""解析 PR 评论中的 `/eval sha= [cases=]` 触发命令。 供 .github/workflows/eval-dispatch.yml 调用：评论体经环境变量 COMMENT_BODY -传入（避免 argv 注入），解析结果以 GitHub Actions output（products / cases_ref） -写出；格式非法时把单行错误写入 output `error` 并以非零退出。 +传入（避免 argv 注入），解析结果以 GitHub Actions output +（products / cases_ref / reviewed_sha）写出；格式非法时把单行错误写入 +output `error` 并以非零退出。 -产品集显式必填；`cases=` 为用例仓库版本逃生舱（破坏性变更配对验证）。 +产品集和完整审核 SHA 显式必填；`cases=` 为用例仓库版本逃生舱 +（破坏性变更配对验证）。 """ import os @@ -14,7 +16,11 @@ import sys PRODUCT_RE = re.compile(r"^[a-z0-9][a-z0-9-]*$") REF_RE = re.compile(r"^[A-Za-z0-9._/-]+$") -USAGE = "用法: /eval [,...] [cases=]；示例: /eval drive,doc cases=feat/x" +SHA_RE = re.compile(r"^[0-9a-fA-F]{40}$") +USAGE = ( + "用法: /eval [,...] sha=<40位PR-head-SHA> [cases=]；" + "示例: /eval drive,doc sha=0123456789abcdef0123456789abcdef01234567 cases=feat/x" +) def parse(body: str): @@ -35,15 +41,27 @@ def parse(body: str): raise ValueError(f"产品名非法: {product}。{USAGE}") cases_ref = "" + reviewed_sha = "" for extra in tokens[2:]: if extra.startswith("cases="): + if cases_ref: + raise ValueError(f"cases 引用只能指定一次。{USAGE}") cases_ref = extra[len("cases="):] if not cases_ref or not REF_RE.match(cases_ref): raise ValueError(f"cases 引用非法: {extra}。{USAGE}") + elif extra.startswith("sha="): + if reviewed_sha: + raise ValueError(f"审核 SHA 只能指定一次。{USAGE}") + reviewed_sha = extra[len("sha="):] + if not SHA_RE.match(reviewed_sha): + raise ValueError(f"审核 SHA 非法: {extra}。{USAGE}") else: raise ValueError(f"未知参数: {extra}。{USAGE}") - return ",".join(products), cases_ref + if not reviewed_sha: + raise ValueError(f"审核 SHA 显式必填。{USAGE}") + + return ",".join(products), cases_ref, reviewed_sha.lower() def _write_outputs(pairs): @@ -57,12 +75,18 @@ def _write_outputs(pairs): def main() -> int: try: - products, cases_ref = parse(os.environ.get("COMMENT_BODY", "")) + products, cases_ref, reviewed_sha = parse(os.environ.get("COMMENT_BODY", "")) except ValueError as exc: _write_outputs([("error", str(exc))]) print(str(exc), file=sys.stderr) return 1 - _write_outputs([("products", products), ("cases_ref", cases_ref)]) + _write_outputs( + [ + ("products", products), + ("cases_ref", cases_ref), + ("reviewed_sha", reviewed_sha), + ] + ) return 0 diff --git a/scripts/ci/eval_comment_parse_test.py b/scripts/ci/eval_comment_parse_test.py index bbf76178..89d8e4d8 100644 --- a/scripts/ci/eval_comment_parse_test.py +++ b/scripts/ci/eval_comment_parse_test.py @@ -2,22 +2,28 @@ import pytest from eval_comment_parse import parse +SHA = "0123456789abcdef0123456789abcdef01234567" + def test_single_product(): - assert parse("/eval drive") == ("drive", "") + assert parse(f"/eval drive sha={SHA}") == ("drive", "", SHA) def test_multiple_products(): - assert parse("/eval drive,doc,edu-app") == ("drive,doc,edu-app", "") + assert parse(f"/eval drive,doc,edu-app sha={SHA}") == ("drive,doc,edu-app", "", SHA) def test_cases_ref_escape_hatch(): - assert parse("/eval drive cases=feat/drive-latest") == ("drive", "feat/drive-latest") + assert parse(f"/eval drive sha={SHA} cases=feat/drive-latest") == ( + "drive", + "feat/drive-latest", + SHA, + ) def test_extra_lines_after_command_are_ignored(): - body = "/eval drive\n\n顺便说明：这个 PR 只动了 drive 的 --latest。" - assert parse(body) == ("drive", "") + body = f"/eval drive sha={SHA}\n\n顺便说明：这个 PR 只动了 drive 的 --latest。" + assert parse(body) == ("drive", "", SHA) def test_missing_products_rejected(): @@ -25,24 +31,34 @@ def test_missing_products_rejected(): parse("/eval") +def test_missing_reviewed_sha_rejected(): + with pytest.raises(ValueError, match="审核 SHA 显式必填"): + parse("/eval drive") + + def test_similar_command_prefix_rejected(): with pytest.raises(ValueError, match="/eval 命令开头"): - parse("/evaluate drive") + parse(f"/evaluate drive sha={SHA}") def test_illegal_product_name_rejected(): with pytest.raises(ValueError, match="产品名非法"): - parse("/eval drive;rm") + parse(f"/eval drive;rm sha={SHA}") def test_unknown_extra_token_rejected(): with pytest.raises(ValueError, match="未知参数"): - parse("/eval drive --force") + parse(f"/eval drive sha={SHA} --force") def test_illegal_cases_ref_rejected(): with pytest.raises(ValueError, match="cases 引用非法"): - parse("/eval drive cases=$(whoami)") + parse(f"/eval drive sha={SHA} cases=$(whoami)") + + +def test_short_reviewed_sha_rejected(): + with pytest.raises(ValueError, match="审核 SHA 非法"): + parse("/eval drive sha=0123456") def test_empty_comment_rejected(): diff --git a/scripts/ci/eval_dispatch_guard.py b/scripts/ci/eval_dispatch_guard.py new file mode 100644 index 00000000..d08e5271 --- /dev/null +++ b/scripts/ci/eval_dispatch_guard.py @@ -0,0 +1,80 @@ +#!/usr/bin/env python3 +"""校验 `/eval` 派发的仓库权限与已审核 PR head。""" + +import json +import os +import re +import sys + + +TRUSTED_PERMISSIONS = frozenset({"write", "maintain", "admin"}) +FULL_SHA_RE = re.compile(r"^[0-9a-fA-F]{40}$") + + +def _read_api_response(): + try: + value = json.load(sys.stdin) + except (json.JSONDecodeError, OSError) as exc: + raise ValueError(f"GitHub API response is not valid JSON: {exc}") from exc + if not isinstance(value, dict): + raise ValueError("GitHub API response must be a JSON object") + return value + + +def require_trusted_permission(response, commenter: str): + permission = response.get("permission") + if permission not in TRUSTED_PERMISSIONS: + raise PermissionError( + f"{commenter or 'commenter'} does not have write, maintain, or admin permission" + ) + return permission + + +def require_reviewed_current_head(response, expected_pr_number: str, reviewed_sha: str): + try: + expected_number = int(expected_pr_number) + except (TypeError, ValueError) as exc: + raise ValueError("expected PR number is invalid") from exc + if response.get("number") != expected_number: + raise ValueError("GitHub API response does not match the requested PR") + if response.get("state") != "open": + raise ValueError("PR is not open") + if not FULL_SHA_RE.fullmatch(reviewed_sha or ""): + raise ValueError("reviewed SHA must be a full 40-character commit SHA") + + head = response.get("head") + current_sha = head.get("sha") if isinstance(head, dict) else "" + if not FULL_SHA_RE.fullmatch(current_sha or ""): + raise ValueError("GitHub API response does not contain a valid PR head SHA") + if current_sha.lower() != reviewed_sha.lower(): + raise ValueError( + f"PR head changed after review: reviewed {reviewed_sha.lower()}, " + f"current {current_sha.lower()}" + ) + return reviewed_sha.lower() + + +def main() -> int: + mode = sys.argv[1] if len(sys.argv) == 2 else "" + try: + response = _read_api_response() + if mode == "permission": + permission = require_trusted_permission(response, os.environ.get("COMMENTER", "")) + print(f"permission={permission}") + return 0 + if mode == "head": + head_sha = require_reviewed_current_head( + response, + os.environ.get("EXPECTED_PR_NUMBER", ""), + os.environ.get("REVIEWED_SHA", ""), + ) + print(f"head_sha={head_sha}") + return 0 + raise ValueError(f"unknown guard mode: {mode}") + except (PermissionError, ValueError) as exc: + print(str(exc), file=sys.stderr) + return 1 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/test/scripts/eval_dispatch_workflow_test.go b/test/scripts/eval_dispatch_workflow_test.go new file mode 100644 index 00000000..07fb6f31 --- /dev/null +++ b/test/scripts/eval_dispatch_workflow_test.go @@ -0,0 +1,173 @@ +// Copyright 2026 Alibaba Group +// Licensed under the Apache License, Version 2.0 (the "License"); + +package scripts_test + +import ( + "os" + "os/exec" + "path/filepath" + "strings" + "testing" +) + +func TestEvalDispatchWorkflowUsesRepositoryPermissionAndReviewedSHA(t *testing.T) { + t.Parallel() + + root, err := filepath.Abs(filepath.Join("..", "..")) + if err != nil { + t.Fatalf("resolve repository root: %v", err) + } + data, err := os.ReadFile(filepath.Join(root, ".github", "workflows", "eval-dispatch.yml")) + if err != nil { + t.Fatalf("read eval-dispatch workflow: %v", err) + } + workflow := string(data) + + for _, want := range []string{ + "/collaborators/${COMMENTER}/permission", + "eval_dispatch_guard.py permission", + "REVIEWED_SHA: ${{ steps.parse.outputs.reviewed_sha }}", + "eval_dispatch_guard.py head", + } { + if !strings.Contains(workflow, want) { + t.Errorf("eval-dispatch workflow missing security contract %q", want) + } + } + if strings.Contains(workflow, "author_association") { + t.Error("eval-dispatch workflow must not authorize from author_association") + } +} + +func TestEvalDispatchRejectsLowRepositoryPermissions(t *testing.T) { + t.Parallel() + + for _, permission := range []string{"read", "triage", "none"} { + permission := permission + t.Run(permission, func(t *testing.T) { + t.Parallel() + output, err := runEvalDispatchGuard(t, "permission", `{"permission":"`+permission+`"}`, "COMMENTER=low-privilege-user") + if err == nil { + t.Fatalf("permission %q unexpectedly allowed; output=%s", permission, output) + } + if !strings.Contains(output, "does not have write, maintain, or admin permission") { + t.Fatalf("permission %q rejection = %q, want trusted-permission error", permission, output) + } + }) + } +} + +func TestEvalDispatchAllowsTrustedRepositoryPermissions(t *testing.T) { + t.Parallel() + + for _, permission := range []string{"write", "maintain", "admin"} { + permission := permission + t.Run(permission, func(t *testing.T) { + t.Parallel() + output, err := runEvalDispatchGuard(t, "permission", `{"permission":"`+permission+`"}`, "COMMENTER=maintainer") + if err != nil { + t.Fatalf("permission %q rejected: %v\n%s", permission, err, output) + } + }) + } +} + +func TestEvalDispatchRejectsChangedPRHead(t *testing.T) { + t.Parallel() + + const reviewedSHA = "1111111111111111111111111111111111111111" + const currentSHA = "2222222222222222222222222222222222222222" + pr := `{"number":934,"state":"open","head":{"sha":"` + currentSHA + `"}}` + output, err := runEvalDispatchGuard( + t, + "head", + pr, + "EXPECTED_PR_NUMBER=934", + "REVIEWED_SHA="+reviewedSHA, + ) + if err == nil { + t.Fatalf("changed PR head unexpectedly allowed; output=%s", output) + } + if !strings.Contains(output, "PR head changed after review") { + t.Fatalf("changed-head rejection = %q, want explicit stale-review error", output) + } +} + +func TestEvalDispatchAcceptsReviewedCurrentPRHead(t *testing.T) { + t.Parallel() + + const reviewedSHA = "1111111111111111111111111111111111111111" + pr := `{"number":934,"state":"open","head":{"sha":"` + reviewedSHA + `"}}` + output, err := runEvalDispatchGuard( + t, + "head", + pr, + "EXPECTED_PR_NUMBER=934", + "REVIEWED_SHA="+reviewedSHA, + ) + if err != nil { + t.Fatalf("reviewed current PR head rejected: %v\n%s", err, output) + } + if !strings.Contains(output, "head_sha="+reviewedSHA) { + t.Fatalf("head guard output = %q, want pinned head SHA", output) + } +} + +func TestEvalCommentRequiresExplicitReviewedSHA(t *testing.T) { + t.Parallel() + + const reviewedSHA = "1111111111111111111111111111111111111111" + output, err := runEvalCommentParser(t, "/eval drive sha="+reviewedSHA) + if err != nil { + t.Fatalf("explicit reviewed SHA rejected: %v\n%s", err, output) + } + for _, want := range []string{"products=drive", "reviewed_sha=" + reviewedSHA} { + if !strings.Contains(output, want) { + t.Errorf("parser output = %q, want %q", output, want) + } + } + + output, err = runEvalCommentParser(t, "/eval drive") + if err == nil { + t.Fatalf("command without reviewed SHA unexpectedly allowed; output=%s", output) + } + if !strings.Contains(output, "审核 SHA 显式必填") { + t.Fatalf("missing-SHA rejection = %q, want explicit reviewed-SHA error", output) + } +} + +func runEvalDispatchGuard(t *testing.T, mode, input string, env ...string) (string, error) { + t.Helper() + + root, err := filepath.Abs(filepath.Join("..", "..")) + if err != nil { + t.Fatalf("resolve repository root: %v", err) + } + cmd := exec.Command("python3", filepath.Join(root, "scripts", "ci", "eval_dispatch_guard.py"), mode) + cmd.Stdin = strings.NewReader(input) + cmd.Env = append(os.Environ(), env...) + output, runErr := cmd.CombinedOutput() + return string(output), runErr +} + +func runEvalCommentParser(t *testing.T, comment string) (string, error) { + t.Helper() + + root, err := filepath.Abs(filepath.Join("..", "..")) + if err != nil { + t.Fatalf("resolve repository root: %v", err) + } + outputPath := filepath.Join(t.TempDir(), "github-output") + cmd := exec.Command("python3", filepath.Join(root, "scripts", "ci", "eval_comment_parse.py")) + cmd.Env = append( + os.Environ(), + "COMMENT_BODY="+comment, + "GITHUB_OUTPUT="+outputPath, + ) + combined, runErr := cmd.CombinedOutput() + fileOutput, readErr := os.ReadFile(outputPath) + if readErr != nil && !os.IsNotExist(readErr) { + t.Fatalf("read parser GitHub output: %v", readErr) + } + return string(combined) + string(fileOutput), runErr +} From b082135e6ec97b206ced72042151471a208d22b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=85=8B=E8=B0=A8?= <2940480744@qq.com> Date: Tue, 11 Aug 2026 15:17:48 +0800 Subject: [PATCH 16/19] test(chat): cover scoped search branches --- internal/helpers/chat_message_search_test.go | 113 ++++++++++++++++++- 1 file changed, 112 insertions(+), 1 deletion(-) diff --git a/internal/helpers/chat_message_search_test.go b/internal/helpers/chat_message_search_test.go index a8d3c1f2..a41711e2 100644 --- a/internal/helpers/chat_message_search_test.go +++ b/internal/helpers/chat_message_search_test.go @@ -438,6 +438,100 @@ func TestNativeScopedSearchValidEmptyResultIsComplete(t *testing.T) { } func TestCrossPlatformCoverageNativeScopedSearchFailureAndPaginationBranches(t *testing.T) { + t.Run("empty scope uses the native search call", func(t *testing.T) { + caller := &chatMessageSearchCaller{} + previousDeps := deps + t.Cleanup(func() { deps = previousDeps }) + InitDeps(caller) + deps.Out.w = io.Discard + + cmd := &cobra.Command{Use: "search"} + err := runConversationScopedMessageSearch( + cmd, + "im", + "search_messages", + "openConversationIds", + map[string]any{"keyword": "周报"}, + []string{"", " "}, + ) + if err != nil { + t.Fatal(err) + } + if len(caller.calls) != 1 || caller.calls[0].toolName != "search_messages" { + t.Fatalf("calls = %#v", caller.calls) + } + }) + + t.Run("invalid page options fail before preflight", func(t *testing.T) { + caller := &chatMessageSearchCaller{} + previousDeps := deps + t.Cleanup(func() { deps = previousDeps }) + InitDeps(caller) + + cmd := &cobra.Command{Use: "search"} + AddPagedMCPFlags(cmd) + if err := cmd.Flags().Set("page-all", "true"); err != nil { + t.Fatal(err) + } + if err := cmd.Flags().Set("page-limit", "0"); err != nil { + t.Fatal(err) + } + err := runConversationScopedMessageSearch( + cmd, + "im", + "search_messages", + "openConversationIds", + map[string]any{"keyword": "周报"}, + []string{"cid-target"}, + ) + if err == nil || !strings.Contains(err.Error(), "--page-limit must be between 1 and 500") { + t.Fatalf("error = %v", err) + } + if len(caller.calls) != 0 { + t.Fatalf("invalid paging made calls: %#v", caller.calls) + } + }) + + t.Run("cancelled context interrupts page delay", func(t *testing.T) { + caller := &chatMessageSearchCaller{searchResponse: `{ + "result": { + "messages": [{"openMessageId":"m1","openConversationId":"cid-target"}], + "hasMore": true, + "nextCursor": "c2" + } + }`} + previousDeps := deps + t.Cleanup(func() { deps = previousDeps }) + InitDeps(caller) + deps.Out.w = io.Discard + + cmd := &cobra.Command{Use: "search"} + AddPagedMCPFlags(cmd) + if err := cmd.Flags().Set("page-all", "true"); err != nil { + t.Fatal(err) + } + if err := cmd.Flags().Set("page-delay", "60000"); err != nil { + t.Fatal(err) + } + ctx, cancel := context.WithCancel(context.Background()) + cancel() + cmd.SetContext(ctx) + err := runConversationScopedMessageSearch( + cmd, + "im", + "search_messages", + "openConversationIds", + map[string]any{"keyword": "周报", "limit": 100, "cursor": "0"}, + []string{"cid-target"}, + ) + if !errors.Is(err, context.Canceled) { + t.Fatalf("error = %v, want context canceled", err) + } + if caller.searchCalls != 1 { + t.Fatalf("search calls = %d, want 1", caller.searchCalls) + } + }) + t.Run("lower search error", func(t *testing.T) { caller := &chatMessageSearchCaller{searchError: errors.New("search unavailable")} _, err := executeNativeScopedSearch(t, caller, @@ -521,13 +615,24 @@ func TestCrossPlatformCoverageNativeScopedSearchFailureAndPaginationBranches(t * } } -func TestNativeScopedSearchDryRunShowsCompositePlanWithoutCallingTools(t *testing.T) { +func TestCrossPlatformCoverageNativeScopedSearchDryRunShowsCompositePlanWithoutCallingTools(t *testing.T) { caller := &chatMessageSearchCaller{} previousDeps := deps t.Cleanup(func() { deps = previousDeps }) InitDeps(caller) cmd := &cobra.Command{Use: "search"} cmd.Flags().Bool("dry-run", true, "") + AddPagedMCPFlags(cmd) + for name, value := range map[string]string{ + "page-all": "true", + "page-limit": "7", + "max-items": "9", + "page-delay": "11", + } { + if err := cmd.Flags().Set(name, value); err != nil { + t.Fatalf("set --%s: %v", name, err) + } + } var output strings.Builder cmd.SetOut(&output) err := runConversationScopedMessageSearch( @@ -562,6 +667,12 @@ func TestNativeScopedSearchDryRunShowsCompositePlanWithoutCallingTools(t *testin if _, exists := arguments["openConversationIds"]; exists { t.Fatalf("dry-run global search still carries scope: %#v", searchStage) } + if searchStage["pageAll"] != true || + searchStage["pageLimit"] != float64(7) || + searchStage["maxItems"] != float64(9) || + searchStage["pageDelay"] != float64(11) { + t.Fatalf("dry-run paging = %#v", searchStage) + } } type chatChangedContractCaller struct { From ad6837d6949644a3a7689e5f0fa703e8ed65ec65 Mon Sep 17 00:00:00 2001 From: chichuan Date: Tue, 11 Aug 2026 15:45:33 +0800 Subject: [PATCH 17/19] feat(eval-dispatch): allowlist tier for self-service PR evaluation Users listed in .github/eval-allowlist.txt (default branch, PR-reviewed) may dispatch /eval for their own PRs only; write/maintain/admin retain dispatch for any PR. Fail-closed on permission API 404/network errors. --- .github/workflows/eval-dispatch.yml | 22 ++++++--- scripts/ci/eval_dispatch_guard.py | 73 ++++++++++++++++++++++++----- 2 files changed, 75 insertions(+), 20 deletions(-) diff --git a/.github/workflows/eval-dispatch.yml b/.github/workflows/eval-dispatch.yml index 1292dd80..70df32e4 100644 --- a/.github/workflows/eval-dispatch.yml +++ b/.github/workflows/eval-dispatch.yml @@ -1,9 +1,13 @@ name: PR Eval Dispatch -# `/eval sha= [cases=]` PR 评论 → 触发内网评测流水线，报告由内网 bot 回贴。 -# 本 workflow 只在默认分支上下文运行，不 checkout、不执行 PR 代码； -# maintainer 必须在 /eval 中显式记录已审阅的 PR head；权限与当前 head 校验 -# 通过后，内网 CI 只构建并执行该已审核 SHA。 +# `/eval [sha=] [cases=]` PR 评论 → 触发内网评测流水线，报告由内网 bot 回贴。 +# 本 workflow 只在默认分支上下文运行，不 checkout、不执行 PR 代码。 +# 审核 SHA 规则：评测他人 PR 必须显式携带 sha=（审阅背书凭据，验证 +# 其恰为当前 open head）；评测自己创建的 PR 可省略，自动钉住派发时刻 +# 的当前 head（作者自背书，无第三方偷换窗口）；内网 CI 另以 +# FETCH_HEAD 校验兜底派发后的变更。 +# 授权两级：仓库 write/maintain/admin 可派发任意 PR；默认分支 +# .github/eval-allowlist.txt 名单内的用户仅可派发自己创建的 PR。 # 触发通道与凭证全部经 secrets 注入，文件内不出现任何内网信息。 on: @@ -33,15 +37,18 @@ jobs: - name: Check out default branch tooling uses: actions/checkout@v4 - - name: Verify commenter repository permission + - name: Verify commenter dispatch authorization env: GH_TOKEN: ${{ github.token }} COMMENTER: ${{ github.event.comment.user.login }} + PR_AUTHOR: ${{ github.event.issue.user.login }} + EVAL_ALLOWLIST_PATH: .github/eval-allowlist.txt run: | - permission_json="$(curl --fail --silent --show-error \ + # 不用 --fail：非协作者查权限返回 404 错误体，交由 guard 走名单分支；硬网络错误降级为空对象同样 fail-closed + permission_json="$(curl --silent --show-error \ -H "Authorization: Bearer ${GH_TOKEN}" \ -H "Accept: application/vnd.github+json" \ - "https://api.github.com/repos/${GITHUB_REPOSITORY}/collaborators/${COMMENTER}/permission")" + "https://api.github.com/repos/${GITHUB_REPOSITORY}/collaborators/${COMMENTER}/permission")" || permission_json='{}' printf '%s' "$permission_json" | python3 scripts/ci/eval_dispatch_guard.py permission - name: Parse /eval command @@ -74,6 +81,7 @@ jobs: PR_NUMBER: ${{ github.event.issue.number }} EXPECTED_PR_NUMBER: ${{ github.event.issue.number }} REVIEWED_SHA: ${{ steps.parse.outputs.reviewed_sha }} + COMMENTER: ${{ github.event.comment.user.login }} run: | pr_json="$(curl --fail --silent --show-error \ -H "Authorization: Bearer ${GH_TOKEN}" \ diff --git a/scripts/ci/eval_dispatch_guard.py b/scripts/ci/eval_dispatch_guard.py index d08e5271..d4be187a 100644 --- a/scripts/ci/eval_dispatch_guard.py +++ b/scripts/ci/eval_dispatch_guard.py @@ -21,16 +21,44 @@ def _read_api_response(): return value -def require_trusted_permission(response, commenter: str): +def load_allowlist(path: str) -> frozenset: + """读取自助触发允许名单（默认分支文件；# 注释与空行忽略；大小写不敏感）。""" + if not path: + return frozenset() + try: + with open(path, encoding="utf-8") as f: + lines = f.read().splitlines() + except FileNotFoundError: + return frozenset() + entries = set() + for line in lines: + entry = line.split("#", 1)[0].strip() + if entry: + entries.add(entry.lower()) + return frozenset(entries) + + +def authorize_dispatch(response, commenter: str, pr_author: str, allowlist: frozenset): + """两级授权：仓库写权限可派发任意 PR；名单内用户仅可派发自己创建的 PR。""" permission = response.get("permission") - if permission not in TRUSTED_PERMISSIONS: + if permission in TRUSTED_PERMISSIONS: + return f"maintainer:{permission}" + commenter_key = (commenter or "").strip().lower() + author_key = (pr_author or "").strip().lower() + if commenter_key and commenter_key in allowlist: + if commenter_key == author_key: + return "allowlisted-author" raise PermissionError( - f"{commenter or 'commenter'} does not have write, maintain, or admin permission" + f"{commenter or 'commenter'} is allowlisted for self-service only " + "and may not dispatch other authors' PRs" ) - return permission + raise PermissionError( + f"{commenter or 'commenter'} does not have write, maintain, or admin permission " + "and is not an allowlisted PR author" + ) -def require_reviewed_current_head(response, expected_pr_number: str, reviewed_sha: str): +def require_reviewed_current_head(response, expected_pr_number: str, reviewed_sha: str, commenter: str): try: expected_number = int(expected_pr_number) except (TypeError, ValueError) as exc: @@ -39,19 +67,32 @@ def require_reviewed_current_head(response, expected_pr_number: str, reviewed_sh raise ValueError("GitHub API response does not match the requested PR") if response.get("state") != "open": raise ValueError("PR is not open") - if not FULL_SHA_RE.fullmatch(reviewed_sha or ""): - raise ValueError("reviewed SHA must be a full 40-character commit SHA") head = response.get("head") current_sha = head.get("sha") if isinstance(head, dict) else "" if not FULL_SHA_RE.fullmatch(current_sha or ""): raise ValueError("GitHub API response does not contain a valid PR head SHA") - if current_sha.lower() != reviewed_sha.lower(): + + if reviewed_sha: + if not FULL_SHA_RE.fullmatch(reviewed_sha): + raise ValueError("reviewed SHA must be a full 40-character commit SHA") + if current_sha.lower() != reviewed_sha.lower(): + raise ValueError( + f"PR head changed after review: reviewed {reviewed_sha.lower()}, " + f"current {current_sha.lower()}" + ) + return reviewed_sha.lower() + + # sha 省略：仅限评论者本人创建的 PR（自助模式）——作者对自己分支的 + # tip 背书不存在第三方偷换窗口；钉住派发时刻的当前 head，内网侧 + # FETCH_HEAD 校验继续兜底派发→执行窗口。评测他人 PR 必须显式带 sha=。 + author = ((response.get("user") or {}).get("login") or "").strip().lower() + commenter_key = (commenter or "").strip().lower() + if not author or commenter_key != author: raise ValueError( - f"PR head changed after review: reviewed {reviewed_sha.lower()}, " - f"current {current_sha.lower()}" + "dispatching another author's PR requires an explicit reviewed sha=" ) - return reviewed_sha.lower() + return current_sha.lower() def main() -> int: @@ -59,14 +100,20 @@ def main() -> int: try: response = _read_api_response() if mode == "permission": - permission = require_trusted_permission(response, os.environ.get("COMMENTER", "")) - print(f"permission={permission}") + role = authorize_dispatch( + response, + os.environ.get("COMMENTER", ""), + os.environ.get("PR_AUTHOR", ""), + load_allowlist(os.environ.get("EVAL_ALLOWLIST_PATH", "")), + ) + print(f"authorized={role}") return 0 if mode == "head": head_sha = require_reviewed_current_head( response, os.environ.get("EXPECTED_PR_NUMBER", ""), os.environ.get("REVIEWED_SHA", ""), + os.environ.get("COMMENTER", ""), ) print(f"head_sha={head_sha}") return 0 From 233e0359e4a0e3796bd1588d1d6b87a702fb18b3 Mon Sep 17 00:00:00 2001 From: chichuan Date: Tue, 11 Aug 2026 15:45:33 +0800 Subject: [PATCH 18/19] chore(eval-dispatch): seed allowlist with 53 internal contributors --- .github/eval-allowlist.txt | 61 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+) create mode 100644 .github/eval-allowlist.txt diff --git a/.github/eval-allowlist.txt b/.github/eval-allowlist.txt new file mode 100644 index 00000000..8cde985f --- /dev/null +++ b/.github/eval-allowlist.txt @@ -0,0 +1,61 @@ +# /eval 自助触发允许名单 +# +# 名单内的 GitHub 登录名可对【自己创建的 PR】触发 /eval 评测； +# 对任意 PR 触发仍需仓库 write/maintain/admin 权限（维护者背书）。 +# 授权读取的始终是默认分支上的本文件，PR 无法修改自身授权。 +# +# 变更本文件必须走 PR 评审。每行一个 GitHub login，# 开头为注释。 + +aftersss +notable-open +EdgarWang0925 +ayunya +yutongshe +qingyang1014 +caiTriumph +xlb1130 +Anonymity-0 +FuShu-Yang +guimingyue +AlwaysLee +TaoJikun +zengyoulingzyl-stack +liyuan333 +huangyoo +lifeihong +nitonitori +cywan1998 +gangwn +junlonghuo2 +aqruan +Freda0909 +ShawnWhite777 +PeterGuy326 +abucraft +pengzhihan47-star +rainyak8 +gongrongyun +huangyuanzhuo-coder +ybcstudy +bigqy +liwang-ai +meng93 +wxianfeng +Patrick-Star-CN +rossluo28-hz +dxy704330469 +gtezg30062 +Neige-Premaire +zhuoyu20 +avicii-chen +typefield +Haofeng0705 +Huwenjiao +liuzeyang +maoqxxmm +FloralTide +lingyun9833 +dxb121 +C0922 +xiaoji121 +H3java From 20d1f7c614433028517e3c55c6133e77e4ea6600 Mon Sep 17 00:00:00 2001 From: chichuan Date: Tue, 11 Aug 2026 15:46:16 +0800 Subject: [PATCH 19/19] feat(eval-dispatch): optional sha= for own-PR dispatch; structural cases ref validation - /eval on one's own PR may omit sha=: the guard auto-pins the dispatch-time head (commenter == PR author leaves no third-party swap window); dispatching another author's PR still requires the explicit reviewed SHA (keeps the P1-2 TOCTOU remedy where the threat lives) - cases= is now validated structurally per git check-ref-format semantics (leading/trailing//double slashes, '..', dot-leading components, .lock suffixes) and rejects '-'-leading values to prevent git fetch option injection (review P2) --- scripts/ci/eval_comment_parse.py | 38 ++++-- scripts/ci/eval_comment_parse_test.py | 32 ++++- test/scripts/eval_dispatch_workflow_test.go | 124 +++++++++++++++++++- 3 files changed, 180 insertions(+), 14 deletions(-) diff --git a/scripts/ci/eval_comment_parse.py b/scripts/ci/eval_comment_parse.py index e015ed5e..24596151 100644 --- a/scripts/ci/eval_comment_parse.py +++ b/scripts/ci/eval_comment_parse.py @@ -1,13 +1,14 @@ #!/usr/bin/env python3 -"""解析 PR 评论中的 `/eval sha= [cases=]` 触发命令。 +"""解析 PR 评论中的 `/eval [sha=] [cases=]` 触发命令。 供 .github/workflows/eval-dispatch.yml 调用：评论体经环境变量 COMMENT_BODY 传入（避免 argv 注入），解析结果以 GitHub Actions output （products / cases_ref / reviewed_sha）写出；格式非法时把单行错误写入 output `error` 并以非零退出。 -产品集和完整审核 SHA 显式必填；`cases=` 为用例仓库版本逃生舱 -（破坏性变更配对验证）。 +产品集显式必填；`sha=` 在评测他人 PR 时必填（审核背书凭据，由 guard 校验）， +评测自己创建的 PR 时可省略（自助模式，自动钉住派发时刻的当前 head）； +`cases=` 为用例仓库版本逃生舱（破坏性变更配对验证）。 """ import os @@ -15,14 +16,34 @@ import re import sys PRODUCT_RE = re.compile(r"^[a-z0-9][a-z0-9-]*$") -REF_RE = re.compile(r"^[A-Za-z0-9._/-]+$") +REF_CHARSET_RE = re.compile(r"^[A-Za-z0-9._/-]+$") SHA_RE = re.compile(r"^[0-9a-fA-F]{40}$") USAGE = ( - "用法: /eval [,...] sha=<40位PR-head-SHA> [cases=]；" - "示例: /eval drive,doc sha=0123456789abcdef0123456789abcdef01234567 cases=feat/x" + "用法: /eval [,...] [sha=<40位PR-head-SHA>] [cases=]；" + "评测他人 PR 时 sha= 必填，自己的 PR 可省略；" + "示例: /eval drive,doc sha=0123456789abcdef0123456789abcdef01234567" ) +def _is_valid_cases_ref(ref: str) -> bool: + """等价于 git check-ref-format --allow-onelevel 的结构校验（不执行 git）。 + + 字符白名单之上补齐结构规则：禁止首尾斜杠与连续斜杠、尾部点号、 + 任意位置的 `..`、以点开头或以 .lock 结尾的路径分量；另禁止 `-` + 开头，避免被下游 `git fetch origin ` 当作选项解析。 + """ + if not ref or not REF_CHARSET_RE.match(ref): + return False + if ref.startswith(("-", "/")) or ref.endswith(("/", ".")): + return False + if ".." in ref or "//" in ref: + return False + for component in ref.split("/"): + if component.startswith(".") or component.endswith(".lock"): + return False + return True + + def parse(body: str): lines = [line.strip() for line in (body or "").splitlines() if line.strip()] if not lines: @@ -47,7 +68,7 @@ def parse(body: str): if cases_ref: raise ValueError(f"cases 引用只能指定一次。{USAGE}") cases_ref = extra[len("cases="):] - if not cases_ref or not REF_RE.match(cases_ref): + if not _is_valid_cases_ref(cases_ref): raise ValueError(f"cases 引用非法: {extra}。{USAGE}") elif extra.startswith("sha="): if reviewed_sha: @@ -59,7 +80,8 @@ def parse(body: str): raise ValueError(f"未知参数: {extra}。{USAGE}") if not reviewed_sha: - raise ValueError(f"审核 SHA 显式必填。{USAGE}") + # 是否允许省略由 guard 按“评论者是否 PR 作者”裁决，解析层不拦 + return ",".join(products), cases_ref, "" return ",".join(products), cases_ref, reviewed_sha.lower() diff --git a/scripts/ci/eval_comment_parse_test.py b/scripts/ci/eval_comment_parse_test.py index 89d8e4d8..419b1f7c 100644 --- a/scripts/ci/eval_comment_parse_test.py +++ b/scripts/ci/eval_comment_parse_test.py @@ -31,9 +31,9 @@ def test_missing_products_rejected(): parse("/eval") -def test_missing_reviewed_sha_rejected(): - with pytest.raises(ValueError, match="审核 SHA 显式必填"): - parse("/eval drive") +def test_omitted_reviewed_sha_defers_to_guard(): + # 是否允许省略由 guard 按“评论者是否 PR 作者”裁决，解析层放行并输出空 reviewed_sha + assert parse("/eval drive") == ("drive", "", "") def test_similar_command_prefix_rejected(): @@ -56,6 +56,32 @@ def test_illegal_cases_ref_rejected(): parse(f"/eval drive sha={SHA} cases=$(whoami)") +def test_structurally_invalid_cases_refs_rejected(): + # git check-ref-format 结构规则：字符合法但结构非法的引用必须拒绝 + invalid_refs = [ + "..", # 以点开头且含连续点号 + "/main", # 首部斜杠 + "feature//x", # 连续斜杠 + "feature/", # 尾部斜杠 + "release.", # 尾部点号 + "a..b", # 任意位置连续点号 + ".hidden", # 分量以点开头 + "a/.hidden", # 非首分量以点开头 + "a.lock", # 分量以 .lock 结尾 + "a/b.lock", # 非首分量以 .lock 结尾 + "-flag", # 以 - 开头，防 git fetch 选项注入 + ] + for ref in invalid_refs: + with pytest.raises(ValueError, match="cases 引用非法"): + parse(f"/eval drive sha={SHA} cases={ref}") + + +def test_structurally_valid_cases_refs_accepted(): + valid_refs = ["main", "feat/drive-latest", "release/v1.0", "a.b/c-d_e", SHA] + for ref in valid_refs: + assert parse(f"/eval drive sha={SHA} cases={ref}") == ("drive", ref, SHA) + + def test_short_reviewed_sha_rejected(): with pytest.raises(ValueError, match="审核 SHA 非法"): parse("/eval drive sha=0123456") diff --git a/test/scripts/eval_dispatch_workflow_test.go b/test/scripts/eval_dispatch_workflow_test.go index 07fb6f31..235d1f31 100644 --- a/test/scripts/eval_dispatch_workflow_test.go +++ b/test/scripts/eval_dispatch_workflow_test.go @@ -27,6 +27,8 @@ func TestEvalDispatchWorkflowUsesRepositoryPermissionAndReviewedSHA(t *testing.T for _, want := range []string{ "/collaborators/${COMMENTER}/permission", "eval_dispatch_guard.py permission", + "PR_AUTHOR: ${{ github.event.issue.user.login }}", + "EVAL_ALLOWLIST_PATH: .github/eval-allowlist.txt", "REVIEWED_SHA: ${{ steps.parse.outputs.reviewed_sha }}", "eval_dispatch_guard.py head", } { @@ -37,6 +39,9 @@ func TestEvalDispatchWorkflowUsesRepositoryPermissionAndReviewedSHA(t *testing.T if strings.Contains(workflow, "author_association") { t.Error("eval-dispatch workflow must not authorize from author_association") } + if _, err := os.Stat(filepath.Join(root, ".github", "eval-allowlist.txt")); err != nil { + t.Errorf("eval allowlist file missing on default branch: %v", err) + } } func TestEvalDispatchRejectsLowRepositoryPermissions(t *testing.T) { @@ -127,15 +132,128 @@ func TestEvalCommentRequiresExplicitReviewedSHA(t *testing.T) { } } + // sha 省略在解析层放行（空 reviewed_sha），由 guard 按评论者是否 PR 作者裁决 output, err = runEvalCommentParser(t, "/eval drive") + if err != nil { + t.Fatalf("omitted reviewed SHA rejected at parse layer: %v\n%s", err, output) + } + if !strings.Contains(output, "reviewed_sha=\n") && !strings.HasSuffix(output, "reviewed_sha=") { + t.Fatalf("parser output = %q, want empty reviewed_sha passthrough", output) + } +} + +func TestEvalDispatchAutoPinsOwnPRHeadWhenShaOmitted(t *testing.T) { + t.Parallel() + + const currentSHA = "3333333333333333333333333333333333333333" + pr := `{"number":934,"state":"open","user":{"login":"Internal-Contributor"},"head":{"sha":"` + currentSHA + `"}}` + output, err := runEvalDispatchGuard( + t, + "head", + pr, + "EXPECTED_PR_NUMBER=934", + "REVIEWED_SHA=", + "COMMENTER=internal-contributor", + ) + if err != nil { + t.Fatalf("own-PR auto pin rejected: %v\n%s", err, output) + } + if !strings.Contains(output, "head_sha="+currentSHA) { + t.Fatalf("guard output = %q, want auto-pinned current head", output) + } +} + +func TestEvalDispatchRequiresShaForOthersPRs(t *testing.T) { + t.Parallel() + + const currentSHA = "3333333333333333333333333333333333333333" + pr := `{"number":934,"state":"open","user":{"login":"someone-else"},"head":{"sha":"` + currentSHA + `"}}` + output, err := runEvalDispatchGuard( + t, + "head", + pr, + "EXPECTED_PR_NUMBER=934", + "REVIEWED_SHA=", + "COMMENTER=internal-contributor", + ) if err == nil { - t.Fatalf("command without reviewed SHA unexpectedly allowed; output=%s", output) + t.Fatalf("other author's PR without sha unexpectedly allowed; output=%s", output) } - if !strings.Contains(output, "审核 SHA 显式必填") { - t.Fatalf("missing-SHA rejection = %q, want explicit reviewed-SHA error", output) + if !strings.Contains(output, "requires an explicit reviewed sha") { + t.Fatalf("missing-sha rejection = %q, want explicit-sha requirement", output) } } +func TestEvalDispatchAllowsAllowlistedAuthorOnOwnPR(t *testing.T) { + t.Parallel() + + allowlist := writeEvalAllowlist(t, "# 自助评测名单\nInternal-Contributor # 内部贡献者\n") + output, err := runEvalDispatchGuard( + t, + "permission", + `{"message":"Not Found"}`, + "COMMENTER=internal-contributor", + "PR_AUTHOR=Internal-Contributor", + "EVAL_ALLOWLIST_PATH="+allowlist, + ) + if err != nil { + t.Fatalf("allowlisted author rejected on own PR: %v\n%s", err, output) + } + if !strings.Contains(output, "authorized=allowlisted-author") { + t.Fatalf("guard output = %q, want allowlisted-author authorization", output) + } +} + +func TestEvalDispatchRejectsAllowlistedUserOnOthersPR(t *testing.T) { + t.Parallel() + + allowlist := writeEvalAllowlist(t, "internal-contributor\n") + output, err := runEvalDispatchGuard( + t, + "permission", + `{"message":"Not Found"}`, + "COMMENTER=internal-contributor", + "PR_AUTHOR=someone-else", + "EVAL_ALLOWLIST_PATH="+allowlist, + ) + if err == nil { + t.Fatalf("allowlisted user unexpectedly dispatched another author's PR; output=%s", output) + } + if !strings.Contains(output, "self-service only") { + t.Fatalf("cross-PR rejection = %q, want self-service-only error", output) + } +} + +func TestEvalDispatchRejectsUnlistedCommenterWithoutWrite(t *testing.T) { + t.Parallel() + + allowlist := writeEvalAllowlist(t, "# 空名单\n") + output, err := runEvalDispatchGuard( + t, + "permission", + `{"permission":"read"}`, + "COMMENTER=stranger", + "PR_AUTHOR=stranger", + "EVAL_ALLOWLIST_PATH="+allowlist, + ) + if err == nil { + t.Fatalf("unlisted commenter unexpectedly allowed; output=%s", output) + } + if !strings.Contains(output, "not an allowlisted PR author") { + t.Fatalf("unlisted rejection = %q, want allowlist-aware error", output) + } +} + +func writeEvalAllowlist(t *testing.T, content string) string { + t.Helper() + + path := filepath.Join(t.TempDir(), "eval-allowlist.txt") + if err := os.WriteFile(path, []byte(content), 0o644); err != nil { + t.Fatalf("write allowlist fixture: %v", err) + } + return path +} + func runEvalDispatchGuard(t *testing.T, mode, input string, env ...string) (string, error) { t.Helper()