From fad41d4d99c13c4ecbf22630b16cfa883c491ddb Mon Sep 17 00:00:00 2001 From: Dennis Date: Wed, 12 Aug 2026 12:29:07 +0800 Subject: [PATCH 1/9] fix(drive): verify upload and move readback --- internal/shortcut/drive/catalog_operations.go | 7 ++ internal/shortcut/drive/common.go | 35 +++++++++ internal/shortcut/drive/drive.go | 19 +++++ .../shortcut/drive/drive_shortcuts_test.go | 76 ++++++++++++++++++- 4 files changed, 136 insertions(+), 1 deletion(-) diff --git a/internal/shortcut/drive/catalog_operations.go b/internal/shortcut/drive/catalog_operations.go index 0a23fbf3..394a1638 100644 --- a/internal/shortcut/drive/catalog_operations.go +++ b/internal/shortcut/drive/catalog_operations.go @@ -489,6 +489,13 @@ var Upload = shortcut.Shortcut{ if remoteName := firstString(verified, "name", "fileName"); remoteName == "" || !strings.HasPrefix(remoteName, strings.TrimSuffix(name, filepath.Ext(name))) { return driveResponseError("drive/commit_upload", "readback_mismatch", fmt.Sprintf("上传后读回名称 %q 与请求 %q 不一致", remoteName, name)) } + remoteSize, ok := firstInt64(verified, "fileSize", "size", "byteSize", "length") + if !ok { + return driveResponseError("drive/commit_upload", "readback_missing_size", "上传后读回缺少有效文件大小；无法证明远端文件完整") + } + if remoteSize != info.Size() { + return driveResponseError("drive/commit_upload", "readback_size_mismatch", fmt.Sprintf("上传后读回大小 %d 与本地文件大小 %d 不一致", remoteSize, info.Size())) + } return rt.Output(map[string]any{"success": true, "nodeId": nodeID, "sizeBytes": info.Size(), "file": verified}) }, } diff --git a/internal/shortcut/drive/common.go b/internal/shortcut/drive/common.go index 220e5ffd..d528b10f 100644 --- a/internal/shortcut/drive/common.go +++ b/internal/shortcut/drive/common.go @@ -6,6 +6,8 @@ package drive import ( "encoding/json" "fmt" + "math" + "strconv" "strings" "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/corecmd" @@ -216,6 +218,39 @@ func nestedString(data map[string]any, keys ...string) string { return "" } +func firstInt64(data map[string]any, keys ...string) (int64, bool) { + for _, key := range keys { + value, present := data[key] + if !present { + continue + } + switch typed := value.(type) { + case int: + return int64(typed), true + case int32: + return int64(typed), true + case int64: + return typed, true + case float64: + if !math.IsNaN(typed) && !math.IsInf(typed, 0) && typed == math.Trunc(typed) && typed >= math.MinInt64 && typed < math.MaxInt64 { + return int64(typed), true + } + case json.Number: + parsed, err := strconv.ParseInt(typed.String(), 10, 64) + if err == nil { + return parsed, true + } + case string: + parsed, err := strconv.ParseInt(strings.TrimSpace(typed), 10, 64) + if err == nil { + return parsed, true + } + } + return 0, false + } + return 0, false +} + func driveResponseError(operation, reason, message string) error { return apperrors.NewAPI(message, apperrors.WithOperation(operation), diff --git a/internal/shortcut/drive/drive.go b/internal/shortcut/drive/drive.go index 4cbf239b..507f7aad 100644 --- a/internal/shortcut/drive/drive.go +++ b/internal/shortcut/drive/drive.go @@ -20,6 +20,7 @@ package drive import ( + "fmt" "strings" "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/corecmd" @@ -546,6 +547,24 @@ var Move = shortcut.Shortcut{ if err != nil { return err } + if rt.Changed("folder") { + remoteFolder := firstString(verified, "folderId", "targetFolderId", "parentId") + if remoteFolder == "" { + return driveResponseError("doc/move_document", "readback_missing_folder", "移动后读回缺少目标文件夹 ID；无法证明移动已到达请求位置") + } + if remoteFolder != rt.Str("folder") { + return driveResponseError("doc/move_document", "readback_folder_mismatch", fmt.Sprintf("移动后读回文件夹 %q 与请求 %q 不一致", remoteFolder, rt.Str("folder"))) + } + } + if rt.Changed("workspace") { + remoteWorkspace := firstString(verified, "workspaceId", "spaceId") + if remoteWorkspace == "" { + return driveResponseError("doc/move_document", "readback_missing_workspace", "移动后读回缺少目标知识库 ID；无法证明移动已到达请求位置") + } + if remoteWorkspace != rt.Str("workspace") { + return driveResponseError("doc/move_document", "readback_workspace_mismatch", fmt.Sprintf("移动后读回知识库 %q 与请求 %q 不一致", remoteWorkspace, rt.Str("workspace"))) + } + } return rt.Output(map[string]any{"success": true, "nodeId": rt.Str("node"), "file": verified}) }, } diff --git a/internal/shortcut/drive/drive_shortcuts_test.go b/internal/shortcut/drive/drive_shortcuts_test.go index 6fddea7a..5ff95517 100644 --- a/internal/shortcut/drive/drive_shortcuts_test.go +++ b/internal/shortcut/drive/drive_shortcuts_test.go @@ -9,6 +9,7 @@ import ( "errors" "fmt" "io" + "math" "os" "path/filepath" "strings" @@ -176,6 +177,25 @@ func TestCrossPlatformCoverageDriveDownloadAndUploadRequireArtifactsAndReadback( if _, _, err := resolveDriveUploadInput("../escape.bin"); err == nil { t.Fatal("upload path escape was accepted") } + for _, tc := range []struct { + name string + readback string + want string + }{ + {"missing remote size", `{"success":true,"result":{"fileId":"uploaded-2","name":"input.bin"}}`, "缺少有效文件大小"}, + {"mismatched remote size", `{"success":true,"result":{"fileId":"uploaded-3","name":"input.bin","fileSize":17}}`, "与本地文件大小 18 不一致"}, + } { + t.Run(tc.name, func(t *testing.T) { + testseam.Swap(t, &uploadDriveFile, func(context.Context, helpers.DriveUploadRequest) (map[string]any, error) { + return map[string]any{"success": true, "result": map[string]any{"fileId": strings.TrimSuffix(tc.name, " remote size")}}, nil + }) + caller := &driveCoverageCaller{responses: map[string][]string{"get_file_info": {tc.readback}}} + err := runDriveCoverage(t, Upload, caller, "--file", "input.bin", "--yes") + if err == nil || !strings.Contains(err.Error(), tc.want) { + t.Fatalf("error = %v, want %q", err, tc.want) + } + }) + } testseam.Swap(t, &driveDownload, func(_ context.Context, _ string, options localio.DownloadOptions) (localio.DownloadResult, error) { if options.Output != "downloads/file.bin" || options.Headers["x-token"] != "secret" { @@ -218,6 +238,39 @@ func TestCrossPlatformCoverageDriveCopyPreservesSchemaProperties(t *testing.T) { } } +func TestCrossPlatformCoverageDriveFirstInt64(t *testing.T) { + for _, tc := range []struct { + name string + value any + want int64 + ok bool + }{ + {"int", int(1), 1, true}, + {"int32", int32(2), 2, true}, + {"int64", int64(3), 3, true}, + {"float", float64(4), 4, true}, + {"json number", json.Number("5"), 5, true}, + {"string", " 6 ", 6, true}, + {"fraction", 1.5, 0, false}, + {"nan", math.NaN(), 0, false}, + {"infinity", math.Inf(1), 0, false}, + {"overflow", float64(math.MaxInt64), 0, false}, + {"bad json number", json.Number("bad"), 0, false}, + {"bad string", "bad", 0, false}, + {"unsupported", true, 0, false}, + } { + t.Run(tc.name, func(t *testing.T) { + got, ok := firstInt64(map[string]any{"size": tc.value}, "missing", "size") + if ok != tc.ok || got != tc.want { + t.Fatalf("firstInt64(%#v) = (%d, %t), want (%d, %t)", tc.value, got, ok, tc.want, tc.ok) + } + }) + } + if got, ok := firstInt64(map[string]any{}, "size"); ok || got != 0 { + t.Fatalf("missing firstInt64 = (%d, %t), want (0, false)", got, ok) + } +} + func TestCrossPlatformCoverageDriveVersionAndPublishContracts(t *testing.T) { versionPayload := `{"success":true,"versions":[{"version":1,"fileSize":3},{"versionNumber":"2","fileSize":4}],"hasMore":false}` caller := &driveCoverageCaller{responses: map[string][]string{"list_file_versions": {versionPayload}}} @@ -449,11 +502,32 @@ func TestCrossPlatformCoverageDriveCreateRestoreCopyMoveRename(t *testing.T) { move := &driveCoverageCaller{responses: map[string][]string{ "move_document": {`{"success":true}`}, - "get_document_info": {`{"success":true,"result":{"nodeId":"n1"}}`}, + "get_document_info": {`{"success":true,"result":{"nodeId":"n1","folderId":"target","workspaceId":"space"}}`}, }} if err := runDriveCoverage(t, Move, move, "--node", "n1", "--folder", "target", "--workspace", "space", "--yes"); err != nil { t.Fatal(err) } + for _, tc := range []struct { + name string + readback string + want string + }{ + {"missing folder", `{"success":true,"result":{"nodeId":"n1","workspaceId":"space"}}`, "缺少目标文件夹 ID"}, + {"wrong folder", `{"success":true,"result":{"nodeId":"n1","folderId":"other","workspaceId":"space"}}`, "与请求 \"target\" 不一致"}, + {"missing workspace", `{"success":true,"result":{"nodeId":"n1","folderId":"target"}}`, "缺少目标知识库 ID"}, + {"wrong workspace", `{"success":true,"result":{"nodeId":"n1","folderId":"target","workspaceId":"other"}}`, "与请求 \"space\" 不一致"}, + } { + t.Run(tc.name, func(t *testing.T) { + caller := &driveCoverageCaller{responses: map[string][]string{ + "move_document": {`{"success":true}`}, + "get_document_info": {tc.readback}, + }} + err := runDriveCoverage(t, Move, caller, "--node", "n1", "--folder", "target", "--workspace", "space", "--yes") + if err == nil || !strings.Contains(err.Error(), tc.want) { + t.Fatalf("error = %v, want %q", err, tc.want) + } + }) + } rename := &driveCoverageCaller{responses: map[string][]string{ "get_file_info": {`{"success":true,"result":{"fileId":"n1","type":"FILE","extension":"md","name":"old.md"}}`, `{"success":true,"result":{"fileId":"n1","name":"new.md"}}`}, From 98d03455b14ad6feeec012cdf81c796c2e928465 Mon Sep 17 00:00:00 2001 From: Dennis Date: Wed, 12 Aug 2026 15:31:27 +0800 Subject: [PATCH 2/9] fix(drive): bind readback to requested objects --- internal/shortcut/drive/catalog_operations.go | 9 +++++- internal/shortcut/drive/common.go | 10 +++++- internal/shortcut/drive/drive.go | 7 ++++ .../shortcut/drive/drive_shortcuts_test.go | 32 +++++++++++++++---- 4 files changed, 50 insertions(+), 8 deletions(-) diff --git a/internal/shortcut/drive/catalog_operations.go b/internal/shortcut/drive/catalog_operations.go index 394a1638..6cfb71dd 100644 --- a/internal/shortcut/drive/catalog_operations.go +++ b/internal/shortcut/drive/catalog_operations.go @@ -486,7 +486,14 @@ var Upload = shortcut.Shortcut{ if err != nil { return err } - if remoteName := firstString(verified, "name", "fileName"); remoteName == "" || !strings.HasPrefix(remoteName, strings.TrimSuffix(name, filepath.Ext(name))) { + remoteID := firstString(verified, "fileId", "dentryUuid", "nodeId", "id") + if remoteID == "" { + return driveResponseError("drive/commit_upload", "readback_missing_id", "上传后读回缺少文件 ID；无法证明读回的是已提交文件") + } + if remoteID != nodeID { + return driveResponseError("drive/commit_upload", "readback_id_mismatch", fmt.Sprintf("上传后读回文件 ID %q 与提交 ID %q 不一致", remoteID, nodeID)) + } + if remoteName := firstString(verified, "name", "fileName"); !driveReadbackNameMatches(verified, name) { return driveResponseError("drive/commit_upload", "readback_mismatch", fmt.Sprintf("上传后读回名称 %q 与请求 %q 不一致", remoteName, name)) } remoteSize, ok := firstInt64(verified, "fileSize", "size", "byteSize", "length") diff --git a/internal/shortcut/drive/common.go b/internal/shortcut/drive/common.go index d528b10f..94b7da41 100644 --- a/internal/shortcut/drive/common.go +++ b/internal/shortcut/drive/common.go @@ -218,6 +218,15 @@ func nestedString(data map[string]any, keys ...string) string { return "" } +func driveReadbackNameMatches(data map[string]any, requested string) bool { + remoteName := firstString(data, "name", "fileName") + if remoteName == requested { + return true + } + extension := strings.TrimLeft(firstString(data, "extension", "fileExtension", "ext"), ".") + return extension != "" && remoteName+"."+extension == requested +} + func firstInt64(data map[string]any, keys ...string) (int64, bool) { for _, key := range keys { value, present := data[key] @@ -246,7 +255,6 @@ func firstInt64(data map[string]any, keys ...string) (int64, bool) { return parsed, true } } - return 0, false } return 0, false } diff --git a/internal/shortcut/drive/drive.go b/internal/shortcut/drive/drive.go index 507f7aad..cbc04f77 100644 --- a/internal/shortcut/drive/drive.go +++ b/internal/shortcut/drive/drive.go @@ -547,6 +547,13 @@ var Move = shortcut.Shortcut{ if err != nil { return err } + remoteID := firstString(verified, "nodeId", "fileId", "dentryUuid", "id") + if remoteID == "" { + return driveResponseError("doc/move_document", "readback_missing_id", "移动后读回缺少节点 ID；无法证明读回的是已移动节点") + } + if remoteID != rt.Str("node") { + return driveResponseError("doc/move_document", "readback_id_mismatch", fmt.Sprintf("移动后读回节点 %q 与请求节点 %q 不一致", remoteID, rt.Str("node"))) + } if rt.Changed("folder") { remoteFolder := firstString(verified, "folderId", "targetFolderId", "parentId") if remoteFolder == "" { diff --git a/internal/shortcut/drive/drive_shortcuts_test.go b/internal/shortcut/drive/drive_shortcuts_test.go index 5ff95517..b383e074 100644 --- a/internal/shortcut/drive/drive_shortcuts_test.go +++ b/internal/shortcut/drive/drive_shortcuts_test.go @@ -178,16 +178,20 @@ func TestCrossPlatformCoverageDriveDownloadAndUploadRequireArtifactsAndReadback( t.Fatal("upload path escape was accepted") } for _, tc := range []struct { - name string - readback string - want string + name string + committedID string + readback string + want string }{ - {"missing remote size", `{"success":true,"result":{"fileId":"uploaded-2","name":"input.bin"}}`, "缺少有效文件大小"}, - {"mismatched remote size", `{"success":true,"result":{"fileId":"uploaded-3","name":"input.bin","fileSize":17}}`, "与本地文件大小 18 不一致"}, + {"missing remote id", "uploaded-2", `{"success":true,"result":{"name":"input.bin","fileSize":18}}`, "缺少文件 ID"}, + {"mismatched remote id", "uploaded-3", `{"success":true,"result":{"fileId":"other","name":"input.bin","fileSize":18}}`, "与提交 ID"}, + {"prefix-only remote name", "uploaded-4", `{"success":true,"result":{"fileId":"uploaded-4","name":"input.bin-old","fileSize":18}}`, "读回名称"}, + {"missing remote size", "uploaded-5", `{"success":true,"result":{"fileId":"uploaded-5","name":"input.bin"}}`, "缺少有效文件大小"}, + {"mismatched remote size", "uploaded-6", `{"success":true,"result":{"fileId":"uploaded-6","name":"input.bin","fileSize":17}}`, "与本地文件大小 18 不一致"}, } { t.Run(tc.name, func(t *testing.T) { testseam.Swap(t, &uploadDriveFile, func(context.Context, helpers.DriveUploadRequest) (map[string]any, error) { - return map[string]any{"success": true, "result": map[string]any{"fileId": strings.TrimSuffix(tc.name, " remote size")}}, nil + return map[string]any{"success": true, "result": map[string]any{"fileId": tc.committedID}}, nil }) caller := &driveCoverageCaller{responses: map[string][]string{"get_file_info": {tc.readback}}} err := runDriveCoverage(t, Upload, caller, "--file", "input.bin", "--yes") @@ -196,6 +200,17 @@ func TestCrossPlatformCoverageDriveDownloadAndUploadRequireArtifactsAndReadback( } }) } + t.Run("split remote extension", func(t *testing.T) { + testseam.Swap(t, &uploadDriveFile, func(context.Context, helpers.DriveUploadRequest) (map[string]any, error) { + return map[string]any{"success": true, "result": map[string]any{"fileId": "uploaded-7"}}, nil + }) + caller := &driveCoverageCaller{responses: map[string][]string{ + "get_file_info": {`{"success":true,"result":{"fileId":"uploaded-7","name":"input","extension":"bin","fileSize":18}}`}, + }} + if err := runDriveCoverage(t, Upload, caller, "--file", "input.bin", "--yes"); err != nil { + t.Fatal(err) + } + }) testseam.Swap(t, &driveDownload, func(_ context.Context, _ string, options localio.DownloadOptions) (localio.DownloadResult, error) { if options.Output != "downloads/file.bin" || options.Headers["x-token"] != "secret" { @@ -269,6 +284,9 @@ func TestCrossPlatformCoverageDriveFirstInt64(t *testing.T) { if got, ok := firstInt64(map[string]any{}, "size"); ok || got != 0 { t.Fatalf("missing firstInt64 = (%d, %t), want (0, false)", got, ok) } + if got, ok := firstInt64(map[string]any{"fileSize": nil, "size": "7"}, "fileSize", "size"); !ok || got != 7 { + t.Fatalf("fallback firstInt64 = (%d, %t), want (7, true)", got, ok) + } } func TestCrossPlatformCoverageDriveVersionAndPublishContracts(t *testing.T) { @@ -512,6 +530,8 @@ func TestCrossPlatformCoverageDriveCreateRestoreCopyMoveRename(t *testing.T) { readback string want string }{ + {"missing node id", `{"success":true,"result":{"folderId":"target","workspaceId":"space"}}`, "缺少节点 ID"}, + {"wrong node id", `{"success":true,"result":{"nodeId":"other","folderId":"target","workspaceId":"space"}}`, "与请求节点 \"n1\" 不一致"}, {"missing folder", `{"success":true,"result":{"nodeId":"n1","workspaceId":"space"}}`, "缺少目标文件夹 ID"}, {"wrong folder", `{"success":true,"result":{"nodeId":"n1","folderId":"other","workspaceId":"space"}}`, "与请求 \"target\" 不一致"}, {"missing workspace", `{"success":true,"result":{"nodeId":"n1","folderId":"target"}}`, "缺少目标知识库 ID"}, From 54dc8fadb71662118b47aea640114c88f658d878 Mon Sep 17 00:00:00 2001 From: wxianfeng Date: Wed, 12 Aug 2026 19:09:37 +0800 Subject: [PATCH 3/9] feat: forward agent version and extension context --- .changes/85384225-agent-version-ext.md | 8 + .env.example | 9 + docs/reference.md | 43 ++ internal/app/agent_host_test.go | 2 +- internal/app/agent_metadata.go | 248 +++++++ internal/app/agent_metadata_test.go | 743 +++++++++++++++++++ internal/app/agent_product_test.go | 2 +- internal/app/auth_registry.go | 25 +- internal/app/coverage_more_test.go | 15 +- internal/app/identity_export.go | 6 +- internal/app/root.go | 152 +++- internal/app/runner.go | 39 +- internal/auth/agent_metadata_headers_test.go | 80 ++ internal/logging/redact.go | 2 + internal/logging/redact_test.go | 29 +- internal/transport/agent_metadata_test.go | 93 +++ internal/transport/client.go | 41 +- internal/transport/redirect_test.go | 140 ++++ pkg/cli/identity.go | 5 +- pkg/cli/identity_test.go | 35 + 20 files changed, 1638 insertions(+), 79 deletions(-) create mode 100644 .changes/85384225-agent-version-ext.md create mode 100644 internal/app/agent_metadata.go create mode 100644 internal/app/agent_metadata_test.go create mode 100644 internal/auth/agent_metadata_headers_test.go create mode 100644 internal/transport/agent_metadata_test.go create mode 100644 internal/transport/redirect_test.go create mode 100644 pkg/cli/identity_test.go diff --git a/.changes/85384225-agent-version-ext.md b/.changes/85384225-agent-version-ext.md new file mode 100644 index 00000000..98649dfd --- /dev/null +++ b/.changes/85384225-agent-version-ext.md @@ -0,0 +1,8 @@ +--- +category: Added +--- + +- **Agent version and extended context passthrough** (Aone 85384225) — adds + validated `DWS_AGENT_VER` and sensitive JSON `DWS_AGENT_EXT` metadata to + ordinary non-plugin MCP requests without forwarding it to A2A, OAuth, + Discovery, or third-party plugins. diff --git a/.env.example b/.env.example index 0707e058..e5451497 100644 --- a/.env.example +++ b/.env.example @@ -19,3 +19,12 @@ # Cache directory (optional, defaults to ~/.dws/cache) # DWS_CACHE_DIR= + +# Agent integration metadata (optional; ordinary non-plugin MCP requests only) +# DWS_AGENT_PRODUCT=example-agent +# DWS_AGENT_HOST=cloud +# DWS_AGENT_VER=0.1.5 +# DWS_AGENT_EXT='{"umt":"example-redacted","miniwua":"example-redacted","ua":"ExampleAgent/0.1.5"}' +# The outer single quotes above are shell syntax and are not part of the value. +# DWS_AGENT_EXT is sensitive caller-declared JSON (max 8 KiB); never put real +# tokens in committed files or use this metadata alone for authentication. diff --git a/docs/reference.md b/docs/reference.md index 3290bb67..b8c7f015 100644 --- a/docs/reference.md +++ b/docs/reference.md @@ -7,6 +7,8 @@ | `DWS_CONFIG_DIR` | Override default config directory / 覆盖默认配置目录 | | `DWS_AGENT_PRODUCT` | Optional, caller-declared Agent product sent as `x-dws-agent-product` (for example `qwenwork`) for downstream logs/BI and used as the IM `clawType` display label when `--ai-tag` is enabled. `--ai-tag` defaults to `true`, so a configured Product changes the displayed label by default. With `--ai-tag=false`, native `chat message send` / `reply` calls send an empty `clawType`, while shortcut calls omit the argument. Surrounding ASCII spaces/tabs are trimmed; the remaining value must be at most 64 bytes and match `^[A-Za-z0-9][A-Za-z0-9_-]*$`. Unset or empty values omit the Header and use the edition's IM display default. This client never uses Product to change the separate HTTP `claw-type` PAT/routing label. / 可选、由调用方声明的 Agent 产品标识，经校验后作为 `x-dws-agent-product` 发送，并用于 IM 小尾巴；`--ai-tag` 默认为 `true`，因此配置 Product 后默认会改变展示标签。使用 `--ai-tag=false` 时，原生 `chat message send` / `reply` 发送空的 `clawType`，shortcut 调用则省略该参数。未设置时省略请求头且 IM 使用发行版默认值；本客户端不会用 Product 修改独立的 HTTP `claw-type` | | `DWS_AGENT_HOST` | Optional, caller-declared Agent runtime form sent as `x-dws-agent-host` (for example `cloud` or `desktop`) for downstream logs/BI. Surrounding ASCII spaces/tabs are trimmed; the remaining value must be at most 64 bytes and match `^[a-z0-9][a-z0-9_-]*$`; unset values are omitted. This client does not use Host for PAT, authentication, Discovery, or MCP endpoint selection. / 可选、由调用方声明的 Agent 运行形态，经校验后作为 `x-dws-agent-host` 发送给下游日志/BI；本客户端不使用该值进行 PAT、鉴权、Discovery 或 MCP 端点选择，未设置时省略 | +| `DWS_AGENT_VER` | Optional caller-declared Agent version / 可选、由调用方声明的 Agent 版本。After trimming surrounding ASCII spaces/tabs, the value must be at most 64 bytes and match `^[A-Za-z0-9][A-Za-z0-9._+-]*$`; a non-empty valid value is sent as `x-dws-agent-ver`, while unset or empty values omit the Header. / 去除首尾 ASCII 空格和 Tab 后，值不得超过 64 字节且必须匹配上述格式；合法非空值通过 `x-dws-agent-ver` 发送，未设置或空值则省略请求头 | +| `DWS_AGENT_EXT` | Optional caller-declared Agent extended context / 可选、由调用方声明的 Agent 扩展上下文。The value must be a UTF-8 JSON object no larger than 8 KiB, is compacted before being sent as the sensitive `x-dws-agent-ext` Header, and may use the recommended keys `umt`, `miniwua`, and `ua`; unknown keys remain supported. Unset or empty values omit the Header. / 值必须是 UTF-8 JSON 对象且不得超过 8 KiB，压缩后通过敏感请求头 `x-dws-agent-ext` 发送；推荐使用 `umt`、`miniwua`、`ua`，同时允许未知扩展键。未设置或空值则省略请求头 | | `DWS__MCP_URL` | Override a product MCP endpoint for local development / 本地开发时覆盖指定产品 MCP endpoint | | `DWS_CLIENT_ID` | OAuth client ID (DingTalk AppKey) | | `DWS_CLIENT_SECRET` | OAuth client secret (DingTalk AppSecret) | @@ -14,6 +16,47 @@ | `DWS_ALLOW_HTTP_ENDPOINTS` | Set `1` to allow HTTP for loopback during dev / 设为 `1` 允许回环地址 HTTP，仅用于开发调试 | | `DWS_DISABLE_KEYCHAIN` | macOS only. Set `1` to skip system Keychain for the encryption key and use file-based storage (same scheme as Linux). For sandboxed runtimes (e.g. Codex App) that block Keychain APIs. Weakens at-rest protection — DEK and ciphertext live in the same directory. / 仅 macOS。设为 `1` 时跳过系统 Keychain，密钥以文件形式存储（与 Linux 一致）。用于 Keychain API 被拦截的沙盒环境（如 Codex App）。代价是 DEK 与密文同目录，保护强度低于默认方案 | +### Agent Version and Extended Context / Agent 版本与扩展上下文 + +`DWS_AGENT_VER` and `DWS_AGENT_EXT` are sent only on the CLI's ordinary, +non-plugin MCP requests. They do not change the standard HTTP `User-Agent` or +the separate `X-Cli-Version` that identifies the DWS CLI version, and they are +not forwarded to A2A, OAuth, Discovery, or third-party plugin requests. + +`DWS_AGENT_EXT` is one JSON-object Header rather than a set of Headers. The +recommended keys are `umt`, `miniwua`, and `ua`, but the open-source CLI keeps +the object extensible and does not enforce a key allowlist. For example, using +fictional, redacted values: + +```bash +DWS_AGENT_VER=0.1.5 +DWS_AGENT_EXT='{"umt":"example-redacted","miniwua":"example-redacted","ua":"ExampleAgent/0.1.5"}' +``` + +The shell's outer single quotes group the JSON and are not part of the +environment-variable value. The CLI trims surrounding ASCII spaces/tabs, +omits either Header when its value is empty, and compacts EXT to a single-line +JSON object. A representative current payload is about 657 bytes, well below +the 8 KiB limit; integrations must still enforce the limit because values can +grow. EXT may contain sensitive device or runtime signals: the CLI masks it in +configuration and logs, and removes it on a cross-host redirect. + +Both values are declared by the caller and are therefore forgeable. They can +support compatibility checks, diagnostics, and observability, but they are not +credentials or attestations and must never be sufficient on their own to +authenticate a caller or authorize access. + +`DWS_AGENT_VER` 与 `DWS_AGENT_EXT` 仅随 CLI 发起的普通非插件 MCP 请求发送，不会 +改变标准 HTTP `User-Agent`，也不会覆盖标识 DWS CLI 自身版本的 `X-Cli-Version`； +二者不会进入 A2A、OAuth、Discovery 或第三方插件请求。EXT 使用单个 JSON 对象 +请求头，不拆成多个子请求头；推荐键为 `umt`、`miniwua`、`ua`，但开源 CLI 不限制 +扩展键。Shell 示例中的外层单引号只用于保护 JSON，不属于环境变量值。当前典型负载 +约为 657 字节，远低于 8 KiB 上限，但集成方仍须遵守大小限制。EXT 可能包含敏感的 +设备或运行时信号，配置展示和日志会对其脱敏，跨主机重定向时也会移除该请求头。 + +这两个值都由调用方自行声明，可以被伪造；它们可用于兼容性判断、诊断和可观测性， +但不是凭据或可信证明，不能单独用于身份认证或访问授权。 + ### Agent Product, Host, and `claw-type` / Agent 产品、运行形态与 `claw-type` `DWS_AGENT_PRODUCT` and `DWS_AGENT_HOST` are caller-declared observation diff --git a/internal/app/agent_host_test.go b/internal/app/agent_host_test.go index 38c5e6a0..82c22abd 100644 --- a/internal/app/agent_host_test.go +++ b/internal/app/agent_host_test.go @@ -165,7 +165,7 @@ func TestResolveIdentityHeadersOmitsAbsentOrInvalidAgentHost(t *testing.T) { } } -func TestRootRejectsInvalidAgentHostBeforeEditionHook(t *testing.T) { +func TestCrossPlatformCoverageRootRejectsInvalidAgentHostBeforeEditionHook(t *testing.T) { t.Setenv("DWS_CONFIG_DIR", t.TempDir()) const invalidValue = "DO_NOT_ECHO" t.Setenv(envDWSAgentHost, invalidValue) diff --git a/internal/app/agent_metadata.go b/internal/app/agent_metadata.go new file mode 100644 index 00000000..c8b7d74d --- /dev/null +++ b/internal/app/agent_metadata.go @@ -0,0 +1,248 @@ +// 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 app + +import ( + "bytes" + "context" + "encoding/json" + "os" + "regexp" + "strings" + "unicode" + "unicode/utf8" + + apperrors "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/errors" + "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/executor" + "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/transport" + "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/pkg/configmeta" +) + +const ( + envDWSAgentVersion = "DWS_AGENT_VER" + envDWSAgentExt = "DWS_AGENT_EXT" + maxAgentVersionBytes = 64 + maxAgentExtensionBytes = 8 * 1024 +) + +var agentVersionPattern = regexp.MustCompile(`^[A-Za-z0-9][A-Za-z0-9._+-]*$`) + +type agentMetadataSnapshot struct { + version string + ext string + versionErr error + extErr error +} + +type agentMetadataSnapshotContextKey struct{} + +func (snapshot agentMetadataSnapshot) validationError() error { + if snapshot.versionErr != nil { + return snapshot.versionErr + } + return snapshot.extErr +} + +func contextWithAgentMetadataSnapshot(ctx context.Context, snapshot agentMetadataSnapshot) context.Context { + return context.WithValue(ctx, agentMetadataSnapshotContextKey{}, snapshot) +} + +func agentMetadataSnapshotFromContext(ctx context.Context) (agentMetadataSnapshot, bool) { + if ctx == nil { + return agentMetadataSnapshot{}, false + } + snapshot, ok := ctx.Value(agentMetadataSnapshotContextKey{}).(agentMetadataSnapshot) + return snapshot, ok +} + +func init() { + configmeta.Register(configmeta.ConfigItem{ + Name: envDWSAgentVersion, + Category: configmeta.CategoryExternal, + Description: "调用 DWS 的 Agent 版本；仅作为 x-dws-agent-ver 透传到非插件 MCP 请求", + Example: "1.2.3-beta.1+build.7", + }) + configmeta.Register(configmeta.ConfigItem{ + Name: envDWSAgentExt, + Category: configmeta.CategoryExternal, + Description: "调用 DWS 的 Agent 扩展上下文 JSON；仅作为 x-dws-agent-ext 透传到非插件 MCP 请求", + Example: `{"umt":"","miniwua":"","ua":"agent/1.0"}`, + Sensitive: true, + }) +} + +// parseAgentVersion normalizes and validates the caller-declared Agent +// version. Only surrounding ASCII spaces and tabs are trimmed. An unset or +// ASCII-whitespace-only value means "do not emit". +func parseAgentVersion(raw string) (string, error) { + value := strings.Trim(raw, " \t") + if value == "" { + return "", nil + } + if len(value) > maxAgentVersionBytes || !agentVersionPattern.MatchString(value) { + return "", invalidAgentVersionError() + } + return value, nil +} + +// parseAgentExt validates one generic JSON object and returns its compact +// one-line representation. Raw control characters other than horizontal tab +// are rejected before JSON parsing; escaped JSON control characters remain +// valid because they are safe on the HTTP header wire. +func parseAgentExt(raw string) (string, error) { + if len(raw) > maxAgentExtensionBytes || !utf8.ValidString(raw) { + return "", invalidAgentExtError() + } + for _, r := range raw { + if unicode.IsControl(r) && r != '\t' { + return "", invalidAgentExtError() + } + } + + value := strings.Trim(raw, " \t") + if value == "" { + return "", nil + } + + var compact bytes.Buffer + if err := json.Compact(&compact, []byte(value)); err != nil { + return "", invalidAgentExtError() + } + compactBytes := compact.Bytes() + if len(compactBytes) > maxAgentExtensionBytes || len(compactBytes) < 2 || compactBytes[0] != '{' { + return "", invalidAgentExtError() + } + return compact.String(), nil +} + +func invalidAgentVersionError() error { + return apperrors.NewValidation( + "DWS_AGENT_VER must be at most 64 bytes and match ^[A-Za-z0-9][A-Za-z0-9._+-]*$", + apperrors.WithReason("invalid_agent_version"), + ) +} + +func invalidAgentExtError() error { + return apperrors.NewValidation( + "DWS_AGENT_EXT must be a UTF-8 JSON object of at most 8192 bytes without raw control characters", + apperrors.WithReason("invalid_agent_ext"), + ) +} + +// readAgentMetadataSnapshot reads both environment variables from one +// os.Environ snapshot, then parses them once. Normal CLI execution retains the +// validated result through the invocation so hooks and transport observe the +// same pair even in an embedding process that mutates its environment. +func readAgentMetadataSnapshot() agentMetadataSnapshot { + var rawVersion, rawExt string + for _, entry := range os.Environ() { + key, value, _ := strings.Cut(entry, "=") + switch key { + case envDWSAgentVersion: + rawVersion = value + case envDWSAgentExt: + rawExt = value + } + } + version, versionErr := parseAgentVersion(rawVersion) + ext, extErr := parseAgentExt(rawExt) + return agentMetadataSnapshot{ + version: version, + ext: ext, + versionErr: versionErr, + extErr: extErr, + } +} + +// removeAgentMetadataHeaders removes every case variant so edition or +// credential hooks cannot smuggle MCP-only metadata into shared transports. +func removeAgentMetadataHeaders(headers map[string]string) { + for key := range headers { + if strings.EqualFold(key, transport.HeaderAgentVersion) || + strings.EqualFold(key, transport.HeaderAgentExt) { + delete(headers, key) + } + } +} + +// applyAgentMetadataHeaders applies validated environment values as the final +// authority for non-plugin MCP requests. Invalid values are omitted on +// library paths that bypass root validation; normal CLI execution rejects +// them before hooks or network access. +func applyAgentMetadataHeaders(headers map[string]string) map[string]string { + return applyAgentMetadataSnapshot(headers, readAgentMetadataSnapshot()) +} + +func applyAgentMetadataSnapshot(headers map[string]string, snapshot agentMetadataSnapshot) map[string]string { + removeAgentMetadataHeaders(headers) + + if (snapshot.versionErr != nil || snapshot.version == "") && (snapshot.extErr != nil || snapshot.ext == "") { + return headers + } + if headers == nil { + headers = make(map[string]string) + } + if snapshot.versionErr == nil && snapshot.version != "" { + headers[transport.HeaderAgentVersion] = snapshot.version + } + if snapshot.extErr == nil && snapshot.ext != "" { + headers[transport.HeaderAgentExt] = snapshot.ext + } + return headers +} + +// resolveMCPRequestHeaders adds Agent version and extension metadata only to +// the built-in DingTalk MCP request path. Shared identity consumers (notably +// A2A) continue to use resolveIdentityHeaders and never receive these fields. +func resolveMCPRequestHeaders() map[string]string { + return resolveMCPRequestHeadersWithSnapshot(readAgentMetadataSnapshot()) +} + +func resolveMCPRequestHeadersWithSnapshot(snapshot agentMetadataSnapshot) map[string]string { + return applyAgentMetadataSnapshot(resolveIdentityHeaders(), snapshot) +} + +// resolveMCPRequestHeadersForInvocation resolves one immutable Header snapshot +// for an invocation. The helper-only mcp-meta server performs endpoint +// discovery rather than an ordinary MCP product call, so caller-declared +// Agent metadata must not cross that boundary. +func resolveMCPRequestHeadersForInvocation(invocation executor.Invocation, snapshots ...agentMetadataSnapshot) map[string]string { + headers := resolveIdentityHeaders() + if strings.EqualFold(strings.TrimSpace(invocation.CanonicalProduct), mcpMetaServerID) { + return headers + } + snapshot := readAgentMetadataSnapshot() + if len(snapshots) > 0 { + snapshot = snapshots[0] + } + return applyAgentMetadataSnapshot(headers, snapshot) +} + +// pluginRequestHeaders returns a private, sanitized copy of plugin-owned +// Headers. Third-party plugins never receive DWS-owned Agent metadata, even if +// their manifest tries to declare the reserved Header names itself. +func pluginRequestHeaders(pluginAuth *PluginAuth) map[string]string { + if pluginAuth == nil || len(pluginAuth.ExtraHeaders) == 0 { + return nil + } + headers := make(map[string]string, len(pluginAuth.ExtraHeaders)) + for key, value := range pluginAuth.ExtraHeaders { + headers[key] = value + } + removeAgentMetadataHeaders(headers) + if len(headers) == 0 { + return nil + } + return headers +} diff --git a/internal/app/agent_metadata_test.go b/internal/app/agent_metadata_test.go new file mode 100644 index 00000000..2bae8254 --- /dev/null +++ b/internal/app/agent_metadata_test.go @@ -0,0 +1,743 @@ +// 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 app + +import ( + "context" + "encoding/json" + "errors" + "io" + "maps" + "os" + "strings" + "testing" + + "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/audit" + apperrors "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/errors" + "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/executor" + outputpkg "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/output" + "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/pipeline" + "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/testseam" + "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/transport" + "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/pkg/agentproduct" + "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/pkg/configmeta" + "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/pkg/edition" + "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/pkg/mcptypes" + "github.com/spf13/cobra" +) + +func TestCrossPlatformCoverageParseAgentVersion(t *testing.T) { + var nilContext context.Context + if _, ok := agentMetadataSnapshotFromContext(nilContext); ok { + t.Fatal("nil context unexpectedly contained Agent metadata") + } + wantSnapshot := agentMetadataSnapshot{version: "context-version", ext: "{}"} + if got, ok := agentMetadataSnapshotFromContext(contextWithAgentMetadataSnapshot(context.Background(), wantSnapshot)); !ok || got != wantSnapshot { + t.Fatalf("context Agent metadata = %#v, %v; want %#v", got, ok, wantSnapshot) + } + + valid := []struct { + name string + raw string + want string + }{ + {name: "unset", raw: "", want: ""}, + {name: "ASCII whitespace only", raw: " \t ", want: ""}, + {name: "semantic version", raw: "1.2.3", want: "1.2.3"}, + {name: "pre-release and build", raw: " v1.2.3-rc.1+build_7 ", want: "v1.2.3-rc.1+build_7"}, + {name: "maximum length", raw: strings.Repeat("a", maxAgentVersionBytes), want: strings.Repeat("a", maxAgentVersionBytes)}, + } + for _, tc := range valid { + t.Run(tc.name, func(t *testing.T) { + got, err := parseAgentVersion(tc.raw) + if err != nil { + t.Fatalf("parseAgentVersion() error = %v", err) + } + if got != tc.want { + t.Fatalf("parseAgentVersion() = %q, want %q", got, tc.want) + } + }) + } + + invalid := []struct { + name string + raw string + }{ + {name: "leading punctuation", raw: "-1.2.3"}, + {name: "internal space", raw: "1.2 3"}, + {name: "slash", raw: "1.2/3"}, + {name: "line feed", raw: "1.2.3\n"}, + {name: "carriage return", raw: "1.2.3\r"}, + {name: "NUL", raw: "1.2\x003"}, + {name: "Unicode", raw: "版本1"}, + {name: "too long", raw: strings.Repeat("a", maxAgentVersionBytes+1)}, + } + for _, tc := range invalid { + t.Run(tc.name, func(t *testing.T) { + got, err := parseAgentVersion(tc.raw) + if err == nil || got != "" { + t.Fatalf("parseAgentVersion(%q) = %q, %v; want validation error", tc.raw, got, err) + } + assertAgentMetadataValidationError(t, err, "invalid_agent_version", tc.raw) + }) + } +} + +func TestCrossPlatformCoverageParseAgentExt(t *testing.T) { + boundary := `{"x":"` + strings.Repeat("a", maxAgentExtensionBytes-8) + `"}` + if len(boundary) != maxAgentExtensionBytes { + t.Fatalf("invalid boundary fixture size: %d", len(boundary)) + } + + valid := []struct { + name string + raw string + want string + }{ + {name: "unset", raw: "", want: ""}, + {name: "ASCII whitespace only", raw: " \t ", want: ""}, + {name: "empty object", raw: "{}", want: "{}"}, + {name: "compact generic object", raw: " \t{ \"umt\": \"masked\",\t \"nested\": { \"ok\": true }, \"unknown\": [1, 2] }\t ", want: `{"umt":"masked","nested":{"ok":true},"unknown":[1,2]}`}, + {name: "Unicode value", raw: `{"ua":"千问办公/1.0"}`, want: `{"ua":"千问办公/1.0"}`}, + {name: "escaped control remains safe", raw: `{"ua":"line\nnext"}`, want: `{"ua":"line\nnext"}`}, + {name: "maximum length", raw: boundary, want: boundary}, + } + for _, tc := range valid { + t.Run(tc.name, func(t *testing.T) { + got, err := parseAgentExt(tc.raw) + if err != nil { + t.Fatalf("parseAgentExt() error = %v", err) + } + if got != tc.want { + t.Fatalf("parseAgentExt() = %q, want %q", got, tc.want) + } + }) + } + + invalidUTF8 := string([]byte{'{', '"', 'x', '"', ':', '"', 0xff, '"', '}'}) + invalid := []struct { + name string + raw string + }{ + {name: "too long raw input", raw: strings.Repeat(" ", maxAgentExtensionBytes+1)}, + {name: "invalid UTF-8", raw: invalidUTF8}, + {name: "array", raw: `[]`}, + {name: "string", raw: `"value"`}, + {name: "number", raw: `1`}, + {name: "boolean", raw: `true`}, + {name: "null", raw: `null`}, + {name: "malformed object", raw: `{"secret":"DO_NOT_ECHO"`}, + {name: "trailing value", raw: `{} {}`}, + {name: "line feed", raw: "{\n}"}, + {name: "carriage return", raw: "{\r}"}, + {name: "NUL", raw: "{\x00}"}, + {name: "vertical tab", raw: "{\v}"}, + {name: "form feed", raw: "{\f}"}, + {name: "DEL", raw: "{\x7f}"}, + {name: "C1 control", raw: "{\u0085}"}, + } + for _, tc := range invalid { + t.Run(tc.name, func(t *testing.T) { + got, err := parseAgentExt(tc.raw) + if err == nil || got != "" { + t.Fatalf("parseAgentExt() = %q, %v; want validation error", got, err) + } + assertAgentMetadataValidationError(t, err, "invalid_agent_ext", tc.raw) + }) + } +} + +func assertAgentMetadataValidationError(t *testing.T, err error, reason, raw string) { + t.Helper() + var appErr *apperrors.Error + if !errors.As(err, &appErr) { + t.Fatalf("error type = %T, want *errors.Error", err) + } + if appErr.Category != apperrors.CategoryValidation || appErr.Reason != reason { + t.Fatalf("error = category %q reason %q, want validation/%s", appErr.Category, appErr.Reason, reason) + } + if strings.Contains(raw, "DO_NOT_ECHO") && strings.Contains(err.Error(), "DO_NOT_ECHO") { + t.Fatalf("error must not echo invalid value: %v", err) + } +} + +func TestCrossPlatformCoverageAgentMetadataConfigRegistrationAndMasking(t *testing.T) { + items := configmeta.All() + var versionItem, extItem *configmeta.ConfigItem + for i := range items { + switch items[i].Name { + case envDWSAgentVersion: + versionItem = &items[i] + case envDWSAgentExt: + extItem = &items[i] + } + } + if versionItem == nil || extItem == nil { + t.Fatalf("Agent metadata config registration missing: version=%v ext=%v", versionItem != nil, extItem != nil) + } + if versionItem.Category != configmeta.CategoryExternal || versionItem.Sensitive { + t.Fatalf("version config metadata = %#v", *versionItem) + } + if extItem.Category != configmeta.CategoryExternal || !extItem.Sensitive { + t.Fatalf("extension config metadata = %#v", *extItem) + } + + const canary = `{"umt":"SENSITIVE_CANARY"}` + t.Setenv(envDWSAgentExt, canary) + got, ok := configmeta.Resolve(envDWSAgentExt) + if !ok || got == "" || strings.Contains(got, "SENSITIVE_CANARY") || got == canary { + t.Fatalf("sensitive extension was not masked: value=%q ok=%v", got, ok) + } + + t.Setenv(envDWSAgentVersion, "9.8.7") + command := newConfigListCommand() + var output strings.Builder + command.SetOut(&output) + command.SetArgs([]string{"--category", string(configmeta.CategoryExternal), "--show-values", "--json"}) + if err := command.Execute(); err != nil { + t.Fatalf("config list failed: %v", err) + } + rawOutput := output.String() + if !json.Valid([]byte(rawOutput)) { + t.Fatalf("config list emitted invalid JSON: %q", rawOutput) + } + if !strings.Contains(rawOutput, envDWSAgentVersion) || !strings.Contains(rawOutput, envDWSAgentExt) { + t.Fatalf("config list omitted Agent metadata variables: %s", rawOutput) + } + if strings.Contains(rawOutput, "SENSITIVE_CANARY") || strings.Contains(rawOutput, canary) { + t.Fatalf("config list leaked Agent extension: %s", rawOutput) + } +} + +func TestCrossPlatformCoverageResolveMCPRequestHeadersScopesAndFinalizesAgentMetadata(t *testing.T) { + t.Setenv("DWS_CONFIG_DIR", t.TempDir()) + t.Setenv(envDWSAgentHost, "") + t.Setenv(agentproduct.EnvName, "") + t.Setenv(envDWSAgentVersion, " 1.2.3-rc.1 ") + t.Setenv(envDWSAgentExt, " { \"umt\": \"masked\", \"unknown\": true } ") + + oldEdition := edition.Get() + t.Cleanup(func() { edition.Override(oldEdition) }) + edition.Override(&edition.Hooks{ + MergeHeaders: func(headers map[string]string) map[string]string { + headers["X-Dws-Agent-Ver"] = "merge-must-not-win" + headers["X-Dws-Agent-Ext"] = `{"source":"merge"}` + return headers + }, + EnterpriseCredentialHeaders: func(headers map[string]string) map[string]string { + headers[transport.HeaderAgentVersion] = "credential-must-not-win" + headers[transport.HeaderAgentExt] = `{"source":"credential"}` + return headers + }, + }) + + for name, headers := range map[string]map[string]string{ + "shared identity": resolveIdentityHeaders(), + "A2A export": MCPIdentityHeaders(), + } { + if hasHeaderFold(headers, transport.HeaderAgentVersion) || hasHeaderFold(headers, transport.HeaderAgentExt) { + t.Fatalf("%s leaked MCP-only metadata: %#v", name, headers) + } + } + + headers := resolveMCPRequestHeaders() + if got := headers[transport.HeaderAgentVersion]; got != "1.2.3-rc.1" { + t.Fatalf("%s = %q, want 1.2.3-rc.1", transport.HeaderAgentVersion, got) + } + if got := headers[transport.HeaderAgentExt]; got != `{"umt":"masked","unknown":true}` { + t.Fatalf("%s = %q", transport.HeaderAgentExt, got) + } + if got := headers[transport.HeaderVersion]; got != version { + t.Fatalf("%s = %q, want CLI version %q", transport.HeaderVersion, got, version) + } + if _, ok := headers["User-Agent"]; ok { + t.Fatal("Agent extension must not create or replace the standard User-Agent header") + } + for _, key := range []string{"umt", "miniwua", "ua", "x-dws-agent-umt", "x-dws-agent-miniwua", "x-dws-agent-ua"} { + if hasHeaderFold(headers, key) { + t.Fatalf("Agent extension was split into an extra header %q: %#v", key, headers) + } + } + + // Library paths are best-effort: one invalid value is omitted without + // suppressing the other valid field or preserving hook-injected values. + t.Setenv(envDWSAgentExt, `{"secret":"DO_NOT_ECHO"`) + headers = resolveMCPRequestHeaders() + if got := headers[transport.HeaderAgentVersion]; got != "1.2.3-rc.1" { + t.Fatalf("valid version was suppressed: %q", got) + } + if hasHeaderFold(headers, transport.HeaderAgentExt) { + t.Fatalf("invalid extension or hook value leaked: %#v", headers) + } + + // Exercise the nil-map and empty-input library paths. An absent environment + // must not allocate a map, while an EXT-only value must allocate one and + // remain a single compact Header. + t.Setenv(envDWSAgentVersion, "") + t.Setenv(envDWSAgentExt, "") + if got := applyAgentMetadataHeaders(nil); got != nil { + t.Fatalf("empty metadata allocated headers: %#v", got) + } + t.Setenv(envDWSAgentExt, " { } ") + headers = applyAgentMetadataHeaders(nil) + if got := headers[transport.HeaderAgentExt]; got != "{}" { + t.Fatalf("EXT-only metadata = %q, want {}", got) + } +} + +func TestCrossPlatformCoverageRootRejectsInvalidAgentMetadataBeforeEditionHook(t *testing.T) { + oldEdition := edition.Get() + t.Cleanup(func() { edition.Override(oldEdition) }) + + tests := []struct { + name string + env string + value string + reason string + }{ + {name: "version", env: envDWSAgentVersion, value: "DO_NOT ECHO", reason: "invalid_agent_version"}, + {name: "extension", env: envDWSAgentExt, value: `{"secret":"DO_NOT_ECHO"`, reason: "invalid_agent_ext"}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Setenv("DWS_CONFIG_DIR", t.TempDir()) + t.Setenv(envDWSAgentHost, "") + t.Setenv(agentproduct.EnvName, "") + t.Setenv(envDWSAgentVersion, "") + t.Setenv(envDWSAgentExt, "") + t.Setenv(tc.env, tc.value) + + headerHookCalled := false + afterHookCalled := false + edition.Override(&edition.Hooks{ + MergeHeaders: func(headers map[string]string) map[string]string { + headerHookCalled = true + return headers + }, + EnterpriseCredentialHeaders: func(headers map[string]string) map[string]string { + headerHookCalled = true + return headers + }, + AfterPersistentPreRun: func(_ *cobra.Command, _ []string) error { + afterHookCalled = true + return nil + }, + }) + + root := NewRootCommand() + root.SetOut(io.Discard) + root.SetErr(io.Discard) + root.SetArgs([]string{"version"}) + err := root.Execute() + if err == nil { + t.Fatalf("root command accepted invalid %s", tc.env) + } + if headerHookCalled || afterHookCalled { + t.Fatalf("edition hook ran before %s validation", tc.env) + } + assertAgentMetadataValidationError(t, err, tc.reason, tc.value) + }) + } +} + +func TestCrossPlatformCoverageAgentMetadataProcessEntryValidationPrecedesRootConstruction(t *testing.T) { + for _, tc := range []struct { + name string + args []string + want bool + }{ + {name: "default JSON", args: []string{"version"}, want: true}, + {name: "long JSON", args: []string{"version", "--format", "JSON"}, want: true}, + {name: "long table", args: []string{"--format=table", "version"}, want: false}, + {name: "short attached JSON", args: []string{"version", "-fjson"}, want: true}, + {name: "short table", args: []string{"version", "-f", "table"}, want: false}, + {name: "last wins", args: []string{"--format", "table", "version", "-f=json"}, want: true}, + {name: "terminator", args: []string{"version", "--format", "table", "--", "--format", "json"}, want: false}, + {name: "missing value", args: []string{"version", "--format"}, want: false}, + } { + t.Run("presentation/"+tc.name, func(t *testing.T) { + if got := processArgsRequestJSON(tc.args); got != tc.want { + t.Fatalf("processArgsRequestJSON(%q) = %v, want %v", tc.args, got, tc.want) + } + }) + } + + t.Setenv("DWS_CONFIG_DIR", t.TempDir()) + t.Setenv(envDWSAgentHost, "") + t.Setenv(agentproduct.EnvName, "") + t.Setenv(envDWSAgentVersion, "") + sensitiveRaw := "{\"umt\":\"must-not-leak\"}\n" + t.Setenv(envDWSAgentExt, sensitiveRaw) + oldEdition := edition.Get() + t.Cleanup(func() { edition.Override(oldEdition) }) + extensionHookCalls := 0 + edition.Override(&edition.Hooks{ + Name: "presentation-test", + RegisterExtraCommands: func(*cobra.Command, edition.ToolCaller) { + extensionHookCalls++ + }, + VisibleProducts: func() []string { + extensionHookCalls++ + return nil + }, + StaticServers: func() []edition.ServerInfo { + extensionHookCalls++ + return nil + }, + }) + + oldArgs := os.Args + os.Args = []string{"dws", "version"} + t.Cleanup(func() { os.Args = oldArgs }) + + rootConstructed := false + preParseCalled := false + testseam.Swap(t, &rootNewRootCommandWithEngine, func(context.Context, *pipeline.Engine) *cobra.Command { + rootConstructed = true + return &cobra.Command{Use: "dws"} + }) + testseam.Swap(t, &rootRunPreParse, func(*cobra.Command, *pipeline.Engine) error { + preParseCalled = true + return nil + }) + + stderrFile, err := os.CreateTemp(t.TempDir(), "agent-metadata-stderr-*") + if err != nil { + t.Fatalf("create stderr capture: %v", err) + } + oldStderr := os.Stderr + os.Stderr = stderrFile + t.Cleanup(func() { + os.Stderr = oldStderr + _ = stderrFile.Close() + }) + + if code := Execute(); code == 0 { + t.Fatal("process entry accepted invalid Agent metadata") + } + if rootConstructed || preParseCalled { + t.Fatalf("invalid Agent metadata reached root hooks: constructed=%v preParse=%v", rootConstructed, preParseCalled) + } + if extensionHookCalls != 0 { + t.Fatalf("invalid Agent metadata executed %d extension hooks", extensionHookCalls) + } + if err := stderrFile.Sync(); err != nil { + t.Fatalf("sync stderr capture: %v", err) + } + stderrOutput, err := os.ReadFile(stderrFile.Name()) + if err != nil { + t.Fatalf("read stderr capture: %v", err) + } + if strings.Contains(string(stderrOutput), "must-not-leak") || strings.Contains(string(stderrOutput), sensitiveRaw) { + t.Fatalf("process validation error leaked raw EXT: %q", stderrOutput) + } + if !json.Valid(stderrOutput) || !strings.Contains(string(stderrOutput), `"reason": "invalid_agent_ext"`) { + t.Fatalf("default JSON error presentation = %q", stderrOutput) + } + + stdoutFile, err := os.CreateTemp(t.TempDir(), "agent-metadata-stdout-*") + if err != nil { + t.Fatalf("create stdout capture: %v", err) + } + oldStdout := os.Stdout + os.Stdout = stdoutFile + t.Cleanup(func() { + os.Stdout = oldStdout + _ = stdoutFile.Close() + }) + emitEarlyAgentMetadataValidationError(invalidAgentExtError(), []string{"drive", "+list", "--format", "json"}) + if err := stdoutFile.Sync(); err != nil { + t.Fatalf("sync stdout capture: %v", err) + } + unifiedOutput, err := os.ReadFile(stdoutFile.Name()) + if err != nil { + t.Fatalf("read stdout capture: %v", err) + } + if !json.Valid(unifiedOutput) || !strings.Contains(string(unifiedOutput), `"outcome": "failure"`) || + !strings.Contains(string(unifiedOutput), `"subtype": "invalid_agent_ext"`) { + t.Fatalf("unified JSON error presentation = %q", unifiedOutput) + } + if extensionHookCalls != 0 { + t.Fatalf("presentation-only root executed %d extension hooks", extensionHookCalls) + } + + if err := stderrFile.Truncate(0); err != nil { + t.Fatalf("truncate fallback stderr capture: %v", err) + } + if _, err := stderrFile.Seek(0, io.SeekStart); err != nil { + t.Fatalf("rewind fallback stderr capture: %v", err) + } + testseam.Swap(t, &rootEmitResult, func(*cobra.Command, outputpkg.CommandResult) (int, error) { + return 0, errors.New("injected result emission failure") + }) + emitEarlyAgentMetadataValidationError(invalidAgentExtError(), []string{"drive", "+list", "--format", "json"}) + if err := stderrFile.Sync(); err != nil { + t.Fatalf("sync fallback stderr capture: %v", err) + } + fallbackOutput, err := os.ReadFile(stderrFile.Name()) + if err != nil { + t.Fatalf("read fallback stderr capture: %v", err) + } + if !json.Valid(fallbackOutput) || !strings.Contains(string(fallbackOutput), `"reason": "invalid_agent_ext"`) || + strings.Contains(string(fallbackOutput), "must-not-leak") { + t.Fatalf("fallback validation error presentation = %q", fallbackOutput) + } + + if err := stderrFile.Truncate(0); err != nil { + t.Fatalf("truncate stderr capture: %v", err) + } + if _, err := stderrFile.Seek(0, io.SeekStart); err != nil { + t.Fatalf("rewind stderr capture: %v", err) + } + emitEarlyAgentMetadataValidationError(invalidAgentExtError(), []string{"version", "--format", "table"}) + if err := stderrFile.Sync(); err != nil { + t.Fatalf("sync human stderr capture: %v", err) + } + humanOutput, err := os.ReadFile(stderrFile.Name()) + if err != nil { + t.Fatalf("read human stderr capture: %v", err) + } + if json.Valid(humanOutput) || !strings.Contains(string(humanOutput), "DWS_AGENT_EXT") || + strings.Contains(string(humanOutput), "must-not-leak") { + t.Fatalf("human validation error presentation = %q", humanOutput) + } + + var capturedRunner *runtimeRunner + testseam.Swap(t, &rootNewCommandRunnerWithFlags, func(flags *GlobalFlags) executor.Runner { + capturedRunner = newCommandRunnerWithFlags(flags).(*runtimeRunner) + return capturedRunner + }) + cachedSnapshot := agentMetadataSnapshot{version: "9.8.7", ext: `{"ua":"cached"}`} + _ = newRootCommandWithMode( + contextWithAgentMetadataSnapshot(context.Background(), cachedSnapshot), + nil, + false, + true, + true, + ) + if capturedRunner == nil || capturedRunner.agentMetadata == nil || *capturedRunner.agentMetadata != cachedSnapshot { + t.Fatalf("root runner Agent metadata = %#v, want %#v", capturedRunner, cachedSnapshot) + } +} + +func TestCrossPlatformCoverageAgentMetadataExcludedFromServiceDiscovery(t *testing.T) { + t.Setenv("DWS_CONFIG_DIR", t.TempDir()) + t.Setenv(envDWSAgentHost, "") + t.Setenv(agentproduct.EnvName, "") + t.Setenv(envDWSAgentVersion, "3.0.0") + t.Setenv(envDWSAgentExt, `{"umt":"test-value"}`) + + headers := resolveMCPRequestHeadersForInvocation(executor.Invocation{ + CanonicalProduct: mcpMetaServerID, + Tool: mcpMetaURLTool, + }) + if hasHeaderFold(headers, transport.HeaderAgentVersion) || hasHeaderFold(headers, transport.HeaderAgentExt) { + t.Fatalf("service-discovery request leaked Agent metadata: %#v", headers) + } + + headers = resolveMCPRequestHeadersForInvocation(executor.Invocation{CanonicalProduct: "doc", Tool: "read"}) + if headers[transport.HeaderAgentVersion] != "3.0.0" || headers[transport.HeaderAgentExt] == "" { + t.Fatalf("ordinary MCP request omitted Agent metadata: %#v", headers) + } + cached := agentMetadataSnapshot{version: "3.1.0", ext: "{}"} + headers = resolveMCPRequestHeadersForInvocation(executor.Invocation{CanonicalProduct: "doc", Tool: "read"}, cached) + if headers[transport.HeaderAgentVersion] != "3.1.0" || headers[transport.HeaderAgentExt] != "{}" { + t.Fatalf("ordinary MCP request ignored its validated snapshot: %#v", headers) + } +} + +func TestCrossPlatformCoverageAgentMetadataMCPAndPluginScoping(t *testing.T) { + t.Setenv("DWS_CONFIG_DIR", t.TempDir()) + t.Setenv(envDWSAgentHost, "") + t.Setenv(agentproduct.EnvName, "") + t.Setenv(envDWSAgentVersion, "2.0.0") + t.Setenv(envDWSAgentExt, `{"ua":"test-agent/2.0"}`) + + oldEdition := edition.Get() + t.Cleanup(func() { edition.Override(oldEdition) }) + edition.Override(&edition.Hooks{}) + + pluginAuthMu.Lock() + oldPluginRegistry := pluginAuthRegistry + pluginAuthRegistry = make(map[string]*PluginAuth) + pluginAuthMu.Unlock() + t.Cleanup(func() { + pluginAuthMu.Lock() + pluginAuthRegistry = oldPluginRegistry + pluginAuthMu.Unlock() + }) + dynamicMu.Lock() + oldDynamicEndpoints := dynamicEndpoints + oldDynamicProducts := dynamicProducts + oldDynamicAliases := dynamicAliases + oldDynamicToolEndpoints := dynamicToolEndpoints + dynamicEndpoints = nil + dynamicProducts = nil + dynamicAliases = nil + dynamicToolEndpoints = nil + dynamicMu.Unlock() + t.Cleanup(func() { + dynamicMu.Lock() + dynamicEndpoints = oldDynamicEndpoints + dynamicProducts = oldDynamicProducts + dynamicAliases = oldDynamicAliases + dynamicToolEndpoints = oldDynamicToolEndpoints + dynamicMu.Unlock() + }) + + testseam.Swap(t, &runnerPreflightDocDownload, func(*runtimeRunner, context.Context, *transport.Client, string, executor.Invocation) error { + return nil + }) + type capturedRequest struct { + headers map[string]string + token string + } + var captured []capturedRequest + testseam.Swap(t, &runnerCallTool, func(client *transport.Client, _ context.Context, _, _ string, _ map[string]any) (transport.ToolCallResult, error) { + copyHeaders := make(map[string]string, len(client.ExtraHeaders)) + for key, value := range client.ExtraHeaders { + copyHeaders[key] = value + } + captured = append(captured, capturedRequest{headers: copyHeaders, token: client.AuthToken}) + return transport.ToolCallResult{Content: map[string]any{"value": "ok"}}, nil + }) + + created := newCommandRunnerWithFlags(&GlobalFlags{}).(*runtimeRunner) + if hasHeaderFold(created.transport.ExtraHeaders, transport.HeaderAgentVersion) || + hasHeaderFold(created.transport.ExtraHeaders, transport.HeaderAgentExt) { + t.Fatalf("new runner resolved Agent metadata before invocation validation: %#v", created.transport.ExtraHeaders) + } + + // runSingle must not cache ambient MCP metadata on the shared base transport. + // Use mock mode to exercise the path without authentication or network I/O. + t.Setenv(envDWSAgentVersion, "2.0.1") + refreshRunner := &runtimeRunner{ + transport: transport.NewClient(nil), + globalFlags: &GlobalFlags{Mock: true}, + auditSink: audit.NopSink{}, + } + refreshInvocation := executor.Invocation{CanonicalProduct: "refresh", Tool: "tool", Params: map[string]any{}} + if _, err := refreshRunner.runSingle(context.Background(), refreshInvocation, false); err != nil { + t.Fatalf("mock runSingle failed: %v", err) + } + if hasHeaderFold(refreshRunner.transport.ExtraHeaders, transport.HeaderAgentVersion) { + t.Fatalf("runSingle mutated the shared transport Header map: %#v", refreshRunner.transport.ExtraHeaders) + } + t.Setenv(envDWSAgentVersion, "2.0.0") + + r := &runtimeRunner{ + transport: transport.NewClient(nil), + globalFlags: &GlobalFlags{Token: "test-token"}, + auditSink: audit.NopSink{}, + agentMetadata: &agentMetadataSnapshot{ + version: "2.0.0", + ext: `{"ua":"test-agent/2.0"}`, + }, + } + builtIn := executor.Invocation{CanonicalProduct: "built-in", Tool: "tool", Params: map[string]any{}} + if _, err := r.executeInvocation(context.Background(), "https://example.test", builtIn); err != nil { + t.Fatalf("built-in invocation failed: %v", err) + } + + pluginDescriptor := mcptypes.ServerDescriptor{ + Key: "third-party", + Endpoint: "https://plugin.example.test", + CLI: mcptypes.CLIOverlay{ID: "third-party"}, + AuthHeaders: map[string]string{ + "X-Plugin": "yes", + "X-Dws-Agent-Ver": "plugin-must-not-forge-version", + "X-Dws-Agent-Ext": `{"source":"plugin"}`, + }, + } + registerPluginHTTPServer(pluginDescriptor) + registeredPlugin, pluginOwned := LookupPluginAuth("third-party") + if !pluginOwned || registeredPlugin == nil || registeredPlugin.Token != "" { + t.Fatalf("anonymous HTTP plugin ownership = %#v, %v", registeredPlugin, pluginOwned) + } + registerPluginHTTPServer(mcptypes.ServerDescriptor{ + Key: "anonymous-empty", + Endpoint: "https://anonymous.example.test", + CLI: mcptypes.CLIOverlay{ID: "anonymous-empty"}, + }) + if emptyPlugin, owned := LookupPluginAuth("anonymous-empty"); !owned || emptyPlugin == nil || emptyPlugin.Token != "" || len(emptyPlugin.ExtraHeaders) != 0 { + t.Fatalf("headerless HTTP plugin ownership = %#v, %v", emptyPlugin, owned) + } + originalPluginHeaders := maps.Clone(registeredPlugin.ExtraHeaders) + pluginInvocation := executor.Invocation{CanonicalProduct: "third-party", Tool: "tool", Params: map[string]any{}} + if _, err := r.executeInvocation(context.Background(), "https://plugin.example.test", pluginInvocation); err != nil { + t.Fatalf("plugin invocation failed: %v", err) + } + + if len(captured) != 2 { + t.Fatalf("captured %d calls, want 2", len(captured)) + } + if captured[0].headers[transport.HeaderAgentVersion] != "2.0.0" || captured[0].headers[transport.HeaderAgentExt] != `{"ua":"test-agent/2.0"}` { + t.Fatalf("built-in MCP metadata = %#v", captured[0].headers) + } + if hasHeaderFold(captured[1].headers, transport.HeaderAgentVersion) || hasHeaderFold(captured[1].headers, transport.HeaderAgentExt) { + t.Fatalf("plugin request leaked Agent metadata: %#v", captured[1].headers) + } + if got := captured[1].headers["X-Plugin"]; got != "yes" { + t.Fatalf("plugin-owned header = %q, want yes", got) + } + if captured[1].token != "" { + t.Fatalf("anonymous plugin unexpectedly received default OAuth token") + } + if !maps.Equal(registeredPlugin.ExtraHeaders, originalPluginHeaders) { + t.Fatalf("plugin Header sanitization mutated registry state: got %#v want %#v", registeredPlugin.ExtraHeaders, originalPluginHeaders) + } + if got := pluginRequestHeaders(nil); got != nil { + t.Fatalf("nil plugin auth produced Headers: %#v", got) + } + if got := pluginRequestHeaders(&PluginAuth{ExtraHeaders: map[string]string{ + "X-DWS-AGENT-VER": "forged", + "X-DWS-AGENT-EXT": `{"forged":true}`, + }}); got != nil { + t.Fatalf("reserved-only plugin Headers survived sanitization: %#v", got) + } + + // Keep the execution-boundary auth guard independently testable: even if a + // future token provider returns an empty token without an error, built-in MCP + // calls must fail before preflight or transport while anonymous plugins remain + // valid above. + resolveCalled := false + testseam.Swap(t, &runnerResolveAuthToken, func(*runtimeRunner, context.Context) (string, error) { + resolveCalled = true + return "", nil + }) + callsBefore := len(captured) + unauthenticated := &runtimeRunner{ + transport: transport.NewClient(nil), + globalFlags: &GlobalFlags{}, + auditSink: audit.NopSink{}, + } + if _, err := unauthenticated.executeInvocation(context.Background(), "https://example.test", executor.Invocation{CanonicalProduct: "built-in-unauthenticated", Tool: "tool"}); err == nil || !isAuthError(err) { + t.Fatalf("unauthenticated built-in request = %v, want auth error", err) + } + if !resolveCalled { + t.Fatal("unauthenticated request did not exercise the token resolver") + } + if len(captured) != callsBefore { + t.Fatalf("unauthenticated built-in request reached transport: calls %d -> %d", callsBefore, len(captured)) + } +} + +func hasHeaderFold(headers map[string]string, want string) bool { + for key := range headers { + if strings.EqualFold(key, want) { + return true + } + } + return false +} diff --git a/internal/app/agent_product_test.go b/internal/app/agent_product_test.go index 2cd8219e..8cc0c92b 100644 --- a/internal/app/agent_product_test.go +++ b/internal/app/agent_product_test.go @@ -158,7 +158,7 @@ func TestApplyAgentProductHeader(t *testing.T) { } } -func TestRootRejectsInvalidAgentProductBeforeEditionHook(t *testing.T) { +func TestCrossPlatformCoverageRootRejectsInvalidAgentProductBeforeEditionHook(t *testing.T) { t.Setenv("DWS_CONFIG_DIR", t.TempDir()) const invalidValue = "DO_NOT ECHO" t.Setenv(agentproduct.EnvName, invalidValue) diff --git a/internal/app/auth_registry.go b/internal/app/auth_registry.go index 1083bd5b..60c55be6 100644 --- a/internal/app/auth_registry.go +++ b/internal/app/auth_registry.go @@ -15,11 +15,10 @@ package app import "sync" -// PluginAuth holds authentication credentials for a plugin-owned -// streamable-http MCP server. Each server is keyed by its canonical -// product ID (CLI.ID) so that different servers can use independent -// tokens without interfering with each other or with the default -// DingTalk OAuth token. +// PluginAuth marks ownership of a plugin-owned streamable-http MCP server and +// holds its optional authentication credentials. Every accepted HTTP plugin, +// including an anonymous one, has a non-nil record keyed by canonical product +// ID (CLI.ID) so execution never falls back to built-in DingTalk OAuth. type PluginAuth struct { // Token is the Bearer token extracted from the plugin's // "Authorization" header (e.g. a third-party API key). @@ -39,27 +38,25 @@ var ( pluginAuthRegistry = make(map[string]*PluginAuth) ) -// RegisterPluginAuth stores authentication credentials for a plugin -// server keyed by its canonical product ID. The runner looks up these -// credentials at execution time to inject the correct Bearer token -// instead of the default DingTalk OAuth token. +// RegisterPluginAuth stores ownership and optional authentication credentials +// for a plugin server keyed by its canonical product ID. func RegisterPluginAuth(productID string, auth *PluginAuth) { pluginAuthMu.Lock() defer pluginAuthMu.Unlock() pluginAuthRegistry[productID] = auth } -// ClearPluginAuth removes credentials for a plugin product. Registration uses -// this before applying an accepted descriptor so a descriptor without custom -// auth cannot inherit stale credentials from an earlier root construction. +// ClearPluginAuth removes the ownership and credential record for a plugin +// product. func ClearPluginAuth(productID string) { pluginAuthMu.Lock() defer pluginAuthMu.Unlock() delete(pluginAuthRegistry, productID) } -// LookupPluginAuth returns the authentication credentials registered -// for the given product ID, or nil if none exists. +// LookupPluginAuth returns plugin ownership and optional authentication +// credentials for the product ID. The bool denotes ownership, not whether a +// Bearer token is present. func LookupPluginAuth(productID string) (*PluginAuth, bool) { pluginAuthMu.RLock() defer pluginAuthMu.RUnlock() diff --git a/internal/app/coverage_more_test.go b/internal/app/coverage_more_test.go index dae675b6..c56c1247 100644 --- a/internal/app/coverage_more_test.go +++ b/internal/app/coverage_more_test.go @@ -61,11 +61,16 @@ func appRPCServer(t *testing.T, initOK, listOK bool) *httptest.Server { } func TestCrossPlatformCoveragePluginAuthCoverage(t *testing.T) { - registerPluginAuthFromHeaders(mcptypes.ServerDescriptor{Key: "fallback", Endpoint: "%", AuthHeaders: map[string]string{"Authorization": "token"}}) - registerPluginAuthFromHeaders(mcptypes.ServerDescriptor{Key: "server", Endpoint: "https://x.test", CLI: mcptypes.CLIOverlay{ID: "cli"}, AuthHeaders: map[string]string{"Authorization": "Bearer token", "X": "Y"}}) - registerPluginAuthFromHeaders(mcptypes.ServerDescriptor{Key: "none"}) - if got, ok := LookupPluginAuth("cli"); !ok || got == nil || got.Token != "token" { - t.Fatalf("registered plugin auth = %#v, %v", got, ok) + fallback := pluginAuthFromServerDescriptor(mcptypes.ServerDescriptor{Key: "fallback", Endpoint: "%", AuthHeaders: map[string]string{"Authorization": "token"}}) + if fallback == nil || fallback.Token != "token" || len(fallback.TrustedDomains) != 0 { + t.Fatalf("fallback plugin auth = %#v", fallback) + } + got := pluginAuthFromServerDescriptor(mcptypes.ServerDescriptor{Key: "server", Endpoint: "https://x.test", CLI: mcptypes.CLIOverlay{ID: "cli"}, AuthHeaders: map[string]string{"Authorization": "Bearer token", "X": "Y"}}) + if got == nil || got.Token != "token" || got.ExtraHeaders["X"] != "Y" || len(got.TrustedDomains) != 2 { + t.Fatalf("plugin auth = %#v", got) + } + if anonymous := pluginAuthFromServerDescriptor(mcptypes.ServerDescriptor{Key: "none"}); anonymous == nil || anonymous.Token != "" { + t.Fatalf("anonymous plugin ownership = %#v", anonymous) } } diff --git a/internal/app/identity_export.go b/internal/app/identity_export.go index 9b2c36f0..dcc5b9a9 100644 --- a/internal/app/identity_export.go +++ b/internal/app/identity_export.go @@ -13,9 +13,9 @@ package app -// MCPIdentityHeaders returns the same header map used for MCP HTTP requests -// (agent identity, env trace headers, edition MergeHeaders). Intended for -// non-MCP transports such as the A2A gateway client. +// MCPIdentityHeaders returns the shared identity header map used by non-MCP +// transports such as the A2A gateway client. MCP-only Agent version and +// extension metadata are intentionally excluded. func MCPIdentityHeaders() map[string]string { return resolveIdentityHeaders() } diff --git a/internal/app/root.go b/internal/app/root.go index f9ad0a9f..cf42e616 100644 --- a/internal/app/root.go +++ b/internal/app/root.go @@ -79,6 +79,7 @@ var ( rootPluginSyncSkills = plugin.SyncSkills rootAuthLoadTokenData = authpkg.LoadTokenData rootNewCommandRunnerWithFlags = newCommandRunnerWithFlags + rootEmitResult = output.EmitResult ) // Execute runs the root command and returns the process exit code. @@ -136,6 +137,16 @@ func Execute() (exitCode int) { restoreArgs := rootNormalizeProcessProfileArgs() defer restoreArgs() + // Validate MCP Agent metadata before constructing the command tree. + // Construction may invoke edition registration/static-server hooks and load + // plugin PreParse handlers, so PersistentPreRunE alone is too late for the + // process entry point. Retain this exact pair for the eventual invocation. + agentMetadata := readAgentMetadataSnapshot() + if err := agentMetadata.validationError(); err != nil { + emitEarlyAgentMetadataValidationError(err, os.Args[1:]) + return apperrors.ExitCode(err) + } + timing := NewTimingCollector() defer func() { rootStopAllStdioClients() // Ensure child processes are terminated on exit @@ -147,6 +158,7 @@ func Execute() (exitCode int) { // Attach timing collector to context for use by child components ctx := WithTimingCollector(context.Background(), timing) + ctx = contextWithAgentMetadataSnapshot(ctx, agentMetadata) ctx, resultStore = output.WithResultStore(ctx) var signalState *processSignalState var stopSignals func() @@ -260,6 +272,70 @@ func Execute() (exitCode int) { return 0 } +// emitEarlyAgentMetadataValidationError preserves each built-in command's +// legacy-vs-unified output contract without running extension hooks. The +// presentation-only tree contains reviewed open-source commands and flags but +// deliberately omits edition registration, plugin loading, and visibility +// hooks; callers therefore still fail before any external hook executes. +func emitEarlyAgentMetadataValidationError(err error, args []string) { + format := processArgsFormat(args) + presentationRoot := newRootPresentationCommand() + _ = presentationRoot.PersistentFlags().Set("format", format) + if target, _, findErr := presentationRoot.Find(args); findErr == nil && target != nil && output.UsesUnifiedResult(target) { + target.SetOut(os.Stdout) + target.SetErr(os.Stderr) + result := output.FailureWithExitCode(errorInfoFromExecutionError(err), apperrors.ExitCode(err)) + if _, emitErr := rootEmitResult(target, result); emitErr == nil { + return + } + } + if strings.EqualFold(strings.TrimSpace(format), "json") { + _ = apperrors.PrintJSON(os.Stderr, err) + return + } + _ = apperrors.PrintHumanAt(os.Stderr, err, apperrors.VerbosityNormal) +} + +// processArgsRequestJSON preserves the CLI's machine-readable error contract +// for validation that must occur before Cobra and its presentation flags exist. +// The global format defaults to JSON; an explicit non-JSON format switches to +// the human diagnostic path. Last occurrence wins, matching pflag semantics. +func processArgsRequestJSON(args []string) bool { + return strings.EqualFold(strings.TrimSpace(processArgsFormat(args)), "json") +} + +func processArgsFormat(args []string) string { + format := "json" + for index := 0; index < len(args); index++ { + arg := args[index] + if arg == "--" { + break + } + if value, ok := strings.CutPrefix(arg, "--format="); ok { + format = value + continue + } + if value, ok := strings.CutPrefix(arg, "-f="); ok { + format = value + continue + } + if strings.HasPrefix(arg, "-f") && len(arg) > len("-f") { + format = strings.TrimPrefix(arg, "-f") + continue + } + if arg != "--format" && arg != "-f" { + continue + } + if index+1 >= len(args) { + format = "" + break + } + index++ + format = args[index] + } + return format +} + // errorInfoFromExecutionError projects the repository error model into the unified // failure body. Exit code and category are derived from the same error value, // preventing the wire and process status from drifting apart. @@ -633,12 +709,25 @@ func NewRootCommandWithEngine(rootCtx context.Context, engine *pipeline.Engine) } func newRootCommandWithEngine(rootCtx context.Context, engine *pipeline.Engine, loadRuntimeExtensions bool, declarationOnly bool) *cobra.Command { + return newRootCommandWithMode(rootCtx, engine, loadRuntimeExtensions, declarationOnly, false) +} + +func newRootPresentationCommand() *cobra.Command { + return newRootCommandWithMode(context.Background(), nil, false, true, true) +} + +func newRootCommandWithMode(rootCtx context.Context, engine *pipeline.Engine, loadRuntimeExtensions bool, declarationOnly bool, presentationOnly bool) *cobra.Command { if rootCtx == nil { rootCtx = context.Background() } flags := &GlobalFlags{} authpkg.SetRuntimeProfile(preparseProfileFlag(os.Args[1:])) runner := rootNewCommandRunnerWithFlags(flags) + if snapshot, ok := agentMetadataSnapshotFromContext(rootCtx); ok { + if runtime, ok := runner.(*runtimeRunner); ok { + runtime.agentMetadata = &snapshot + } + } root := &cobra.Command{ Use: "dws", @@ -672,15 +761,29 @@ func newRootCommandWithEngine(rootCtx context.Context, engine *pipeline.Engine, // --output sink instead opens at Run entry (after Cobra's own // validation), so validation failures still cannot strand a // temporary file. - // Validate caller-provided identity labels before any edition hook - // or command network activity can run. Header-only library callers - // use the best-effort path in resolveIdentityHeaders instead. + // Validate caller-provided identity and MCP metadata before command + // execution hooks or network activity. The process entry point additionally + // validates Agent metadata before command-tree construction; direct Cobra + // embedding retains this execution-boundary guard. if _, err := parseAgentHost(os.Getenv(envDWSAgentHost)); err != nil { return err } if _, err := parseAgentProduct(os.Getenv(agentproduct.EnvName)); err != nil { return err } + agentMetadata, cached := agentMetadataSnapshotFromContext(cmd.Context()) + if !cached { + agentMetadata = readAgentMetadataSnapshot() + } + if err := agentMetadata.validationError(); err != nil { + return err + } + if runtime, ok := runner.(*runtimeRunner); ok { + // Retain the exact validated pair for this command execution so a + // concurrently mutating embedding environment cannot change what is + // later applied after edition and credential hooks. + runtime.agentMetadata = &agentMetadata + } if shouldDetectNestedSkillLayout(cmd) { if found, err := detectNestedMultiSkillLayout(); err == nil && found { fmt.Fprintln(cmd.ErrOrStderr(), "⚠️ 检测到旧升级器留下的嵌套 Skill；请运行 dws skill setup --mode multi 查看迁移计划并确认") @@ -777,10 +880,12 @@ func newRootCommandWithEngine(rootCtx context.Context, engine *pipeline.Engine, // PAT authorization commands (open-source core) pat.RegisterCommands(root, patCaller) - if fn := edition.Get().RegisterExtraCommands; fn != nil { - caller := newToolCallerAdapter(runner, flags) - fn(root, caller) - deduplicateCommands(root) + if !presentationOnly { + if fn := edition.Get().RegisterExtraCommands; fn != nil { + caller := newToolCallerAdapter(runner, flags) + fn(root, caller) + deduplicateCommands(root) + } } if loadRuntimeExtensions { // Resolve plugins only after the complete distribution command tree is @@ -791,7 +896,9 @@ func newRootCommandWithEngine(rootCtx context.Context, engine *pipeline.Engine, addPluginCommandsSafe(root, pluginCmds) } } - hideNonDirectRuntimeCommands(root) + if !presentationOnly { + hideNonDirectRuntimeCommands(root) + } configureRootHelp(root) // Set custom flag error handler for better UX root.SetFlagErrorFunc(flagErrorWithSuggestions) @@ -1755,17 +1862,16 @@ func distributionRootOwns(root *cobra.Command, name string) bool { func registerPluginHTTPServer(srv mcptypes.ServerDescriptor) { AppendDynamicServer(srv) productID := firstNonEmptyPluginString(srv.CLI.ID, srv.Key) - ClearPluginAuth(productID) - if len(srv.AuthHeaders) > 0 { - registerPluginAuthFromHeaders(srv) - } + // Register ownership for every accepted HTTP plugin, including anonymous + // plugins. Execution must never fall back to the built-in DingTalk OAuth or + // Agent-metadata path merely because a plugin has no Authorization Header. + RegisterPluginAuth(productID, pluginAuthFromServerDescriptor(srv)) } -// registerPluginAuthFromHeaders extracts authentication credentials from -// a server descriptor's AuthHeaders and registers them in the global -// PluginAuth registry. The runner uses this registry at execution time -// to inject the correct Bearer token for third-party MCP servers. -func registerPluginAuthFromHeaders(srv mcptypes.ServerDescriptor) { +// pluginAuthFromServerDescriptor extracts plugin-owned credentials and custom +// Headers. A non-nil result also acts as the HTTP plugin ownership marker for +// anonymous plugins. +func pluginAuthFromServerDescriptor(srv mcptypes.ServerDescriptor) *PluginAuth { authToken := "" extraHeaders := make(map[string]string) for key, value := range srv.AuthHeaders { @@ -1776,20 +1882,18 @@ func registerPluginAuthFromHeaders(srv mcptypes.ServerDescriptor) { extraHeaders[key] = value } } - if authToken == "" { - return - } var trustedDomains []string if parsed, err := url.Parse(srv.Endpoint); err == nil { host := parsed.Hostname() - trustedDomains = []string{host, "*." + host} + if host != "" { + trustedDomains = []string{host, "*." + host} + } } - productID := firstNonEmptyPluginString(srv.CLI.ID, srv.Key) - RegisterPluginAuth(productID, &PluginAuth{ + return &PluginAuth{ Token: authToken, ExtraHeaders: extraHeaders, TrustedDomains: trustedDomains, - }) + } } // newPipelineEngine creates and configures the pipeline engine with diff --git a/internal/app/runner.go b/internal/app/runner.go index 3e9290c4..175f70b9 100644 --- a/internal/app/runner.go +++ b/internal/app/runner.go @@ -137,7 +137,6 @@ func newCommandRunnerWithFlags(flags *GlobalFlags) executor.Runner { httpClient = &http.Client{Timeout: time.Duration(flags.Timeout) * time.Second} } transportClient := transport.NewClient(httpClient) - transportClient.ExtraHeaders = resolveIdentityHeaders() transportClient.FileLogger = FileLoggerInstance() return &runtimeRunner{ transport: transportClient, @@ -156,12 +155,14 @@ type runtimeRunner struct { enforceContentScan bool includeScanReport bool auditSink audit.Sink + agentMetadata *agentMetadataSnapshot } var ( runnerResolveMultiProfileSelections = resolveMultiProfileSelections runnerResolveProfile = authpkg.ResolveProfile runnerGetCachedRuntimeToken = getCachedRuntimeToken + runnerResolveAuthToken = (*runtimeRunner).resolveAuthToken runnerPreflightDocDownload = (*runtimeRunner).preflightDocDownload runnerCallTool = (*transport.Client).CallTool runnerStdioEnsureInitialized = (*transport.StdioClient).EnsureInitialized @@ -237,7 +238,6 @@ func (r *runtimeRunner) runSingle(ctx context.Context, invocation executor.Invoc if r.transport == nil { return r.fallback.Run(ctx, invocation) } - r.transport.ExtraHeaders = resolveIdentityHeaders() // Mock mode: skip endpoint resolution, use a placeholder endpoint. if r.globalFlags != nil && r.globalFlags.Mock { @@ -543,9 +543,9 @@ func (r *runtimeRunner) executeInvocation(ctx context.Context, endpoint string, emitAudit(auditSink, execID, invokeStart, invocation, endpoint, retErr, version) }() - // Check if this product has plugin-level auth credentials registered. - // If so, use the plugin's token instead of the default DingTalk OAuth token. - // This allows third-party MCP servers (e.g. Bailian) to use their own API keys. + // Check whether this product belongs to an HTTP plugin. Every accepted + // plugin has an ownership record; credentials within that record are + // optional. Plugin requests never fall back to the default DingTalk OAuth. pluginAuth, hasPluginAuth := LookupPluginAuth(invocation.CanonicalProduct) authToken := "" @@ -553,7 +553,7 @@ func (r *runtimeRunner) executeInvocation(ctx context.Context, endpoint string, authToken = pluginAuth.Token } else if !invocation.DryRun && (r.globalFlags == nil || !r.globalFlags.Mock) { var tokenErr error - authToken, tokenErr = r.resolveAuthToken(ctx) + authToken, tokenErr = runnerResolveAuthToken(r, ctx) if tokenErr != nil { return executor.Result{}, tokenResolutionError(tokenErr) } @@ -603,9 +603,10 @@ func (r *runtimeRunner) executeInvocation(ctx context.Context, endpoint string, }, nil } - // Fail-fast: reject unauthenticated requests before making network calls. - // This provides a clear error message instead of cryptic HTTP 400 from MCP. - if strings.TrimSpace(authToken) == "" { + // Preserve a final execution-boundary guard even though the built-in token + // resolver normally returns either a non-empty token or an error. HTTP + // plugins are ownership-scoped separately and may intentionally be anonymous. + if !hasPluginAuth && strings.TrimSpace(authToken) == "" { return executor.Result{}, apperrors.NewAuth( "未登录，请先执行 dws auth login", apperrors.WithReason("not_authenticated"), @@ -616,12 +617,20 @@ func (r *runtimeRunner) executeInvocation(ctx context.Context, endpoint string, var tc *transport.Client if hasPluginAuth { - // Use plugin-level auth: inject the plugin's token and trust its domains. - tc = r.transport.WithAuth(authToken, pluginAuth.ExtraHeaders) + // Plugin ownership is authoritative even when the plugin is anonymous. + // Copy and sanitize manifest headers so plugins cannot opt themselves into + // DWS-owned Agent metadata by declaring the reserved names directly. + tc = r.transport.WithAuth(authToken, pluginRequestHeaders(pluginAuth)) tc.TrustedDomains = pluginAuth.TrustedDomains } else { - // Default path: use DingTalk OAuth token with identity headers. - tc = r.transport.WithAuth(authToken, resolveIdentityHeaders()) + // Default path: use DingTalk OAuth token with identity headers. Agent + // metadata is resolved exactly once per invocation and is excluded from + // helper-only service-discovery requests. + if r.agentMetadata != nil { + tc = r.transport.WithAuth(authToken, resolveMCPRequestHeadersForInvocation(invocation, *r.agentMetadata)) + } else { + tc = r.transport.WithAuth(authToken, resolveMCPRequestHeadersForInvocation(invocation)) + } } callCtx := ctx @@ -1069,6 +1078,10 @@ func resolveIdentityHeaders() map[string]string { } else { delete(headers, agentproduct.HeaderName) } + // Agent version and extension are intentionally MCP-request-only. Remove + // every case variant potentially supplied by an edition or credential hook + // so shared consumers such as A2A cannot inherit them. + removeAgentMetadataHeaders(headers) return headers } diff --git a/internal/auth/agent_metadata_headers_test.go b/internal/auth/agent_metadata_headers_test.go new file mode 100644 index 00000000..adab2952 --- /dev/null +++ b/internal/auth/agent_metadata_headers_test.go @@ -0,0 +1,80 @@ +// 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 auth + +import ( + "context" + "io" + "net/http" + "strings" + "testing" +) + +type agentMetadataHeaderRoundTripFunc func(*http.Request) (*http.Response, error) + +func (f agentMetadataHeaderRoundTripFunc) RoundTrip(req *http.Request) (*http.Response, error) { + return f(req) +} + +func TestCrossPlatformCoverageOAuthRequestsExcludeAgentMetadataHeaders(t *testing.T) { + t.Setenv("DWS_AGENT_VER", "1.2.3-test") + t.Setenv("DWS_AGENT_EXT", `{"umt":"test-umt","miniwua":"test-wua","ua":"test-agent"}`) + + assertExcluded := func(t *testing.T, header http.Header) { + t.Helper() + for _, name := range []string{"x-dws-agent-ver", "x-dws-agent-ext"} { + if values := header.Values(name); len(values) != 0 { + t.Fatalf("OAuth request header %q = %q, want absent", name, values) + } + } + } + + newCaptureClient := func(responseBody string) (*http.Client, <-chan http.Header) { + seen := make(chan http.Header, 1) + client := &http.Client{Transport: agentMetadataHeaderRoundTripFunc(func(req *http.Request) (*http.Response, error) { + seen <- req.Header.Clone() + return &http.Response{ + StatusCode: http.StatusOK, + Header: http.Header{"Content-Type": {"application/json"}}, + Body: io.NopCloser(strings.NewReader(responseBody)), + Request: req, + }, nil + })} + return client, seen + } + + t.Run("JSON token request", func(t *testing.T) { + client, seen := newCaptureClient(`{}`) + provider := &OAuthProvider{httpClient: client} + if _, err := provider.postJSON(context.Background(), "https://oauth.test/token", map[string]string{ + "code": "test-code", + "grantType": "authorization_code", + }); err != nil { + t.Fatalf("postJSON() error = %v", err) + } + assertExcluded(t, <-seen) + }) + + t.Run("device code request", func(t *testing.T) { + client, seen := newCaptureClient(`{"success":true,"result":{"deviceCode":"test-device-code","userCode":"TEST-CODE","verificationUri":"https://example.test/device","expiresIn":900,"interval":1}}`) + provider := NewDeviceFlowProvider(t.TempDir(), newDeviceFlowTestLogger()) + provider.clientID = "test-client-id" + provider.httpClient = client + provider.SetBaseURL("https://oauth.test") + if _, err := provider.requestDeviceCode(context.Background()); err != nil { + t.Fatalf("requestDeviceCode() error = %v", err) + } + assertExcluded(t, <-seen) + }) +} diff --git a/internal/logging/redact.go b/internal/logging/redact.go index d84003f7..adb6b40c 100644 --- a/internal/logging/redact.go +++ b/internal/logging/redact.go @@ -26,6 +26,8 @@ import ( var sensitiveKeys = map[string]bool{ "authorization": true, "x-user-access-token": true, + "dws_agent_ext": true, + "x-dws-agent-ext": true, "client_secret": true, "client-secret": true, "token": true, diff --git a/internal/logging/redact_test.go b/internal/logging/redact_test.go index e3830df8..eb4578fc 100644 --- a/internal/logging/redact_test.go +++ b/internal/logging/redact_test.go @@ -30,6 +30,12 @@ func TestIsSensitiveKey(t *testing.T) { {"authorization", true}, {"x-user-access-token", true}, {"X-User-Access-Token", true}, + {"DWS_AGENT_EXT", true}, + {"dws_agent_ext", true}, + {"x-dws-agent-ext", true}, + {"X-Dws-Agent-Ext", true}, + {"DWS_AGENT_VER", false}, + {"x-dws-agent-ver", false}, {"client_secret", true}, {"client-secret", true}, {"token", true}, @@ -128,17 +134,24 @@ func TestSanitizeArguments_Empty(t *testing.T) { func TestRedactHeaders(t *testing.T) { t.Parallel() - headers := http.Header{ - "Authorization": {"Bearer token123456"}, - "Content-Type": {"application/json"}, - } + headers := make(http.Header) + headers.Set("Authorization", "Bearer test-credential-value") + headers.Set("Content-Type", "application/json") + headers.Set("DWS_AGENT_EXT", `{"umt":"test-umt-value"}`) + headers.Set("x-dws-agent-ext", `{"ua":"test-agent-value"}`) attrs := RedactHeaders(headers) - if len(attrs) != 2 { - t.Fatalf("expected 2 attrs, got %d", len(attrs)) + if len(attrs) != 4 { + t.Fatalf("expected 4 attrs, got %d", len(attrs)) } for _, attr := range attrs { - if attr.Key == "header.authorization" && !strings.Contains(attr.Value.String(), "***") { - t.Fatalf("authorization should be redacted: %s", attr.Value.String()) + switch attr.Key { + case "header.authorization", "header.dws_agent_ext", "header.x-dws-agent-ext": + if !strings.Contains(attr.Value.String(), "***") { + t.Fatalf("%s should be redacted: %s", attr.Key, attr.Value.String()) + } + if strings.Contains(attr.Value.String(), "test-") { + t.Fatalf("%s leaked its original value: %s", attr.Key, attr.Value.String()) + } } if attr.Key == "header.content-type" && attr.Value.String() != "application/json" { t.Fatalf("content-type should not be redacted: %s", attr.Value.String()) diff --git a/internal/transport/agent_metadata_test.go b/internal/transport/agent_metadata_test.go new file mode 100644 index 00000000..3767ce87 --- /dev/null +++ b/internal/transport/agent_metadata_test.go @@ -0,0 +1,93 @@ +// 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 transport + +import ( + "context" + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "testing" +) + +func TestCrossPlatformCoverageAgentMetadataReachesEveryMCPMethod(t *testing.T) { + t.Parallel() + + const ( + agentVersion = "1.2.3-test+7" + agentExt = `{"umt":"test-umt","nested":{"enabled":true}}` + cliVersion = "9.8.7-cli" + userAgent = "dws-user-agent-test/1.0" + ) + + seen := make(map[string]int) + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + var request requestEnvelope + if err := json.NewDecoder(r.Body).Decode(&request); err != nil { + http.Error(w, err.Error(), http.StatusBadRequest) + return + } + seen[request.Method]++ + if got := r.Header.Get(HeaderAgentVersion); got != agentVersion { + t.Errorf("%s %s = %q, want %q", request.Method, HeaderAgentVersion, got, agentVersion) + } + if got := r.Header.Get(HeaderAgentExt); got != agentExt { + t.Errorf("%s %s = %q, want %q", request.Method, HeaderAgentExt, got, agentExt) + } + if got := r.Header.Get(HeaderVersion); got != cliVersion { + t.Errorf("%s %s = %q, want %q", request.Method, HeaderVersion, got, cliVersion) + } + if got := r.Header.Get("User-Agent"); got != userAgent { + t.Errorf("%s User-Agent = %q, want preset value %q", request.Method, got, userAgent) + } + + w.Header().Set("Content-Type", "application/json") + switch request.Method { + case "initialize": + _, _ = fmt.Fprintf(w, `{"jsonrpc":"2.0","id":1,"result":{"protocolVersion":"%s","capabilities":{}}}`, supportedProtocolVersions[0]) + case "tools/list": + _, _ = fmt.Fprint(w, `{"jsonrpc":"2.0","id":2,"result":{"tools":[]}}`) + case "tools/call": + _, _ = fmt.Fprint(w, `{"jsonrpc":"2.0","id":3,"result":{"content":{"success":true}}}`) + default: + http.Error(w, "unexpected method", http.StatusBadRequest) + } + })) + defer server.Close() + + client := NewClient(server.Client()) + client.ExtraHeaders = map[string]string{ + HeaderAgentVersion: agentVersion, + HeaderAgentExt: agentExt, + HeaderVersion: cliVersion, + "User-Agent": userAgent, + } + ctx := context.Background() + if _, err := client.Initialize(ctx, server.URL); err != nil { + t.Fatalf("Initialize() error = %v", err) + } + if _, err := client.ListTools(ctx, server.URL); err != nil { + t.Fatalf("ListTools() error = %v", err) + } + if _, err := client.CallTool(ctx, server.URL, "test_tool", map[string]any{"value": "safe"}); err != nil { + t.Fatalf("CallTool() error = %v", err) + } + + for _, method := range []string{"initialize", "tools/list", "tools/call"} { + if got := seen[method]; got != 1 { + t.Errorf("%s request count = %d, want 1", method, got) + } + } +} diff --git a/internal/transport/client.go b/internal/transport/client.go index c9184a41..ddbae10d 100644 --- a/internal/transport/client.go +++ b/internal/transport/client.go @@ -68,10 +68,12 @@ const ( defaultRetryMaxDelay = 5 * time.Second // Security headers - HeaderSource = "X-Cli-Source" - HeaderVersion = "X-Cli-Version" - HeaderExecutionId = "X-Cli-Execution-Id" - SourceValue = "dws-cli" + HeaderSource = "X-Cli-Source" + HeaderVersion = "X-Cli-Version" + HeaderExecutionId = "X-Cli-Execution-Id" + HeaderAgentVersion = "x-dws-agent-ver" + HeaderAgentExt = "x-dws-agent-ext" + SourceValue = "dws-cli" ) // Supported MCP protocol versions, ordered from newest to oldest. @@ -256,20 +258,43 @@ func NewClient(httpClient *http.Client) *Client { // safeRedirectPolicy prevents credential headers from being forwarded // when a response redirects to a different host (e.g. API 302 → CDN). -// Strips Authorization, x-user-access-token on cross-host redirects; -// other headers like X-Cli-* pass through. +// Credentials and Agent extension context are bound to the initial origin: +// once a redirect chain leaves that origin they remain stripped for every +// subsequent hop, including a redirect back to the initial origin. +// Non-sensitive headers like X-Cli-* and x-dws-agent-ver pass through. func safeRedirectPolicy(req *http.Request, via []*http.Request) error { if len(via) >= 10 { return fmt.Errorf("too many redirects") } - if len(via) > 0 && req.URL.Host != via[0].URL.Host { - // Cross-host redirect: strip sensitive headers to prevent credential leakage + if redirectChainLeftInitialOrigin(req, via) { + req.Header.Del(HeaderAgentExt) req.Header.Del("Authorization") req.Header.Del("x-user-access-token") } return nil } +func redirectChainLeftInitialOrigin(req *http.Request, via []*http.Request) bool { + if len(via) == 0 { + return false + } + initialURL := via[0].URL + if !sameOrigin(req.URL, initialURL) { + return true + } + for _, previous := range via[1:] { + if !sameOrigin(previous.URL, initialURL) { + return true + } + } + return false +} + +func sameOrigin(left, right *url.URL) bool { + return strings.EqualFold(left.Scheme, right.Scheme) && + strings.EqualFold(left.Host, right.Host) +} + // WithAuth returns a shallow copy of c with the given auth token and extra // headers. The returned client shares the underlying HTTP client but is safe // to use concurrently with the original. diff --git a/internal/transport/redirect_test.go b/internal/transport/redirect_test.go new file mode 100644 index 00000000..0614174f --- /dev/null +++ b/internal/transport/redirect_test.go @@ -0,0 +1,140 @@ +// 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 transport + +import ( + "net/http" + "strings" + "testing" +) + +func TestCrossPlatformCoverageSafeRedirectPolicyAgentMetadataHeaders(t *testing.T) { + t.Parallel() + + newRequest := func(t *testing.T, rawURL string) *http.Request { + t.Helper() + req, err := http.NewRequest(http.MethodGet, rawURL, nil) + if err != nil { + t.Fatal(err) + } + req.Header.Set(HeaderAgentVersion, "1.2.3-test") + req.Header.Set(HeaderAgentExt, `{"ua":"test-agent-value"}`) + req.Header.Set("Authorization", "Bearer test-token") + req.Header.Set("x-user-access-token", "test-token") + return req + } + + assertHeaders := func(t *testing.T, request *http.Request, wantExt bool) { + t.Helper() + if got := request.Header.Get(HeaderAgentVersion); got != "1.2.3-test" { + t.Fatalf("agent version = %q, want retained", got) + } + gotExt := request.Header.Get(HeaderAgentExt) + if wantExt && gotExt == "" { + t.Fatal("agent extension was removed") + } + if !wantExt && gotExt != "" { + t.Fatalf("agent extension leaked across origins: %q", gotExt) + } + for _, key := range []string{"Authorization", "x-user-access-token"} { + got := request.Header.Get(key) + if wantExt && got == "" { + t.Fatalf("same-origin redirect removed %s", key) + } + if !wantExt && got != "" { + t.Fatalf("credential %s leaked across origins", key) + } + } + } + + t.Run("same origin retains version and extension", func(t *testing.T) { + t.Parallel() + previous := newRequest(t, "https://api.example.test/start") + redirected := newRequest(t, "https://api.example.test/next") + + if err := safeRedirectPolicy(redirected, []*http.Request{previous}); err != nil { + t.Fatal(err) + } + assertHeaders(t, redirected, true) + }) + + t.Run("cross host strips extension and retains version", func(t *testing.T) { + t.Parallel() + previous := newRequest(t, "https://api.example.test/start") + redirected := newRequest(t, "https://cdn.example.test/asset") + + if err := safeRedirectPolicy(redirected, []*http.Request{previous}); err != nil { + t.Fatal(err) + } + assertHeaders(t, redirected, false) + }) + + t.Run("scheme downgrade on same host strips extension", func(t *testing.T) { + t.Parallel() + previous := newRequest(t, "https://api.example.test/start") + redirected := newRequest(t, "http://api.example.test/next") + + if err := safeRedirectPolicy(redirected, []*http.Request{previous}); err != nil { + t.Fatal(err) + } + assertHeaders(t, redirected, false) + }) + + t.Run("extension remains stripped after returning to initial origin", func(t *testing.T) { + t.Parallel() + initial := newRequest(t, "https://api.example.test/start") + crossOrigin := newRequest(t, "https://cdn.example.test/asset") + redirectedBack := newRequest(t, "https://api.example.test/final") + + if err := safeRedirectPolicy(redirectedBack, []*http.Request{initial, crossOrigin}); err != nil { + t.Fatal(err) + } + assertHeaders(t, redirectedBack, false) + }) + + t.Run("extension remains stripped on later cross-origin hop", func(t *testing.T) { + t.Parallel() + initial := newRequest(t, "https://api.example.test/start") + crossOrigin := newRequest(t, "https://cdn.example.test/asset") + redirected := newRequest(t, "https://cdn.example.test/final") + + if err := safeRedirectPolicy(redirected, []*http.Request{initial, crossOrigin}); err != nil { + t.Fatal(err) + } + assertHeaders(t, redirected, false) + }) + + t.Run("initial request is unchanged", func(t *testing.T) { + t.Parallel() + request := newRequest(t, "https://api.example.test/start") + + if err := safeRedirectPolicy(request, nil); err != nil { + t.Fatal(err) + } + assertHeaders(t, request, true) + }) + + t.Run("redirect limit is enforced", func(t *testing.T) { + t.Parallel() + request := newRequest(t, "https://api.example.test/final") + via := make([]*http.Request, 10) + for index := range via { + via[index] = newRequest(t, "https://api.example.test/"+strings.Repeat("x", index+1)) + } + + if err := safeRedirectPolicy(request, via); err == nil { + t.Fatal("safeRedirectPolicy() error = nil, want redirect limit error") + } + }) +} diff --git a/pkg/cli/identity.go b/pkg/cli/identity.go index e42afa4e..98adcc77 100644 --- a/pkg/cli/identity.go +++ b/pkg/cli/identity.go @@ -15,8 +15,9 @@ package cli import "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/app" -// MCPIdentityHeaders returns HTTP headers aligned with MCP tool calls -// (identity + edition merge). Overlays may pass this to auxiliary clients. +// MCPIdentityHeaders returns the shared identity and edition headers that +// overlays may pass to auxiliary clients. MCP-request-only metadata such as +// DWS_AGENT_VER and DWS_AGENT_EXT is intentionally excluded. func MCPIdentityHeaders() map[string]string { return app.MCPIdentityHeaders() } diff --git a/pkg/cli/identity_test.go b/pkg/cli/identity_test.go new file mode 100644 index 00000000..4ca6c44e --- /dev/null +++ b/pkg/cli/identity_test.go @@ -0,0 +1,35 @@ +// 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 cli + +import ( + "strings" + "testing" +) + +func TestCrossPlatformCoverageMCPIdentityHeadersExcludeMCPOnlyMetadata(t *testing.T) { + t.Setenv("DWS_CONFIG_DIR", t.TempDir()) + t.Setenv("DWS_AGENT_VER", "1.2.3") + t.Setenv("DWS_AGENT_EXT", `{"ua":"test-agent/1.2.3"}`) + + headers := MCPIdentityHeaders() + if headers == nil { + t.Fatal("MCPIdentityHeaders() returned nil") + } + for key := range headers { + if strings.EqualFold(key, "x-dws-agent-ver") || strings.EqualFold(key, "x-dws-agent-ext") { + t.Fatalf("shared identity export leaked MCP-only Header %q", key) + } + } +} From a203572ee3ed991c016c95109af232e62be39b57 Mon Sep 17 00:00:00 2001 From: Dennis Date: Wed, 12 Aug 2026 14:46:20 +0800 Subject: [PATCH 4/9] feat(chat): unify shortcut auto-pagination controls --- internal/shortcut/chat/chat_conversation.go | 68 ++++++++++++----- .../chat/chat_conversation_projection_test.go | 22 ++++++ internal/shortcut/chat/chat_group.go | 47 +++++++++--- internal/shortcut/chat/chat_message.go | 1 + .../chat/flag_list_pagination_test.go | 28 ++++++- internal/shortcut/chat/lark_alignment.go | 69 ++++++++++++----- .../chat/remaining_read_pagination_test.go | 24 +++++- internal/shortcut/chatmsg/chatmsg.go | 13 ++++ internal/shortcut/pagination.go | 75 +++++++++++++++++++ internal/shortcut/smart/at_me.go | 45 +++++++++-- internal/shortcut/smart/chat_messages.go | 44 ++++++++--- internal/shortcut/smart/chat_messages_test.go | 24 +++--- internal/shortcut/smart/my_groups.go | 58 ++++++++++++-- .../smart/remaining_read_pagination_test.go | 48 ++++++++++++ internal/shortcut/smart/thread_replies.go | 1 + .../dingtalk-chat/references/contracts.md | 4 +- 16 files changed, 482 insertions(+), 89 deletions(-) create mode 100644 internal/shortcut/pagination.go diff --git a/internal/shortcut/chat/chat_conversation.go b/internal/shortcut/chat/chat_conversation.go index a45094b7..3c77f8f9 100644 --- a/internal/shortcut/chat/chat_conversation.go +++ b/internal/shortcut/chat/chat_conversation.go @@ -313,17 +313,17 @@ var ConversationList = shortcut.Shortcut{ Examples: []string{"dws chat +conversation-list --limit 50"}, }, }, - Flags: []shortcut.Flag{ + Flags: append([]shortcut.Flag{ {Name: "limit", Type: shortcut.FlagInt, Default: "100", Desc: "每页数量；--limit 必须在 1-100"}, {Name: "cursor", Type: shortcut.FlagInt, Desc: "分页游标（首次不传或 0）"}, {Name: "exclude-muted", Type: shortcut.FlagBool, Desc: "排除已免打扰会话"}, - {Name: "page-all", Type: shortcut.FlagBool, Desc: "自动读取全部分页；--page-limit 仅与 --page-all 一起使用且范围 1-500"}, + {Name: "page-all", Type: shortcut.FlagBool, Desc: "自动读取全部分页；--page-limit 仅与 --page-all 一起使用且范围 1-500；--max-items/--page-delay 仅与 --page-all 一起使用；值必须大于等于 0"}, {Name: "page-limit", Type: shortcut.FlagInt, Default: "50", Desc: "--page-limit 仅与 --page-all 一起使用且范围 1-500"}, - }, - Constraints: []shortcut.Constraint{ + }, shortcut.AutoPageControlFlags()...), + Constraints: append([]shortcut.Constraint{ {Kind: shortcut.ConstraintCustom, Flags: []string{"limit"}, Description: "--limit 必须在 1-100"}, {Kind: shortcut.ConstraintCustom, Flags: []string{"page-all", "page-limit"}, Description: "--page-limit 仅与 --page-all 一起使用且范围 1-500"}, - }, + }, shortcut.AutoPageControlConstraints()...), Tips: []string{ `dws chat +conversation-list --limit 50`, `dws chat +conversation-list --page-all --limit 100`, @@ -338,6 +338,9 @@ var ConversationList = shortcut.Shortcut{ if pageLimit := rt.Int("page-limit"); pageLimit < 1 || pageLimit > 500 { return apperrors.NewValidation("--page-limit 必须在 1-500 之间") } + if err := shortcut.ValidateAutoPageControls(rt); err != nil { + return apperrors.NewValidation(err.Error()) + } return nil }, Execute: func(rt *shortcut.RuntimeContext) error { @@ -353,8 +356,18 @@ var ConversationList = shortcut.Shortcut{ complete := false hasMore := false nextCursor := int64(0) + stopReason := "source_complete" + truncatedByPageLimit := false + truncatedByResultLimit := false failures := make([]map[string]any, 0) for pagesFetched < pageLimit { + if pagesFetched > 0 { + if err := shortcut.WaitAutoPageDelay(rt); err != nil { + failures = append(failures, map[string]any{"stage": "conversation-page-delay", "cursor": cursor, "error": err.Error()}) + stopReason = "delay_interrupted" + break + } + } params := map[string]any{"limit": rt.Int("limit")} if cursor > 0 { params["cursor"] = cursor @@ -379,6 +392,10 @@ var ConversationList = shortcut.Shortcut{ } seenConversations[id] = true } + if maxItems := rt.Int("max-items"); maxItems > 0 && len(convs) >= maxItems { + truncatedByResultLimit = true + continue + } convs = append(convs, conversation) } page := chatmsg.Pagination(data) @@ -389,7 +406,11 @@ var ConversationList = shortcut.Shortcut{ break } if !hasMore { - complete = true + complete = !truncatedByResultLimit + if truncatedByResultLimit { + hasMore = true + stopReason = "result_limit" + } break } nextCursor, err = conversationPaginationCursor(page["nextCursor"]) @@ -398,26 +419,37 @@ var ConversationList = shortcut.Shortcut{ break } if !rt.Bool("page-all") { + stopReason = "single_page" + break + } + if maxItems := rt.Int("max-items"); maxItems > 0 && len(convs) >= maxItems { + truncatedByResultLimit = true + stopReason = "result_limit" break } seenCursors[nextCursor] = true cursor = nextCursor } - if rt.Bool("page-all") && hasMore && pagesFetched == pageLimit { - failures = append(failures, map[string]any{"stage": "conversation-page-limit", "error": fmt.Sprintf("达到 --page-limit=%d，仍有更多会话", pageLimit)}) + if rt.Bool("page-all") && hasMore && pagesFetched == pageLimit && !truncatedByResultLimit { + truncatedByPageLimit = true + stopReason = "page_limit" } payload := map[string]any{ - "count": len(convs), - "conversations": convs, - "pagesFetched": pagesFetched, - "complete": complete, - "hasMore": hasMore, - "nextCursor": nextCursor, - "paginationKnown": len(failures) == 0 || hasMore, - "failedCount": len(failures), - "failures": failures, - "partial": len(failures) > 0, + "count": len(convs), + "conversations": convs, + "pagesFetched": pagesFetched, + "complete": complete, + "hasMore": hasMore, + "nextCursor": nextCursor, + "paginationKnown": len(failures) == 0 || hasMore, + "stopReason": stopReason, + "truncatedByPageLimit": truncatedByPageLimit, + "truncatedByResultLimit": truncatedByResultLimit, + "failedCount": len(failures), + "failures": failures, + "partial": len(failures) > 0, } + chatmsg.ApplyTruncation(payload) return rt.Output(payload) }, } diff --git a/internal/shortcut/chat/chat_conversation_projection_test.go b/internal/shortcut/chat/chat_conversation_projection_test.go index dee998aa..cec59a68 100644 --- a/internal/shortcut/chat/chat_conversation_projection_test.go +++ b/internal/shortcut/chat/chat_conversation_projection_test.go @@ -148,6 +148,28 @@ func TestCrossPlatformCoverageConversationListSinglePagePreservesTypedCursor(t * } } +func TestCrossPlatformCoverageConversationListMaxItemsPublishesStableTruncation(t *testing.T) { + fake := &larkAlignmentCaller{responses: map[string]string{ + "im/list_all_conversations": `{"result":{"conversationList":[{"openConversationId":"cid-1"},{"openConversationId":"cid-2"}],"hasMore":true,"nextCursor":2}}`, + }} + helpers.InitDeps(fake) + root := newPlatformCoverageRoot() + var output bytes.Buffer + root.SetOut(&output) + root.SetArgs([]string{"chat", "+conversation-list", "--page-all", "--max-items", "1", "--page-delay", "0"}) + 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) + } + if payload["count"] != float64(1) || payload["truncated"] != true || + payload["truncatedByResultLimit"] != true || payload["stopReason"] != "result_limit" { + t.Fatalf("payload = %#v", payload) + } +} + func TestCrossPlatformCoverageConversationListDeduplicatesStableIDs(t *testing.T) { fake := &larkAlignmentCaller{responses: map[string]string{ "im/list_all_conversations": `{"result":{"conversationList":[{"openConversationId":"cid-1","title":"一"},{"openConversationId":"cid-1","title":"重复"}],"hasMore":false}}`, diff --git a/internal/shortcut/chat/chat_group.go b/internal/shortcut/chat/chat_group.go index 215b3f87..6bba8430 100644 --- a/internal/shortcut/chat/chat_group.go +++ b/internal/shortcut/chat/chat_group.go @@ -878,16 +878,16 @@ var ChatListAll = shortcut.Shortcut{ Examples: []string{"dws chat +chat-list-all --limit 50"}, }, }, - Flags: []shortcut.Flag{ + Flags: append([]shortcut.Flag{ {Name: "limit", Type: shortcut.FlagInt, Default: "100", Desc: "每页返回数量；--limit 必须在 1-200 之间"}, {Name: "cursor", Type: shortcut.FlagString, Desc: "分页游标，翻页传 nextCursor"}, - {Name: "page-all", Type: shortcut.FlagBool, Desc: "沿 nextCursor 自动读取全部已加入群；--page-limit 仅与 --page-all 一起使用且范围 1-500"}, + {Name: "page-all", Type: shortcut.FlagBool, Desc: "沿 nextCursor 自动读取全部已加入群；--page-limit 仅与 --page-all 一起使用且范围 1-500；--max-items/--page-delay 仅与 --page-all 一起使用；值必须大于等于 0"}, {Name: "page-limit", Type: shortcut.FlagInt, Default: "50", Desc: "--page-limit 仅与 --page-all 一起使用且范围 1-500"}, - }, - Constraints: []shortcut.Constraint{ + }, shortcut.AutoPageControlFlags()...), + Constraints: append([]shortcut.Constraint{ {Kind: shortcut.ConstraintCustom, Flags: []string{"limit"}, Description: "--limit 必须在 1-200 之间"}, {Kind: shortcut.ConstraintCustom, Flags: []string{"page-all", "page-limit"}, Description: "--page-limit 仅与 --page-all 一起使用且范围 1-500"}, - }, + }, shortcut.AutoPageControlConstraints()...), Tips: []string{ `dws chat +chat-list-all --limit 50`, `dws chat +chat-list-all --limit 200 --page-all --page-limit 50`, @@ -908,6 +908,9 @@ func validateChatListAll(rt *shortcut.RuntimeContext) error { return apperrors.NewValidation("--page-limit 必须在 1-500 之间") } } + if err := shortcut.ValidateAutoPageControls(rt); err != nil { + return apperrors.NewValidation(err.Error()) + } return nil } @@ -958,9 +961,19 @@ func readAllChatListAll(rt *shortcut.RuntimeContext, baseParams map[string]any) hasMore := false stopReason := "source_complete" truncatedByPageLimit := false + truncatedByResultLimit := false var nextCursor any for pagesFetched < pageLimit { + if pagesFetched > 0 { + if err := shortcut.WaitAutoPageDelay(rt); err != nil { + failures = append(failures, map[string]any{ + "page": pagesFetched + 1, "stage": "delay", "cursor": cursorKey, "error": err.Error(), + }) + stopReason = "delay_interrupted" + break + } + } params := map[string]any{"limit": baseParams["limit"]} if cursorKey != "0" { params["cursor"] = cursorValue @@ -989,6 +1002,10 @@ func readAllChatListAll(rt *shortcut.RuntimeContext, baseParams map[string]any) if id != "" { seenGroups[id] = true } + if maxItems := rt.Int("max-items"); maxItems > 0 && len(allGroups) >= maxItems { + truncatedByResultLimit = true + continue + } allGroups = append(allGroups, group) } @@ -1004,9 +1021,14 @@ func readAllChatListAll(rt *shortcut.RuntimeContext, baseParams map[string]any) } hasMore = pageHasMore if !hasMore { - complete = true + complete = !truncatedByResultLimit nextCursor = nil - stopReason = "source_complete" + if truncatedByResultLimit { + hasMore = true + stopReason = "result_limit" + } else { + stopReason = "source_complete" + } break } nextCursor = page["nextCursor"] @@ -1022,8 +1044,13 @@ func readAllChatListAll(rt *shortcut.RuntimeContext, baseParams map[string]any) seenCursors[nextKey] = true cursorKey = nextKey cursorValue = nextCursor + if maxItems := rt.Int("max-items"); maxItems > 0 && len(allGroups) >= maxItems { + truncatedByResultLimit = true + stopReason = "result_limit" + break + } } - if !complete && hasMore && len(failures) == 0 && pagesFetched >= pageLimit { + if !complete && hasMore && len(failures) == 0 && pagesFetched >= pageLimit && !truncatedByResultLimit { truncatedByPageLimit = true stopReason = "page_limit" } @@ -1033,9 +1060,11 @@ func readAllChatListAll(rt *shortcut.RuntimeContext, baseParams map[string]any) "pagesFetched": pagesFetched, "paginationKnown": true, "complete": complete && len(failures) == 0, "hasMore": hasMore, "stopReason": stopReason, "truncatedByPageLimit": truncatedByPageLimit, - "failedCount": len(failures), "failures": failures, + "truncatedByResultLimit": truncatedByResultLimit, + "failedCount": len(failures), "failures": failures, "partial": len(failures) > 0 && len(allGroups) > 0, } + chatmsg.ApplyTruncation(payload) if hasMore && nextCursor != nil { payload["nextCursor"] = nextCursor } diff --git a/internal/shortcut/chat/chat_message.go b/internal/shortcut/chat/chat_message.go index b74761cd..536686e6 100644 --- a/internal/shortcut/chat/chat_message.go +++ b/internal/shortcut/chat/chat_message.go @@ -634,6 +634,7 @@ func readAllDirectMessages(rt *shortcut.RuntimeContext, params map[string]any) ( payload["hasMore"] = hasMore payload["stopReason"] = stopReason payload["truncatedByPageLimit"] = truncatedByPageLimit + chatmsg.ApplyTruncation(payload) payload["failedCount"] = len(failures) payload["failures"] = failures payload["partial"] = len(failures) > 0 && len(messages) > 0 diff --git a/internal/shortcut/chat/flag_list_pagination_test.go b/internal/shortcut/chat/flag_list_pagination_test.go index b9b99611..9f58795b 100644 --- a/internal/shortcut/chat/flag_list_pagination_test.go +++ b/internal/shortcut/chat/flag_list_pagination_test.go @@ -73,7 +73,29 @@ func TestCrossPlatformCoverageFlagListPageTokenAndPageLimit(t *testing.T) { if err := json.Unmarshal(output.Bytes(), &payload); err != nil { t.Fatal(err) } - if payload["complete"] != false || payload["nextCursor"] != float64(9) || payload["truncatedByPageLimit"] != true || payload["stopReason"] != "page_limit" { + if payload["complete"] != false || payload["nextCursor"] != float64(9) || payload["truncated"] != true || payload["truncatedByPageLimit"] != true || payload["stopReason"] != "page_limit" { + t.Fatalf("payload = %#v", payload) + } +} + +func TestCrossPlatformCoverageFlagListMaxItemsPublishesStableTruncation(t *testing.T) { + fake := &larkAlignmentCaller{responses: map[string]string{ + "im/list_message_favorites": `{"result":{"items":[{"openMessageId":"msg-1"},{"openMessageId":"msg-2"}],"hasMore":true,"nextCursor":9}}`, + }} + helpers.InitDeps(fake) + root := newPlatformCoverageRoot() + var output bytes.Buffer + root.SetOut(&output) + root.SetArgs([]string{"chat", "+flag-list", "--page-all", "--max-items", "1", "--page-delay", "0"}) + 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) + } + if payload["count"] != float64(1) || payload["truncated"] != true || + payload["truncatedByResultLimit"] != true || payload["stopReason"] != "result_limit" { t.Fatalf("payload = %#v", payload) } } @@ -137,7 +159,11 @@ func TestCrossPlatformCoverageFlagListPaginationValidation(t *testing.T) { {"--size", "31"}, {"--page-token", "not-a-number"}, {"--page-limit", "2"}, + {"--max-items", "1"}, + {"--page-delay", "1"}, {"--page-all", "--page-limit", "0"}, + {"--page-all", "--max-items", "-1"}, + {"--page-all", "--page-delay", "-1"}, {"--page-all", "--page-limit", "501"}, {"--cursor", "1", "--page-token", "2"}, } { diff --git a/internal/shortcut/chat/lark_alignment.go b/internal/shortcut/chat/lark_alignment.go index d741f039..04d7706b 100644 --- a/internal/shortcut/chat/lark_alignment.go +++ b/internal/shortcut/chat/lark_alignment.go @@ -510,21 +510,21 @@ var FlagList = shortcut.Shortcut{ Description: "分页查询当前用户收藏的消息，支持有界自动翻页", Intent: "当你要查看当前用户的 DingTalk message favorite 列表时使用；默认读取一页，明确要求全部收藏时加 --page-all，并用 --page-limit 保持有界。底层实际使用数字 cursor，结果按 openMessageId 去重并公开 complete、hasMore、nextCursor、stopReason 和 failures；它不把 message favorite 与 Pin、会话置顶或 Lark feed-layer thread flag 混为一谈。", Risk: shortcut.RiskRead, - Flags: []shortcut.Flag{ + Flags: append([]shortcut.Flag{ {Name: "page-size", Type: shortcut.FlagInt, Default: "20", Desc: "每页数量；下游真实上限为 30，显式页大小必须在 1-30 之间"}, {Name: "size", Type: shortcut.FlagInt, Default: "20", Desc: "--page-size 的兼容别名；下游真实上限为 30，显式页大小必须在 1-30 之间"}, {Name: "page-token", Type: shortcut.FlagString, Desc: "Lark 对齐的起始分页参数；起始 cursor 必须是非负整数"}, {Name: "cursor", Type: shortcut.FlagInt, Default: "0", Desc: "钉钉数字分页游标；起始 cursor 必须是非负整数"}, - {Name: "page-all", Type: shortcut.FlagBool, Desc: "自动读取全部收藏分页；--page-limit 仅与 --page-all 一起使用且范围 1-500"}, + {Name: "page-all", Type: shortcut.FlagBool, Desc: "自动读取全部收藏分页；--page-limit 仅与 --page-all 一起使用且范围 1-500；--max-items/--page-delay 仅与 --page-all 一起使用；值必须大于等于 0"}, {Name: "page-limit", Type: shortcut.FlagInt, Default: "20", Desc: "--page-limit 仅与 --page-all 一起使用且范围 1-500"}, - }, - Constraints: []shortcut.Constraint{ + }, shortcut.AutoPageControlFlags()...), + Constraints: append([]shortcut.Constraint{ {Kind: shortcut.ConstraintMutuallyExclusive, Flags: []string{"page-size", "size"}}, {Kind: shortcut.ConstraintMutuallyExclusive, Flags: []string{"page-token", "cursor"}}, {Kind: shortcut.ConstraintCustom, Flags: []string{"page-size", "size"}, Description: "显式页大小必须在 1-30 之间"}, {Kind: shortcut.ConstraintCustom, Flags: []string{"page-token", "cursor"}, Description: "起始 cursor 必须是非负整数"}, {Kind: shortcut.ConstraintCustom, Flags: []string{"page-all", "page-limit"}, Description: "--page-limit 仅与 --page-all 一起使用且范围 1-500"}, - }, + }, shortcut.AutoPageControlConstraints()...), Tips: []string{ `dws chat +flag-list --cursor 0 --page-size 20`, `dws chat +flag-list --page-size 30 --page-all --page-limit 20`, @@ -548,6 +548,9 @@ func validateFlagList(rt *shortcut.RuntimeContext) error { return apperrors.NewValidation("--page-limit 必须在 1-500 之间") } } + if err := shortcut.ValidateAutoPageControls(rt); err != nil { + return apperrors.NewValidation(err.Error()) + } return nil } @@ -598,8 +601,18 @@ func executeFlagList(rt *shortcut.RuntimeContext) error { var cursorErr error stopReason := "source_complete" truncatedByPageLimit := false + truncatedByResultLimit := false for pagesFetched < pageLimit { + if pagesFetched > 0 { + if err := shortcut.WaitAutoPageDelay(rt); err != nil { + failures = append(failures, map[string]any{ + "page": pagesFetched + 1, "stage": "delay", "cursor": cursor, "error": err.Error(), + }) + stopReason = "delay_interrupted" + break + } + } data, callErr := rt.CallMCPData("im", "list_message_favorites", flagListRequestParams(cursor, pageSize)) if callErr != nil { if pagesFetched == 0 { @@ -621,6 +634,10 @@ func executeFlagList(rt *shortcut.RuntimeContext) error { if messageID != "" { seenMessages[messageID] = true } + if maxItems := rt.Int("max-items"); maxItems > 0 && len(items) >= maxItems { + truncatedByResultLimit = true + continue + } items = append(items, item) } @@ -650,9 +667,14 @@ func executeFlagList(rt *shortcut.RuntimeContext) error { } hasMore = pageHasMore if !hasMore { - complete = true + complete = !truncatedByResultLimit nextCursor = 0 - stopReason = "source_complete" + if truncatedByResultLimit { + hasMore = true + stopReason = "result_limit" + } else { + stopReason = "source_complete" + } break } if cursorErr != nil || nextCursor <= 0 || seenCursors[nextCursor] { @@ -669,28 +691,35 @@ func executeFlagList(rt *shortcut.RuntimeContext) error { } seenCursors[nextCursor] = true cursor = nextCursor + if maxItems := rt.Int("max-items"); maxItems > 0 && len(items) >= maxItems { + truncatedByResultLimit = true + stopReason = "result_limit" + break + } } - if !complete && hasMore && len(failures) == 0 && pagesFetched >= pageLimit { + if !complete && hasMore && len(failures) == 0 && pagesFetched >= pageLimit && !truncatedByResultLimit { truncatedByPageLimit = rt.Bool("page-all") if truncatedByPageLimit { stopReason = "page_limit" } } payload := map[string]any{ - "count": len(items), - "items": items, - "pagesFetched": pagesFetched, - "paginationKnown": paginationKnown, - "complete": complete && len(failures) == 0, - "hasMore": hasMore, - "nextCursor": nextCursor, - "stopReason": stopReason, - "truncatedByPageLimit": truncatedByPageLimit, - "failedCount": len(failures), - "failures": failures, - "partial": len(failures) > 0 && len(items) > 0, + "count": len(items), + "items": items, + "pagesFetched": pagesFetched, + "paginationKnown": paginationKnown, + "complete": complete && len(failures) == 0, + "hasMore": hasMore, + "nextCursor": nextCursor, + "stopReason": stopReason, + "truncatedByPageLimit": truncatedByPageLimit, + "truncatedByResultLimit": truncatedByResultLimit, + "failedCount": len(failures), + "failures": failures, + "partial": len(failures) > 0 && len(items) > 0, } + chatmsg.ApplyTruncation(payload) if outputErr := rt.Output(payload); outputErr != nil { return outputErr } diff --git a/internal/shortcut/chat/remaining_read_pagination_test.go b/internal/shortcut/chat/remaining_read_pagination_test.go index 821ba023..66b8d235 100644 --- a/internal/shortcut/chat/remaining_read_pagination_test.go +++ b/internal/shortcut/chat/remaining_read_pagination_test.go @@ -95,7 +95,29 @@ func TestCrossPlatformCoverageDirectMessagesPageLimitPublishesExecutableContinua t.Fatal(err) } if payload["complete"] != false || payload["hasMore"] != true || - payload["truncatedByPageLimit"] != true || payload["stopReason"] != "page_limit" || payload["nextPage"] == nil { + payload["truncated"] != true || payload["truncatedByPageLimit"] != true || payload["stopReason"] != "page_limit" || payload["nextPage"] == nil { + t.Fatalf("payload = %#v", payload) + } +} + +func TestCrossPlatformCoverageChatListAllMaxItemsPublishesStableTruncation(t *testing.T) { + fake := &larkAlignmentCaller{responses: map[string]string{ + "im/list_my_groups_pagination": `{"result":{"groups":[{"openConversationId":"g1"},{"openConversationId":"g2"}],"hasMore":true,"nextCursor":88}}`, + }} + helpers.InitDeps(fake) + root := newPlatformCoverageRoot() + var output bytes.Buffer + root.SetOut(&output) + root.SetArgs([]string{"chat", "+chat-list-all", "--page-all", "--max-items", "1", "--page-delay", "0"}) + 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) + } + if payload["count"] != float64(1) || payload["truncated"] != true || + payload["truncatedByResultLimit"] != true || payload["stopReason"] != "result_limit" { t.Fatalf("payload = %#v", payload) } } diff --git a/internal/shortcut/chatmsg/chatmsg.go b/internal/shortcut/chatmsg/chatmsg.go index eab58e2f..a94ed2c5 100644 --- a/internal/shortcut/chatmsg/chatmsg.go +++ b/internal/shortcut/chatmsg/chatmsg.go @@ -86,6 +86,7 @@ var messageResultContractV1 = MessageResultContract{ "hasMore", "nextPage", "stopReason", + "truncated", "truncatedByPageLimit", "truncatedByResultLimit", "failedCount", @@ -122,9 +123,21 @@ func NewMessageListPayload(messages []map[string]any) map[string]any { "failedCount": 0, "failures": []map[string]any{}, "partial": false, + "truncated": false, } } +// ApplyTruncation publishes the stable aggregate bit while preserving the +// established reason-specific fields for compatibility and diagnosis. +func ApplyTruncation(payload map[string]any) { + if payload == nil { + return + } + byPage, _ := payload["truncatedByPageLimit"].(bool) + byItems, _ := payload["truncatedByResultLimit"].(bool) + payload["truncated"] = byPage || byItems +} + // ListMessageItems returns message rows from the common list response envelopes. func ListMessageItems(data map[string]any) []map[string]any { if data == nil { diff --git a/internal/shortcut/pagination.go b/internal/shortcut/pagination.go new file mode 100644 index 00000000..451d8ec1 --- /dev/null +++ b/internal/shortcut/pagination.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 shortcut + +import ( + "fmt" + "time" +) + +// AutoPageControlFlags returns the shared item and pacing controls used by +// cursor-based shortcuts. Callers retain ownership of --page-all and their +// product-specific --page-limit defaults. +func AutoPageControlFlags() []Flag { + const evidence = "--max-items/--page-delay 仅与 --page-all 一起使用；值必须大于等于 0" + return []Flag{ + {Name: "max-items", Type: FlagInt, Desc: "自动翻页最多返回条数（默认 0 表示不限制）。" + evidence}, + {Name: "page-delay", Type: FlagInt, Desc: "自动翻页每页之间等待毫秒数（默认 0 表示不等待）。" + evidence}, + } +} + +// AutoPageControlConstraints publishes the runtime-only relationship between +// the pagination switch and its shared controls into Help and Schema. +func AutoPageControlConstraints() []Constraint { + return []Constraint{{ + Kind: ConstraintCustom, + Flags: []string{"page-all", "max-items", "page-delay"}, + Description: "--max-items/--page-delay 仅与 --page-all 一起使用；值必须大于等于 0", + }} +} + +// ValidateAutoPageControls enforces the shared contract while preserving the +// historical behavior that defaulted controls do not activate pagination. +func ValidateAutoPageControls(rt *RuntimeContext) error { + if !rt.Bool("page-all") { + if rt.Changed("max-items") || rt.Changed("page-delay") { + return fmt.Errorf("--max-items/--page-delay 仅与 --page-all 一起使用") + } + return nil + } + if rt.Int("max-items") < 0 { + return fmt.Errorf("--max-items 必须大于等于 0") + } + if rt.Int("page-delay") < 0 { + return fmt.Errorf("--page-delay 必须大于等于 0") + } + return nil +} + +// WaitAutoPageDelay waits between successful pages and remains cancellable so +// a throttled pagination run cannot ignore command cancellation. +func WaitAutoPageDelay(rt *RuntimeContext) error { + delayMS := rt.Int("page-delay") + if delayMS <= 0 { + return nil + } + timer := time.NewTimer(time.Duration(delayMS) * time.Millisecond) + defer timer.Stop() + select { + case <-rt.Command().Context().Done(): + return rt.Command().Context().Err() + case <-timer.C: + return nil + } +} diff --git a/internal/shortcut/smart/at_me.go b/internal/shortcut/smart/at_me.go index 881c0206..155100f5 100644 --- a/internal/shortcut/smart/at_me.go +++ b/internal/shortcut/smart/at_me.go @@ -93,23 +93,23 @@ var AtMe = shortcut.Shortcut{ }, }, }, - Flags: append([]shortcut.Flag{ + Flags: append(append([]shortcut.Flag{ {Name: "group", Type: shortcut.FlagString, Desc: "仅查看指定群；可传 openConversationId 或群名"}, {Name: "chat-query", Type: shortcut.FlagString, Desc: "--group 的旧版自然名称入口", Hidden: true}, {Name: "group-query", Type: shortcut.FlagString, Desc: "--chat-query 的兼容别名", Hidden: true}, {Name: "days", Type: shortcut.FlagInt, Desc: "回溯天数（默认 7）；--days 必须在 1-3650 之间", Default: "7", Required: false}, {Name: "limit", Type: shortcut.FlagInt, Desc: "每页返回数量（默认 50）；--limit 必须大于 0", Default: "50"}, {Name: "cursor", Type: shortcut.FlagString, Desc: "分页游标，翻页传上次的 nextCursor", Default: "0"}, - {Name: "page-all", Type: shortcut.FlagBool, Desc: "沿 nextCursor 自动读取全部 @我 消息；--page-limit 仅与 --page-all 一起使用且范围 1-500"}, + {Name: "page-all", Type: shortcut.FlagBool, Desc: "沿 nextCursor 自动读取全部 @我 消息；--page-limit 仅与 --page-all 一起使用且范围 1-500；--max-items/--page-delay 仅与 --page-all 一起使用；值必须大于等于 0"}, {Name: "page-limit", Type: shortcut.FlagInt, Default: "50", Desc: "--page-limit 仅与 --page-all 一起使用且范围 1-500"}, {Name: "no-reactions", Type: shortcut.FlagBool, Desc: "不输出消息 reaction（默认输出）"}, - }, chatshortcut.MessageResourceDownloadFlags()...), - Constraints: append([]shortcut.Constraint{ + }, shortcut.AutoPageControlFlags()...), chatshortcut.MessageResourceDownloadFlags()...), + Constraints: append(append([]shortcut.Constraint{ {Kind: shortcut.ConstraintMutuallyExclusive, Flags: []string{"group", "chat-query", "group-query"}}, {Kind: shortcut.ConstraintCustom, Flags: []string{"days"}, Description: "--days 必须在 1-3650 之间"}, {Kind: shortcut.ConstraintCustom, Flags: []string{"limit"}, Description: "--limit 必须大于 0"}, {Kind: shortcut.ConstraintCustom, Flags: []string{"page-all", "page-limit"}, Description: "--page-limit 仅与 --page-all 一起使用且范围 1-500"}, - }, chatshortcut.MessageResourceDownloadConstraints()...), + }, shortcut.AutoPageControlConstraints()...), chatshortcut.MessageResourceDownloadConstraints()...), Tips: []string{ `dws chat +at-me`, `dws chat +at-me --days 3`, @@ -211,6 +211,9 @@ func validateAtMe(rt *shortcut.RuntimeContext) error { return apperrors.NewValidation("--page-limit 必须在 1-500 之间") } } + if err := shortcut.ValidateAutoPageControls(rt); err != nil { + return apperrors.NewValidation(err.Error()) + } return nil } @@ -240,8 +243,18 @@ func readAllAtMePages(rt *shortcut.RuntimeContext, baseParams map[string]any) (m nextCursor := "" stopReason := "source_complete" truncatedByPageLimit := false + truncatedByResultLimit := false for pagesFetched < pageLimit { + if pagesFetched > 0 { + if err := shortcut.WaitAutoPageDelay(rt); err != nil { + failures = append(failures, map[string]any{ + "page": pagesFetched + 1, "stage": "delay", "cursor": cursor, "error": err.Error(), + }) + stopReason = "delay_interrupted" + break + } + } params := make(map[string]any, len(baseParams)) for key, value := range baseParams { params[key] = value @@ -268,6 +281,10 @@ func readAllAtMePages(rt *shortcut.RuntimeContext, baseParams map[string]any) (m if id != "" { seenMessages[id] = true } + if maxItems := rt.Int("max-items"); maxItems > 0 && len(allItems) >= maxItems { + truncatedByResultLimit = true + continue + } allItems = append(allItems, item) } @@ -283,9 +300,14 @@ func readAllAtMePages(rt *shortcut.RuntimeContext, baseParams map[string]any) (m } hasMore = pageHasMore if !hasMore { - complete = true + complete = !truncatedByResultLimit nextCursor = "" - stopReason = "source_complete" + if truncatedByResultLimit { + hasMore = true + stopReason = "result_limit" + } else { + stopReason = "source_complete" + } break } nextCursor = atMeCursorString(page["nextCursor"]) @@ -299,8 +321,13 @@ func readAllAtMePages(rt *shortcut.RuntimeContext, baseParams map[string]any) (m } seenCursors[nextCursor] = true cursor = nextCursor + if maxItems := rt.Int("max-items"); maxItems > 0 && len(allItems) >= maxItems { + truncatedByResultLimit = true + stopReason = "result_limit" + break + } } - if !complete && hasMore && len(failures) == 0 && pagesFetched >= pageLimit { + if !complete && hasMore && len(failures) == 0 && pagesFetched >= pageLimit && !truncatedByResultLimit { truncatedByPageLimit = true stopReason = "page_limit" } @@ -312,9 +339,11 @@ func readAllAtMePages(rt *shortcut.RuntimeContext, baseParams map[string]any) (m payload["hasMore"] = hasMore payload["stopReason"] = stopReason payload["truncatedByPageLimit"] = truncatedByPageLimit + payload["truncatedByResultLimit"] = truncatedByResultLimit payload["failedCount"] = len(failures) payload["failures"] = failures payload["partial"] = len(failures) > 0 && len(allItems) > 0 + chatmsg.ApplyTruncation(payload) if hasMore && nextCursor != "" { payload["nextCursor"] = nextCursor } diff --git a/internal/shortcut/smart/chat_messages.go b/internal/shortcut/smart/chat_messages.go index 7a1e9fe1..461f0f2e 100644 --- a/internal/shortcut/smart/chat_messages.go +++ b/internal/shortcut/smart/chat_messages.go @@ -60,7 +60,7 @@ var ChatMessages = shortcut.Shortcut{ Intent: "当你要读取或导出一个指定群聊或单聊的消息记录时使用；可附带发送者姓名解析，无稳定身份时保留全部消息，唯一解析出稳定身份后按 senderId 筛选同一次读取结果；" + "群聊的 --group 可传群名或 openConversationId，单聊可传 --user 或 --open-dingtalk-id，所有目标参数互斥且必须选一个。自然群名只在唯一解析后读取，多候选会返回结构化 candidates。" + "省略时间参数时默认从当前时间向前读取最近消息；兼容模式可用 --time/--direction，范围模式可用公开可选的 --start/--end/--order（兼容 --start-time/--end-time/--sort），范围语义为 [start,end)。" + - "全量读取用 --page-all，并由 --page-limit/--max-results 保持有界；结果公开 complete、hasMore、nextPage、stopReason、截断和逐页失败，不能把部分结果称为完整。--output 把同一 ledger 原子写为工作目录内 JSON。" + + "全量读取用 --page-all，并由 --page-limit/--max-items 保持有界；结果公开 complete、hasMore、nextPage、stopReason、截断和逐页失败，不能把部分结果称为完整。--output 把同一 ledger 原子写为工作目录内 JSON。" + "默认只读；--download-resources 使用工作目录内安全路径、默认不覆盖和原子落盘。", Risk: shortcut.RiskRead, Safety: contract.SafetySpec{ @@ -86,7 +86,7 @@ var ChatMessages = shortcut.Shortcut{ UseWhen: []string{"当你要读取或导出一个指定群聊或单聊的消息记录时使用；可附带发送者姓名解析，无稳定身份时保留全部消息，唯一解析出稳定身份后按 senderId 筛选同一次读取结果；" + "群聊的 --group 可传群名或 openConversationId，单聊可传 --user 或 --open-dingtalk-id，所有目标参数互斥且必须选一个。自然群名只在唯一解析后读取，多候选会返回结构化 candidates。" + "省略时间参数时默认从当前时间向前读取最近消息；兼容模式可用 --time/--direction，范围模式可用公开可选的 --start/--end/--order（兼容 --start-time/--end-time/--sort），范围语义为 [start,end)。" + - "全量读取用 --page-all，并由 --page-limit/--max-results 保持有界；结果公开 complete、hasMore、nextPage、stopReason、截断和逐页失败，不能把部分结果称为完整。--output 把同一 ledger 原子写为工作目录内 JSON。" + + "全量读取用 --page-all，并由 --page-limit/--max-items 保持有界；结果公开 complete、hasMore、nextPage、stopReason、截断和逐页失败，不能把部分结果称为完整。--output 把同一 ledger 原子写为工作目录内 JSON。" + "默认只读；--download-resources 使用工作目录内安全路径、默认不覆盖和原子落盘。"}, AvoidWhen: []string{"以发送者、关键词、@对象或消息类型为主的直接条件检索优先使用 +search-msg；已有一批精确消息 ID 时使用 +messages-mget。已选择会话读取时可在同一次调用附带发送者姓名，不需要再搜索消息"}, Examples: []string{ @@ -117,9 +117,11 @@ var ChatMessages = shortcut.Shortcut{ {Name: "page-size", Type: shortcut.FlagInt, Desc: "--limit 的兼容别名", Hidden: true}, {Name: "direction", Type: shortcut.FlagString, Enum: []string{"newer", "older"}, Desc: "时间方向 newer/older；省略时为 older，从时间边界向前读取"}, {Name: "no-reactions", Type: shortcut.FlagBool, Desc: "不输出消息 reaction（默认输出）"}, - {Name: "page-all", Type: shortcut.FlagBool, Desc: "沿 typed nextPage.time 自动读取后续页；--page-limit 仅与 --page-all 一起使用且范围 1-500；--max-results 仅与 --page-all 一起使用且不能为负数"}, + {Name: "page-all", Type: shortcut.FlagBool, Desc: "沿 typed nextPage.time 自动读取后续页；--page-limit 仅与 --page-all 一起使用且范围 1-500；--max-items 仅与 --page-all 一起使用且不能为负数；--page-delay 仅与 --page-all 一起使用且不能为负数"}, {Name: "page-limit", Type: shortcut.FlagInt, Default: "50", Desc: "--page-limit 仅与 --page-all 一起使用且范围 1-500"}, - {Name: "max-results", Type: shortcut.FlagInt, Desc: "--max-results 仅与 --page-all 一起使用且不能为负数；0 表示仅受页数上限约束"}, + {Name: "max-items", Type: shortcut.FlagInt, Desc: "自动翻页最多返回条数（默认 0 表示不限制）；--max-items 仅与 --page-all 一起使用且不能为负数"}, + {Name: "max-results", Type: shortcut.FlagInt, Desc: "--max-items 的兼容别名", Hidden: true}, + {Name: "page-delay", Type: shortcut.FlagInt, Desc: "自动翻页每页之间等待毫秒数（默认 0 表示不等待）；--page-delay 仅与 --page-all 一起使用且不能为负数"}, {Name: "output", Shorthand: "o", Type: shortcut.FlagString, Desc: "把完整结构化 ledger 原子写入工作目录内的相对 JSON 文件"}, }, chatshortcut.MessageResourceDownloadFlags()...), Constraints: append([]shortcut.Constraint{ @@ -138,7 +140,8 @@ var ChatMessages = shortcut.Shortcut{ {Kind: shortcut.ConstraintCustom, Flags: []string{"order", "sort"}, Description: "asc 必须指定 --start/--start-time"}, {Kind: shortcut.ConstraintCustom, Flags: []string{"limit"}, Description: "显式页大小必须大于 0"}, {Kind: shortcut.ConstraintCustom, Flags: []string{"page-all", "page-limit"}, Description: "--page-limit 仅与 --page-all 一起使用且范围 1-500"}, - {Kind: shortcut.ConstraintCustom, Flags: []string{"page-all", "max-results"}, Description: "--max-results 仅与 --page-all 一起使用且不能为负数"}, + {Kind: shortcut.ConstraintCustom, Flags: []string{"page-all", "max-items"}, Description: "--max-items 仅与 --page-all 一起使用且不能为负数"}, + {Kind: shortcut.ConstraintCustom, Flags: []string{"page-all", "page-delay"}, Description: "--page-delay 仅与 --page-all 一起使用且不能为负数"}, { Kind: shortcut.ConstraintCustom, Flags: []string{"output", "overwrite"}, @@ -179,16 +182,25 @@ func validateChatMessages(rt *shortcut.RuntimeContext) error { return localChatOptionError("invalid_page_size", "+chat-messages 的 --"+name+" 必须大于 0", "--"+name) } } - if !rt.Bool("page-all") && (rt.Changed("page-limit") || rt.Changed("max-results")) { - return apperrors.NewValidation("--page-limit/--max-results 仅与 --page-all 一起使用") + if !rt.Bool("page-all") && rt.Changed("page-limit") { + return apperrors.NewValidation("--page-limit 仅与 --page-all 一起使用") + } + if !rt.Bool("page-all") && rt.Changed("max-results") { + return apperrors.NewValidation("--max-results 仅与 --page-all 一起使用") } if rt.Bool("page-all") { if pageLimit := rt.Int("page-limit"); pageLimit < 1 || pageLimit > chatMessagesHardPageLimit { return apperrors.NewValidation("--page-limit 必须在 1-500 之间") } - if rt.Int("max-results") < 0 { - return apperrors.NewValidation("--max-results 不能小于 0") - } + } + if err := shortcut.ValidateAutoPageControls(rt); err != nil { + return apperrors.NewValidation(err.Error()) + } + if rt.Int("max-results") < 0 { + return apperrors.NewValidation("--max-results 不能小于 0") + } + if rt.Changed("max-items") && rt.Changed("max-results") { + return apperrors.NewValidation("--max-items 与 --max-results 不能同时使用") } if rt.Changed("output") { if err := chatshortcut.ValidateMessageExportOutput(rt.Str("output")); err != nil { @@ -477,7 +489,7 @@ func collectOneChatMessagesPage(rt *shortcut.RuntimeContext, request chatMessage func collectAllChatMessages(rt *shortcut.RuntimeContext, request chatMessagesRequest) (map[string]any, []map[string]any, error) { pageLimit := defaultChatPageLimit(rt.Int("page-limit"), chatMessagesDefaultPageLimit) - maxResults := rt.Int("max-results") + maxResults := rt.IntFirst("max-items", "max-results") basePageSize, _ := request.params["limit"].(int) if basePageSize <= 0 { basePageSize = chatMessagesAllPageSize @@ -496,6 +508,15 @@ func collectAllChatMessages(rt *shortcut.RuntimeContext, request chatMessagesReq var nextPage map[string]any for pagesFetched < pageLimit { + if pagesFetched > 0 { + if delayErr := shortcut.WaitAutoPageDelay(rt); delayErr != nil { + failures = append(failures, map[string]any{ + "page": pagesFetched + 1, "stage": "delay", "error": delayErr.Error(), + }) + stopReason = "delay_interrupted" + break + } + } request.params["limit"] = basePageSize if maxResults > 0 { remaining := maxResults - len(allItems) @@ -656,6 +677,7 @@ func collectAllChatMessages(rt *shortcut.RuntimeContext, request chatMessagesReq payload["stopReason"] = stopReason payload["truncatedByPageLimit"] = truncatedByPageLimit payload["truncatedByResultLimit"] = truncatedByResultLimit + chatmsg.ApplyTruncation(payload) payload["failedCount"] = len(failures) payload["failures"] = failures payload["partial"] = len(failures) > 0 && len(results) > 0 diff --git a/internal/shortcut/smart/chat_messages_test.go b/internal/shortcut/smart/chat_messages_test.go index f94438fb..9576755a 100644 --- a/internal/shortcut/smart/chat_messages_test.go +++ b/internal/shortcut/smart/chat_messages_test.go @@ -613,21 +613,23 @@ func TestCrossPlatformCoverageChatMessagesAdditionalCollectionEdges(t *testing.T }) t.Run("terminal result limit and unsafe continuation", func(t *testing.T) { - caller := &chatMessagesPagingCaller{responses: []string{ - `{"result":{"hasMore":true,"messages":[{"openMessageId":"m2","createTime":"2026-01-03 00:00:00"},{"openMessageId":"m1","createTime":"2026-01-02 00:00:00"},{"openMessageId":"old","createTime":"2026-01-01 00:00:00"}]}}`, - }} - payload, _, err := collectAllChatMessages( - runtimeWith(t, caller, map[string]string{"max-results": "1"}), - chatMessagesRequest{tool: "list_conversation_message_v2", params: map[string]any{}, direction: "older", timeRange: configuredRange}, - ) - if err != nil || payload["truncatedByResultLimit"] != true || payload["stopReason"] != "result_limit" { - t.Fatalf("payload=%#v err=%v", payload, err) + for _, flag := range []string{"max-items", "max-results"} { + caller := &chatMessagesPagingCaller{responses: []string{ + `{"result":{"hasMore":true,"messages":[{"openMessageId":"m2","createTime":"2026-01-03 00:00:00"},{"openMessageId":"m1","createTime":"2026-01-02 00:00:00"},{"openMessageId":"old","createTime":"2026-01-01 00:00:00"}]}}`, + }} + payload, _, err := collectAllChatMessages( + runtimeWith(t, caller, map[string]string{flag: "1"}), + chatMessagesRequest{tool: "list_conversation_message_v2", params: map[string]any{}, direction: "older", timeRange: configuredRange}, + ) + if err != nil || payload["truncated"] != true || payload["truncatedByResultLimit"] != true || payload["stopReason"] != "result_limit" { + t.Fatalf("%s payload=%#v err=%v", flag, payload, err) + } } - caller = &chatMessagesPagingCaller{responses: []string{ + caller := &chatMessagesPagingCaller{responses: []string{ `{"result":{"hasMore":true,"messages":[{"openMessageId":"m1","createTime":"2026-01-03 00:00:00"}]}}`, }} - payload, _, err = collectAllChatMessages( + payload, _, err := collectAllChatMessages( runtimeWith(t, caller, map[string]string{"max-results": "1"}), chatMessagesRequest{tool: "list_conversation_message_v2", params: map[string]any{}, direction: "older"}, ) diff --git a/internal/shortcut/smart/my_groups.go b/internal/shortcut/smart/my_groups.go index 1c2d3ae9..072fdcbb 100644 --- a/internal/shortcut/smart/my_groups.go +++ b/internal/shortcut/smart/my_groups.go @@ -84,17 +84,17 @@ var MyGroups = shortcut.Shortcut{ }, }, }, - Flags: []shortcut.Flag{ + Flags: append([]shortcut.Flag{ {Name: "type", Type: shortcut.FlagString, Desc: "按群类型过滤（可选，如返回中的 groupType/conversationType，大小写不敏感）", Required: false}, {Name: "limit", Type: shortcut.FlagInt, Desc: "每页返回数量（默认 200）；--limit 必须在 1-200 之间", Default: "200"}, {Name: "cursor", Type: shortcut.FlagString, Desc: "分页游标，翻页传上次的 nextCursor"}, - {Name: "page-all", Type: shortcut.FlagBool, Desc: "沿 nextCursor 自动读取全部已加入群；--page-limit 仅与 --page-all 一起使用且范围 1-500"}, + {Name: "page-all", Type: shortcut.FlagBool, Desc: "沿 nextCursor 自动读取全部已加入群；--page-limit 仅与 --page-all 一起使用且范围 1-500；--max-items/--page-delay 仅与 --page-all 一起使用；值必须大于等于 0"}, {Name: "page-limit", Type: shortcut.FlagInt, Default: "50", Desc: "--page-limit 仅与 --page-all 一起使用且范围 1-500"}, - }, - Constraints: []shortcut.Constraint{ + }, shortcut.AutoPageControlFlags()...), + Constraints: append([]shortcut.Constraint{ {Kind: shortcut.ConstraintCustom, Flags: []string{"limit"}, Description: "--limit 必须在 1-200 之间"}, {Kind: shortcut.ConstraintCustom, Flags: []string{"page-all", "page-limit"}, Description: "--page-limit 仅与 --page-all 一起使用且范围 1-500"}, - }, + }, shortcut.AutoPageControlConstraints()...), Tips: []string{ `dws chat +my-groups`, `dws chat +my-groups --type group`, @@ -116,6 +116,9 @@ func validateMyGroups(rt *shortcut.RuntimeContext) error { return apperrors.NewValidation("--page-limit 必须在 1-500 之间") } } + if err := shortcut.ValidateAutoPageControls(rt); err != nil { + return apperrors.NewValidation(err.Error()) + } return nil } @@ -180,9 +183,20 @@ func readAllMyGroups(rt *shortcut.RuntimeContext, baseParams map[string]any) (ma hasMore := false stopReason := "source_complete" truncatedByPageLimit := false + truncatedByResultLimit := false + eligibleCount := 0 var nextCursor any for pagesFetched < pageLimit { + if pagesFetched > 0 { + if err := shortcut.WaitAutoPageDelay(rt); err != nil { + failures = append(failures, map[string]any{ + "page": pagesFetched + 1, "stage": "delay", "cursor": cursorKey, "error": err.Error(), + }) + stopReason = "delay_interrupted" + break + } + } params := map[string]any{"limit": baseParams["limit"]} if cursorKey != "0" { params["cursor"] = cursorValue @@ -208,7 +222,14 @@ func readAllMyGroups(rt *shortcut.RuntimeContext, baseParams map[string]any) (ma if id != "" { seenGroups[id] = true } + if maxItems := rt.Int("max-items"); maxItems > 0 && myGroupsMatchesFilter(rt, group) && eligibleCount >= maxItems { + truncatedByResultLimit = true + continue + } allGroups = append(allGroups, group) + if myGroupsMatchesFilter(rt, group) { + eligibleCount++ + } } page := chatmsg.Pagination(data) @@ -223,9 +244,14 @@ func readAllMyGroups(rt *shortcut.RuntimeContext, baseParams map[string]any) (ma } hasMore = pageHasMore if !hasMore { - complete = true + complete = !truncatedByResultLimit nextCursor = nil - stopReason = "source_complete" + if truncatedByResultLimit { + hasMore = true + stopReason = "result_limit" + } else { + stopReason = "source_complete" + } break } nextCursor = page["nextCursor"] @@ -241,8 +267,13 @@ func readAllMyGroups(rt *shortcut.RuntimeContext, baseParams map[string]any) (ma seenCursors[nextKey] = true cursorKey = nextKey cursorValue = nextCursor + if maxItems := rt.Int("max-items"); maxItems > 0 && eligibleCount >= maxItems { + truncatedByResultLimit = true + stopReason = "result_limit" + break + } } - if !complete && hasMore && len(failures) == 0 && pagesFetched >= pageLimit { + if !complete && hasMore && len(failures) == 0 && pagesFetched >= pageLimit && !truncatedByResultLimit { truncatedByPageLimit = true stopReason = "page_limit" } @@ -254,9 +285,11 @@ func readAllMyGroups(rt *shortcut.RuntimeContext, baseParams map[string]any) (ma payload["hasMore"] = hasMore payload["stopReason"] = stopReason payload["truncatedByPageLimit"] = truncatedByPageLimit + payload["truncatedByResultLimit"] = truncatedByResultLimit payload["failedCount"] = len(failures) payload["failures"] = failures payload["partial"] = len(failures) > 0 && len(allGroups) > 0 + chatmsg.ApplyTruncation(payload) if hasMore && nextCursor != nil { payload["nextCursor"] = nextCursor } @@ -275,6 +308,15 @@ func readAllMyGroups(rt *shortcut.RuntimeContext, baseParams map[string]any) (ma ) } +func myGroupsMatchesFilter(rt *shortcut.RuntimeContext, group map[string]any) bool { + typeFilter := strings.TrimSpace(rt.Str("type")) + if typeFilter == "" { + return true + } + groupType, _ := myGroupsProject(group)["type"].(string) + return strings.EqualFold(strings.TrimSpace(groupType), typeFilter) +} + func myGroupsCursorString(value any) string { if value == nil { return "" diff --git a/internal/shortcut/smart/remaining_read_pagination_test.go b/internal/shortcut/smart/remaining_read_pagination_test.go index 2400d989..65f29f6f 100644 --- a/internal/shortcut/smart/remaining_read_pagination_test.go +++ b/internal/shortcut/smart/remaining_read_pagination_test.go @@ -83,6 +83,28 @@ func TestCrossPlatformCoverageAtMePageAllContinuesAcrossEmptyIntermediatePage(t } } +func TestCrossPlatformCoverageAtMeMaxItemsPublishesStableTruncation(t *testing.T) { + caller := &chatMessagesPagingCaller{responses: []string{ + `{"result":{"conversationMessagesList":[{"messages":[{"openMessageId":"m1"},{"openMessageId":"m2"}]}],"hasMore":true,"nextCursor":"cursor-2"}}`, + }} + helpers.InitDeps(caller) + root := newPlatformCoverageRoot() + var output bytes.Buffer + root.SetOut(&output) + root.SetArgs([]string{"chat", "+at-me", "--page-all", "--max-items", "1", "--page-delay", "0"}) + 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) + } + if payload["count"] != float64(1) || payload["truncated"] != true || + payload["truncatedByResultLimit"] != true || payload["stopReason"] != "result_limit" { + t.Fatalf("payload = %#v", payload) + } +} + func TestCrossPlatformCoverageMyGroupsPageAllUsesNumericCursorAndFiltersAfterMerge(t *testing.T) { caller := &chatMessagesPagingCaller{responses: []string{ `{"result":{"groups":[{"openConversationId":"g1","title":"群一","groupType":"group"}],"hasMore":true,"nextCursor":88}}`, @@ -109,12 +131,38 @@ func TestCrossPlatformCoverageMyGroupsPageAllUsesNumericCursorAndFiltersAfterMer } } +func TestCrossPlatformCoverageMyGroupsMaxItemsAppliesAfterTypeFilter(t *testing.T) { + caller := &chatMessagesPagingCaller{responses: []string{ + `{"result":{"groups":[{"openConversationId":"p1","groupType":"p2p"},{"openConversationId":"g1","groupType":"group"},{"openConversationId":"g2","groupType":"group"}],"hasMore":false}}`, + }} + helpers.InitDeps(caller) + root := newPlatformCoverageRoot() + var output bytes.Buffer + root.SetOut(&output) + root.SetArgs([]string{"chat", "+my-groups", "--type", "group", "--page-all", "--max-items", "1", "--page-delay", "0"}) + 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) + } + if payload["count"] != float64(1) || payload["truncated"] != true || + payload["truncatedByResultLimit"] != true || payload["stopReason"] != "result_limit" { + t.Fatalf("payload = %#v", payload) + } +} + func TestCrossPlatformCoverageRemainingReadPaginationValidation(t *testing.T) { for _, args := range [][]string{ {"chat", "+at-me", "--page-limit", "2"}, + {"chat", "+at-me", "--max-items", "1"}, + {"chat", "+at-me", "--page-delay", "1"}, {"chat", "+at-me", "--page-all", "--page-limit", "501"}, + {"chat", "+at-me", "--page-all", "--max-items", "-1"}, {"chat", "+my-groups", "--limit", "201"}, {"chat", "+my-groups", "--page-limit", "2"}, + {"chat", "+my-groups", "--max-items", "1"}, {"chat", "+my-groups", "--page-all", "--page-limit", "501"}, } { helpers.InitDeps(&chatMessagesPagingCaller{}) diff --git a/internal/shortcut/smart/thread_replies.go b/internal/shortcut/smart/thread_replies.go index 53697622..e98aae20 100644 --- a/internal/shortcut/smart/thread_replies.go +++ b/internal/shortcut/smart/thread_replies.go @@ -463,6 +463,7 @@ func collectAllThreadReplies(rt *shortcut.RuntimeContext, params map[string]any) payload["hasMore"] = hasMore payload["stopReason"] = stopReason payload["truncatedByPageLimit"] = truncatedByPageLimit + chatmsg.ApplyTruncation(payload) payload["failedCount"] = len(failures) payload["failures"] = failures payload["partial"] = len(failures) > 0 && len(allItems) > 0 diff --git a/skills/multi/dingtalk-chat/references/contracts.md b/skills/multi/dingtalk-chat/references/contracts.md index 9c1fb723..9fce4598 100644 --- a/skills/multi/dingtalk-chat/references/contracts.md +++ b/skills/multi/dingtalk-chat/references/contracts.md @@ -12,11 +12,11 @@ - `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`, `resolvedFilters`, `queryRange`, `pagesFetched`, `paginationKnown`, `complete`, `hasMore`, `nextPage`, `stopReason`, `truncatedByPageLimit`, `truncatedByResultLimit`, `failedCount`, `failures`, `partial`, `scope`, `resourceDownloads` +- `envelope_fields`: `contractVersion`, `messages`, `count`, `resolvedFilters`, `queryRange`, `pagesFetched`, `paginationKnown`, `complete`, `hasMore`, `nextPage`, `stopReason`, `truncated`, `truncatedByPageLimit`, `truncatedByResultLimit`, `failedCount`, `failures`, `partial`, `scope`, `resourceDownloads` 当 `complete=false` 时不能称为全量成功。`nextPage` 只能来自真实 lower boundary； -`failedCount/failures`、`partial` 和两个 truncated 字段必须原样保留。 +`failedCount/failures`、`partial`、总 `truncated` 和两个原因字段必须原样保留。 当 Runtime 解析并应用自然发送者条件时，`resolvedFilters.senders[]` 保留原查询及选中的 `userId/openDingTalkId`。消息展示名可以与通讯录姓名不同；只能用稳定 `senderId` 与解析结果关联， 不得重新做姓名字符串比较。 From 896801634f2de6fe41de2348e61bbb2dda5ccc86 Mon Sep 17 00:00:00 2001 From: Dennis Date: Wed, 12 Aug 2026 14:59:18 +0800 Subject: [PATCH 5/9] fix(chat): preserve max-results visibility --- internal/shortcut/smart/chat_messages.go | 4 +++- internal/shortcut/smart/chat_messages_test.go | 12 ++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/internal/shortcut/smart/chat_messages.go b/internal/shortcut/smart/chat_messages.go index 461f0f2e..1f7bf413 100644 --- a/internal/shortcut/smart/chat_messages.go +++ b/internal/shortcut/smart/chat_messages.go @@ -120,7 +120,7 @@ var ChatMessages = shortcut.Shortcut{ {Name: "page-all", Type: shortcut.FlagBool, Desc: "沿 typed nextPage.time 自动读取后续页；--page-limit 仅与 --page-all 一起使用且范围 1-500；--max-items 仅与 --page-all 一起使用且不能为负数；--page-delay 仅与 --page-all 一起使用且不能为负数"}, {Name: "page-limit", Type: shortcut.FlagInt, Default: "50", Desc: "--page-limit 仅与 --page-all 一起使用且范围 1-500"}, {Name: "max-items", Type: shortcut.FlagInt, Desc: "自动翻页最多返回条数（默认 0 表示不限制）；--max-items 仅与 --page-all 一起使用且不能为负数"}, - {Name: "max-results", Type: shortcut.FlagInt, Desc: "--max-items 的兼容别名", Hidden: true}, + {Name: "max-results", Type: shortcut.FlagInt, Desc: "--max-items 的公开兼容别名；仅与 --page-all 一起使用且不能为负数"}, {Name: "page-delay", Type: shortcut.FlagInt, Desc: "自动翻页每页之间等待毫秒数（默认 0 表示不等待）；--page-delay 仅与 --page-all 一起使用且不能为负数"}, {Name: "output", Shorthand: "o", Type: shortcut.FlagString, Desc: "把完整结构化 ledger 原子写入工作目录内的相对 JSON 文件"}, }, chatshortcut.MessageResourceDownloadFlags()...), @@ -141,6 +141,8 @@ var ChatMessages = shortcut.Shortcut{ {Kind: shortcut.ConstraintCustom, Flags: []string{"limit"}, Description: "显式页大小必须大于 0"}, {Kind: shortcut.ConstraintCustom, Flags: []string{"page-all", "page-limit"}, Description: "--page-limit 仅与 --page-all 一起使用且范围 1-500"}, {Kind: shortcut.ConstraintCustom, Flags: []string{"page-all", "max-items"}, Description: "--max-items 仅与 --page-all 一起使用且不能为负数"}, + {Kind: shortcut.ConstraintCustom, Flags: []string{"page-all", "max-results"}, Description: "--max-results 仅与 --page-all 一起使用且不能为负数"}, + {Kind: shortcut.ConstraintMutuallyExclusive, Flags: []string{"max-items", "max-results"}}, {Kind: shortcut.ConstraintCustom, Flags: []string{"page-all", "page-delay"}, Description: "--page-delay 仅与 --page-all 一起使用且不能为负数"}, { Kind: shortcut.ConstraintCustom, diff --git a/internal/shortcut/smart/chat_messages_test.go b/internal/shortcut/smart/chat_messages_test.go index 9576755a..c79abd76 100644 --- a/internal/shortcut/smart/chat_messages_test.go +++ b/internal/shortcut/smart/chat_messages_test.go @@ -503,6 +503,18 @@ func chatMessagesRuntimeForTest(t *testing.T, values map[string]string) *shortcu return shortcut.RuntimeContextForTest(cmd, ChatMessages) } +func TestCrossPlatformCoverageChatMessagesKeepsMaxResultsPublic(t *testing.T) { + root := newPlatformCoverageRoot() + cmd, _, err := root.Find([]string{"chat", "+chat-messages"}) + if err != nil { + t.Fatal(err) + } + flag := cmd.Flags().Lookup("max-results") + if flag == nil || flag.Hidden { + t.Fatalf("--max-results must remain a visible compatibility flag: %#v", flag) + } +} + func TestCrossPlatformCoverageChatMessagesAdditionalValidationAndHelpers(t *testing.T) { for _, values := range []map[string]string{ {"time": "2026-01-01", "start": "2026-01-01"}, From eee41a9b459cd96bdf141101a13e0094d4c1a2cc Mon Sep 17 00:00:00 2001 From: Dennis Date: Wed, 12 Aug 2026 15:39:25 +0800 Subject: [PATCH 6/9] fix(chat): preserve safe pagination continuations --- internal/shortcut/chat/chat_conversation.go | 33 +++++++++++- .../chat/chat_conversation_projection_test.go | 51 ++++++++++++++++++- internal/shortcut/chat/chat_group.go | 15 +++++- .../chat/flag_list_pagination_test.go | 5 +- internal/shortcut/chat/lark_alignment.go | 15 +++++- .../chat/remaining_read_pagination_test.go | 5 +- internal/shortcut/pagination.go | 15 ++++++ internal/shortcut/smart/at_me.go | 14 +++++ internal/shortcut/smart/chat_messages.go | 4 +- internal/shortcut/smart/my_groups.go | 15 +++++- .../smart/remaining_read_pagination_test.go | 11 +++- 11 files changed, 171 insertions(+), 12 deletions(-) diff --git a/internal/shortcut/chat/chat_conversation.go b/internal/shortcut/chat/chat_conversation.go index 3c77f8f9..edad1f81 100644 --- a/internal/shortcut/chat/chat_conversation.go +++ b/internal/shortcut/chat/chat_conversation.go @@ -359,6 +359,7 @@ var ConversationList = shortcut.Shortcut{ stopReason := "source_complete" truncatedByPageLimit := false truncatedByResultLimit := false + unsafeResultContinuation := false failures := make([]map[string]any, 0) for pagesFetched < pageLimit { if pagesFetched > 0 { @@ -368,7 +369,7 @@ var ConversationList = shortcut.Shortcut{ break } } - params := map[string]any{"limit": rt.Int("limit")} + params := map[string]any{"limit": shortcut.AutoPageRequestSize(rt, rt.Int("limit"), len(convs))} if cursor > 0 { params["cursor"] = cursor } @@ -384,6 +385,7 @@ var ConversationList = shortcut.Shortcut{ break } pagesFetched++ + overflowOnPage := false for _, conversation := range conversationListProject(data) { id := strings.TrimSpace(fmt.Sprint(conversation["openConversationId"])) if id != "" && id != "" { @@ -394,6 +396,7 @@ var ConversationList = shortcut.Shortcut{ } if maxItems := rt.Int("max-items"); maxItems > 0 && len(convs) >= maxItems { truncatedByResultLimit = true + overflowOnPage = true continue } convs = append(convs, conversation) @@ -405,6 +408,14 @@ var ConversationList = shortcut.Shortcut{ failures = append(failures, map[string]any{"stage": "conversation-pagination", "error": "下层未返回 hasMore，无法证明结果完整"}) break } + if overflowOnPage { + hasMore = true + nextCursor = 0 + unsafeResultContinuation = true + failures = append(failures, map[string]any{"stage": "conversation-pagination", "error": "下层返回条数超过请求的剩余额度，无法生成不跳项的安全续页游标"}) + stopReason = "pagination_error" + break + } if !hasMore { complete = !truncatedByResultLimit if truncatedByResultLimit { @@ -450,7 +461,25 @@ var ConversationList = shortcut.Shortcut{ "partial": len(failures) > 0, } chatmsg.ApplyTruncation(payload) - return rt.Output(payload) + if err := rt.Output(payload); err != nil { + return err + } + if stopReason == "delay_interrupted" && rt.Command().Context().Err() != nil { + return rt.Command().Context().Err() + } + if unsafeResultContinuation { + return apperrors.NewAPI( + fmt.Sprintf("会话列表分页未完成：成功读取 %d 页，存在 %d 个失败项", pagesFetched, len(failures)), + apperrors.WithOperation("im/list_all_conversations"), + apperrors.WithReason("conversation_list_incomplete"), + apperrors.WithOrigin("mcp_gateway"), + apperrors.WithFailureStage("pagination"), + apperrors.WithExecutionStarted(true), + apperrors.WithRetryable(true), + apperrors.WithHint("请根据 failures 和 nextCursor 重试"), + ) + } + return nil }, } diff --git a/internal/shortcut/chat/chat_conversation_projection_test.go b/internal/shortcut/chat/chat_conversation_projection_test.go index cec59a68..320807e1 100644 --- a/internal/shortcut/chat/chat_conversation_projection_test.go +++ b/internal/shortcut/chat/chat_conversation_projection_test.go @@ -15,6 +15,7 @@ package chat import ( "bytes" + "context" "encoding/json" "reflect" "testing" @@ -150,7 +151,7 @@ func TestCrossPlatformCoverageConversationListSinglePagePreservesTypedCursor(t * func TestCrossPlatformCoverageConversationListMaxItemsPublishesStableTruncation(t *testing.T) { fake := &larkAlignmentCaller{responses: map[string]string{ - "im/list_all_conversations": `{"result":{"conversationList":[{"openConversationId":"cid-1"},{"openConversationId":"cid-2"}],"hasMore":true,"nextCursor":2}}`, + "im/list_all_conversations": `{"result":{"conversationList":[{"openConversationId":"cid-1"}],"hasMore":true,"nextCursor":2}}`, }} helpers.InitDeps(fake) root := newPlatformCoverageRoot() @@ -168,6 +169,54 @@ func TestCrossPlatformCoverageConversationListMaxItemsPublishesStableTruncation( payload["truncatedByResultLimit"] != true || payload["stopReason"] != "result_limit" { t.Fatalf("payload = %#v", payload) } + if len(fake.calls) != 1 || fake.calls[0].args["limit"] != 1 || payload["nextCursor"] != float64(2) { + t.Fatalf("unsafe continuation: calls=%#v payload=%#v", fake.calls, payload) + } +} + +func TestCrossPlatformCoverageConversationListRejectsOversizedLimitPage(t *testing.T) { + fake := &larkAlignmentCaller{responses: map[string]string{ + "im/list_all_conversations": `{"result":{"conversationList":[{"openConversationId":"cid-1"},{"openConversationId":"cid-2"}],"hasMore":true,"nextCursor":2}}`, + }} + helpers.InitDeps(fake) + root := newPlatformCoverageRoot() + var output bytes.Buffer + root.SetOut(&output) + root.SetArgs([]string{"chat", "+conversation-list", "--page-all", "--max-items", "1"}) + if err := root.Execute(); err == nil { + t.Fatal("oversized lower page unexpectedly published a safe continuation") + } + var payload map[string]any + if err := json.Unmarshal(output.Bytes(), &payload); err != nil { + t.Fatal(err) + } + if payload["stopReason"] != "pagination_error" || payload["failedCount"] != float64(1) || payload["nextCursor"] != float64(0) { + t.Fatalf("payload = %#v", payload) + } +} + +func TestCrossPlatformCoverageConversationListPropagatesDelayCancellation(t *testing.T) { + fake := &larkAlignmentCaller{responses: map[string]string{ + "im/list_all_conversations": `{"result":{"conversationList":[{"openConversationId":"cid-1"}],"hasMore":true,"nextCursor":2}}`, + }} + helpers.InitDeps(fake) + root := newPlatformCoverageRoot() + ctx, cancel := context.WithCancel(context.Background()) + cancel() + root.SetContext(ctx) + var output bytes.Buffer + root.SetOut(&output) + root.SetArgs([]string{"chat", "+conversation-list", "--page-all", "--page-delay", "1"}) + if err := root.Execute(); err == nil || err != context.Canceled { + t.Fatalf("delay cancellation error = %v, want context.Canceled", err) + } + var payload map[string]any + if err := json.Unmarshal(output.Bytes(), &payload); err != nil { + t.Fatal(err) + } + if payload["stopReason"] != "delay_interrupted" || payload["failedCount"] != float64(1) { + t.Fatalf("payload = %#v", payload) + } } func TestCrossPlatformCoverageConversationListDeduplicatesStableIDs(t *testing.T) { diff --git a/internal/shortcut/chat/chat_group.go b/internal/shortcut/chat/chat_group.go index 6bba8430..731c9c40 100644 --- a/internal/shortcut/chat/chat_group.go +++ b/internal/shortcut/chat/chat_group.go @@ -974,7 +974,8 @@ func readAllChatListAll(rt *shortcut.RuntimeContext, baseParams map[string]any) break } } - params := map[string]any{"limit": baseParams["limit"]} + pageSize, _ := baseParams["limit"].(int) + params := map[string]any{"limit": shortcut.AutoPageRequestSize(rt, pageSize, len(allGroups))} if cursorKey != "0" { params["cursor"] = cursorValue } @@ -991,6 +992,7 @@ func readAllChatListAll(rt *shortcut.RuntimeContext, baseParams map[string]any) } pagesFetched++ pageGroups := chatListAllProject(data) + overflowOnPage := false for _, group := range pageGroups { id := strings.TrimSpace(fmt.Sprint(group["openConversationId"])) if id == "" { @@ -1004,6 +1006,7 @@ func readAllChatListAll(rt *shortcut.RuntimeContext, baseParams map[string]any) } if maxItems := rt.Int("max-items"); maxItems > 0 && len(allGroups) >= maxItems { truncatedByResultLimit = true + overflowOnPage = true continue } allGroups = append(allGroups, group) @@ -1020,6 +1023,16 @@ func readAllChatListAll(rt *shortcut.RuntimeContext, baseParams map[string]any) break } hasMore = pageHasMore + if overflowOnPage { + hasMore = true + nextCursor = nil + failures = append(failures, map[string]any{ + "page": pagesFetched, "stage": "pagination", + "error": "已加入群列表下层返回条数超过请求的剩余额度，无法生成不跳项的安全续页游标", + }) + stopReason = "pagination_error" + break + } if !hasMore { complete = !truncatedByResultLimit nextCursor = nil diff --git a/internal/shortcut/chat/flag_list_pagination_test.go b/internal/shortcut/chat/flag_list_pagination_test.go index 9f58795b..cc7c2ad3 100644 --- a/internal/shortcut/chat/flag_list_pagination_test.go +++ b/internal/shortcut/chat/flag_list_pagination_test.go @@ -80,7 +80,7 @@ func TestCrossPlatformCoverageFlagListPageTokenAndPageLimit(t *testing.T) { func TestCrossPlatformCoverageFlagListMaxItemsPublishesStableTruncation(t *testing.T) { fake := &larkAlignmentCaller{responses: map[string]string{ - "im/list_message_favorites": `{"result":{"items":[{"openMessageId":"msg-1"},{"openMessageId":"msg-2"}],"hasMore":true,"nextCursor":9}}`, + "im/list_message_favorites": `{"result":{"items":[{"openMessageId":"msg-1"}],"hasMore":true,"nextCursor":9}}`, }} helpers.InitDeps(fake) root := newPlatformCoverageRoot() @@ -98,6 +98,9 @@ func TestCrossPlatformCoverageFlagListMaxItemsPublishesStableTruncation(t *testi payload["truncatedByResultLimit"] != true || payload["stopReason"] != "result_limit" { t.Fatalf("payload = %#v", payload) } + if len(fake.calls) != 1 || fake.calls[0].args["size"] != "1" || payload["nextCursor"] != float64(9) { + t.Fatalf("unsafe continuation: calls=%#v payload=%#v", fake.calls, payload) + } } func TestCrossPlatformCoverageFlagListFailureModes(t *testing.T) { diff --git a/internal/shortcut/chat/lark_alignment.go b/internal/shortcut/chat/lark_alignment.go index 04d7706b..c8e01234 100644 --- a/internal/shortcut/chat/lark_alignment.go +++ b/internal/shortcut/chat/lark_alignment.go @@ -613,7 +613,8 @@ func executeFlagList(rt *shortcut.RuntimeContext) error { break } } - data, callErr := rt.CallMCPData("im", "list_message_favorites", flagListRequestParams(cursor, pageSize)) + requestPageSize := shortcut.AutoPageRequestSize(rt, pageSize, len(items)) + data, callErr := rt.CallMCPData("im", "list_message_favorites", flagListRequestParams(cursor, requestPageSize)) if callErr != nil { if pagesFetched == 0 { return callErr @@ -626,6 +627,7 @@ func executeFlagList(rt *shortcut.RuntimeContext) error { } pagesFetched++ pageItems := flagListItems(data) + overflowOnPage := false for _, item := range pageItems { messageID := firstNonEmptyMapString(item, "openMessageId", "messageId", "itemId", "id") if messageID != "" && seenMessages[messageID] { @@ -636,6 +638,7 @@ func executeFlagList(rt *shortcut.RuntimeContext) error { } if maxItems := rt.Int("max-items"); maxItems > 0 && len(items) >= maxItems { truncatedByResultLimit = true + overflowOnPage = true continue } items = append(items, item) @@ -666,6 +669,16 @@ func executeFlagList(rt *shortcut.RuntimeContext) error { } } hasMore = pageHasMore + if overflowOnPage { + hasMore = true + nextCursor = 0 + failures = append(failures, map[string]any{ + "page": pagesFetched, "stage": "pagination", + "error": "收藏列表下层返回条数超过请求的剩余额度，无法生成不跳项的安全续页游标", + }) + stopReason = "pagination_error" + break + } if !hasMore { complete = !truncatedByResultLimit nextCursor = 0 diff --git a/internal/shortcut/chat/remaining_read_pagination_test.go b/internal/shortcut/chat/remaining_read_pagination_test.go index 66b8d235..878d989b 100644 --- a/internal/shortcut/chat/remaining_read_pagination_test.go +++ b/internal/shortcut/chat/remaining_read_pagination_test.go @@ -102,7 +102,7 @@ func TestCrossPlatformCoverageDirectMessagesPageLimitPublishesExecutableContinua func TestCrossPlatformCoverageChatListAllMaxItemsPublishesStableTruncation(t *testing.T) { fake := &larkAlignmentCaller{responses: map[string]string{ - "im/list_my_groups_pagination": `{"result":{"groups":[{"openConversationId":"g1"},{"openConversationId":"g2"}],"hasMore":true,"nextCursor":88}}`, + "im/list_my_groups_pagination": `{"result":{"groups":[{"openConversationId":"g1"}],"hasMore":true,"nextCursor":88}}`, }} helpers.InitDeps(fake) root := newPlatformCoverageRoot() @@ -120,6 +120,9 @@ func TestCrossPlatformCoverageChatListAllMaxItemsPublishesStableTruncation(t *te payload["truncatedByResultLimit"] != true || payload["stopReason"] != "result_limit" { t.Fatalf("payload = %#v", payload) } + if len(fake.calls) != 1 || fake.calls[0].args["limit"] != 1 || payload["nextCursor"] != float64(88) { + t.Fatalf("unsafe continuation: calls=%#v payload=%#v", fake.calls, payload) + } } func TestCrossPlatformCoverageDirectMessagesPageAllFailsClosedWithoutMillisecondCursor(t *testing.T) { diff --git a/internal/shortcut/pagination.go b/internal/shortcut/pagination.go index 451d8ec1..81973344 100644 --- a/internal/shortcut/pagination.go +++ b/internal/shortcut/pagination.go @@ -57,6 +57,21 @@ func ValidateAutoPageControls(rt *RuntimeContext) error { return nil } +// AutoPageRequestSize caps the next lower-page request to the remaining item +// budget. A cursor returned for that request is therefore safe to resume from: +// the CLI did not intentionally discard a suffix of the lower page. +func AutoPageRequestSize(rt *RuntimeContext, pageSize, itemCount int) int { + maxItems := rt.Int("max-items") + if maxItems <= 0 { + return pageSize + } + remaining := maxItems - itemCount + if remaining > 0 && remaining < pageSize { + return remaining + } + return pageSize +} + // WaitAutoPageDelay waits between successful pages and remains cancellable so // a throttled pagination run cannot ignore command cancellation. func WaitAutoPageDelay(rt *RuntimeContext) error { diff --git a/internal/shortcut/smart/at_me.go b/internal/shortcut/smart/at_me.go index 155100f5..1723d4e6 100644 --- a/internal/shortcut/smart/at_me.go +++ b/internal/shortcut/smart/at_me.go @@ -259,6 +259,8 @@ func readAllAtMePages(rt *shortcut.RuntimeContext, baseParams map[string]any) (m for key, value := range baseParams { params[key] = value } + pageSize, _ := baseParams["limit"].(int) + params["limit"] = shortcut.AutoPageRequestSize(rt, pageSize, len(allItems)) params["cursor"] = cursor data, err := rt.CallMCPData("chat", "search_at_me_message", params) if err != nil { @@ -273,6 +275,7 @@ func readAllAtMePages(rt *shortcut.RuntimeContext, baseParams map[string]any) (m } pagesFetched++ pageItems := atMeMessageItems(data) + overflowOnPage := false for _, item := range pageItems { id := chatmsg.StableMessageID(item) if id != "" && seenMessages[id] { @@ -283,6 +286,7 @@ func readAllAtMePages(rt *shortcut.RuntimeContext, baseParams map[string]any) (m } if maxItems := rt.Int("max-items"); maxItems > 0 && len(allItems) >= maxItems { truncatedByResultLimit = true + overflowOnPage = true continue } allItems = append(allItems, item) @@ -299,6 +303,16 @@ func readAllAtMePages(rt *shortcut.RuntimeContext, baseParams map[string]any) (m break } hasMore = pageHasMore + if overflowOnPage { + hasMore = true + nextCursor = "" + failures = append(failures, map[string]any{ + "page": pagesFetched, "stage": "pagination", + "error": "@我消息下层返回条数超过请求的剩余额度，无法生成不跳项的安全续页游标", + }) + stopReason = "pagination_error" + break + } if !hasMore { complete = !truncatedByResultLimit nextCursor = "" diff --git a/internal/shortcut/smart/chat_messages.go b/internal/shortcut/smart/chat_messages.go index 1f7bf413..ed03dd4c 100644 --- a/internal/shortcut/smart/chat_messages.go +++ b/internal/shortcut/smart/chat_messages.go @@ -117,10 +117,10 @@ var ChatMessages = shortcut.Shortcut{ {Name: "page-size", Type: shortcut.FlagInt, Desc: "--limit 的兼容别名", Hidden: true}, {Name: "direction", Type: shortcut.FlagString, Enum: []string{"newer", "older"}, Desc: "时间方向 newer/older；省略时为 older，从时间边界向前读取"}, {Name: "no-reactions", Type: shortcut.FlagBool, Desc: "不输出消息 reaction（默认输出）"}, - {Name: "page-all", Type: shortcut.FlagBool, Desc: "沿 typed nextPage.time 自动读取后续页；--page-limit 仅与 --page-all 一起使用且范围 1-500；--max-items 仅与 --page-all 一起使用且不能为负数；--page-delay 仅与 --page-all 一起使用且不能为负数"}, + {Name: "page-all", Type: shortcut.FlagBool, Desc: "沿 typed nextPage.time 自动读取后续页；--page-limit 仅与 --page-all 一起使用且范围 1-500；--max-items 仅与 --page-all 一起使用且不能为负数；--max-results 仅与 --page-all 一起使用且不能为负数；--page-delay 仅与 --page-all 一起使用且不能为负数"}, {Name: "page-limit", Type: shortcut.FlagInt, Default: "50", Desc: "--page-limit 仅与 --page-all 一起使用且范围 1-500"}, {Name: "max-items", Type: shortcut.FlagInt, Desc: "自动翻页最多返回条数（默认 0 表示不限制）；--max-items 仅与 --page-all 一起使用且不能为负数"}, - {Name: "max-results", Type: shortcut.FlagInt, Desc: "--max-items 的公开兼容别名；仅与 --page-all 一起使用且不能为负数"}, + {Name: "max-results", Type: shortcut.FlagInt, Desc: "--max-items 的公开兼容别名；--max-results 仅与 --page-all 一起使用且不能为负数"}, {Name: "page-delay", Type: shortcut.FlagInt, Desc: "自动翻页每页之间等待毫秒数（默认 0 表示不等待）；--page-delay 仅与 --page-all 一起使用且不能为负数"}, {Name: "output", Shorthand: "o", Type: shortcut.FlagString, Desc: "把完整结构化 ledger 原子写入工作目录内的相对 JSON 文件"}, }, chatshortcut.MessageResourceDownloadFlags()...), diff --git a/internal/shortcut/smart/my_groups.go b/internal/shortcut/smart/my_groups.go index 072fdcbb..0f84432a 100644 --- a/internal/shortcut/smart/my_groups.go +++ b/internal/shortcut/smart/my_groups.go @@ -197,7 +197,8 @@ func readAllMyGroups(rt *shortcut.RuntimeContext, baseParams map[string]any) (ma break } } - params := map[string]any{"limit": baseParams["limit"]} + pageSize, _ := baseParams["limit"].(int) + params := map[string]any{"limit": shortcut.AutoPageRequestSize(rt, pageSize, eligibleCount)} if cursorKey != "0" { params["cursor"] = cursorValue } @@ -214,6 +215,7 @@ func readAllMyGroups(rt *shortcut.RuntimeContext, baseParams map[string]any) (ma } pagesFetched++ pageGroups := myGroupsExtract(data) + overflowOnPage := false for _, group := range pageGroups { id := myGroupsStr(group, "openConversationId", "openConversationID", "conversationId", "openCid", "cid", "id") if id != "" && seenGroups[id] { @@ -224,6 +226,7 @@ func readAllMyGroups(rt *shortcut.RuntimeContext, baseParams map[string]any) (ma } if maxItems := rt.Int("max-items"); maxItems > 0 && myGroupsMatchesFilter(rt, group) && eligibleCount >= maxItems { truncatedByResultLimit = true + overflowOnPage = true continue } allGroups = append(allGroups, group) @@ -243,6 +246,16 @@ func readAllMyGroups(rt *shortcut.RuntimeContext, baseParams map[string]any) (ma break } hasMore = pageHasMore + if overflowOnPage { + hasMore = true + nextCursor = nil + failures = append(failures, map[string]any{ + "page": pagesFetched, "stage": "pagination", + "error": "我的群列表下层返回的匹配条数超过请求的剩余额度，无法生成不跳项的安全续页游标", + }) + stopReason = "pagination_error" + break + } if !hasMore { complete = !truncatedByResultLimit nextCursor = nil diff --git a/internal/shortcut/smart/remaining_read_pagination_test.go b/internal/shortcut/smart/remaining_read_pagination_test.go index 65f29f6f..220da567 100644 --- a/internal/shortcut/smart/remaining_read_pagination_test.go +++ b/internal/shortcut/smart/remaining_read_pagination_test.go @@ -85,7 +85,7 @@ func TestCrossPlatformCoverageAtMePageAllContinuesAcrossEmptyIntermediatePage(t func TestCrossPlatformCoverageAtMeMaxItemsPublishesStableTruncation(t *testing.T) { caller := &chatMessagesPagingCaller{responses: []string{ - `{"result":{"conversationMessagesList":[{"messages":[{"openMessageId":"m1"},{"openMessageId":"m2"}]}],"hasMore":true,"nextCursor":"cursor-2"}}`, + `{"result":{"conversationMessagesList":[{"messages":[{"openMessageId":"m1"}]}],"hasMore":true,"nextCursor":"cursor-2"}}`, }} helpers.InitDeps(caller) root := newPlatformCoverageRoot() @@ -103,6 +103,9 @@ func TestCrossPlatformCoverageAtMeMaxItemsPublishesStableTruncation(t *testing.T payload["truncatedByResultLimit"] != true || payload["stopReason"] != "result_limit" { t.Fatalf("payload = %#v", payload) } + if len(caller.args) != 1 || caller.args[0]["limit"] != 1 || payload["nextCursor"] != "cursor-2" { + t.Fatalf("unsafe continuation: calls=%#v payload=%#v", caller.args, payload) + } } func TestCrossPlatformCoverageMyGroupsPageAllUsesNumericCursorAndFiltersAfterMerge(t *testing.T) { @@ -133,7 +136,8 @@ func TestCrossPlatformCoverageMyGroupsPageAllUsesNumericCursorAndFiltersAfterMer func TestCrossPlatformCoverageMyGroupsMaxItemsAppliesAfterTypeFilter(t *testing.T) { caller := &chatMessagesPagingCaller{responses: []string{ - `{"result":{"groups":[{"openConversationId":"p1","groupType":"p2p"},{"openConversationId":"g1","groupType":"group"},{"openConversationId":"g2","groupType":"group"}],"hasMore":false}}`, + `{"result":{"groups":[{"openConversationId":"p1","groupType":"p2p"}],"hasMore":true,"nextCursor":2}}`, + `{"result":{"groups":[{"openConversationId":"g1","groupType":"group"}],"hasMore":true,"nextCursor":3}}`, }} helpers.InitDeps(caller) root := newPlatformCoverageRoot() @@ -151,6 +155,9 @@ func TestCrossPlatformCoverageMyGroupsMaxItemsAppliesAfterTypeFilter(t *testing. payload["truncatedByResultLimit"] != true || payload["stopReason"] != "result_limit" { t.Fatalf("payload = %#v", payload) } + if len(caller.args) != 2 || caller.args[0]["limit"] != 1 || caller.args[1]["limit"] != 1 || payload["nextCursor"] != float64(3) { + t.Fatalf("unsafe filtered continuation: calls=%#v payload=%#v", caller.args, payload) + } } func TestCrossPlatformCoverageRemainingReadPaginationValidation(t *testing.T) { From d07bf39586c3162047a47f78c6264afe3792059d Mon Sep 17 00:00:00 2001 From: Dennis Date: Wed, 12 Aug 2026 16:43:55 +0800 Subject: [PATCH 7/9] fix(chat): bound automatic page delays --- internal/shortcut/chat/chat_conversation.go | 6 +- .../chat/chat_conversation_projection_test.go | 21 +++++ internal/shortcut/chat/chat_group.go | 9 +- .../chat/flag_list_pagination_test.go | 51 +++++++++++ internal/shortcut/chat/lark_alignment.go | 9 +- .../chat/remaining_read_pagination_test.go | 63 +++++++++++++ internal/shortcut/chatmsg/chatmsg_test.go | 1 + internal/shortcut/pagination.go | 11 ++- internal/shortcut/pagination_test.go | 91 +++++++++++++++++++ internal/shortcut/smart/at_me.go | 9 +- internal/shortcut/smart/chat_messages_test.go | 27 ++++++ internal/shortcut/smart/my_groups.go | 9 +- .../smart/remaining_read_pagination_test.go | 78 ++++++++++++++++ 13 files changed, 350 insertions(+), 35 deletions(-) create mode 100644 internal/shortcut/pagination_test.go diff --git a/internal/shortcut/chat/chat_conversation.go b/internal/shortcut/chat/chat_conversation.go index edad1f81..a575f9a2 100644 --- a/internal/shortcut/chat/chat_conversation.go +++ b/internal/shortcut/chat/chat_conversation.go @@ -417,11 +417,7 @@ var ConversationList = shortcut.Shortcut{ break } if !hasMore { - complete = !truncatedByResultLimit - if truncatedByResultLimit { - hasMore = true - stopReason = "result_limit" - } + complete = true break } nextCursor, err = conversationPaginationCursor(page["nextCursor"]) diff --git a/internal/shortcut/chat/chat_conversation_projection_test.go b/internal/shortcut/chat/chat_conversation_projection_test.go index 320807e1..acddbd0b 100644 --- a/internal/shortcut/chat/chat_conversation_projection_test.go +++ b/internal/shortcut/chat/chat_conversation_projection_test.go @@ -17,6 +17,7 @@ import ( "bytes" "context" "encoding/json" + "errors" "reflect" "testing" @@ -219,6 +220,26 @@ func TestCrossPlatformCoverageConversationListPropagatesDelayCancellation(t *tes } } +func TestCrossPlatformCoverageConversationListAutoPageValidationAndOutputFailure(t *testing.T) { + helpers.InitDeps(&larkAlignmentCaller{}) + root := newPlatformCoverageRoot() + root.SetArgs([]string{"chat", "+conversation-list", "--max-items", "1"}) + if err := root.Execute(); err == nil { + t.Fatal("max-items without page-all unexpectedly succeeded") + } + + fake := &larkAlignmentCaller{responses: map[string]string{ + "im/list_all_conversations": `{"result":{"conversationList":[],"hasMore":false}}`, + }} + helpers.InitDeps(fake) + root = newPlatformCoverageRoot() + root.SetOut(chatOutputErrorWriter{err: errors.New("fixture output")}) + root.SetArgs([]string{"chat", "+conversation-list"}) + if err := root.Execute(); err == nil { + t.Fatal("output error was swallowed") + } +} + func TestCrossPlatformCoverageConversationListDeduplicatesStableIDs(t *testing.T) { fake := &larkAlignmentCaller{responses: map[string]string{ "im/list_all_conversations": `{"result":{"conversationList":[{"openConversationId":"cid-1","title":"一"},{"openConversationId":"cid-1","title":"重复"}],"hasMore":false}}`, diff --git a/internal/shortcut/chat/chat_group.go b/internal/shortcut/chat/chat_group.go index 731c9c40..d9b5d31c 100644 --- a/internal/shortcut/chat/chat_group.go +++ b/internal/shortcut/chat/chat_group.go @@ -1034,14 +1034,9 @@ func readAllChatListAll(rt *shortcut.RuntimeContext, baseParams map[string]any) break } if !hasMore { - complete = !truncatedByResultLimit + complete = true nextCursor = nil - if truncatedByResultLimit { - hasMore = true - stopReason = "result_limit" - } else { - stopReason = "source_complete" - } + stopReason = "source_complete" break } nextCursor = page["nextCursor"] diff --git a/internal/shortcut/chat/flag_list_pagination_test.go b/internal/shortcut/chat/flag_list_pagination_test.go index cc7c2ad3..b9a4c5b7 100644 --- a/internal/shortcut/chat/flag_list_pagination_test.go +++ b/internal/shortcut/chat/flag_list_pagination_test.go @@ -5,6 +5,7 @@ package chat import ( "bytes" + "context" "encoding/json" "errors" "testing" @@ -103,6 +104,56 @@ func TestCrossPlatformCoverageFlagListMaxItemsPublishesStableTruncation(t *testi } } +func TestCrossPlatformCoverageFlagListFailsClosedOnOversizeAndCanceledDelay(t *testing.T) { + t.Run("oversized lower page", func(t *testing.T) { + fake := &larkAlignmentCaller{responses: map[string]string{ + "im/list_message_favorites": `{"result":{"items":[{"openMessageId":"m1"},{"openMessageId":"m2"}],"hasMore":true,"nextCursor":9}}`, + }} + helpers.InitDeps(fake) + root := newPlatformCoverageRoot() + var output bytes.Buffer + root.SetOut(&output) + root.SetArgs([]string{"chat", "+flag-list", "--page-all", "--max-items", "1"}) + if err := root.Execute(); err == nil { + t.Fatal("oversized lower page unexpectedly published a continuation") + } + var payload map[string]any + if err := json.Unmarshal(output.Bytes(), &payload); err != nil { + t.Fatal(err) + } + if payload["stopReason"] != "pagination_error" || payload["failedCount"] != float64(1) || payload["nextCursor"] != float64(0) { + t.Fatalf("payload = %#v", payload) + } + if len(fake.calls) != 1 || fake.calls[0].args["size"] != "1" { + t.Fatalf("calls = %#v", fake.calls) + } + }) + + t.Run("canceled delay", func(t *testing.T) { + fake := &larkAlignmentCaller{responses: map[string]string{ + "im/list_message_favorites": `{"result":{"items":[{"openMessageId":"m1"}],"hasMore":true,"nextCursor":9}}`, + }} + helpers.InitDeps(fake) + root := newPlatformCoverageRoot() + ctx, cancel := context.WithCancel(context.Background()) + cancel() + root.SetContext(ctx) + var output bytes.Buffer + root.SetOut(&output) + root.SetArgs([]string{"chat", "+flag-list", "--page-all", "--page-delay", "1"}) + if err := root.Execute(); err == nil { + t.Fatal("canceled delay unexpectedly succeeded") + } + var payload map[string]any + if err := json.Unmarshal(output.Bytes(), &payload); err != nil { + t.Fatal(err) + } + if payload["stopReason"] != "delay_interrupted" || payload["failedCount"] != float64(1) { + t.Fatalf("payload = %#v", payload) + } + }) +} + func TestCrossPlatformCoverageFlagListFailureModes(t *testing.T) { t.Run("later read failure keeps partial result", func(t *testing.T) { fake := &larkAlignmentCaller{ diff --git a/internal/shortcut/chat/lark_alignment.go b/internal/shortcut/chat/lark_alignment.go index c8e01234..10b238e3 100644 --- a/internal/shortcut/chat/lark_alignment.go +++ b/internal/shortcut/chat/lark_alignment.go @@ -680,14 +680,9 @@ func executeFlagList(rt *shortcut.RuntimeContext) error { break } if !hasMore { - complete = !truncatedByResultLimit + complete = true nextCursor = 0 - if truncatedByResultLimit { - hasMore = true - stopReason = "result_limit" - } else { - stopReason = "source_complete" - } + stopReason = "source_complete" break } if cursorErr != nil || nextCursor <= 0 || seenCursors[nextCursor] { diff --git a/internal/shortcut/chat/remaining_read_pagination_test.go b/internal/shortcut/chat/remaining_read_pagination_test.go index 878d989b..d5506dbc 100644 --- a/internal/shortcut/chat/remaining_read_pagination_test.go +++ b/internal/shortcut/chat/remaining_read_pagination_test.go @@ -5,12 +5,14 @@ package chat import ( "bytes" + "context" "encoding/json" "errors" "testing" "time" "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/helpers" + "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/shortcut" ) func TestCrossPlatformCoverageChatListAllPageAllUsesNumericCursorAndDeduplicates(t *testing.T) { @@ -125,6 +127,56 @@ func TestCrossPlatformCoverageChatListAllMaxItemsPublishesStableTruncation(t *te } } +func TestCrossPlatformCoverageChatListAllFailsClosedOnOversizeAndCanceledDelay(t *testing.T) { + t.Run("oversized lower page", func(t *testing.T) { + fake := &larkAlignmentCaller{responses: map[string]string{ + "im/list_my_groups_pagination": `{"result":{"groups":[{"openConversationId":"g1"},{"openConversationId":"g2"}],"hasMore":true,"nextCursor":88}}`, + }} + helpers.InitDeps(fake) + root := newPlatformCoverageRoot() + var output bytes.Buffer + root.SetOut(&output) + root.SetArgs([]string{"chat", "+chat-list-all", "--page-all", "--max-items", "1"}) + if err := root.Execute(); err == nil { + t.Fatal("oversized lower page unexpectedly published a continuation") + } + var payload map[string]any + if err := json.Unmarshal(output.Bytes(), &payload); err != nil { + t.Fatal(err) + } + if payload["stopReason"] != "pagination_error" || payload["failedCount"] != float64(1) { + t.Fatalf("payload = %#v", payload) + } + if len(fake.calls) != 1 || fake.calls[0].args["limit"] != 1 || payload["nextCursor"] != nil { + t.Fatalf("unsafe continuation: calls=%#v payload=%#v", fake.calls, payload) + } + }) + + t.Run("canceled delay", func(t *testing.T) { + fake := &larkAlignmentCaller{responses: map[string]string{ + "im/list_my_groups_pagination": `{"result":{"groups":[{"openConversationId":"g1"}],"hasMore":true,"nextCursor":88}}`, + }} + helpers.InitDeps(fake) + root := newPlatformCoverageRoot() + ctx, cancel := context.WithCancel(context.Background()) + cancel() + root.SetContext(ctx) + var output bytes.Buffer + root.SetOut(&output) + root.SetArgs([]string{"chat", "+chat-list-all", "--page-all", "--page-delay", "1"}) + if err := root.Execute(); err == nil { + t.Fatal("canceled delay unexpectedly succeeded") + } + var payload map[string]any + if err := json.Unmarshal(output.Bytes(), &payload); err != nil { + t.Fatal(err) + } + if payload["stopReason"] != "delay_interrupted" || payload["failedCount"] != float64(1) { + t.Fatalf("payload = %#v", payload) + } + }) +} + func TestCrossPlatformCoverageDirectMessagesPageAllFailsClosedWithoutMillisecondCursor(t *testing.T) { fake := &larkAlignmentCaller{responses: map[string]string{ "chat/list_individual_chat_message": `{"result":{"messages":[{"openMessageId":"m1"}],"hasMore":true}}`, @@ -188,6 +240,17 @@ func TestCrossPlatformCoverageChatRemainingPaginationValidation(t *testing.T) { t.Fatalf("invalid args succeeded: %v", args) } } + root := newPlatformCoverageRoot() + cmd, _, err := root.Find([]string{"chat", "+chat-list-all"}) + if err != nil { + t.Fatal(err) + } + if err := cmd.Flags().Set("max-items", "1"); err != nil { + t.Fatal(err) + } + if err := validateChatListAll(shortcut.RuntimeContextForTest(cmd, ChatListAll)); err == nil { + t.Fatal("direct auto-page validation unexpectedly succeeded") + } } func TestCrossPlatformCoverageChatListAllAdditionalEdges(t *testing.T) { diff --git a/internal/shortcut/chatmsg/chatmsg_test.go b/internal/shortcut/chatmsg/chatmsg_test.go index 73eadbb9..baf4a9f2 100644 --- a/internal/shortcut/chatmsg/chatmsg_test.go +++ b/internal/shortcut/chatmsg/chatmsg_test.go @@ -441,6 +441,7 @@ func TestCrossPlatformCoverageReactionsNormalizesEmotionReplyList(t *testing.T) } func TestCrossPlatformCoverageApplyPaginationReadsNestedEnvelope(t *testing.T) { + ApplyTruncation(nil) payload := map[string]any{"count": 98} ApplyPagination(payload, map[string]any{ "result": map[string]any{ diff --git a/internal/shortcut/pagination.go b/internal/shortcut/pagination.go index 81973344..c11839a6 100644 --- a/internal/shortcut/pagination.go +++ b/internal/shortcut/pagination.go @@ -15,9 +15,12 @@ package shortcut import ( "fmt" + "math" "time" ) +const maxAutoPageDelayMS = int64(math.MaxInt64) / int64(time.Millisecond) + // AutoPageControlFlags returns the shared item and pacing controls used by // cursor-based shortcuts. Callers retain ownership of --page-all and their // product-specific --page-limit defaults. @@ -51,9 +54,13 @@ func ValidateAutoPageControls(rt *RuntimeContext) error { if rt.Int("max-items") < 0 { return fmt.Errorf("--max-items 必须大于等于 0") } - if rt.Int("page-delay") < 0 { + delayMS := rt.Int("page-delay") + if delayMS < 0 { return fmt.Errorf("--page-delay 必须大于等于 0") } + if int64(delayMS) > maxAutoPageDelayMS { + return fmt.Errorf("--page-delay 不能大于 %d 毫秒", maxAutoPageDelayMS) + } return nil } @@ -79,7 +86,7 @@ func WaitAutoPageDelay(rt *RuntimeContext) error { if delayMS <= 0 { return nil } - timer := time.NewTimer(time.Duration(delayMS) * time.Millisecond) + timer := time.NewTimer(time.Duration(int64(delayMS)) * time.Millisecond) defer timer.Stop() select { case <-rt.Command().Context().Done(): diff --git a/internal/shortcut/pagination_test.go b/internal/shortcut/pagination_test.go new file mode 100644 index 00000000..20fd3445 --- /dev/null +++ b/internal/shortcut/pagination_test.go @@ -0,0 +1,91 @@ +// 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 shortcut + +import ( + "context" + "strconv" + "testing" + + "github.com/spf13/cobra" +) + +func autoPageRuntimeForTest(t *testing.T, pageAll bool, maxItems, pageDelay string) *RuntimeContext { + t.Helper() + cmd := &cobra.Command{Use: "page"} + cmd.SetContext(context.Background()) + cmd.Flags().Bool("page-all", false, "") + cmd.Flags().Int("max-items", 0, "") + cmd.Flags().Int("page-delay", 0, "") + if pageAll { + if err := cmd.Flags().Set("page-all", "true"); err != nil { + t.Fatal(err) + } + } + for name, value := range map[string]string{"max-items": maxItems, "page-delay": pageDelay} { + if value != "" { + if err := cmd.Flags().Set(name, value); err != nil { + t.Fatal(err) + } + } + } + return RuntimeContextForTest(cmd, Shortcut{}) +} + +func TestCrossPlatformCoverageAutoPageControlsBoundDelayAndRequestSize(t *testing.T) { + if err := ValidateAutoPageControls(autoPageRuntimeForTest(t, false, "1", "")); err == nil { + t.Fatal("max-items without page-all unexpectedly succeeded") + } + if err := ValidateAutoPageControls(autoPageRuntimeForTest(t, true, "-1", "")); err == nil { + t.Fatal("negative max-items unexpectedly succeeded") + } + if err := ValidateAutoPageControls(autoPageRuntimeForTest(t, true, "", "-1")); err == nil { + t.Fatal("negative page-delay unexpectedly succeeded") + } + if err := ValidateAutoPageControls(autoPageRuntimeForTest(t, true, "", strconv.FormatInt(maxAutoPageDelayMS, 10))); err != nil { + t.Fatalf("maximum safe page-delay failed: %v", err) + } + if strconv.IntSize == 64 { + tooLarge := strconv.FormatInt(maxAutoPageDelayMS+1, 10) + if err := ValidateAutoPageControls(autoPageRuntimeForTest(t, true, "", tooLarge)); err == nil { + t.Fatal("overflowing page-delay unexpectedly succeeded") + } + } + + if got := AutoPageRequestSize(autoPageRuntimeForTest(t, true, "0", ""), 100, 40); got != 100 { + t.Fatalf("unlimited request size = %d", got) + } + if got := AutoPageRequestSize(autoPageRuntimeForTest(t, true, "50", ""), 100, 40); got != 10 { + t.Fatalf("remaining request size = %d", got) + } + if got := AutoPageRequestSize(autoPageRuntimeForTest(t, true, "50", ""), 100, 50); got != 100 { + t.Fatalf("exhausted request size = %d", got) + } +} + +func TestCrossPlatformCoverageWaitAutoPageDelayNoopAndCancellation(t *testing.T) { + if err := WaitAutoPageDelay(autoPageRuntimeForTest(t, true, "", "0")); err != nil { + t.Fatalf("zero delay = %v", err) + } + if err := WaitAutoPageDelay(autoPageRuntimeForTest(t, true, "", "1")); err != nil { + t.Fatalf("elapsed delay = %v", err) + } + ctx, cancel := context.WithCancel(context.Background()) + cancel() + rt := autoPageRuntimeForTest(t, true, "", "1") + rt.Command().SetContext(ctx) + if err := WaitAutoPageDelay(rt); err != context.Canceled { + t.Fatalf("canceled delay = %v, want context.Canceled", err) + } +} diff --git a/internal/shortcut/smart/at_me.go b/internal/shortcut/smart/at_me.go index 1723d4e6..e67330a5 100644 --- a/internal/shortcut/smart/at_me.go +++ b/internal/shortcut/smart/at_me.go @@ -314,14 +314,9 @@ func readAllAtMePages(rt *shortcut.RuntimeContext, baseParams map[string]any) (m break } if !hasMore { - complete = !truncatedByResultLimit + complete = true nextCursor = "" - if truncatedByResultLimit { - hasMore = true - stopReason = "result_limit" - } else { - stopReason = "source_complete" - } + stopReason = "source_complete" break } nextCursor = atMeCursorString(page["nextCursor"]) diff --git a/internal/shortcut/smart/chat_messages_test.go b/internal/shortcut/smart/chat_messages_test.go index c79abd76..0c3918c9 100644 --- a/internal/shortcut/smart/chat_messages_test.go +++ b/internal/shortcut/smart/chat_messages_test.go @@ -516,6 +516,16 @@ func TestCrossPlatformCoverageChatMessagesKeepsMaxResultsPublic(t *testing.T) { } func TestCrossPlatformCoverageChatMessagesAdditionalValidationAndHelpers(t *testing.T) { + for _, values := range []map[string]string{ + {"max-results": "1"}, + {"max-items": "1"}, + {"page-all": "true", "max-results": "-1"}, + {"page-all": "true", "max-items": "1", "max-results": "1"}, + } { + if err := validateChatMessages(chatMessagesRuntimeForTest(t, values)); err == nil { + t.Fatalf("pagination validation unexpectedly accepted %#v", values) + } + } for _, values := range []map[string]string{ {"time": "2026-01-01", "start": "2026-01-01"}, {"direction": "older", "start": "2026-01-01"}, @@ -664,6 +674,23 @@ func TestCrossPlatformCoverageChatMessagesAdditionalCollectionEdges(t *testing.T } }) + t.Run("canceled delay", func(t *testing.T) { + caller := &chatMessagesPagingCaller{responses: []string{ + `{"result":{"hasMore":true,"nextCursor":1234,"messages":[{"openMessageId":"m1"}]}}`, + }} + rt := runtimeWith(t, caller, map[string]string{"page-delay": "1"}) + ctx, cancel := context.WithCancel(context.Background()) + cancel() + rt.Command().SetContext(ctx) + payload, _, err := collectAllChatMessages( + rt, + chatMessagesRequest{tool: "list_conversation_message_v2", params: map[string]any{}, direction: "older"}, + ) + if err == nil || payload["stopReason"] != "delay_interrupted" || payload["failedCount"] != 1 { + t.Fatalf("payload=%#v err=%v", payload, err) + } + }) + t.Run("first failure ledger output error", func(t *testing.T) { helpers.InitDeps(&chatMessagesPagingCaller{failAt: 1}) root := newPlatformCoverageRoot() diff --git a/internal/shortcut/smart/my_groups.go b/internal/shortcut/smart/my_groups.go index 0f84432a..b4f6f57e 100644 --- a/internal/shortcut/smart/my_groups.go +++ b/internal/shortcut/smart/my_groups.go @@ -257,14 +257,9 @@ func readAllMyGroups(rt *shortcut.RuntimeContext, baseParams map[string]any) (ma break } if !hasMore { - complete = !truncatedByResultLimit + complete = true nextCursor = nil - if truncatedByResultLimit { - hasMore = true - stopReason = "result_limit" - } else { - stopReason = "source_complete" - } + stopReason = "source_complete" break } nextCursor = page["nextCursor"] diff --git a/internal/shortcut/smart/remaining_read_pagination_test.go b/internal/shortcut/smart/remaining_read_pagination_test.go index 220da567..b4ab47b7 100644 --- a/internal/shortcut/smart/remaining_read_pagination_test.go +++ b/internal/shortcut/smart/remaining_read_pagination_test.go @@ -5,6 +5,7 @@ package smart import ( "bytes" + "context" "encoding/json" "testing" @@ -160,6 +161,83 @@ func TestCrossPlatformCoverageMyGroupsMaxItemsAppliesAfterTypeFilter(t *testing. } } +func TestCrossPlatformCoverageRemainingListsFailClosedOnOversizeAndCanceledDelay(t *testing.T) { + tests := []struct { + name string + command string + response string + extra []string + }{ + { + name: "at-me", + command: "+at-me", + response: `{"result":{"conversationMessagesList":[{"messages":[{"openMessageId":"m1"},{"openMessageId":"m2"}]}],` + + `"hasMore":true,"nextCursor":"next"}}`, + }, + { + name: "my-groups", + command: "+my-groups", + response: `{"result":{"groups":[{"openConversationId":"g1","groupType":"group"},{"openConversationId":"g2","groupType":"group"}],"hasMore":true,"nextCursor":2}}`, + extra: []string{"--type", "group"}, + }, + } + for _, tc := range tests { + t.Run(tc.name+" oversized lower page", func(t *testing.T) { + caller := &chatMessagesPagingCaller{responses: []string{tc.response}} + helpers.InitDeps(caller) + root := newPlatformCoverageRoot() + var output bytes.Buffer + root.SetOut(&output) + args := append([]string{"chat", tc.command}, tc.extra...) + args = append(args, "--page-all", "--max-items", "1") + root.SetArgs(args) + if err := root.Execute(); err == nil { + t.Fatal("oversized lower page unexpectedly published a continuation") + } + var payload map[string]any + if err := json.Unmarshal(output.Bytes(), &payload); err != nil { + t.Fatal(err) + } + if payload["stopReason"] != "pagination_error" || payload["failedCount"] != float64(1) { + t.Fatalf("payload = %#v", payload) + } + if len(caller.args) != 1 || caller.args[0]["limit"] != 1 || payload["nextCursor"] != nil { + t.Fatalf("unsafe continuation: calls=%#v payload=%#v", caller.args, payload) + } + }) + + t.Run(tc.name+" canceled delay", func(t *testing.T) { + continuing := tc.response + if tc.command == "+at-me" { + continuing = `{"result":{"conversationMessagesList":[{"messages":[{"openMessageId":"m1"}]}],"hasMore":true,"nextCursor":"next"}}` + } else { + continuing = `{"result":{"groups":[{"openConversationId":"g1","groupType":"group"}],"hasMore":true,"nextCursor":2}}` + } + caller := &chatMessagesPagingCaller{responses: []string{continuing}} + helpers.InitDeps(caller) + root := newPlatformCoverageRoot() + ctx, cancel := context.WithCancel(context.Background()) + cancel() + root.SetContext(ctx) + var output bytes.Buffer + root.SetOut(&output) + args := append([]string{"chat", tc.command}, tc.extra...) + args = append(args, "--page-all", "--page-delay", "1") + root.SetArgs(args) + if err := root.Execute(); err == nil { + t.Fatal("canceled delay unexpectedly succeeded") + } + var payload map[string]any + if err := json.Unmarshal(output.Bytes(), &payload); err != nil { + t.Fatal(err) + } + if payload["stopReason"] != "delay_interrupted" || payload["failedCount"] != float64(1) { + t.Fatalf("payload = %#v", payload) + } + }) + } +} + func TestCrossPlatformCoverageRemainingReadPaginationValidation(t *testing.T) { for _, args := range [][]string{ {"chat", "+at-me", "--page-limit", "2"}, From fdbd11e0eaa1c21986e38f494277c240db5b989e Mon Sep 17 00:00:00 2001 From: Dennis Date: Wed, 12 Aug 2026 19:28:29 +0800 Subject: [PATCH 8/9] docs(changelog): add IM pagination release note --- .changes/970-chat-auto-pagination.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changes/970-chat-auto-pagination.md diff --git a/.changes/970-chat-auto-pagination.md b/.changes/970-chat-auto-pagination.md new file mode 100644 index 00000000..4b665e47 --- /dev/null +++ b/.changes/970-chat-auto-pagination.md @@ -0,0 +1,5 @@ +--- +category: Added +--- + +- **Chat automatic pagination controls** (#970) — adds bounded `--max-items` and cancellable `--page-delay` support to the core IM list shortcuts, with safe continuation metadata and truncation reporting. From 2f8e136dc08f7c0aa63cf3aa1057a428f9714d84 Mon Sep 17 00:00:00 2001 From: Dennis Date: Wed, 12 Aug 2026 19:51:00 +0800 Subject: [PATCH 9/9] fix(chat): fail closed on bounded legacy pages --- .../chat/flag_list_pagination_test.go | 29 +++++++++++++++++++ internal/shortcut/chat/lark_alignment.go | 2 +- 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/internal/shortcut/chat/flag_list_pagination_test.go b/internal/shortcut/chat/flag_list_pagination_test.go index b9a4c5b7..2f997956 100644 --- a/internal/shortcut/chat/flag_list_pagination_test.go +++ b/internal/shortcut/chat/flag_list_pagination_test.go @@ -104,6 +104,35 @@ func TestCrossPlatformCoverageFlagListMaxItemsPublishesStableTruncation(t *testi } } +func TestCrossPlatformCoverageFlagListLegacyFullRemainingPageFailsClosed(t *testing.T) { + fake := &larkAlignmentCaller{sequenceResponses: map[string][]string{ + "im/list_message_favorites": { + `{"result":{"items":[{"openMessageId":"m1"},{"openMessageId":"m2"}],"hasMore":true,"nextCursor":7}}`, + `{"result":{"items":[{"openMessageId":"m3"}]}}`, + }, + }} + helpers.InitDeps(fake) + root := newPlatformCoverageRoot() + var output bytes.Buffer + root.SetOut(&output) + root.SetArgs([]string{"chat", "+flag-list", "--page-size", "2", "--page-all", "--max-items", "3"}) + if err := root.Execute(); err == nil { + t.Fatal("full remaining-budget legacy page unexpectedly declared a complete result") + } + if len(fake.calls) != 2 || fake.calls[0].args["size"] != "2" || fake.calls[1].args["size"] != "1" { + t.Fatalf("request sizes = %#v", fake.calls) + } + var payload map[string]any + if err := json.Unmarshal(output.Bytes(), &payload); err != nil { + t.Fatal(err) + } + if payload["count"] != float64(3) || payload["complete"] != false || + payload["paginationKnown"] != false || payload["stopReason"] != "pagination_error" || + payload["failedCount"] != float64(1) || payload["nextCursor"] != float64(0) { + t.Fatalf("payload = %#v", payload) + } +} + func TestCrossPlatformCoverageFlagListFailsClosedOnOversizeAndCanceledDelay(t *testing.T) { t.Run("oversized lower page", func(t *testing.T) { fake := &larkAlignmentCaller{responses: map[string]string{ diff --git a/internal/shortcut/chat/lark_alignment.go b/internal/shortcut/chat/lark_alignment.go index 10b238e3..f8a7a605 100644 --- a/internal/shortcut/chat/lark_alignment.go +++ b/internal/shortcut/chat/lark_alignment.go @@ -651,7 +651,7 @@ func executeFlagList(rt *shortcut.RuntimeContext) error { switch { case nextCursor > 0: pageHasMore = true - case len(pageItems) < pageSize: + case len(pageItems) < requestPageSize: paginationKnown = false complete = true hasMore = false