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] 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)