From 2561652759c51faed6846f19b154600b5cae4936 Mon Sep 17 00:00:00 2001 From: yangr-happy <301323675+yangr-happy@users.noreply.github.com> Date: Tue, 4 Aug 2026 15:23:12 +0800 Subject: [PATCH 1/5] fix(mail): complete rule IDs before reorder Fetch the current mailbox rules with the same service command context before calling reorder, validate the requested IDs locally, and submit the completed rule ID order. Document partial reorder input behavior for the mail skill reference. Test: go test ./cmd/service Co-authored-by: TRAE CLI --- cmd/service/mail_rules_reorder.go | 197 ++++++++++++ cmd/service/mail_rules_reorder_test.go | 293 ++++++++++++++++++ cmd/service/service.go | 4 + .../lark-mail/references/lark-mail-rules.md | 12 + 4 files changed, 506 insertions(+) create mode 100644 cmd/service/mail_rules_reorder.go create mode 100644 cmd/service/mail_rules_reorder_test.go diff --git a/cmd/service/mail_rules_reorder.go b/cmd/service/mail_rules_reorder.go new file mode 100644 index 0000000000..e8b4d7fed8 --- /dev/null +++ b/cmd/service/mail_rules_reorder.go @@ -0,0 +1,197 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package service + +import ( + "context" + "strings" + + "github.com/larksuite/cli/errs" + "github.com/larksuite/cli/internal/client" + "github.com/larksuite/cli/internal/core" +) + +const mailRulesReorderSchemaPath = "mail.user_mailbox.rules.reorder" + +func maybeCompleteMailRulesReorderIDs(ctx context.Context, ac *client.APIClient, opts *ServiceMethodOptions, request *client.RawApiRequest, checkErr func(interface{}, core.Identity) error) error { + if opts.SchemaPath != mailRulesReorderSchemaPath { + return nil + } + + body, ok := toStringAnyMap(request.Data) + if !ok { + return errs.NewValidationError(errs.SubtypeInvalidArgument, "mail rules reorder requires a JSON object body").WithParam("--data") + } + + requestedIDs, err := mailRuleIDsFromBody(body) + if err != nil { + return err + } + if err := validateRequestedMailRuleIDs(requestedIDs); err != nil { + return err + } + + existingIDs, err := listAllMailRuleIDs(ctx, ac, *request, checkErr) + if err != nil { + return err + } + + completedIDs, err := completeMailRuleIDs(requestedIDs, existingIDs) + if err != nil { + return err + } + body["rule_ids"] = completedIDs + request.Data = body + return nil +} + +func toStringAnyMap(v any) (map[string]any, bool) { + if m, ok := v.(map[string]any); ok { + return m, true + } + return nil, false +} + +func mailRuleIDsFromBody(body map[string]any) ([]string, error) { + raw, ok := body["rule_ids"] + if !ok { + return nil, errs.NewValidationError(errs.SubtypeInvalidArgument, "rule_ids is required").WithParam("rule_ids") + } + switch ids := raw.(type) { + case []string: + return append([]string(nil), ids...), nil + case []any: + out := make([]string, 0, len(ids)) + for i, id := range ids { + s, ok := id.(string) + if !ok { + return nil, errs.NewValidationError(errs.SubtypeInvalidArgument, "rule_ids[%d] must be a string", i).WithParam("rule_ids") + } + out = append(out, s) + } + return out, nil + default: + return nil, errs.NewValidationError(errs.SubtypeInvalidArgument, "rule_ids must be an array of strings").WithParam("rule_ids") + } +} + +func validateRequestedMailRuleIDs(requestedIDs []string) error { + if len(requestedIDs) == 0 { + return errs.NewValidationError(errs.SubtypeInvalidArgument, "at least one rule_id is required for reorder").WithParam("rule_ids") + } + seen := map[string]struct{}{} + for _, id := range requestedIDs { + if id == "" { + return errs.NewValidationError(errs.SubtypeInvalidArgument, "rule_ids cannot contain an empty string").WithParam("rule_ids") + } + if _, ok := seen[id]; ok { + return errs.NewValidationError(errs.SubtypeInvalidArgument, "duplicate rule_id: %s", id).WithParam("rule_ids") + } + seen[id] = struct{}{} + } + return nil +} + +func completeMailRuleIDs(requestedIDs, existingIDs []string) ([]string, error) { + existingSet := make(map[string]struct{}, len(existingIDs)) + for _, id := range existingIDs { + if id == "" { + continue + } + existingSet[id] = struct{}{} + } + for _, id := range requestedIDs { + if _, ok := existingSet[id]; !ok { + return nil, errs.NewValidationError(errs.SubtypeInvalidArgument, "unknown rule_id: %s", id).WithParam("rule_ids") + } + } + + requestedSet := make(map[string]struct{}, len(requestedIDs)) + completed := make([]string, 0, len(existingIDs)) + for _, id := range requestedIDs { + requestedSet[id] = struct{}{} + completed = append(completed, id) + } + for _, id := range existingIDs { + if _, ok := requestedSet[id]; !ok { + completed = append(completed, id) + } + } + return completed, nil +} + +func listAllMailRuleIDs(ctx context.Context, ac *client.APIClient, reorderRequest client.RawApiRequest, checkErr func(interface{}, core.Identity) error) ([]string, error) { + listRequest := client.RawApiRequest{ + Method: "GET", + URL: mailRulesListURL(reorderRequest.URL), + Params: copyRequestParamsWithoutPageToken(reorderRequest.Params), + As: reorderRequest.As, + } + result, err := ac.PaginateAll(ctx, listRequest, client.PaginationOptions{ + PageLimit: 0, + PageDelay: -1, + Identity: reorderRequest.As, + }) + if err != nil { + return nil, err + } + if apiErr := checkErr(result, reorderRequest.As); apiErr != nil { + return nil, apiErr + } + return extractMailRuleIDs(result) +} + +func mailRulesListURL(reorderURL string) string { + return strings.TrimSuffix(reorderURL, "/reorder") +} + +func copyRequestParamsWithoutPageToken(in map[string]any) map[string]any { + out := make(map[string]any, len(in)) + for k, v := range in { + if k == "page_token" { + continue + } + out[k] = v + } + return out +} + +func extractMailRuleIDs(result any) ([]string, error) { + data, ok := nestedMap(result, "data") + if !ok { + return nil, errs.NewInternalError(errs.SubtypeInvalidResponse, "mail rules list response missing data") + } + rules, ok := data["items"].([]any) + if !ok { + rules, ok = data["rules"].([]any) + } + if !ok { + return nil, errs.NewInternalError(errs.SubtypeInvalidResponse, "mail rules list response missing items") + } + + ids := make([]string, 0, len(rules)) + for i, item := range rules { + rule, ok := item.(map[string]any) + if !ok { + return nil, errs.NewInternalError(errs.SubtypeInvalidResponse, "mail rules list item %d is not an object", i) + } + id, ok := rule["rule_id"].(string) + if !ok || id == "" { + id, ok = rule["id"].(string) + } + if !ok || id == "" { + return nil, errs.NewInternalError(errs.SubtypeInvalidResponse, "mail rules list item %d missing rule_id", i) + } + ids = append(ids, id) + } + return ids, nil +} + +func nestedMap(result any, key string) (map[string]any, bool) { + m, ok := toStringAnyMap(result) + if !ok { + return nil, false + } + return toStringAnyMap(m[key]) +} diff --git a/cmd/service/mail_rules_reorder_test.go b/cmd/service/mail_rules_reorder_test.go new file mode 100644 index 0000000000..e1ee61f4b8 --- /dev/null +++ b/cmd/service/mail_rules_reorder_test.go @@ -0,0 +1,293 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package service + +import ( + "bytes" + "encoding/json" + "errors" + "net/http" + "reflect" + "strings" + "testing" + + "github.com/larksuite/cli/errs" + "github.com/larksuite/cli/internal/cmdutil" + "github.com/larksuite/cli/internal/httpmock" + "github.com/larksuite/cli/internal/meta" +) + +func mailSpec() meta.Service { + return meta.ServiceFromMap(map[string]interface{}{ + "name": "mail", + "servicePath": "/open-apis/mail/v1", + }) +} + +func mailRulesReorderMethod() meta.Method { + return meta.FromMap(map[string]interface{}{ + "path": "user_mailboxes/{user_mailbox_id}/rules", + "httpMethod": "PATCH", + "parameters": map[string]interface{}{ + "user_mailbox_id": map[string]interface{}{"type": "string", "location": "path", "required": true}, + }, + "requestBody": map[string]interface{}{ + "rule_ids": map[string]interface{}{"type": "list", "required": true}, + }, + }) +} + +func mailRulesReorderSubpathMethod() meta.Method { + return meta.FromMap(map[string]interface{}{ + "path": "user_mailboxes/{user_mailbox_id}/rules/reorder", + "httpMethod": "PATCH", + "parameters": map[string]interface{}{ + "user_mailbox_id": map[string]interface{}{"type": "string", "location": "path", "required": true}, + }, + "requestBody": map[string]interface{}{ + "rule_ids": map[string]interface{}{"type": "list", "required": true}, + }, + }) +} + +func newMailRulesReorderCommand(t *testing.T) (*cmdutil.Factory, *bytes.Buffer, *httpmock.Registry, *cobraCommandShim) { + t.Helper() + f, stdout, _, reg := cmdutil.TestFactory(t, testConfig) + cmd := NewCmdServiceMethod(f, mailSpec(), mailRulesReorderMethod(), "reorder", "user_mailbox.rules", nil) + return f, stdout, reg, &cobraCommandShim{setArgs: cmd.SetArgs, execute: cmd.Execute} +} + +type cobraCommandShim struct { + setArgs func([]string) + execute func() error +} + +func registerMailRulesListPage(reg *httpmock.Registry, mailboxID string, body map[string]interface{}, onMatch func(*http.Request)) { + reg.Register(&httpmock.Stub{ + Method: "GET", + URL: "/open-apis/mail/v1/user_mailboxes/" + mailboxID + "/rules", + OnMatch: onMatch, + Body: map[string]interface{}{ + "code": 0, + "msg": "ok", + "data": body, + }, + }) +} + +func registerMailRulesReorder(reg *httpmock.Registry, mailboxID string, onMatch func(*http.Request, map[string]interface{}), body map[string]interface{}) { + reg.Register(&httpmock.Stub{ + Method: "PATCH", + URL: "/open-apis/mail/v1/user_mailboxes/" + mailboxID + "/rules", + OnMatch: func(req *http.Request) { + var got map[string]interface{} + if err := json.NewDecoder(req.Body).Decode(&got); err != nil { + panic(err) + } + onMatch(req, got) + }, + Body: body, + }) +} + +func TestMailRulesReorder_CompletesPartialIDsBeforeReorder(t *testing.T) { + _, _, reg, cmd := newMailRulesReorderCommand(t) + var listMailbox, reorderMailbox string + registerMailRulesListPage(reg, "shared@example.com", map[string]interface{}{ + "items": []interface{}{ + map[string]interface{}{"rule_id": "r1"}, + map[string]interface{}{"rule_id": "r2"}, + map[string]interface{}{"rule_id": "r3"}, + }, + "has_more": false, + }, func(req *http.Request) { + listMailbox = req.URL.Path + }) + registerMailRulesReorder(reg, "shared@example.com", func(req *http.Request, got map[string]interface{}) { + reorderMailbox = req.URL.Path + if gotIDs := interfaceSliceToStrings(got["rule_ids"]); !reflect.DeepEqual(gotIDs, []string{"r2", "r1", "r3"}) { + t.Fatalf("rule_ids = %#v, want [r2 r1 r3]", got["rule_ids"]) + } + }, map[string]interface{}{"code": 0, "msg": "ok", "data": map[string]interface{}{"ok": true}}) + + cmd.setArgs([]string{ + "--as", "bot", + "--params", `{"user_mailbox_id":"shared@example.com"}`, + "--data", `{"rule_ids":["r2"]}`, + }) + if err := cmd.execute(); err != nil { + t.Fatalf("Execute() error = %v", err) + } + if listMailbox != reorderMailbox { + t.Fatalf("list path = %q, reorder path = %q; want same mailbox context", listMailbox, reorderMailbox) + } +} + +func TestMailRulesReorder_FullIDsRemainInRequestedOrder(t *testing.T) { + _, _, reg, cmd := newMailRulesReorderCommand(t) + registerMailRulesListPage(reg, "me", map[string]interface{}{ + "items": []interface{}{ + map[string]interface{}{"rule_id": "r1"}, + map[string]interface{}{"rule_id": "r2"}, + map[string]interface{}{"rule_id": "r3"}, + }, + "has_more": false, + }, nil) + registerMailRulesReorder(reg, "me", func(req *http.Request, got map[string]interface{}) { + if gotIDs := interfaceSliceToStrings(got["rule_ids"]); !reflect.DeepEqual(gotIDs, []string{"r3", "r1", "r2"}) { + t.Fatalf("rule_ids = %#v, want [r3 r1 r2]", got["rule_ids"]) + } + }, map[string]interface{}{"code": 0, "msg": "ok", "data": map[string]interface{}{}}) + + cmd.setArgs([]string{"--as", "bot", "--params", `{"user_mailbox_id":"me"}`, "--data", `{"rule_ids":["r3","r1","r2"]}`}) + if err := cmd.execute(); err != nil { + t.Fatalf("Execute() error = %v", err) + } +} + +func TestMailRulesReorder_ValidationErrorsDoNotCallAPIs(t *testing.T) { + tests := []struct { + name string + data string + wantMsg string + }{ + {name: "empty", data: `{"rule_ids":[]}`, wantMsg: "at least one"}, + {name: "duplicate", data: `{"rule_ids":["r1","r1"]}`, wantMsg: "duplicate rule_id: r1"}, + {name: "empty string", data: `{"rule_ids":[""]}`, wantMsg: "empty string"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + _, _, _, cmd := newMailRulesReorderCommand(t) + cmd.setArgs([]string{"--as", "bot", "--params", `{"user_mailbox_id":"me"}`, "--data", tt.data}) + err := cmd.execute() + assertServiceValidationError(t, err, tt.wantMsg) + }) + } +} + +func TestMailRulesReorder_UnknownIDDoesNotCallReorder(t *testing.T) { + _, _, reg, cmd := newMailRulesReorderCommand(t) + registerMailRulesListPage(reg, "me", map[string]interface{}{ + "items": []interface{}{map[string]interface{}{"rule_id": "known"}}, + }, nil) + cmd.setArgs([]string{"--as", "bot", "--params", `{"user_mailbox_id":"me"}`, "--data", `{"rule_ids":["missing"]}`}) + + err := cmd.execute() + assertServiceValidationError(t, err, "unknown rule_id: missing") +} + +func TestMailRulesReorder_ListFailureDoesNotCallReorder(t *testing.T) { + _, _, reg, cmd := newMailRulesReorderCommand(t) + reg.Register(&httpmock.Stub{ + Method: "GET", + URL: "/open-apis/mail/v1/user_mailboxes/me/rules", + Body: map[string]interface{}{ + "code": 999, + "msg": "list failed", + }, + }) + cmd.setArgs([]string{"--as", "bot", "--params", `{"user_mailbox_id":"me"}`, "--data", `{"rule_ids":["r1"]}`}) + + err := cmd.execute() + var apiErr *errs.APIError + if !errors.As(err, &apiErr) || !strings.Contains(err.Error(), "list failed") { + t.Fatalf("expected list API error, got %T: %v", err, err) + } +} + +func TestMailRulesReorder_ReorderFailureIsSurfaced(t *testing.T) { + _, _, reg, cmd := newMailRulesReorderCommand(t) + registerMailRulesListPage(reg, "me", map[string]interface{}{ + "items": []interface{}{map[string]interface{}{"rule_id": "r1"}}, + }, nil) + registerMailRulesReorder(reg, "me", func(req *http.Request, got map[string]interface{}) {}, map[string]interface{}{ + "code": 998, + "msg": "reorder failed", + }) + cmd.setArgs([]string{"--as", "bot", "--params", `{"user_mailbox_id":"me"}`, "--data", `{"rule_ids":["r1"]}`}) + + err := cmd.execute() + var apiErr *errs.APIError + if !errors.As(err, &apiErr) || !strings.Contains(err.Error(), "reorder failed") { + t.Fatalf("expected reorder API error, got %T: %v", err, err) + } +} + +func TestMailRulesReorder_ListPaginationFetchesAllRules(t *testing.T) { + _, _, reg, cmd := newMailRulesReorderCommand(t) + var tokens []string + registerMailRulesListPage(reg, "me", map[string]interface{}{ + "items": []interface{}{map[string]interface{}{"rule_id": "r1"}}, + "has_more": true, + "page_token": "next-1", + }, func(req *http.Request) { + tokens = append(tokens, req.URL.Query().Get("page_token")) + }) + registerMailRulesListPage(reg, "me", map[string]interface{}{ + "items": []interface{}{map[string]interface{}{"rule_id": "r2"}}, + "has_more": false, + }, func(req *http.Request) { + tokens = append(tokens, req.URL.Query().Get("page_token")) + }) + registerMailRulesReorder(reg, "me", func(req *http.Request, got map[string]interface{}) { + if gotIDs := interfaceSliceToStrings(got["rule_ids"]); !reflect.DeepEqual(gotIDs, []string{"r2", "r1"}) { + t.Fatalf("rule_ids = %#v, want [r2 r1]", got["rule_ids"]) + } + }, map[string]interface{}{"code": 0, "msg": "ok", "data": map[string]interface{}{}}) + + cmd.setArgs([]string{"--as", "bot", "--params", `{"user_mailbox_id":"me"}`, "--data", `{"rule_ids":["r2"]}`}) + if err := cmd.execute(); err != nil { + t.Fatalf("Execute() error = %v", err) + } + if !reflect.DeepEqual(tokens, []string{"", "next-1"}) { + t.Fatalf("page tokens = %v, want [ next-1]", tokens) + } +} + +func TestMailRulesReorder_ListUsesRulesBaseWhenReorderHasSubpath(t *testing.T) { + f, _, _, reg := cmdutil.TestFactory(t, testConfig) + cmd := NewCmdServiceMethod(f, mailSpec(), mailRulesReorderSubpathMethod(), "reorder", "user_mailbox.rules", nil) + var listed, reordered bool + registerMailRulesListPage(reg, "me", map[string]interface{}{ + "items": []interface{}{map[string]interface{}{"rule_id": "r1"}}, + }, func(req *http.Request) { + listed = true + }) + reg.Register(&httpmock.Stub{ + Method: "PATCH", + URL: "/open-apis/mail/v1/user_mailboxes/me/rules/reorder", + OnMatch: func(req *http.Request) { + reordered = true + }, + Body: map[string]interface{}{"code": 0, "msg": "ok", "data": map[string]interface{}{}}, + }) + + cmd.SetArgs([]string{"--as", "bot", "--params", `{"user_mailbox_id":"me"}`, "--data", `{"rule_ids":["r1"]}`}) + if err := cmd.Execute(); err != nil { + t.Fatalf("Execute() error = %v", err) + } + if !listed || !reordered { + t.Fatalf("listed=%v reordered=%v, want both true", listed, reordered) + } +} + +func assertServiceValidationError(t *testing.T, err error, wantSubstr string) { + t.Helper() + var validationErr *errs.ValidationError + if !errors.As(err, &validationErr) { + t.Fatalf("expected validation error, got %T: %v", err, err) + } + if !strings.Contains(err.Error(), wantSubstr) { + t.Fatalf("validation error = %q, want substring %q", err.Error(), wantSubstr) + } +} + +func interfaceSliceToStrings(v interface{}) []string { + items, _ := v.([]interface{}) + out := make([]string, 0, len(items)) + for _, item := range items { + out = append(out, item.(string)) + } + return out +} diff --git a/cmd/service/service.go b/cmd/service/service.go index 3cb6ab5d2f..3c220856ab 100644 --- a/cmd/service/service.go +++ b/cmd/service/service.go @@ -430,6 +430,10 @@ func serviceMethodRun(opts *ServiceMethodOptions) error { // with MissingScopes / Identity / ConsoleURL populated from the response. checkErr := ac.CheckResponse + if err := maybeCompleteMailRulesReorderIDs(opts.Ctx, ac, opts, &request, checkErr); err != nil { + return err + } + if opts.PageAll { return servicePaginate(opts.Ctx, ac, request, format, opts.JqExpr, out, f.IOStreams.ErrOut, opts.Cmd.CommandPath(), client.PaginationOptions{PageLimit: opts.PageLimit, PageDelay: opts.PageDelay}, checkErr) diff --git a/skills/lark-mail/references/lark-mail-rules.md b/skills/lark-mail/references/lark-mail-rules.md index 3c7bef5df5..556accf4d6 100644 --- a/skills/lark-mail/references/lark-mail-rules.md +++ b/skills/lark-mail/references/lark-mail-rules.md @@ -2,6 +2,18 @@ 管理自动处理收到邮件的规则。规则写操作需使用真实 `rule_id`，不要猜测 ID。规则写操作执行前需按 SKILL.md 的写操作确认规则获得用户确认。 +## 重排序 + +`user_mailbox.rules reorder` 可以只传需要优先排序的一部分规则 ID。CLI 会先用同一个 `user_mailbox_id` / `--as` 身份上下文调用 `user_mailbox.rules list` 读取当前全部规则，再按“用户输入顺序优先 + 未输入规则保持当前相对顺序”补齐完整 `rule_ids` 后调用 reorder。 + +```bash +lark-cli mail user_mailbox.rules reorder --as user \ + --params '{"user_mailbox_id":"me"}' \ + --data '{"rule_ids":["",""]}' +``` + +空 `rule_ids`、重复 ID、未知 ID 会在 CLI 侧报 validation error，且不会调用 reorder。list 失败时也不会调用 reorder；reorder 失败时透传 API error。 + ## 主题包含文本 → 标记为已读 ```bash From 98c5edb4e602af00d29b3628ea2e7db70b102ed9 Mon Sep 17 00:00:00 2001 From: yangr-happy <301323675+yangr-happy@users.noreply.github.com> Date: Tue, 4 Aug 2026 21:45:26 +0800 Subject: [PATCH 2/5] fix(mail): address reorder review feedback Complete mail rule reorder IDs before dry-run output so printed requests match executed requests. Move HTTP mock assertions out of OnMatch callbacks and assert typed error metadata for reorder failures. Change-Type: ci-fix Co-authored-by: TRAE CLI --- cmd/service/mail_rules_reorder_test.go | 83 ++++++++++++++++++++------ cmd/service/service.go | 27 +++++++-- 2 files changed, 87 insertions(+), 23 deletions(-) diff --git a/cmd/service/mail_rules_reorder_test.go b/cmd/service/mail_rules_reorder_test.go index e1ee61f4b8..11f762e34b 100644 --- a/cmd/service/mail_rules_reorder_test.go +++ b/cmd/service/mail_rules_reorder_test.go @@ -7,6 +7,7 @@ import ( "bytes" "encoding/json" "errors" + "fmt" "net/http" "reflect" "strings" @@ -94,6 +95,7 @@ func registerMailRulesReorder(reg *httpmock.Registry, mailboxID string, onMatch func TestMailRulesReorder_CompletesPartialIDsBeforeReorder(t *testing.T) { _, _, reg, cmd := newMailRulesReorderCommand(t) var listMailbox, reorderMailbox string + var gotRuleIDs []string registerMailRulesListPage(reg, "shared@example.com", map[string]interface{}{ "items": []interface{}{ map[string]interface{}{"rule_id": "r1"}, @@ -106,9 +108,7 @@ func TestMailRulesReorder_CompletesPartialIDsBeforeReorder(t *testing.T) { }) registerMailRulesReorder(reg, "shared@example.com", func(req *http.Request, got map[string]interface{}) { reorderMailbox = req.URL.Path - if gotIDs := interfaceSliceToStrings(got["rule_ids"]); !reflect.DeepEqual(gotIDs, []string{"r2", "r1", "r3"}) { - t.Fatalf("rule_ids = %#v, want [r2 r1 r3]", got["rule_ids"]) - } + gotRuleIDs = interfaceSliceToStrings(got["rule_ids"]) }, map[string]interface{}{"code": 0, "msg": "ok", "data": map[string]interface{}{"ok": true}}) cmd.setArgs([]string{ @@ -119,6 +119,9 @@ func TestMailRulesReorder_CompletesPartialIDsBeforeReorder(t *testing.T) { if err := cmd.execute(); err != nil { t.Fatalf("Execute() error = %v", err) } + if !reflect.DeepEqual(gotRuleIDs, []string{"r2", "r1", "r3"}) { + t.Fatalf("rule_ids = %#v, want [r2 r1 r3]", gotRuleIDs) + } if listMailbox != reorderMailbox { t.Fatalf("list path = %q, reorder path = %q; want same mailbox context", listMailbox, reorderMailbox) } @@ -126,6 +129,7 @@ func TestMailRulesReorder_CompletesPartialIDsBeforeReorder(t *testing.T) { func TestMailRulesReorder_FullIDsRemainInRequestedOrder(t *testing.T) { _, _, reg, cmd := newMailRulesReorderCommand(t) + var gotRuleIDs []string registerMailRulesListPage(reg, "me", map[string]interface{}{ "items": []interface{}{ map[string]interface{}{"rule_id": "r1"}, @@ -135,15 +139,46 @@ func TestMailRulesReorder_FullIDsRemainInRequestedOrder(t *testing.T) { "has_more": false, }, nil) registerMailRulesReorder(reg, "me", func(req *http.Request, got map[string]interface{}) { - if gotIDs := interfaceSliceToStrings(got["rule_ids"]); !reflect.DeepEqual(gotIDs, []string{"r3", "r1", "r2"}) { - t.Fatalf("rule_ids = %#v, want [r3 r1 r2]", got["rule_ids"]) - } + gotRuleIDs = interfaceSliceToStrings(got["rule_ids"]) }, map[string]interface{}{"code": 0, "msg": "ok", "data": map[string]interface{}{}}) cmd.setArgs([]string{"--as", "bot", "--params", `{"user_mailbox_id":"me"}`, "--data", `{"rule_ids":["r3","r1","r2"]}`}) if err := cmd.execute(); err != nil { t.Fatalf("Execute() error = %v", err) } + if !reflect.DeepEqual(gotRuleIDs, []string{"r3", "r1", "r2"}) { + t.Fatalf("rule_ids = %#v, want [r3 r1 r2]", gotRuleIDs) + } +} + +func TestMailRulesReorder_DryRunCompletesPartialIDs(t *testing.T) { + _, stdout, reg, cmd := newMailRulesReorderCommand(t) + registerMailRulesListPage(reg, "me", map[string]interface{}{ + "items": []interface{}{ + map[string]interface{}{"rule_id": "r1"}, + map[string]interface{}{"rule_id": "r2"}, + map[string]interface{}{"rule_id": "r3"}, + }, + "has_more": false, + }, nil) + + cmd.setArgs([]string{"--as", "bot", "--params", `{"user_mailbox_id":"me"}`, "--data", `{"rule_ids":["r2"]}`, "--dry-run"}) + if err := cmd.execute(); err != nil { + t.Fatalf("Execute() error = %v", err) + } + + var dryRun map[string]any + decoder := json.NewDecoder(strings.NewReader(strings.TrimPrefix(stdout.String(), "=== Dry Run ===\n"))) + if err := decoder.Decode(&dryRun); err != nil { + t.Fatalf("decode dry-run output: %v\n%s", err, stdout.String()) + } + body, ok := dryRun["body"].(map[string]any) + if !ok { + t.Fatalf("dry-run body = %#v, want object", dryRun["body"]) + } + if gotIDs := interfaceSliceToStrings(body["rule_ids"]); !reflect.DeepEqual(gotIDs, []string{"r2", "r1", "r3"}) { + t.Fatalf("dry-run rule_ids = %#v, want [r2 r1 r3]", gotIDs) + } } func TestMailRulesReorder_ValidationErrorsDoNotCallAPIs(t *testing.T) { @@ -190,10 +225,7 @@ func TestMailRulesReorder_ListFailureDoesNotCallReorder(t *testing.T) { cmd.setArgs([]string{"--as", "bot", "--params", `{"user_mailbox_id":"me"}`, "--data", `{"rule_ids":["r1"]}`}) err := cmd.execute() - var apiErr *errs.APIError - if !errors.As(err, &apiErr) || !strings.Contains(err.Error(), "list failed") { - t.Fatalf("expected list API error, got %T: %v", err, err) - } + assertServiceAPIError(t, err, 999, "list failed") } func TestMailRulesReorder_ReorderFailureIsSurfaced(t *testing.T) { @@ -208,15 +240,13 @@ func TestMailRulesReorder_ReorderFailureIsSurfaced(t *testing.T) { cmd.setArgs([]string{"--as", "bot", "--params", `{"user_mailbox_id":"me"}`, "--data", `{"rule_ids":["r1"]}`}) err := cmd.execute() - var apiErr *errs.APIError - if !errors.As(err, &apiErr) || !strings.Contains(err.Error(), "reorder failed") { - t.Fatalf("expected reorder API error, got %T: %v", err, err) - } + assertServiceAPIError(t, err, 998, "reorder failed") } func TestMailRulesReorder_ListPaginationFetchesAllRules(t *testing.T) { _, _, reg, cmd := newMailRulesReorderCommand(t) var tokens []string + var gotRuleIDs []string registerMailRulesListPage(reg, "me", map[string]interface{}{ "items": []interface{}{map[string]interface{}{"rule_id": "r1"}}, "has_more": true, @@ -231,15 +261,16 @@ func TestMailRulesReorder_ListPaginationFetchesAllRules(t *testing.T) { tokens = append(tokens, req.URL.Query().Get("page_token")) }) registerMailRulesReorder(reg, "me", func(req *http.Request, got map[string]interface{}) { - if gotIDs := interfaceSliceToStrings(got["rule_ids"]); !reflect.DeepEqual(gotIDs, []string{"r2", "r1"}) { - t.Fatalf("rule_ids = %#v, want [r2 r1]", got["rule_ids"]) - } + gotRuleIDs = interfaceSliceToStrings(got["rule_ids"]) }, map[string]interface{}{"code": 0, "msg": "ok", "data": map[string]interface{}{}}) cmd.setArgs([]string{"--as", "bot", "--params", `{"user_mailbox_id":"me"}`, "--data", `{"rule_ids":["r2"]}`}) if err := cmd.execute(); err != nil { t.Fatalf("Execute() error = %v", err) } + if !reflect.DeepEqual(gotRuleIDs, []string{"r2", "r1"}) { + t.Fatalf("rule_ids = %#v, want [r2 r1]", gotRuleIDs) + } if !reflect.DeepEqual(tokens, []string{"", "next-1"}) { t.Fatalf("page tokens = %v, want [ next-1]", tokens) } @@ -278,16 +309,32 @@ func assertServiceValidationError(t *testing.T, err error, wantSubstr string) { if !errors.As(err, &validationErr) { t.Fatalf("expected validation error, got %T: %v", err, err) } + requireProblem(t, err, errs.CategoryValidation, errs.SubtypeInvalidArgument, 0) + if validationErr.Param != "rule_ids" { + t.Fatalf("validation error param = %q, want rule_ids", validationErr.Param) + } if !strings.Contains(err.Error(), wantSubstr) { t.Fatalf("validation error = %q, want substring %q", err.Error(), wantSubstr) } } +func assertServiceAPIError(t *testing.T, err error, wantCode int, wantSubstr string) { + t.Helper() + var apiErr *errs.APIError + if !errors.As(err, &apiErr) { + t.Fatalf("expected API error, got %T: %v", err, err) + } + requireProblem(t, err, errs.CategoryAPI, errs.SubtypeUnknown, wantCode) + if !strings.Contains(err.Error(), wantSubstr) { + t.Fatalf("api error = %q, want substring %q", err.Error(), wantSubstr) + } +} + func interfaceSliceToStrings(v interface{}) []string { items, _ := v.([]interface{}) out := make([]string, 0, len(items)) for _, item := range items { - out = append(out, item.(string)) + out = append(out, fmt.Sprint(item)) } return out } diff --git a/cmd/service/service.go b/cmd/service/service.go index 3c220856ab..0d9ca49b33 100644 --- a/cmd/service/service.go +++ b/cmd/service/service.go @@ -401,6 +401,19 @@ func serviceMethodRun(opts *ServiceMethodOptions) error { return err } + var ac *client.APIClient + mailRulesReorderCompleted := false + if opts.SchemaPath == mailRulesReorderSchemaPath { + ac, err = f.NewAPIClientWithConfig(config) + if err != nil { + return err + } + if err := maybeCompleteMailRulesReorderIDs(opts.Ctx, ac, opts, &request, ac.CheckResponse); err != nil { + return err + } + mailRulesReorderCompleted = true + } + if opts.DryRun { if fileMeta != nil { return cmdutil.PrintDryRunWithFile(f.IOStreams.Out, request, config, opts.Format, fileMeta.FieldName, fileMeta.FilePath, fileMeta.FormFields) @@ -414,9 +427,11 @@ func serviceMethodRun(opts *ServiceMethodOptions) error { } } - ac, err := f.NewAPIClientWithConfig(config) - if err != nil { - return err + if ac == nil { + ac, err = f.NewAPIClientWithConfig(config) + if err != nil { + return err + } } out := f.IOStreams.Out @@ -430,8 +445,10 @@ func serviceMethodRun(opts *ServiceMethodOptions) error { // with MissingScopes / Identity / ConsoleURL populated from the response. checkErr := ac.CheckResponse - if err := maybeCompleteMailRulesReorderIDs(opts.Ctx, ac, opts, &request, checkErr); err != nil { - return err + if !mailRulesReorderCompleted { + if err := maybeCompleteMailRulesReorderIDs(opts.Ctx, ac, opts, &request, checkErr); err != nil { + return err + } } if opts.PageAll { From 25fe8bbda651ed8eff750ba335431585cfa7d823 Mon Sep 17 00:00:00 2001 From: yangr-happy <301323675+yangr-happy@users.noreply.github.com> Date: Wed, 5 Aug 2026 11:59:52 +0800 Subject: [PATCH 3/5] test(mail): require string rule IDs in reorder assertions Reject non-string rule_ids in reorder test helpers so mocked request assertions fail on invalid JSON element types. Change-Type: ci-fix Co-authored-by: TRAE CLI --- cmd/service/mail_rules_reorder_test.go | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/cmd/service/mail_rules_reorder_test.go b/cmd/service/mail_rules_reorder_test.go index 11f762e34b..aa738bec77 100644 --- a/cmd/service/mail_rules_reorder_test.go +++ b/cmd/service/mail_rules_reorder_test.go @@ -7,7 +7,6 @@ import ( "bytes" "encoding/json" "errors" - "fmt" "net/http" "reflect" "strings" @@ -108,7 +107,7 @@ func TestMailRulesReorder_CompletesPartialIDsBeforeReorder(t *testing.T) { }) registerMailRulesReorder(reg, "shared@example.com", func(req *http.Request, got map[string]interface{}) { reorderMailbox = req.URL.Path - gotRuleIDs = interfaceSliceToStrings(got["rule_ids"]) + gotRuleIDs = interfaceSliceToStrings(t, got["rule_ids"]) }, map[string]interface{}{"code": 0, "msg": "ok", "data": map[string]interface{}{"ok": true}}) cmd.setArgs([]string{ @@ -139,7 +138,7 @@ func TestMailRulesReorder_FullIDsRemainInRequestedOrder(t *testing.T) { "has_more": false, }, nil) registerMailRulesReorder(reg, "me", func(req *http.Request, got map[string]interface{}) { - gotRuleIDs = interfaceSliceToStrings(got["rule_ids"]) + gotRuleIDs = interfaceSliceToStrings(t, got["rule_ids"]) }, map[string]interface{}{"code": 0, "msg": "ok", "data": map[string]interface{}{}}) cmd.setArgs([]string{"--as", "bot", "--params", `{"user_mailbox_id":"me"}`, "--data", `{"rule_ids":["r3","r1","r2"]}`}) @@ -176,7 +175,7 @@ func TestMailRulesReorder_DryRunCompletesPartialIDs(t *testing.T) { if !ok { t.Fatalf("dry-run body = %#v, want object", dryRun["body"]) } - if gotIDs := interfaceSliceToStrings(body["rule_ids"]); !reflect.DeepEqual(gotIDs, []string{"r2", "r1", "r3"}) { + if gotIDs := interfaceSliceToStrings(t, body["rule_ids"]); !reflect.DeepEqual(gotIDs, []string{"r2", "r1", "r3"}) { t.Fatalf("dry-run rule_ids = %#v, want [r2 r1 r3]", gotIDs) } } @@ -261,7 +260,7 @@ func TestMailRulesReorder_ListPaginationFetchesAllRules(t *testing.T) { tokens = append(tokens, req.URL.Query().Get("page_token")) }) registerMailRulesReorder(reg, "me", func(req *http.Request, got map[string]interface{}) { - gotRuleIDs = interfaceSliceToStrings(got["rule_ids"]) + gotRuleIDs = interfaceSliceToStrings(t, got["rule_ids"]) }, map[string]interface{}{"code": 0, "msg": "ok", "data": map[string]interface{}{}}) cmd.setArgs([]string{"--as", "bot", "--params", `{"user_mailbox_id":"me"}`, "--data", `{"rule_ids":["r2"]}`}) @@ -330,11 +329,19 @@ func assertServiceAPIError(t *testing.T, err error, wantCode int, wantSubstr str } } -func interfaceSliceToStrings(v interface{}) []string { - items, _ := v.([]interface{}) +func interfaceSliceToStrings(t *testing.T, v interface{}) []string { + t.Helper() + items, ok := v.([]interface{}) + if !ok { + t.Fatalf("rule_ids = %#v, want []interface{}", v) + } out := make([]string, 0, len(items)) - for _, item := range items { - out = append(out, fmt.Sprint(item)) + for i, item := range items { + s, ok := item.(string) + if !ok { + t.Fatalf("rule_ids[%d] = %#v, want string", i, item) + } + out = append(out, s) } return out } From a3b1d7b6100aededc022241a930922ba06c19fd7 Mon Sep 17 00:00:00 2001 From: yangr-happy <301323675+yangr-happy@users.noreply.github.com> Date: Wed, 5 Aug 2026 12:20:51 +0800 Subject: [PATCH 4/5] test(mail): align reorder dry-run assertion Read the reordered request body from the dry-run API call envelope so the test validates the completed rule_ids actually sent by the command. Change-Type: ci-fix Co-authored-by: TRAE CLI --- cmd/service/mail_rules_reorder_test.go | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/cmd/service/mail_rules_reorder_test.go b/cmd/service/mail_rules_reorder_test.go index aa738bec77..c6716ca170 100644 --- a/cmd/service/mail_rules_reorder_test.go +++ b/cmd/service/mail_rules_reorder_test.go @@ -171,9 +171,17 @@ func TestMailRulesReorder_DryRunCompletesPartialIDs(t *testing.T) { if err := decoder.Decode(&dryRun); err != nil { t.Fatalf("decode dry-run output: %v\n%s", err, stdout.String()) } - body, ok := dryRun["body"].(map[string]any) + calls, ok := dryRun["api"].([]any) + if !ok || len(calls) != 1 { + t.Fatalf("dry-run api = %#v, want one call", dryRun["api"]) + } + call, ok := calls[0].(map[string]any) + if !ok { + t.Fatalf("dry-run api[0] = %#v, want object", calls[0]) + } + body, ok := call["body"].(map[string]any) if !ok { - t.Fatalf("dry-run body = %#v, want object", dryRun["body"]) + t.Fatalf("dry-run body = %#v, want object", call["body"]) } if gotIDs := interfaceSliceToStrings(t, body["rule_ids"]); !reflect.DeepEqual(gotIDs, []string{"r2", "r1", "r3"}) { t.Fatalf("dry-run rule_ids = %#v, want [r2 r1 r3]", gotIDs) From 92eea87bf53b2f4c2febe93d850d87ada68c3f8c Mon Sep 17 00:00:00 2001 From: yangr-happy <301323675+yangr-happy@users.noreply.github.com> Date: Wed, 5 Aug 2026 12:45:28 +0800 Subject: [PATCH 5/5] test(mail): accept dry-run envelope for reorder Change-Type: ci-fix Co-authored-by: TRAE CLI --- cmd/service/mail_rules_reorder_test.go | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/cmd/service/mail_rules_reorder_test.go b/cmd/service/mail_rules_reorder_test.go index c6716ca170..7a249c0397 100644 --- a/cmd/service/mail_rules_reorder_test.go +++ b/cmd/service/mail_rules_reorder_test.go @@ -171,9 +171,9 @@ func TestMailRulesReorder_DryRunCompletesPartialIDs(t *testing.T) { if err := decoder.Decode(&dryRun); err != nil { t.Fatalf("decode dry-run output: %v\n%s", err, stdout.String()) } - calls, ok := dryRun["api"].([]any) + calls, ok := dryRunAPICalls(dryRun) if !ok || len(calls) != 1 { - t.Fatalf("dry-run api = %#v, want one call", dryRun["api"]) + t.Fatalf("dry-run api = %#v, want one call", dryRun) } call, ok := calls[0].(map[string]any) if !ok { @@ -188,6 +188,18 @@ func TestMailRulesReorder_DryRunCompletesPartialIDs(t *testing.T) { } } +func dryRunAPICalls(dryRun map[string]any) ([]any, bool) { + if calls, ok := dryRun["api"].([]any); ok { + return calls, true + } + data, ok := dryRun["data"].(map[string]any) + if !ok { + return nil, false + } + calls, ok := data["api"].([]any) + return calls, ok +} + func TestMailRulesReorder_ValidationErrorsDoNotCallAPIs(t *testing.T) { tests := []struct { name string