fix(mail): resolve folder/label filter once per +triage list call (#1512)

buildListParams used to re-call resolveFolderID / resolveFolderName (and the
label counterparts) on every list page to assemble folder_id / label_id.
Because resolveListFilter already resolves the filter once before the
pagination loop, the second pass hit the folders/labels list API again on
every page — 1 + page_count calls total, which easily trips rate limits.

buildListParams now only assembles API params from the already-resolved
FolderID / LabelID produced by resolveListFilter; it no longer resolves
names or aliases. The default folder_id=INBOX is still applied when no
explicit filter is present, and only overridden when the caller supplied a
canonical folder ID. The runtime / mailboxID / dryRun parameters are kept
for signature stability (resolveListFilter and buildSearchParams share the
same call shape).

Adds TestMailTriageCustomFolderResolvesOnceAcrossListPages: a custom-folder
filter forced across two messages-list pages, with a non-reusable folders
list stub so any second folders API call fails the test. Updated the two
existing buildListParams alias tests to run resolveListFilter first, mirroring
the real DryRun/Execute call order.

sprint: S1

Co-authored-by: xukuncx <283114605+xukuncx@users.noreply.github.com>

Author: xukuncx

shortcuts/mail/mail_triage.go

@@ -770,13 +770,7 @@ func buildListParams(runtime *common.RuntimeContext, mailboxID string, f triageF
 				params["folder_id"] = folderIDFromFilter
 			}
 		} else {
-			resolved, err := resolveFolderID(runtime, mailboxID, folderIDFromFilter)
-			if err != nil {
-				return nil, err
-			}
-			if resolved != "" {
-				params["folder_id"] = resolved
-			}
+			params["folder_id"] = folderIDFromFilter
 		}
 	} else if folderFromFilter != "" {
 		if dryRun {
@@ -786,13 +780,7 @@ func buildListParams(runtime *common.RuntimeContext, mailboxID string, f triageF
 				params["folder_id"] = folderFromFilter
 			}
 		} else {
-			resolved, err := resolveFolderName(runtime, mailboxID, folderFromFilter)
-			if err != nil {
-				return nil, err
-			}
-			if resolved != "" {
-				params["folder_id"] = resolved
-			}
+			params["folder_id"] = folderFromFilter
 		}
 	}
 
@@ -811,13 +799,7 @@ func buildListParams(runtime *common.RuntimeContext, mailboxID string, f triageF
 				params["label_id"] = labelIDFromFilter
 			}
 		} else {
-			resolved, err := resolveLabelID(runtime, mailboxID, labelIDFromFilter)
-			if err != nil {
-				return nil, err
-			}
-			if resolved != "" {
-				params["label_id"] = resolved
-			}
+			params["label_id"] = labelIDFromFilter
 		}
 	} else if labelFromFilter != "" {
 		if dryRun {
@@ -827,13 +809,7 @@ func buildListParams(runtime *common.RuntimeContext, mailboxID string, f triageF
 				params["label_id"] = labelFromFilter
 			}
 		} else {
-			resolved, err := resolveLabelName(runtime, mailboxID, labelFromFilter)
-			if err != nil {
-				return nil, err
-			}
-			if resolved != "" {
-				params["label_id"] = resolved
-			}
+			params["label_id"] = labelFromFilter
 		}
 	}
 

shortcuts/mail/mail_triage_test.go

@@ -12,6 +12,7 @@ import (
 	"testing"
 
 	"github.com/larksuite/cli/errs"
+	"github.com/larksuite/cli/internal/auth"
 	"github.com/larksuite/cli/internal/cmdutil"
 	"github.com/larksuite/cli/internal/httpmock"
 	"github.com/larksuite/cli/shortcuts/common"
@@ -974,7 +975,11 @@ func TestBuildListParamsDryRunOnlyUnread(t *testing.T) {
 func TestBuildListParamsDryRunFolderAlias(t *testing.T) {
 	rt := runtimeForMailTriageTest(t, nil)
 	f := triageFilter{Folder: "sent"}
-	got, err := buildListParams(rt, "me", f, 20, "", true)
+	resolved, err := resolveListFilter(rt, "me", f, true)
+	if err != nil {
+		t.Fatalf("resolveListFilter: %v", err)
+	}
+	got, err := buildListParams(rt, "me", resolved, 20, "", true)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -983,10 +988,30 @@ func TestBuildListParamsDryRunFolderAlias(t *testing.T) {
 	}
 }
 
+func TestBuildListParamsDryRunCustomFolderPreservesInput(t *testing.T) {
+	rt := runtimeForMailTriageTest(t, nil)
+	f := triageFilter{Folder: "team-folder"}
+	resolved, err := resolveListFilter(rt, "me", f, true)
+	if err != nil {
+		t.Fatalf("resolveListFilter: %v", err)
+	}
+	got, err := buildListParams(rt, "me", resolved, 20, "", true)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if got["folder_id"] != "team-folder" {
+		t.Fatalf("expected dry-run folder_id=team-folder, got %v", got["folder_id"])
+	}
+}
+
 func TestBuildListParamsDryRunLabelAlias(t *testing.T) {
 	rt := runtimeForMailTriageTest(t, nil)
 	f := triageFilter{Label: "flagged"}
-	got, err := buildListParams(rt, "me", f, 10, "", true)
+	resolved, err := resolveListFilter(rt, "me", f, true)
+	if err != nil {
+		t.Fatalf("resolveListFilter: %v", err)
+	}
+	got, err := buildListParams(rt, "me", resolved, 10, "", true)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -995,6 +1020,25 @@ func TestBuildListParamsDryRunLabelAlias(t *testing.T) {
 	}
 }
 
+func TestBuildListParamsDryRunCustomLabelPreservesInput(t *testing.T) {
+	rt := runtimeForMailTriageTest(t, nil)
+	f := triageFilter{Label: "custom-label"}
+	resolved, err := resolveListFilter(rt, "me", f, true)
+	if err != nil {
+		t.Fatalf("resolveListFilter: %v", err)
+	}
+	got, err := buildListParams(rt, "me", resolved, 10, "", true)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if _, ok := got["folder_id"]; ok {
+		t.Fatalf("folder_id should not be set when label is specified, got %v", got["folder_id"])
+	}
+	if got["label_id"] != "custom-label" {
+		t.Fatalf("expected dry-run label_id=custom-label, got %v", got["label_id"])
+	}
+}
+
 // --- buildSearchParams additional coverage ---
 
 func TestBuildSearchParamsAllFilterFields(t *testing.T) {
@@ -1791,3 +1835,137 @@ func mailTriageSearchItem(messageID, subject string) map[string]interface{} {
 		},
 	}
 }
+
+// registerMailTriageFoldersListStub registers a NON-reusable stub for the
+// mailbox folders list API. Because it is non-reusable, any second hit returns
+// "httpmock: no stub for GET .../folders" — which is exactly the assertion we
+// use to prove resolveListFilter runs once and buildListParams does NOT
+// re-resolve. folderID/folderName is the single custom folder the API reports.
+func registerMailTriageFoldersListStub(reg *httpmock.Registry, mailbox, folderID, folderName string) {
+	reg.Register(&httpmock.Stub{
+		Method: "GET",
+		URL:    mailboxPath(mailbox, "folders"),
+		Body: map[string]interface{}{
+			"code": 0,
+			"data": map[string]interface{}{
+				"items": []interface{}{
+					map[string]interface{}{
+						"id":   folderID,
+						"name": folderName,
+					},
+				},
+			},
+		},
+	})
+}
+
+// registerMailTriageListPageStub registers one page of the messages list API,
+// disambiguated from sibling pages by a URL substring unique to that page
+// (e.g. "page_size=5" for page 1 vs "page_size=2" for page 2). The substring
+// must NOT depend on query-param ordering: map iteration makes param order
+// nondeterministic, so prefer a value-only token like "page_size=N" (the N
+// differs per page because pageSize = maxCount - fetched_so_far). Non-reusable
+// so reg.Verify catches under- or over-consumption.
+func registerMailTriageListPageStub(reg *httpmock.Registry, urlSubstring string, items []string, hasMore bool, pageToken string) {
+	data := map[string]interface{}{
+		"items":    items,
+		"has_more": hasMore,
+	}
+	if pageToken != "" {
+		data["page_token"] = pageToken
+	}
+	reg.Register(&httpmock.Stub{
+		Method: "GET",
+		URL:    urlSubstring,
+		Body: map[string]interface{}{
+			"code": 0,
+			"data": data,
+		},
+	})
+}
+
+// TestMailTriageCustomFolderResolvesOnceAcrossListPages is the regression test
+// for the bug where buildListParams re-called resolveFolderID on every list
+// page, turning "resolve once" into "1 + page_count" folder-list API calls and
+// easily tripping rate limits.
+//
+// Setup: a custom folder filter that forces resolveListFilter to hit the
+// folders list API once (to map folder name "team-folder" to folder_id), then two
+// messages-list pages. The folders list stub is non-reusable, so if
+// buildListParams re-resolves, the second hit fails with "no stub". The
+// messages-list stubs are page-specific (disambiguated by page_size in the
+// URL), so both pages are served and Verify asserts each fired exactly once.
+func TestMailTriageCustomFolderResolvesOnceAcrossListPages(t *testing.T) {
+	f, stdout, _, reg := mailShortcutTestFactory(t)
+	defer reg.Verify(t)
+
+	// listMailboxFolders (called once by resolveListFilter) gates on the
+	// mail:user_mailbox.folder:read scope, which the default test token does
+	// not carry. Re-store the token with that scope appended so the folders
+	// API call is actually exercised (and thus the non-reusable folders stub
+	// is the load-bearing "exactly once" assertion).
+	const folderScope = "mail:user_mailbox.folder:read"
+	cfg := mailTestConfig()
+	if stored := auth.GetStoredToken(cfg.AppID, cfg.UserOpenId); stored != nil {
+		if !strings.Contains(stored.Scope, folderScope) {
+			stored.Scope = stored.Scope + " " + folderScope
+			if err := auth.SetStoredToken(stored); err != nil {
+				t.Fatalf("re-store token with folder scope: %v", err)
+			}
+		}
+	}
+
+	const (
+		mailbox    = "me"
+		folderName = "team-folder"
+		folderID   = "fld_custom_team"
+		page2Token = "tok_page2"
+	)
+	// --max 5 with listPageMax=20 → pageSize = 5-0 = 5 on page 1, then 5-3 = 2
+	// on page 2. The page_size query value disambiguates the two list stubs.
+	page1IDs := []string{"msg_a", "msg_b", "msg_c"}
+	page2IDs := []string{"msg_d", "msg_e"}
+
+	// Folders list: registered exactly once, non-reusable. Any second folder
+	// lookup (the bug) fails the test with "no stub for GET .../folders".
+	registerMailTriageFoldersListStub(reg, mailbox, folderID, folderName)
+	// Messages list, page 1: 3 ids, has_more, hands off a page-2 token. The
+	// page_size value (5 = maxCount - 0) is unique to page 1; page 2 uses 2.
+	registerMailTriageListPageStub(reg, "page_size=5", page1IDs, true, page2Token)
+	// Messages list, page 2: 2 ids, terminal.
+	registerMailTriageListPageStub(reg, "page_size=2", page2IDs, false, "")
+	// Batch metadata fetch for all 5 ids.
+	registerMailTriageBatchStub(reg, mailbox, []map[string]interface{}{
+		mailTriageBatchMessage("msg_a", "Subject A"),
+		mailTriageBatchMessage("msg_b", "Subject B"),
+		mailTriageBatchMessage("msg_c", "Subject C"),
+		mailTriageBatchMessage("msg_d", "Subject D"),
+		mailTriageBatchMessage("msg_e", "Subject E"),
+	})
+
+	args := []string{
+		"+triage",
+		"--as", "user",
+		"--mailbox", mailbox,
+		"--filter", `{"folder":"` + folderName + `"}`,
+		"--max", "5",
+		"--format", "json",
+	}
+	if err := runMountedMailShortcut(t, MailTriage, args, f, stdout); err != nil {
+		t.Fatalf("unexpected error running +triage (likely a second folders API call — the bug): %v", err)
+	}
+
+	data := decodeMailTriageJSONOutput(t, stdout)
+	messages := mailTriageMessagesFromOutput(t, data)
+	if len(messages) != 5 {
+		t.Fatalf("expected 5 messages across 2 pages, got %d (stdout=%s)", len(messages), stdout.String())
+	}
+	if got := data["has_more"]; got != false {
+		t.Fatalf("expected has_more=false after exhausting pages, got %v", got)
+	}
+	// All registered stubs (1 folders + 2 list pages + 1 batch_get) are
+	// non-reusable; reg.Verify (deferred above) asserts each was matched
+	// exactly once. Combined with the non-reusable folders stub, this is the
+	// proof that the folders list API was called exactly once across both
+	// pages — the core invariant the fix restores.
+}