From f0f9509b6198ba26e37a5ffe95e7f295dfa3e76f Mon Sep 17 00:00:00 2001 From: kyalpha313 Date: Tue, 26 May 2026 21:56:07 +0800 Subject: [PATCH 1/4] feat(drive): add +list-comments shortcut with anchor-aware filtering --- shortcuts/drive/drive_list_comments.go | 535 ++++++++++++++++++ shortcuts/drive/drive_list_comments_test.go | 593 ++++++++++++++++++++ shortcuts/drive/shortcuts.go | 1 + shortcuts/drive/shortcuts_test.go | 1 + 4 files changed, 1130 insertions(+) create mode 100644 shortcuts/drive/drive_list_comments.go create mode 100644 shortcuts/drive/drive_list_comments_test.go diff --git a/shortcuts/drive/drive_list_comments.go b/shortcuts/drive/drive_list_comments.go new file mode 100644 index 0000000000..657f0383d8 --- /dev/null +++ b/shortcuts/drive/drive_list_comments.go @@ -0,0 +1,535 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package drive + +import ( + "context" + "fmt" + "html" + "io" + "regexp" + "sort" + "strings" + + "github.com/larksuite/cli/internal/output" + "github.com/larksuite/cli/internal/validate" + "github.com/larksuite/cli/shortcuts/common" +) + +// listCommentsPageSize is the page size used when listing file comments. +const listCommentsPageSize = 100 + +// listCommentsMaxPages caps pagination depth to prevent runaway loops on +// documents with very large comment histories. +const listCommentsMaxPages = 100 + +// stickyNoteQuote is the placeholder quote text the comments API returns when +// a comment is anchored to a sticky-note element (XML element type +// ). The actual sticky-note content is not exposed via the +// comments list API, so orphan detection falls back to "does the document +// still contain any readonly-block element?". +const stickyNoteQuote = "[Sticky note]" + +// structuralAnchorTag is the XML prefix used to detect the existence of any +// sticky-note element when validating a [Sticky note] comment. +const structuralAnchorTag = "]*>`) + +// whitespacePattern matches one or more whitespace characters (incl. +// newlines) for normalization to a single space. +var whitespacePattern = regexp.MustCompile(`\s+`) + +// DriveListComments lists comments on a docx with smart defaults: filter to +// unresolved + non-orphan anchors, ordered by anchor position, including +// replies and reactions. See https://github.com/larksuite/cli/issues/1111. +var DriveListComments = common.Shortcut{ + Service: "drive", + Command: "+list-comments", + Description: "List docx comments with smart defaults (unresolved + non-orphan + anchor-ordered + replies + reactions); wiki URLs are auto-unwrapped", + Risk: "read", + Scopes: []string{"docs:document.comment:read"}, + ConditionalScopes: []string{"wiki:node:retrieve"}, + AuthTypes: []string{"user", "bot"}, + HasFormat: true, + Flags: []common.Flag{ + { + Name: "doc", + Desc: "document URL or token (docx URL, bare token + --type=docx, or wiki URL resolving to docx)", + Required: true, + }, + { + Name: "type", + Desc: "document type (required when --doc is a bare token; MVP supports docx only)", + Enum: []string{"docx", "wiki"}, + }, + { + Name: "include-orphaned", + Type: "bool", + Desc: "include comments whose anchor text was deleted/rewritten (default: hide, matching the Lark UI)", + }, + { + Name: "include-resolved", + Type: "bool", + Desc: "include resolved comments (default: hide, i.e. is_solved=false only)", + }, + { + Name: "no-reactions", + Type: "bool", + Desc: "do not include reaction details on replies (default: reactions are fetched)", + }, + { + Name: "order", + Desc: "result order: anchor (by position in document, default) | created (by create_time)", + Default: "anchor", + Enum: []string{"anchor", "created"}, + }, + }, + Validate: validateListComments, + DryRun: dryRunListComments, + Execute: executeListComments, +} + +// validateListComments enforces docx-only scope for MVP and checks that --doc +// is either a recognized URL or a bare token paired with --type. +func validateListComments(_ context.Context, runtime *common.RuntimeContext) error { + raw := strings.TrimSpace(runtime.Str("doc")) + if raw == "" { + return output.ErrValidation("--doc cannot be empty") + } + ref, ok := parseListCommentsDocRef(raw, runtime.Str("type")) + if !ok { + if strings.Contains(raw, "://") { + return output.ErrValidation("unsupported --doc input %q: use a docx URL, a bare token with --type=docx, or a wiki URL", raw) + } + if strings.TrimSpace(runtime.Str("type")) == "" { + return output.ErrValidation("--type is required when --doc is a bare token (allowed: docx, wiki)") + } + return output.ErrValidation("--type %q is not supported by drive +list-comments (MVP supports docx and wiki resolving to docx)", runtime.Str("type")) + } + if ref.Type != "docx" && ref.Type != "wiki" { + return output.ErrValidation("--doc must resolve to docx (got %q); sheet/slides/file/doc are not supported by drive +list-comments yet", ref.Type) + } + if v := strings.TrimSpace(runtime.Str("order")); v != "" && v != "anchor" && v != "created" { + return output.ErrValidation("--order must be one of: anchor, created (got %q)", v) + } + return nil +} + +// parseListCommentsDocRef recognizes a docx or wiki URL, or a bare token +// combined with --type. Returns (ref, true) on success. +func parseListCommentsDocRef(raw, docType string) (common.ResourceRef, bool) { + ref, ok := common.ParseResourceURL(raw) + if ok { + return ref, true + } + if strings.Contains(raw, "://") { + return common.ResourceRef{}, false + } + t := strings.TrimSpace(docType) + if t == "" { + return common.ResourceRef{}, false + } + if t != "docx" && t != "wiki" { + return common.ResourceRef{}, false + } + return common.ResourceRef{Type: t, Token: raw}, true +} + +func dryRunListComments(_ context.Context, runtime *common.RuntimeContext) *common.DryRunAPI { + raw := strings.TrimSpace(runtime.Str("doc")) + ref, _ := parseListCommentsDocRef(raw, runtime.Str("type")) + isWiki := ref.Type == "wiki" + + dry := common.NewDryRunAPI() + stepCount := 0 + + if isWiki { + stepCount++ + dry.GET("/open-apis/wiki/v2/spaces/get_node"). + Desc(fmt.Sprintf("[%d] Resolve wiki node to underlying docx", stepCount)). + Params(map[string]interface{}{"token": ref.Token}) + } + + listToken := ref.Token + if isWiki { + listToken = "" + } + stepCount++ + listParams := map[string]interface{}{ + "file_type": "docx", + "page_size": listCommentsPageSize, + } + if !runtime.Bool("include-resolved") { + listParams["is_solved"] = false + } + if !runtime.Bool("no-reactions") { + listParams["need_reaction"] = true + } + dry.GET(fmt.Sprintf("/open-apis/drive/v1/files/%s/comments", listToken)). + Desc(fmt.Sprintf("[%d] List comments (paginated until exhausted)", stepCount)). + Params(listParams) + + stepCount++ + dry.POST(fmt.Sprintf("/open-apis/docs_ai/v1/documents/%s/fetch", listToken)). + Desc(fmt.Sprintf("[%d] Fetch document XML for orphan detection", stepCount)). + Body(map[string]interface{}{"format": "xml"}) + + dry.Desc(fmt.Sprintf("%d-step orchestration: list comments + fetch document + cross-check anchors", stepCount)) + return dry +} + +// commentItem holds a single comment card plus our derived anchor metadata. +type commentItem struct { + Raw map[string]interface{} + CommentID string + Quote string + IsWhole bool + IsSolved bool + CreateTime int64 + AnchorState string // "valid" | "structural" | "orphaned" + AnchorPosition int64 // byte offset within normalized doc text; -1 for orphan +} + +func executeListComments(_ context.Context, runtime *common.RuntimeContext) error { + raw := strings.TrimSpace(runtime.Str("doc")) + ref, ok := parseListCommentsDocRef(raw, runtime.Str("type")) + if !ok { + // Validate should have caught this; keep guard for safety. + return output.ErrValidation("unable to parse --doc input %q", raw) + } + + // Step 1: Resolve wiki to docx token if needed. + fileToken := ref.Token + var wikiToken string + if ref.Type == "wiki" { + wikiToken = ref.Token + fmt.Fprintf(runtime.IO().ErrOut, "Resolving wiki node: %s\n", common.MaskToken(wikiToken)) + data, err := runtime.CallAPI( + "GET", + "/open-apis/wiki/v2/spaces/get_node", + map[string]interface{}{"token": wikiToken}, + nil, + ) + if err != nil { + return err + } + node := common.GetMap(data, "node") + objType := common.GetString(node, "obj_type") + objToken := common.GetString(node, "obj_token") + if objType != "docx" { + return output.ErrValidation("wiki resolved to %q; drive +list-comments only supports docx (MVP)", objType) + } + if objToken == "" { + return output.Errorf(output.ExitAPI, "api_error", "wiki get_node returned empty obj_token") + } + fileToken = objToken + fmt.Fprintf(runtime.IO().ErrOut, "Wiki resolved to docx: %s\n", common.MaskToken(fileToken)) + } + + // Step 2: Paginate through all comments. + includeResolved := runtime.Bool("include-resolved") + needReactions := !runtime.Bool("no-reactions") + allRaw, err := listAllComments(runtime, fileToken, includeResolved, needReactions) + if err != nil { + return err + } + + // Step 3: Fetch document XML for orphan detection. + docXML, err := fetchDocxXML(runtime, fileToken) + if err != nil { + return err + } + normalized := normalizeDocContent(docXML) + hasStructuralAnchor := strings.Contains(docXML, structuralAnchorTag) + // Approximate position of the FIRST sticky-note anchor in normalized space. + // All sticky-anchored comments share this position for sorting purposes. + structuralPos := projectRawPosToNormalized(docXML, normalized, structuralAnchorTag) + + // Step 4: Build commentItem list with anchor_state + anchor_position. + items := make([]commentItem, 0, len(allRaw)) + for _, r := range allRaw { + items = append(items, buildCommentItem(r, normalized, hasStructuralAnchor, structuralPos)) + } + + // Step 5: Filter by include-orphaned. + includeOrphaned := runtime.Bool("include-orphaned") + filtered := items + if !includeOrphaned { + filtered = make([]commentItem, 0, len(items)) + for _, it := range items { + if it.AnchorState == "orphaned" { + continue + } + filtered = append(filtered, it) + } + } + + // Step 6: Sort. + orderKey := strings.TrimSpace(runtime.Str("order")) + if orderKey == "" { + orderKey = "anchor" + } + sortCommentItems(filtered, orderKey) + + // Step 7: Build output envelope. + outItems := make([]map[string]interface{}, 0, len(filtered)) + for _, it := range filtered { + m := cloneCommentMap(it.Raw) + m["anchor_state"] = it.AnchorState + m["anchor_position"] = it.AnchorPosition + outItems = append(outItems, m) + } + + counts := countByAnchorState(items) + result := map[string]interface{}{ + "items": outItems, + "file_token": fileToken, + "counts": counts, + } + if wikiToken != "" { + result["wiki_token"] = wikiToken + } + + fmt.Fprintf(runtime.IO().ErrOut, + "Comments: %d total (valid=%d, structural=%d, orphaned=%d); returned %d\n", + counts["total"], counts["valid"], counts["structural"], counts["orphaned"], len(filtered)) + + runtime.OutFormatRaw(result, nil, func(w io.Writer) { + for i, it := range filtered { + fmt.Fprintf(w, "[%d] %s (%s, pos=%d) %s\n", + i+1, it.CommentID, it.AnchorState, it.AnchorPosition, truncateQuote(it.Quote, 60)) + } + }) + return nil +} + +// listAllComments paginates through all comment cards on a docx, returning +// the raw maps from the API. Pagination is bounded by listCommentsMaxPages. +func listAllComments(runtime *common.RuntimeContext, fileToken string, includeResolved, needReactions bool) ([]map[string]interface{}, error) { + encodedToken := validate.EncodePathSegment(fileToken) + apiPath := fmt.Sprintf("/open-apis/drive/v1/files/%s/comments", encodedToken) + var pageToken string + var all []map[string]interface{} + for page := 0; page < listCommentsMaxPages; page++ { + params := map[string]interface{}{ + "file_type": "docx", + "page_size": listCommentsPageSize, + } + if !includeResolved { + params["is_solved"] = false + } + if needReactions { + params["need_reaction"] = true + } + if pageToken != "" { + params["page_token"] = pageToken + } + data, err := runtime.CallAPI("GET", apiPath, params, nil) + if err != nil { + return nil, err + } + if rawItems, ok := data["items"].([]interface{}); ok { + for _, it := range rawItems { + if m, ok := it.(map[string]interface{}); ok { + all = append(all, m) + } + } + } + hasMore, _ := data["has_more"].(bool) + if !hasMore { + return all, nil + } + next, _ := data["page_token"].(string) + if next == "" || next == pageToken { + return all, nil + } + pageToken = next + } + return all, nil +} + +// fetchDocxXML returns the document content as XML for orphan detection. +// `text` format was removed from the docs +fetch enum (see PR #1109), so we +// use `xml` and strip tags client-side. Markdown would also work but XML is +// preferred because it preserves structural elements like . +func fetchDocxXML(runtime *common.RuntimeContext, fileToken string) (string, error) { + apiPath := fmt.Sprintf("/open-apis/docs_ai/v1/documents/%s/fetch", + validate.EncodePathSegment(fileToken)) + body := map[string]interface{}{"format": "xml"} + data, err := runtime.CallAPI("POST", apiPath, nil, body) + if err != nil { + return "", err + } + doc := common.GetMap(data, "document") + return common.GetString(doc, "content"), nil +} + +// normalizeDocContent prepares a haystack for substring matching by stripping +// XML tags, decoding HTML entities, and removing ALL whitespace. +// +// Whitespace is removed (not just collapsed) so that text broken by inline +// tags matches correctly. Example: +// +// "

由 P2 / P3 事件聚类触发：…

" +// +// becomes "由P2/P3事件聚类触发：…" — the quote "由 P2 / P3 事件聚类触发：" then +// matches after the same normalization is applied to the needle. +func normalizeDocContent(content string) string { + stripped := xmlTagPattern.ReplaceAllString(content, "") + decoded := html.UnescapeString(stripped) + return whitespacePattern.ReplaceAllString(decoded, "") +} + +// normalizeQuoteNeedle prepares the first line of a comment quote for +// substring matching against a normalized document. Whitespace and HTML +// entities are stripped to keep the same coordinate system as +// normalizeDocContent. +func normalizeQuoteNeedle(quote string) string { + firstLine := strings.TrimSpace(strings.SplitN(quote, "\n", 2)[0]) + if firstLine == "" { + return "" + } + decoded := html.UnescapeString(firstLine) + return whitespacePattern.ReplaceAllString(decoded, "") +} + +// projectRawPosToNormalized returns the position of `marker` in `raw` +// mapped onto the normalized coordinate system by re-normalizing the prefix +// that precedes the marker. Returns -1 if the marker is not found. +// +// This is exact rather than a linear projection: it spends one extra +// normalize pass on the prefix to get the true position of the marker in +// normalized space, which keeps structural anchors (sticky notes etc.) +// correctly ordered relative to text anchors. +func projectRawPosToNormalized(raw, _ string, marker string) int64 { + idx := strings.Index(raw, marker) + if idx < 0 { + return -1 + } + return int64(len(normalizeDocContent(raw[:idx]))) +} + +// buildCommentItem parses an API comment card and computes anchor_state + +// anchor_position. The position is a byte offset in normalized doc text. +// +// Known limitations: +// - Short quote text that appears multiple times in the doc cannot be +// uniquely localized; the first occurrence wins. This can yield +// false-positive "valid" classification when the specific anchor +// instance has been deleted but the text still appears elsewhere. +// - Quotes broken by XML tags (e.g. "..." wrapping a substring) +// are matched after tag stripping; partial wrapping is handled. +func buildCommentItem(raw map[string]interface{}, normalized string, hasStructuralAnchor bool, structuralPos int64) commentItem { + quote, _ := raw["quote"].(string) + isWhole, _ := raw["is_whole"].(bool) + isSolved, _ := raw["is_solved"].(bool) + commentID, _ := raw["comment_id"].(string) + var createTime int64 + if v, ok := raw["create_time"].(float64); ok { + createTime = int64(v) + } + + item := commentItem{ + Raw: raw, + CommentID: commentID, + Quote: quote, + IsWhole: isWhole, + IsSolved: isSolved, + CreateTime: createTime, + } + + switch { + case isWhole: + item.AnchorState = "valid" + item.AnchorPosition = 0 + case quote == stickyNoteQuote: + if hasStructuralAnchor { + item.AnchorState = "structural" + if structuralPos >= 0 { + item.AnchorPosition = structuralPos + } else { + item.AnchorPosition = 0 + } + } else { + item.AnchorState = "orphaned" + item.AnchorPosition = -1 + } + default: + needle := normalizeQuoteNeedle(quote) + if needle == "" { + item.AnchorState = "orphaned" + item.AnchorPosition = -1 + break + } + pos := strings.Index(normalized, needle) + if pos >= 0 { + item.AnchorState = "valid" + item.AnchorPosition = int64(pos) + } else { + item.AnchorState = "orphaned" + item.AnchorPosition = -1 + } + } + return item +} + +// sortCommentItems sorts items in-place. Orphans always sort to the end. +// Within the non-orphan group: +// - order=anchor (default): by AnchorPosition ascending +// - order=created: by CreateTime ascending +func sortCommentItems(items []commentItem, orderKey string) { + sort.SliceStable(items, func(i, j int) bool { + a, b := items[i], items[j] + aOrphan := a.AnchorState == "orphaned" + bOrphan := b.AnchorState == "orphaned" + if aOrphan != bOrphan { + return !aOrphan + } + if orderKey == "created" { + return a.CreateTime < b.CreateTime + } + // anchor order + if a.AnchorPosition != b.AnchorPosition { + return a.AnchorPosition < b.AnchorPosition + } + // stable tiebreaker + return a.CreateTime < b.CreateTime + }) +} + +func countByAnchorState(items []commentItem) map[string]int { + out := map[string]int{ + "total": len(items), + "valid": 0, + "structural": 0, + "orphaned": 0, + } + for _, it := range items { + out[it.AnchorState]++ + } + return out +} + +func cloneCommentMap(m map[string]interface{}) map[string]interface{} { + if m == nil { + return nil + } + out := make(map[string]interface{}, len(m)) + for k, v := range m { + out[k] = v + } + return out +} + +func truncateQuote(s string, n int) string { + s = whitespacePattern.ReplaceAllString(s, " ") + runes := []rune(s) + if len(runes) <= n { + return s + } + return string(runes[:n]) + "..." +} diff --git a/shortcuts/drive/drive_list_comments_test.go b/shortcuts/drive/drive_list_comments_test.go new file mode 100644 index 0000000000..91f3a3ffeb --- /dev/null +++ b/shortcuts/drive/drive_list_comments_test.go @@ -0,0 +1,593 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package drive + +import ( + "context" + "encoding/json" + "strings" + "testing" + + "github.com/spf13/cobra" + + "github.com/larksuite/cli/internal/cmdutil" + "github.com/larksuite/cli/internal/core" + "github.com/larksuite/cli/internal/httpmock" + "github.com/larksuite/cli/shortcuts/common" +) + +// --- parseListCommentsDocRef --- + +func TestParseListCommentsDocRef_DocxURL(t *testing.T) { + t.Parallel() + ref, ok := parseListCommentsDocRef("https://example.feishu.cn/docx/doxAbc123", "") + if !ok { + t.Fatal("expected ok=true for docx URL") + } + if ref.Type != "docx" || ref.Token != "doxAbc123" { + t.Fatalf("got %+v", ref) + } +} + +func TestParseListCommentsDocRef_WikiURL(t *testing.T) { + t.Parallel() + ref, ok := parseListCommentsDocRef("https://example.feishu.cn/wiki/wikXyz789", "") + if !ok { + t.Fatal("expected ok=true for wiki URL") + } + if ref.Type != "wiki" || ref.Token != "wikXyz789" { + t.Fatalf("got %+v", ref) + } +} + +func TestParseListCommentsDocRef_BareTokenRequiresType(t *testing.T) { + t.Parallel() + _, ok := parseListCommentsDocRef("doxAbc123", "") + if ok { + t.Fatal("expected ok=false for bare token without --type") + } + ref, ok := parseListCommentsDocRef("doxAbc123", "docx") + if !ok { + t.Fatal("expected ok=true for bare token with --type=docx") + } + if ref.Type != "docx" || ref.Token != "doxAbc123" { + t.Fatalf("got %+v", ref) + } +} + +func TestParseListCommentsDocRef_RejectsUnsupportedType(t *testing.T) { + t.Parallel() + _, ok := parseListCommentsDocRef("token", "sheet") + if ok { + t.Fatal("expected ok=false for --type=sheet (not in MVP)") + } +} + +// --- validateListComments --- + +func newListCommentsCmd(t *testing.T, flags map[string]string) *cobra.Command { + t.Helper() + cmd := &cobra.Command{Use: "drive +list-comments"} + cmd.Flags().String("doc", "", "") + cmd.Flags().String("type", "", "") + cmd.Flags().Bool("include-orphaned", false, "") + cmd.Flags().Bool("include-resolved", false, "") + cmd.Flags().Bool("no-reactions", false, "") + cmd.Flags().String("order", "anchor", "") + for k, v := range flags { + if err := cmd.Flags().Set(k, v); err != nil { + t.Fatalf("set --%s=%q: %v", k, v, err) + } + } + return cmd +} + +func TestValidateListComments_EmptyDoc(t *testing.T) { + t.Parallel() + cmd := newListCommentsCmd(t, nil) + runtime := common.TestNewRuntimeContext(cmd, &core.CliConfig{}) + err := validateListComments(context.Background(), runtime) + if err == nil || !strings.Contains(err.Error(), "--doc cannot be empty") { + t.Fatalf("err = %v, want empty-doc error", err) + } +} + +func TestValidateListComments_UnsupportedURL(t *testing.T) { + t.Parallel() + cmd := newListCommentsCmd(t, map[string]string{"doc": "https://example.feishu.cn/sheets/shtAbc"}) + runtime := common.TestNewRuntimeContext(cmd, &core.CliConfig{}) + err := validateListComments(context.Background(), runtime) + if err == nil || !strings.Contains(err.Error(), "must resolve to docx") { + t.Fatalf("err = %v, want docx-only error", err) + } +} + +func TestValidateListComments_BareTokenWithoutType(t *testing.T) { + t.Parallel() + cmd := newListCommentsCmd(t, map[string]string{"doc": "doxAbc123"}) + runtime := common.TestNewRuntimeContext(cmd, &core.CliConfig{}) + err := validateListComments(context.Background(), runtime) + if err == nil || !strings.Contains(err.Error(), "--type is required") { + t.Fatalf("err = %v, want missing-type error", err) + } +} + +func TestValidateListComments_DocxURLAccepted(t *testing.T) { + t.Parallel() + cmd := newListCommentsCmd(t, map[string]string{"doc": "https://example.feishu.cn/docx/doxAbc123"}) + runtime := common.TestNewRuntimeContext(cmd, &core.CliConfig{}) + if err := validateListComments(context.Background(), runtime); err != nil { + t.Fatalf("unexpected error: %v", err) + } +} + +func TestValidateListComments_WikiURLAccepted(t *testing.T) { + t.Parallel() + cmd := newListCommentsCmd(t, map[string]string{"doc": "https://example.feishu.cn/wiki/wikAbc"}) + runtime := common.TestNewRuntimeContext(cmd, &core.CliConfig{}) + if err := validateListComments(context.Background(), runtime); err != nil { + t.Fatalf("unexpected error: %v", err) + } +} + +// --- normalizeDocContent / normalizeQuoteNeedle --- + +func TestNormalizeDocContent_StripsTagsEntitiesAndWhitespace(t *testing.T) { + t.Parallel() + in := `

由 P2 / P3 事件聚类触发/关联

` + got := normalizeDocContent(in) + // Tags stripped, entity decoded, ALL whitespace removed. + // Verifies that text broken by an inline tag is recombined and matches. + want := "由P2/P3事件聚类触发/关联" + if !strings.Contains(got, want) { + t.Fatalf("normalize failed: %q, want substring %q", got, want) + } +} + +func TestNormalizeQuoteNeedle_UsesFirstLine(t *testing.T) { + t.Parallel() + multiline := "体验驱动\n随单/年度满意度调查\n舆情反馈" + got := normalizeQuoteNeedle(multiline) + if got != "体验驱动" { + t.Fatalf("got %q, want 体验驱动", got) + } +} + +func TestNormalizeQuoteNeedle_RemovesAllWhitespace(t *testing.T) { + t.Parallel() + in := " hello world " + got := normalizeQuoteNeedle(in) + if got != "helloworld" { + t.Fatalf("got %q, want helloworld", got) + } +} + +func TestNormalizeQuoteNeedle_MatchesAcrossInlineTags(t *testing.T) { + t.Parallel() + // This test documents the cross-tag matching behavior: a needle that has + // whitespace in the original quote text must match content that was + // broken by inline tags in the document XML. + quote := "由 P2 / P3 事件聚类触发" + docContent := normalizeDocContent(`

由 P2 / P3 事件聚类触发：详细描述...

`) + needle := normalizeQuoteNeedle(quote) + if !strings.Contains(docContent, needle) { + t.Fatalf("needle %q not found in normalized doc %q (cross-tag match should succeed)", needle, docContent) + } +} + +func TestNormalizeQuoteNeedle_EmptyQuote(t *testing.T) { + t.Parallel() + if got := normalizeQuoteNeedle(""); got != "" { + t.Fatalf("got %q, want empty", got) + } + if got := normalizeQuoteNeedle("\n\n \n"); got != "" { + t.Fatalf("got %q, want empty for whitespace-only", got) + } +} + +// --- buildCommentItem --- + +func TestBuildCommentItem_WholeDocComment(t *testing.T) { + t.Parallel() + raw := map[string]interface{}{ + "comment_id": "c1", + "is_whole": true, + "create_time": float64(1700000000), + } + it := buildCommentItem(raw, "doc body", false, -1) + if it.AnchorState != "valid" || it.AnchorPosition != 0 { + t.Fatalf("got state=%q pos=%d, want valid/0", it.AnchorState, it.AnchorPosition) + } +} + +func TestBuildCommentItem_StickyWithReadonlyBlockPresent(t *testing.T) { + t.Parallel() + raw := map[string]interface{}{ + "comment_id": "c2", + "is_whole": false, + "quote": stickyNoteQuote, + "create_time": float64(1700000000), + } + it := buildCommentItem(raw, "ignored", true, 4800) + if it.AnchorState != "structural" { + t.Fatalf("got state=%q, want structural", it.AnchorState) + } + if it.AnchorPosition != 4800 { + t.Fatalf("got pos=%d, want 4800", it.AnchorPosition) + } +} + +func TestBuildCommentItem_StickyOrphan(t *testing.T) { + t.Parallel() + raw := map[string]interface{}{ + "comment_id": "c3", + "is_whole": false, + "quote": stickyNoteQuote, + } + it := buildCommentItem(raw, "ignored", false, -1) + if it.AnchorState != "orphaned" { + t.Fatalf("got state=%q, want orphaned", it.AnchorState) + } +} + +func TestBuildCommentItem_TextQuoteFound(t *testing.T) { + t.Parallel() + raw := map[string]interface{}{ + "comment_id": "c4", + "is_whole": false, + "quote": "硬件、软件", + } + doc := "some intro text 硬件、软件 trailing text" + it := buildCommentItem(raw, doc, false, -1) + if it.AnchorState != "valid" { + t.Fatalf("got state=%q, want valid", it.AnchorState) + } + wantPos := int64(strings.Index(doc, "硬件、软件")) + if it.AnchorPosition != wantPos { + t.Fatalf("got pos=%d, want %d", it.AnchorPosition, wantPos) + } +} + +func TestBuildCommentItem_TextQuoteOrphan(t *testing.T) { + t.Parallel() + raw := map[string]interface{}{ + "comment_id": "c5", + "is_whole": false, + "quote": "Initiate Project", + } + doc := "this doc does not mention it" + it := buildCommentItem(raw, doc, false, -1) + if it.AnchorState != "orphaned" || it.AnchorPosition != -1 { + t.Fatalf("got state=%q pos=%d, want orphaned/-1", it.AnchorState, it.AnchorPosition) + } +} + +func TestBuildCommentItem_MultilineQuoteUsesFirstLine(t *testing.T) { + t.Parallel() + raw := map[string]interface{}{ + "comment_id": "c6", + "is_whole": false, + "quote": "体验驱动\n随单/年度满意度调查\n舆情反馈", + } + // Doc still contains 体验驱动 elsewhere but not the sub-bullets — should + // still classify as valid based on first-line match. + doc := "earlier 体验驱动和管理驱动 later" + it := buildCommentItem(raw, doc, false, -1) + if it.AnchorState != "valid" { + t.Fatalf("got state=%q, want valid (first-line match)", it.AnchorState) + } +} + +// --- sortCommentItems --- + +func TestSortCommentItems_OrphansAtEnd(t *testing.T) { + t.Parallel() + items := []commentItem{ + {CommentID: "a", AnchorState: "valid", AnchorPosition: 100, CreateTime: 1}, + {CommentID: "b", AnchorState: "orphaned", AnchorPosition: -1, CreateTime: 2}, + {CommentID: "c", AnchorState: "valid", AnchorPosition: 50, CreateTime: 3}, + {CommentID: "d", AnchorState: "structural", AnchorPosition: 75, CreateTime: 4}, + } + sortCommentItems(items, "anchor") + gotIDs := make([]string, len(items)) + for i, it := range items { + gotIDs[i] = it.CommentID + } + // Expected: c (50), d (75 structural), a (100), b (orphan) + wantIDs := []string{"c", "d", "a", "b"} + for i, want := range wantIDs { + if gotIDs[i] != want { + t.Fatalf("position %d: got %q want %q (full: %v)", i, gotIDs[i], want, gotIDs) + } + } +} + +func TestSortCommentItems_CreatedOrder(t *testing.T) { + t.Parallel() + items := []commentItem{ + {CommentID: "a", AnchorState: "valid", AnchorPosition: 100, CreateTime: 3}, + {CommentID: "b", AnchorState: "valid", AnchorPosition: 50, CreateTime: 1}, + {CommentID: "c", AnchorState: "orphaned", AnchorPosition: -1, CreateTime: 2}, + } + sortCommentItems(items, "created") + // Expected: b (1), a (3), c (orphan at end despite time=2) + gotIDs := make([]string, len(items)) + for i, it := range items { + gotIDs[i] = it.CommentID + } + wantIDs := []string{"b", "a", "c"} + for i, want := range wantIDs { + if gotIDs[i] != want { + t.Fatalf("position %d: got %q want %q (full: %v)", i, gotIDs[i], want, gotIDs) + } + } +} + +// --- projectRawPosToNormalized --- + +func TestProjectRawPosToNormalized_ExactPrefixNormalize(t *testing.T) { + t.Parallel() + raw := "

01234

56789

" + got := projectRawPosToNormalized(raw, "", "01234

" normalizes to "01234" (length 5), so the marker's + // position in normalized space is 5. + if got != 5 { + t.Fatalf("got %d, want 5", got) + } +} + +func TestProjectRawPosToNormalized_NotFound(t *testing.T) { + t.Parallel() + got := projectRawPosToNormalized("no marker here", "normalized", "introduction

硬件、软件 are technical

trailing

`, + }, + }, + }, + }) + + err := mountAndRunDrive(t, DriveListComments, []string{ + "+list-comments", + "--doc", "doxTest", + "--type", "docx", + "--as", "bot", + }, f, stdout) + if err != nil { + t.Fatalf("expected success, got: %v", err) + } + + out := decodeDriveEnvelope(t, stdout) + items, ok := out["items"].([]interface{}) + if !ok { + t.Fatalf("missing items array: %#v", out) + } + if len(items) != 2 { + t.Fatalf("expected 2 items after orphan filter, got %d", len(items)) + } + counts, _ := out["counts"].(map[string]interface{}) + if counts["total"] != float64(3) { + t.Fatalf("counts.total = %#v, want 3", counts["total"]) + } + if counts["valid"] != float64(1) { + t.Fatalf("counts.valid = %#v, want 1", counts["valid"]) + } + if counts["structural"] != float64(1) { + t.Fatalf("counts.structural = %#v, want 1", counts["structural"]) + } + if counts["orphaned"] != float64(1) { + t.Fatalf("counts.orphaned = %#v, want 1", counts["orphaned"]) + } +} + +func TestDriveListComments_E2E_IncludeOrphanedKeepsAll(t *testing.T) { + cfg := &core.CliConfig{AppID: "drive-listcomments-include-orphan", AppSecret: "test-secret", Brand: core.BrandFeishu} + f, stdout, _, reg := cmdutil.TestFactory(t, cfg) + + reg.Register(&httpmock.Stub{ + Method: "GET", + URL: "/open-apis/drive/v1/files/doxTest2/comments", + Body: map[string]interface{}{ + "code": 0, "msg": "ok", + "data": map[string]interface{}{ + "has_more": false, + "items": []map[string]interface{}{ + {"comment_id": "v1", "quote": "硬件", "is_whole": false, "create_time": float64(1)}, + {"comment_id": "o1", "quote": "missing", "is_whole": false, "create_time": float64(2)}, + }, + }, + }, + }) + reg.Register(&httpmock.Stub{ + Method: "POST", + URL: "/open-apis/docs_ai/v1/documents/doxTest2/fetch", + Body: map[string]interface{}{ + "code": 0, "msg": "ok", + "data": map[string]interface{}{ + "document": map[string]interface{}{ + "content": "

硬件 only

", + }, + }, + }, + }) + + err := mountAndRunDrive(t, DriveListComments, []string{ + "+list-comments", + "--doc", "doxTest2", + "--type", "docx", + "--include-orphaned", + "--as", "bot", + }, f, stdout) + if err != nil { + t.Fatalf("expected success, got: %v", err) + } + + out := decodeDriveEnvelope(t, stdout) + items, _ := out["items"].([]interface{}) + if len(items) != 2 { + t.Fatalf("expected 2 items with --include-orphaned, got %d", len(items)) + } +} diff --git a/shortcuts/drive/shortcuts.go b/shortcuts/drive/shortcuts.go index 3da5fdf583..c621720470 100644 --- a/shortcuts/drive/shortcuts.go +++ b/shortcuts/drive/shortcuts.go @@ -34,5 +34,6 @@ func Shortcuts() []common.Shortcut { DriveSecureLabelUpdate, DriveSearch, DriveInspect, + DriveListComments, } } diff --git a/shortcuts/drive/shortcuts_test.go b/shortcuts/drive/shortcuts_test.go index a38a5c0f4e..7b26aa1130 100644 --- a/shortcuts/drive/shortcuts_test.go +++ b/shortcuts/drive/shortcuts_test.go @@ -37,6 +37,7 @@ func TestShortcutsIncludesExpectedCommands(t *testing.T) { "+secure-label-update", "+search", "+inspect", + "+list-comments", } if len(got) != len(want) { From 247ad03444bd6ce5078bfcb4b3a1981f70b71d06 Mon Sep 17 00:00:00 2001 From: kyalpha313 Date: Tue, 26 May 2026 22:18:17 +0800 Subject: [PATCH 2/4] test(drive): isolate config dir in +list-comments E2E tests Add t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) to the two httpmock E2E tests for drive +list-comments, matching the pattern used by drive_inspect_test.go and AGENTS.md guidance for config-state isolation. Addresses CodeRabbit review on #1114. --- shortcuts/drive/drive_list_comments_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/shortcuts/drive/drive_list_comments_test.go b/shortcuts/drive/drive_list_comments_test.go index 91f3a3ffeb..5b8fa4b04c 100644 --- a/shortcuts/drive/drive_list_comments_test.go +++ b/shortcuts/drive/drive_list_comments_test.go @@ -456,6 +456,7 @@ func TestDryRunListComments_NoReactionsDropsParam(t *testing.T) { // --- End-to-end execution with httpmock --- func TestDriveListComments_E2E_FiltersOrphanedByDefault(t *testing.T) { + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) cfg := &core.CliConfig{AppID: "drive-listcomments-e2e", AppSecret: "test-secret", Brand: core.BrandFeishu} f, stdout, _, reg := cmdutil.TestFactory(t, cfg) @@ -544,6 +545,7 @@ func TestDriveListComments_E2E_FiltersOrphanedByDefault(t *testing.T) { } func TestDriveListComments_E2E_IncludeOrphanedKeepsAll(t *testing.T) { + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) cfg := &core.CliConfig{AppID: "drive-listcomments-include-orphan", AppSecret: "test-secret", Brand: core.BrandFeishu} f, stdout, _, reg := cmdutil.TestFactory(t, cfg) From 656e9b02ee55fd99a3a023e39b479ba7bdf2b00b Mon Sep 17 00:00:00 2001 From: kyalpha313 Date: Wed, 27 May 2026 10:49:52 +0800 Subject: [PATCH 3/4] test(drive): drop unnecessary CONFIG_DIR isolation in +list-comments tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Revert the t.Setenv("LARKSUITE_CLI_CONFIG_DIR", ...) added in 64e65b6. Per the repo learning from PR #343, tests built via cmdutil.TestFactory(t, config) use an in-memory config closure and never touch the filesystem, so isolating CONFIG_DIR has no effect. Only the real NewDefault() factory path needs it. Verified: cmdutil.TestFactory sets Config: func() (*core.CliConfig, error) { return config, nil } — no filesystem access. --- shortcuts/drive/drive_list_comments_test.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/shortcuts/drive/drive_list_comments_test.go b/shortcuts/drive/drive_list_comments_test.go index 5b8fa4b04c..91f3a3ffeb 100644 --- a/shortcuts/drive/drive_list_comments_test.go +++ b/shortcuts/drive/drive_list_comments_test.go @@ -456,7 +456,6 @@ func TestDryRunListComments_NoReactionsDropsParam(t *testing.T) { // --- End-to-end execution with httpmock --- func TestDriveListComments_E2E_FiltersOrphanedByDefault(t *testing.T) { - t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) cfg := &core.CliConfig{AppID: "drive-listcomments-e2e", AppSecret: "test-secret", Brand: core.BrandFeishu} f, stdout, _, reg := cmdutil.TestFactory(t, cfg) @@ -545,7 +544,6 @@ func TestDriveListComments_E2E_FiltersOrphanedByDefault(t *testing.T) { } func TestDriveListComments_E2E_IncludeOrphanedKeepsAll(t *testing.T) { - t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) cfg := &core.CliConfig{AppID: "drive-listcomments-include-orphan", AppSecret: "test-secret", Brand: core.BrandFeishu} f, stdout, _, reg := cmdutil.TestFactory(t, cfg) From b86407ac11e94aae576ba52788e1bdc49b1b826a Mon Sep 17 00:00:00 2001 From: kyalpha313 Date: Mon, 8 Jun 2026 18:18:22 +0800 Subject: [PATCH 4/4] feat(drive): rewrite +list-comments using need_relation per #1258 Replaces the client-side quote/text-matching orphan detection with the server-supplied anchor identifiers from `need_relation=true`, following the field design documented in #1258. Behavior: - list comments with `need_relation=true` to obtain `relation.relation` (containing `positionInfo.blockID`) and `content_deleted` - fetch docs XML with `export_block_id=true`; build a block-id index for stable anchor ordering (replaces the prior text-position projection that broke on multi-occurrence and tag-broken anchors) - use `content_deleted` as the orphan signal (resolves the prior "short anchor multi-occurrence" false-positive limitation) - use `parent_type` / `parent_token` to place embedded-resource comments (Sheet / Base / Whiteboard); intentionally downgrade these to `parent_resource_exact` rather than claiming exact internal ordering, since docs XML only identifies the embedding block Output contract additions: - `anchor_block_id`: per-comment anchor (from positionInfo.blockID) - `location_accuracy`: relation_exact | parent_resource_exact | weak_inferred | content_deleted | whole_document - `content_deleted`: server-side orphan signal Tests: - unit: relation parsing, malformed-JSON fallback, content-deleted orphan classification, relation-exact / parent-resource-exact anchoring, embedded-relation downgrade, sorted output, dry-run request shape, httpmock E2E - dry-run E2E (`tests/cli_e2e/drive/drive_list_comments_dryrun_test.go`): asserts `need_relation=true`, `need_reaction=true`, `is_solved=false`, and fetch body `format=xml` with `export_block_id=true` - opt-in live E2E (`tests/cli_e2e/drive/drive_list_comments_workflow_test.go`): gated by `LARK_DRIVE_LIST_COMMENTS_E2E=1`; user identity; creates a docx, adds a block comment, lists, asserts relation metadata, cleans up - coverage doc updated to 11 / 32 = 34.4% Live validation against a real docx: composition now matches the Lark UI side panel for 21 of 22 originally observed comments (the prior algorithm had a 2-item swap error from text-matching false positives); `[Sticky note]` and embedded `` comments now land at their correct positions. --- shortcuts/drive/drive_list_comments.go | 376 +++++++++++------- shortcuts/drive/drive_list_comments_test.go | 363 ++++++++++------- tests/cli_e2e/drive/coverage.md | 8 +- .../drive/drive_list_comments_dryrun_test.go | 42 ++ .../drive_list_comments_workflow_test.go | 200 ++++++++++ 5 files changed, 706 insertions(+), 283 deletions(-) create mode 100644 tests/cli_e2e/drive/drive_list_comments_dryrun_test.go create mode 100644 tests/cli_e2e/drive/drive_list_comments_workflow_test.go diff --git a/shortcuts/drive/drive_list_comments.go b/shortcuts/drive/drive_list_comments.go index 657f0383d8..9a277dd9c7 100644 --- a/shortcuts/drive/drive_list_comments.go +++ b/shortcuts/drive/drive_list_comments.go @@ -5,10 +5,10 @@ package drive import ( "context" + "encoding/json" + "encoding/xml" "fmt" - "html" "io" - "regexp" "sort" "strings" @@ -24,25 +24,6 @@ const listCommentsPageSize = 100 // documents with very large comment histories. const listCommentsMaxPages = 100 -// stickyNoteQuote is the placeholder quote text the comments API returns when -// a comment is anchored to a sticky-note element (XML element type -// ). The actual sticky-note content is not exposed via the -// comments list API, so orphan detection falls back to "does the document -// still contain any readonly-block element?". -const stickyNoteQuote = "[Sticky note]" - -// structuralAnchorTag is the XML prefix used to detect the existence of any -// sticky-note element when validating a [Sticky note] comment. -const structuralAnchorTag = "]*>`) - -// whitespacePattern matches one or more whitespace characters (incl. -// newlines) for normalization to a single space. -var whitespacePattern = regexp.MustCompile(`\s+`) - // DriveListComments lists comments on a docx with smart defaults: filter to // unresolved + non-orphan anchors, ordered by anchor position, including // replies and reactions. See https://github.com/larksuite/cli/issues/1111. @@ -51,7 +32,7 @@ var DriveListComments = common.Shortcut{ Command: "+list-comments", Description: "List docx comments with smart defaults (unresolved + non-orphan + anchor-ordered + replies + reactions); wiki URLs are auto-unwrapped", Risk: "read", - Scopes: []string{"docs:document.comment:read"}, + Scopes: []string{"docs:document.comment:read", "docx:document:readonly"}, ConditionalScopes: []string{"wiki:node:retrieve"}, AuthTypes: []string{"user", "bot"}, HasFormat: true, @@ -160,8 +141,9 @@ func dryRunListComments(_ context.Context, runtime *common.RuntimeContext) *comm } stepCount++ listParams := map[string]interface{}{ - "file_type": "docx", - "page_size": listCommentsPageSize, + "file_type": "docx", + "page_size": listCommentsPageSize, + "need_relation": true, } if !runtime.Bool("include-resolved") { listParams["is_solved"] = false @@ -175,23 +157,28 @@ func dryRunListComments(_ context.Context, runtime *common.RuntimeContext) *comm stepCount++ dry.POST(fmt.Sprintf("/open-apis/docs_ai/v1/documents/%s/fetch", listToken)). - Desc(fmt.Sprintf("[%d] Fetch document XML for orphan detection", stepCount)). - Body(map[string]interface{}{"format": "xml"}) + Desc(fmt.Sprintf("[%d] Fetch document XML with block IDs for anchor ordering", stepCount)). + Body(buildListCommentsFetchBody()) - dry.Desc(fmt.Sprintf("%d-step orchestration: list comments + fetch document + cross-check anchors", stepCount)) + dry.Desc(fmt.Sprintf("%d-step orchestration: list comments with relation + fetch block IDs + order anchors", stepCount)) return dry } // commentItem holds a single comment card plus our derived anchor metadata. type commentItem struct { - Raw map[string]interface{} - CommentID string - Quote string - IsWhole bool - IsSolved bool - CreateTime int64 - AnchorState string // "valid" | "structural" | "orphaned" - AnchorPosition int64 // byte offset within normalized doc text; -1 for orphan + Raw map[string]interface{} + CommentID string + Quote string + IsWhole bool + IsSolved bool + CreateTime int64 + AnchorState string // "valid" | "structural" | "orphaned" + AnchorPosition int64 // block order in docs fetch XML; -1 when unknown/orphaned + AnchorBlockID string + LocationAccuracy string // relation_exact | parent_resource_exact | weak_inferred | content_deleted + ParentType string + ParentToken string + ContentDeleted bool } func executeListComments(_ context.Context, runtime *common.RuntimeContext) error { @@ -238,21 +225,17 @@ func executeListComments(_ context.Context, runtime *common.RuntimeContext) erro return err } - // Step 3: Fetch document XML for orphan detection. + // Step 3: Fetch document XML with block IDs for stable anchor ordering. docXML, err := fetchDocxXML(runtime, fileToken) if err != nil { return err } - normalized := normalizeDocContent(docXML) - hasStructuralAnchor := strings.Contains(docXML, structuralAnchorTag) - // Approximate position of the FIRST sticky-note anchor in normalized space. - // All sticky-anchored comments share this position for sorting purposes. - structuralPos := projectRawPosToNormalized(docXML, normalized, structuralAnchorTag) + blockIndex := buildDocBlockIndex(docXML) - // Step 4: Build commentItem list with anchor_state + anchor_position. + // Step 4: Build commentItem list with relation-derived location metadata. items := make([]commentItem, 0, len(allRaw)) for _, r := range allRaw { - items = append(items, buildCommentItem(r, normalized, hasStructuralAnchor, structuralPos)) + items = append(items, buildCommentItem(r, blockIndex)) } // Step 5: Filter by include-orphaned. @@ -281,6 +264,11 @@ func executeListComments(_ context.Context, runtime *common.RuntimeContext) erro m := cloneCommentMap(it.Raw) m["anchor_state"] = it.AnchorState m["anchor_position"] = it.AnchorPosition + m["location_accuracy"] = it.LocationAccuracy + m["content_deleted"] = it.ContentDeleted + if it.AnchorBlockID != "" { + m["anchor_block_id"] = it.AnchorBlockID + } outItems = append(outItems, m) } @@ -316,8 +304,9 @@ func listAllComments(runtime *common.RuntimeContext, fileToken string, includeRe var all []map[string]interface{} for page := 0; page < listCommentsMaxPages; page++ { params := map[string]interface{}{ - "file_type": "docx", - "page_size": listCommentsPageSize, + "file_type": "docx", + "page_size": listCommentsPageSize, + "need_relation": true, } if !includeResolved { params["is_solved"] = false @@ -352,14 +341,13 @@ func listAllComments(runtime *common.RuntimeContext, fileToken string, includeRe return all, nil } -// fetchDocxXML returns the document content as XML for orphan detection. -// `text` format was removed from the docs +fetch enum (see PR #1109), so we -// use `xml` and strip tags client-side. Markdown would also work but XML is -// preferred because it preserves structural elements like . +// fetchDocxXML returns document XML with block IDs exported for comment anchor +// ordering. The comment API supplies the anchor block ID via need_relation; the +// docs fetch call supplies the current document block order. func fetchDocxXML(runtime *common.RuntimeContext, fileToken string) (string, error) { apiPath := fmt.Sprintf("/open-apis/docs_ai/v1/documents/%s/fetch", validate.EncodePathSegment(fileToken)) - body := map[string]interface{}{"format": "xml"} + body := buildListCommentsFetchBody() data, err := runtime.CallAPI("POST", apiPath, nil, body) if err != nil { return "", err @@ -368,118 +356,231 @@ func fetchDocxXML(runtime *common.RuntimeContext, fileToken string) (string, err return common.GetString(doc, "content"), nil } -// normalizeDocContent prepares a haystack for substring matching by stripping -// XML tags, decoding HTML entities, and removing ALL whitespace. -// -// Whitespace is removed (not just collapsed) so that text broken by inline -// tags matches correctly. Example: -// -// "

由 P2 / P3 事件聚类触发：…

" -// -// becomes "由P2/P3事件聚类触发：…" — the quote "由 P2 / P3 事件聚类触发：" then -// matches after the same normalization is applied to the needle. -func normalizeDocContent(content string) string { - stripped := xmlTagPattern.ReplaceAllString(content, "") - decoded := html.UnescapeString(stripped) - return whitespacePattern.ReplaceAllString(decoded, "") +func buildListCommentsFetchBody() map[string]interface{} { + return map[string]interface{}{ + "format": "xml", + "export_option": map[string]interface{}{ + "export_block_id": true, + }, + } } -// normalizeQuoteNeedle prepares the first line of a comment quote for -// substring matching against a normalized document. Whitespace and HTML -// entities are stripped to keep the same coordinate system as -// normalizeDocContent. -func normalizeQuoteNeedle(quote string) string { - firstLine := strings.TrimSpace(strings.SplitN(quote, "\n", 2)[0]) - if firstLine == "" { - return "" - } - decoded := html.UnescapeString(firstLine) - return whitespacePattern.ReplaceAllString(decoded, "") +type docBlockIndex struct { + orderByBlockID map[string]int64 + blockByToken map[string]string } -// projectRawPosToNormalized returns the position of `marker` in `raw` -// mapped onto the normalized coordinate system by re-normalizing the prefix -// that precedes the marker. Returns -1 if the marker is not found. -// -// This is exact rather than a linear projection: it spends one extra -// normalize pass on the prefix to get the true position of the marker in -// normalized space, which keeps structural anchors (sticky notes etc.) -// correctly ordered relative to text anchors. -func projectRawPosToNormalized(raw, _ string, marker string) int64 { - idx := strings.Index(raw, marker) - if idx < 0 { +func buildDocBlockIndex(content string) docBlockIndex { + idx := docBlockIndex{ + orderByBlockID: map[string]int64{}, + blockByToken: map[string]string{}, + } + decoder := xml.NewDecoder(strings.NewReader(content)) + var order int64 + for { + token, err := decoder.Token() + if err != nil { + break + } + start, ok := token.(xml.StartElement) + if !ok { + continue + } + blockID := firstXMLAttr(start, "id", "block_id") + if blockID == "" { + continue + } + if _, exists := idx.orderByBlockID[blockID]; !exists { + idx.orderByBlockID[blockID] = order + order++ + } + for _, attr := range start.Attr { + if attr.Value == "" { + continue + } + switch attr.Name.Local { + case "token", "spreadsheet_token", "base_token", "app_token", "whiteboard_token": + idx.blockByToken[attr.Value] = blockID + } + } + } + return idx +} + +func firstXMLAttr(start xml.StartElement, names ...string) string { + for _, want := range names { + for _, attr := range start.Attr { + if attr.Name.Local == want { + return attr.Value + } + } + } + return "" +} + +func (idx docBlockIndex) orderOfBlock(blockID string) int64 { + if blockID == "" { + return -1 + } + order, ok := idx.orderByBlockID[blockID] + if !ok { return -1 } - return int64(len(normalizeDocContent(raw[:idx]))) + return order +} + +func (idx docBlockIndex) lookupEmbeddedToken(parentToken string) (string, int64, bool) { + for _, candidate := range parentTokenCandidates(parentToken) { + if blockID := idx.blockByToken[candidate]; blockID != "" { + return blockID, idx.orderOfBlock(blockID), true + } + } + return "", -1, false } -// buildCommentItem parses an API comment card and computes anchor_state + -// anchor_position. The position is a byte offset in normalized doc text. -// -// Known limitations: -// - Short quote text that appears multiple times in the doc cannot be -// uniquely localized; the first occurrence wins. This can yield -// false-positive "valid" classification when the specific anchor -// instance has been deleted but the text still appears elsewhere. -// - Quotes broken by XML tags (e.g. "..." wrapping a substring) -// are matched after tag stripping; partial wrapping is handled. -func buildCommentItem(raw map[string]interface{}, normalized string, hasStructuralAnchor bool, structuralPos int64) commentItem { +func parentTokenCandidates(parentToken string) []string { + parentToken = strings.TrimSpace(parentToken) + if parentToken == "" { + return nil + } + candidates := []string{parentToken} + if idx := strings.LastIndex(parentToken, "_tbl"); idx > 0 { + candidates = append(candidates, parentToken[:idx]) + } + if idx := strings.LastIndex(parentToken, "_"); idx > 0 { + candidates = append(candidates, parentToken[:idx]) + } + if idx := strings.Index(parentToken, "_"); idx > 0 { + candidates = append(candidates, parentToken[:idx]) + } + return candidates +} + +// extractCommentRelation parses the relation payload returned by +// need_relation=true. The nested relation.relation value is itself a JSON +// string, keyed by server object identity; positionInfo.blockID is the stable +// docx block anchor we need. +func extractCommentRelation(raw map[string]interface{}) (blockID string, contentDeleted bool, ok bool) { + relation := common.GetMap(raw, "relation") + if relation == nil { + return "", false, false + } + contentDeleted = common.GetBool(relation, "content_deleted") + relJSON := strings.TrimSpace(common.GetString(relation, "relation")) + if relJSON == "" { + return "", contentDeleted, false + } + var parsed map[string]interface{} + if err := json.Unmarshal([]byte(relJSON), &parsed); err != nil { + return "", contentDeleted, false + } + blockID = findRelationBlockID(parsed) + return blockID, contentDeleted, blockID != "" +} + +func findRelationBlockID(v interface{}) string { + switch node := v.(type) { + case map[string]interface{}: + if pos, ok := node["positionInfo"].(map[string]interface{}); ok { + if blockID, _ := pos["blockID"].(string); blockID != "" { + return blockID + } + if blockID, _ := pos["block_id"].(string); blockID != "" { + return blockID + } + } + for _, child := range node { + if blockID := findRelationBlockID(child); blockID != "" { + return blockID + } + } + case []interface{}: + for _, child := range node { + if blockID := findRelationBlockID(child); blockID != "" { + return blockID + } + } + } + return "" +} + +// buildCommentItem parses an API comment card and computes relation-derived +// anchor_state + anchor_position. Position is current document block order, not +// a byte offset, and -1 means the location is unknown or deleted. +func buildCommentItem(raw map[string]interface{}, idx docBlockIndex) commentItem { quote, _ := raw["quote"].(string) isWhole, _ := raw["is_whole"].(bool) isSolved, _ := raw["is_solved"].(bool) commentID, _ := raw["comment_id"].(string) + parentType, _ := raw["parent_type"].(string) + parentToken, _ := raw["parent_token"].(string) var createTime int64 if v, ok := raw["create_time"].(float64); ok { createTime = int64(v) } item := commentItem{ - Raw: raw, - CommentID: commentID, - Quote: quote, - IsWhole: isWhole, - IsSolved: isSolved, - CreateTime: createTime, - } - - switch { - case isWhole: - item.AnchorState = "valid" - item.AnchorPosition = 0 - case quote == stickyNoteQuote: - if hasStructuralAnchor { - item.AnchorState = "structural" - if structuralPos >= 0 { - item.AnchorPosition = structuralPos + Raw: raw, + CommentID: commentID, + Quote: quote, + IsWhole: isWhole, + IsSolved: isSolved, + CreateTime: createTime, + AnchorPosition: -1, + LocationAccuracy: "weak_inferred", + ParentType: parentType, + ParentToken: parentToken, + } + + blockID, deleted, hasRelation := extractCommentRelation(raw) + item.ContentDeleted = deleted + if deleted { + item.AnchorState = "orphaned" + item.LocationAccuracy = "content_deleted" + item.AnchorBlockID = blockID + return item + } + if hasRelation { + item.AnchorBlockID = blockID + if order := idx.orderOfBlock(blockID); order >= 0 { + if parentType != "" { + item.AnchorState = "structural" + item.LocationAccuracy = "parent_resource_exact" } else { - item.AnchorPosition = 0 + item.AnchorState = "valid" + item.LocationAccuracy = "relation_exact" } - } else { - item.AnchorState = "orphaned" - item.AnchorPosition = -1 - } - default: - needle := normalizeQuoteNeedle(quote) - if needle == "" { - item.AnchorState = "orphaned" - item.AnchorPosition = -1 - break + item.AnchorPosition = order + return item } - pos := strings.Index(normalized, needle) - if pos >= 0 { - item.AnchorState = "valid" - item.AnchorPosition = int64(pos) - } else { - item.AnchorState = "orphaned" - item.AnchorPosition = -1 + } + if parentType != "" && parentToken != "" { + if blockID, order, ok := idx.lookupEmbeddedToken(parentToken); ok { + item.AnchorState = "structural" + item.AnchorBlockID = blockID + item.AnchorPosition = order + item.LocationAccuracy = "parent_resource_exact" + return item } } + if hasRelation { + item.AnchorState = "valid" + item.LocationAccuracy = "weak_inferred" + return item + } + if isWhole { + item.AnchorState = "valid" + item.AnchorPosition = 0 + item.LocationAccuracy = "whole_document" + return item + } + item.AnchorState = "valid" return item } // sortCommentItems sorts items in-place. Orphans always sort to the end. // Within the non-orphan group: -// - order=anchor (default): by AnchorPosition ascending +// - order=anchor (default): by AnchorPosition ascending, unknown locations after anchored comments // - order=created: by CreateTime ascending func sortCommentItems(items []commentItem, orderKey string) { sort.SliceStable(items, func(i, j int) bool { @@ -493,6 +594,11 @@ func sortCommentItems(items []commentItem, orderKey string) { return a.CreateTime < b.CreateTime } // anchor order + aUnknown := a.AnchorPosition < 0 + bUnknown := b.AnchorPosition < 0 + if aUnknown != bUnknown { + return !aUnknown + } if a.AnchorPosition != b.AnchorPosition { return a.AnchorPosition < b.AnchorPosition } @@ -526,7 +632,7 @@ func cloneCommentMap(m map[string]interface{}) map[string]interface{} { } func truncateQuote(s string, n int) string { - s = whitespacePattern.ReplaceAllString(s, " ") + s = strings.Join(strings.Fields(s), " ") runes := []rune(s) if len(runes) <= n { return s diff --git a/shortcuts/drive/drive_list_comments_test.go b/shortcuts/drive/drive_list_comments_test.go index 91f3a3ffeb..1cb8306290 100644 --- a/shortcuts/drive/drive_list_comments_test.go +++ b/shortcuts/drive/drive_list_comments_test.go @@ -131,151 +131,159 @@ func TestValidateListComments_WikiURLAccepted(t *testing.T) { } } -// --- normalizeDocContent / normalizeQuoteNeedle --- +// --- relation / block location helpers --- -func TestNormalizeDocContent_StripsTagsEntitiesAndWhitespace(t *testing.T) { +func TestExtractRelationBlockID(t *testing.T) { t.Parallel() - in := `

由 P2 / P3 事件聚类触发/关联

` - got := normalizeDocContent(in) - // Tags stripped, entity decoded, ALL whitespace removed. - // Verifies that text broken by an inline tag is recombined and matches. - want := "由P2/P3事件聚类触发/关联" - if !strings.Contains(got, want) { - t.Fatalf("normalize failed: %q, want substring %q", got, want) + raw := map[string]interface{}{ + "relation": map[string]interface{}{ + "content_deleted": false, + "relation": `{"22-dox":{"objType":22,"positionInfo":{"blockID":"blk_123"}}}`, + }, } -} - -func TestNormalizeQuoteNeedle_UsesFirstLine(t *testing.T) { - t.Parallel() - multiline := "体验驱动\n随单/年度满意度调查\n舆情反馈" - got := normalizeQuoteNeedle(multiline) - if got != "体验驱动" { - t.Fatalf("got %q, want 体验驱动", got) + blockID, deleted, ok := extractCommentRelation(raw) + if !ok { + t.Fatal("expected relation to parse") } -} - -func TestNormalizeQuoteNeedle_RemovesAllWhitespace(t *testing.T) { - t.Parallel() - in := " hello world " - got := normalizeQuoteNeedle(in) - if got != "helloworld" { - t.Fatalf("got %q, want helloworld", got) + if deleted { + t.Fatal("content_deleted should be false") + } + if blockID != "blk_123" { + t.Fatalf("blockID = %q, want blk_123", blockID) } } -func TestNormalizeQuoteNeedle_MatchesAcrossInlineTags(t *testing.T) { +func TestExtractRelationBlockID_MalformedRelationDoesNotDelete(t *testing.T) { t.Parallel() - // This test documents the cross-tag matching behavior: a needle that has - // whitespace in the original quote text must match content that was - // broken by inline tags in the document XML. - quote := "由 P2 / P3 事件聚类触发" - docContent := normalizeDocContent(`

由 P2 / P3 事件聚类触发：详细描述...

`) - needle := normalizeQuoteNeedle(quote) - if !strings.Contains(docContent, needle) { - t.Fatalf("needle %q not found in normalized doc %q (cross-tag match should succeed)", needle, docContent) + raw := map[string]interface{}{ + "relation": map[string]interface{}{ + "content_deleted": true, + "relation": `{not-json`, + }, + } + blockID, deleted, ok := extractCommentRelation(raw) + if ok { + t.Fatal("malformed relation JSON should not report a parsed block id") + } + if !deleted { + t.Fatal("content_deleted should still be honored when relation JSON is malformed") + } + if blockID != "" { + t.Fatalf("blockID = %q, want empty", blockID) } } -func TestNormalizeQuoteNeedle_EmptyQuote(t *testing.T) { +func TestBuildDocBlockIndex_OrdersBlockIDsAndEmbeddedTokens(t *testing.T) { t.Parallel() - if got := normalizeQuoteNeedle(""); got != "" { - t.Fatalf("got %q, want empty", got) + xml := ` + A + + B + ` + idx := buildDocBlockIndex(xml) + if got := idx.orderOfBlock("blk_a"); got != 0 { + t.Fatalf("blk_a order = %d, want 0", got) + } + if got := idx.orderOfBlock("blk_b"); got != 2 { + t.Fatalf("blk_b order = %d, want 2", got) + } + blockID, order, ok := idx.lookupEmbeddedToken("wbd_123") + if !ok { + t.Fatal("expected embedded token lookup to succeed") } - if got := normalizeQuoteNeedle("\n\n \n"); got != "" { - t.Fatalf("got %q, want empty for whitespace-only", got) + if blockID != "blk_whiteboard" || order != 1 { + t.Fatalf("embedded lookup = (%q,%d), want (blk_whiteboard,1)", blockID, order) } } -// --- buildCommentItem --- - -func TestBuildCommentItem_WholeDocComment(t *testing.T) { +func TestBuildCommentItem_ContentDeletedIsOrphaned(t *testing.T) { t.Parallel() raw := map[string]interface{}{ - "comment_id": "c1", - "is_whole": true, - "create_time": float64(1700000000), + "comment_id": "c_deleted", + "relation": map[string]interface{}{ + "content_deleted": true, + "relation": `{"22-dox":{"positionInfo":{"blockID":"blk_deleted"}}}`, + }, } - it := buildCommentItem(raw, "doc body", false, -1) - if it.AnchorState != "valid" || it.AnchorPosition != 0 { - t.Fatalf("got state=%q pos=%d, want valid/0", it.AnchorState, it.AnchorPosition) + idx := buildDocBlockIndex(`deleted anchor gone`) + it := buildCommentItem(raw, idx) + if it.AnchorState != "orphaned" || it.LocationAccuracy != "content_deleted" { + t.Fatalf("got state=%q accuracy=%q, want orphaned/content_deleted", it.AnchorState, it.LocationAccuracy) } } -func TestBuildCommentItem_StickyWithReadonlyBlockPresent(t *testing.T) { +func TestBuildCommentItem_RelationExact(t *testing.T) { t.Parallel() raw := map[string]interface{}{ - "comment_id": "c2", - "is_whole": false, - "quote": stickyNoteQuote, + "comment_id": "c_exact", "create_time": float64(1700000000), + "relation": map[string]interface{}{ + "content_deleted": false, + "relation": `{"22-dox":{"positionInfo":{"blockID":"blk_a"}}}`, + }, } - it := buildCommentItem(raw, "ignored", true, 4800) - if it.AnchorState != "structural" { - t.Fatalf("got state=%q, want structural", it.AnchorState) + idx := buildDocBlockIndex(`A`) + it := buildCommentItem(raw, idx) + if it.AnchorState != "valid" || it.LocationAccuracy != "relation_exact" { + t.Fatalf("got state=%q accuracy=%q, want valid/relation_exact", it.AnchorState, it.LocationAccuracy) } - if it.AnchorPosition != 4800 { - t.Fatalf("got pos=%d, want 4800", it.AnchorPosition) + if it.AnchorBlockID != "blk_a" || it.AnchorPosition != 0 { + t.Fatalf("got block=%q pos=%d, want blk_a/0", it.AnchorBlockID, it.AnchorPosition) } } -func TestBuildCommentItem_StickyOrphan(t *testing.T) { +func TestBuildCommentItem_ParentResourceExact(t *testing.T) { t.Parallel() raw := map[string]interface{}{ - "comment_id": "c3", - "is_whole": false, - "quote": stickyNoteQuote, + "comment_id": "c_embed", + "parent_type": "WHITEBOARD_BLOCK", + "parent_token": "wbd_123", + } + idx := buildDocBlockIndex(``) + it := buildCommentItem(raw, idx) + if it.AnchorState != "structural" || it.LocationAccuracy != "parent_resource_exact" { + t.Fatalf("got state=%q accuracy=%q, want structural/parent_resource_exact", it.AnchorState, it.LocationAccuracy) } - it := buildCommentItem(raw, "ignored", false, -1) - if it.AnchorState != "orphaned" { - t.Fatalf("got state=%q, want orphaned", it.AnchorState) + if it.AnchorBlockID != "blk_whiteboard" || it.AnchorPosition != 0 { + t.Fatalf("got block=%q pos=%d, want blk_whiteboard/0", it.AnchorBlockID, it.AnchorPosition) } } -func TestBuildCommentItem_TextQuoteFound(t *testing.T) { +func TestBuildCommentItem_ParentResourceTokenUsesLastUnderscore(t *testing.T) { t.Parallel() raw := map[string]interface{}{ - "comment_id": "c4", - "is_whole": false, - "quote": "硬件、软件", + "comment_id": "c_sheet_parent", + "parent_type": "SHEET_BLOCK", + "parent_token": "sht_token_123_sheet1", } - doc := "some intro text 硬件、软件 trailing text" - it := buildCommentItem(raw, doc, false, -1) - if it.AnchorState != "valid" { - t.Fatalf("got state=%q, want valid", it.AnchorState) + idx := buildDocBlockIndex(``) + it := buildCommentItem(raw, idx) + if it.AnchorState != "structural" || it.LocationAccuracy != "parent_resource_exact" { + t.Fatalf("got state=%q accuracy=%q, want structural/parent_resource_exact", it.AnchorState, it.LocationAccuracy) } - wantPos := int64(strings.Index(doc, "硬件、软件")) - if it.AnchorPosition != wantPos { - t.Fatalf("got pos=%d, want %d", it.AnchorPosition, wantPos) + if it.AnchorBlockID != "blk_sheet" || it.AnchorPosition != 0 { + t.Fatalf("got block=%q pos=%d, want blk_sheet/0", it.AnchorBlockID, it.AnchorPosition) } } -func TestBuildCommentItem_TextQuoteOrphan(t *testing.T) { +func TestBuildCommentItem_EmbeddedRelationUsesParentResourceAccuracy(t *testing.T) { t.Parallel() raw := map[string]interface{}{ - "comment_id": "c5", - "is_whole": false, - "quote": "Initiate Project", - } - doc := "this doc does not mention it" - it := buildCommentItem(raw, doc, false, -1) - if it.AnchorState != "orphaned" || it.AnchorPosition != -1 { - t.Fatalf("got state=%q pos=%d, want orphaned/-1", it.AnchorState, it.AnchorPosition) + "comment_id": "c_sheet", + "parent_type": "SHEET_BLOCK", + "parent_token": "sht_123_sheet1", + "relation": map[string]interface{}{ + "content_deleted": false, + "relation": `{"22-dox":{"positionInfo":{"blockID":"blk_sheet"}}}`, + }, } -} - -func TestBuildCommentItem_MultilineQuoteUsesFirstLine(t *testing.T) { - t.Parallel() - raw := map[string]interface{}{ - "comment_id": "c6", - "is_whole": false, - "quote": "体验驱动\n随单/年度满意度调查\n舆情反馈", + idx := buildDocBlockIndex(``) + it := buildCommentItem(raw, idx) + if it.AnchorState != "structural" || it.LocationAccuracy != "parent_resource_exact" { + t.Fatalf("got state=%q accuracy=%q, want structural/parent_resource_exact", it.AnchorState, it.LocationAccuracy) } - // Doc still contains 体验驱动 elsewhere but not the sub-bullets — should - // still classify as valid based on first-line match. - doc := "earlier 体验驱动和管理驱动 later" - it := buildCommentItem(raw, doc, false, -1) - if it.AnchorState != "valid" { - t.Fatalf("got state=%q, want valid (first-line match)", it.AnchorState) + if it.AnchorBlockID != "blk_sheet" || it.AnchorPosition != 0 { + t.Fatalf("got block=%q pos=%d, want blk_sheet/0", it.AnchorBlockID, it.AnchorPosition) } } @@ -284,17 +292,17 @@ func TestBuildCommentItem_MultilineQuoteUsesFirstLine(t *testing.T) { func TestSortCommentItems_OrphansAtEnd(t *testing.T) { t.Parallel() items := []commentItem{ - {CommentID: "a", AnchorState: "valid", AnchorPosition: 100, CreateTime: 1}, + {CommentID: "a", AnchorState: "valid", AnchorPosition: 2, CreateTime: 1}, {CommentID: "b", AnchorState: "orphaned", AnchorPosition: -1, CreateTime: 2}, - {CommentID: "c", AnchorState: "valid", AnchorPosition: 50, CreateTime: 3}, - {CommentID: "d", AnchorState: "structural", AnchorPosition: 75, CreateTime: 4}, + {CommentID: "c", AnchorState: "valid", AnchorPosition: 0, CreateTime: 3}, + {CommentID: "d", AnchorState: "structural", AnchorPosition: 1, CreateTime: 4}, } sortCommentItems(items, "anchor") gotIDs := make([]string, len(items)) for i, it := range items { gotIDs[i] = it.CommentID } - // Expected: c (50), d (75 structural), a (100), b (orphan) + // Expected: c (0), d (1 structural), a (2), b (orphan) wantIDs := []string{"c", "d", "a", "b"} for i, want := range wantIDs { if gotIDs[i] != want { @@ -303,6 +311,27 @@ func TestSortCommentItems_OrphansAtEnd(t *testing.T) { } } +func TestSortCommentItems_UnknownLocationsAfterAnchoredBeforeOrphans(t *testing.T) { + t.Parallel() + items := []commentItem{ + {CommentID: "a", AnchorState: "valid", AnchorPosition: -1, CreateTime: 1}, + {CommentID: "b", AnchorState: "valid", AnchorPosition: 2, CreateTime: 2}, + {CommentID: "c", AnchorState: "orphaned", AnchorPosition: -1, CreateTime: 3}, + {CommentID: "d", AnchorState: "valid", AnchorPosition: 1, CreateTime: 4}, + } + sortCommentItems(items, "anchor") + gotIDs := make([]string, len(items)) + for i, it := range items { + gotIDs[i] = it.CommentID + } + wantIDs := []string{"d", "b", "a", "c"} + for i, want := range wantIDs { + if gotIDs[i] != want { + t.Fatalf("position %d: got %q want %q (full: %v)", i, gotIDs[i], want, gotIDs) + } + } +} + func TestSortCommentItems_CreatedOrder(t *testing.T) { t.Parallel() items := []commentItem{ @@ -324,28 +353,6 @@ func TestSortCommentItems_CreatedOrder(t *testing.T) { } } -// --- projectRawPosToNormalized --- - -func TestProjectRawPosToNormalized_ExactPrefixNormalize(t *testing.T) { - t.Parallel() - raw := "

01234

56789

" - got := projectRawPosToNormalized(raw, "", "01234

" normalizes to "01234" (length 5), so the marker's - // position in normalized space is 5. - if got != 5 { - t.Fatalf("got %d, want 5", got) - } -} - -func TestProjectRawPosToNormalized_NotFound(t *testing.T) { - t.Parallel() - got := projectRawPosToNormalized("no marker here", "normalized", "introduction

硬件、软件 are technical

trailing

`, + "content": `firstlater`, }, }, }, @@ -525,15 +562,33 @@ func TestDriveListComments_E2E_FiltersOrphanedByDefault(t *testing.T) { if !ok { t.Fatalf("missing items array: %#v", out) } - if len(items) != 2 { - t.Fatalf("expected 2 items after orphan filter, got %d", len(items)) + if len(items) != 3 { + t.Fatalf("expected 3 items after orphan filter, got %d", len(items)) + } + gotIDs := make([]string, 0, len(items)) + for _, raw := range items { + item, _ := raw.(map[string]interface{}) + gotIDs = append(gotIDs, item["comment_id"].(string)) + } + wantIDs := []string{"earlier", "embed", "later"} + for i, want := range wantIDs { + if gotIDs[i] != want { + t.Fatalf("item order = %v, want %v", gotIDs, wantIDs) + } + } + embed, _ := items[1].(map[string]interface{}) + if embed["anchor_block_id"] != "blk_whiteboard" { + t.Fatalf("embedded anchor_block_id = %#v, want blk_whiteboard", embed["anchor_block_id"]) + } + if embed["location_accuracy"] != "parent_resource_exact" { + t.Fatalf("embedded location_accuracy = %#v, want parent_resource_exact", embed["location_accuracy"]) } counts, _ := out["counts"].(map[string]interface{}) - if counts["total"] != float64(3) { - t.Fatalf("counts.total = %#v, want 3", counts["total"]) + if counts["total"] != float64(4) { + t.Fatalf("counts.total = %#v, want 4", counts["total"]) } - if counts["valid"] != float64(1) { - t.Fatalf("counts.valid = %#v, want 1", counts["valid"]) + if counts["valid"] != float64(2) { + t.Fatalf("counts.valid = %#v, want 2", counts["valid"]) } if counts["structural"] != float64(1) { t.Fatalf("counts.structural = %#v, want 1", counts["structural"]) @@ -555,8 +610,26 @@ func TestDriveListComments_E2E_IncludeOrphanedKeepsAll(t *testing.T) { "data": map[string]interface{}{ "has_more": false, "items": []map[string]interface{}{ - {"comment_id": "v1", "quote": "硬件", "is_whole": false, "create_time": float64(1)}, - {"comment_id": "o1", "quote": "missing", "is_whole": false, "create_time": float64(2)}, + { + "comment_id": "v1", + "quote": "硬件", + "is_whole": false, + "create_time": float64(1), + "relation": map[string]interface{}{ + "content_deleted": false, + "relation": `{"22-doxTest2":{"positionInfo":{"blockID":"blk_a"}}}`, + }, + }, + { + "comment_id": "o1", + "quote": "missing", + "is_whole": false, + "create_time": float64(2), + "relation": map[string]interface{}{ + "content_deleted": true, + "relation": `{"22-doxTest2":{"positionInfo":{"blockID":"blk_missing"}}}`, + }, + }, }, }, }, @@ -568,7 +641,7 @@ func TestDriveListComments_E2E_IncludeOrphanedKeepsAll(t *testing.T) { "code": 0, "msg": "ok", "data": map[string]interface{}{ "document": map[string]interface{}{ - "content": "

硬件 only

", + "content": `硬件 only`, }, }, }, diff --git a/tests/cli_e2e/drive/coverage.md b/tests/cli_e2e/drive/coverage.md index 1da1029984..da2e2fbadf 100644 --- a/tests/cli_e2e/drive/coverage.md +++ b/tests/cli_e2e/drive/coverage.md @@ -1,9 +1,9 @@ # Drive CLI E2E Coverage ## Metrics -- Denominator: 31 leaf commands -- Covered: 10 -- Coverage: 32.3% +- Denominator: 32 leaf commands +- Covered: 11 +- Coverage: 34.4% ## Summary - TestDrive_FilesCreateFolderWorkflow: proves `drive files create_folder` in `create_folder as bot`; helper asserts the returned folder token and registers best-effort cleanup via `drive files delete`. @@ -14,6 +14,7 @@ - TestDriveAddCommentDryRun_File: dry-run coverage for `drive +add-comment` on supported Drive file targets; pins the `metas.batch_query -> files/:token/new_comments` request chain, `file_type=file`, and the required placeholder `anchor.block_id`. - TestDriveAddCommentMarkdownFileWorkflow: opt-in live workflow skeleton for the same path, gated by `LARK_DRIVE_MD_COMMENT_E2E=1`. - TestDrive_SecureLabelDryRun: dry-run coverage for `drive +secure-label-list` and `drive +secure-label-update`; asserts label-list query params and update URL→type inference, request method/URL/type query, and `label-id` body shape. Runs without hitting live APIs because update can trigger document-level security approval flows. +- TestDriveListCommentsDryRun_RequestsRelationAndBlockIDs / TestDriveListCommentsWorkflow: dry-run coverage for `drive +list-comments` plus an opt-in live docx workflow; pins `need_relation=true` on comment listing, `export_block_id=true` on docs fetch, and relation-exact block mapping after creating a real comment. - TestDriveExportDryRun_FileNameMetadata: dry-run coverage for `drive +export`; asserts export task request shape and local `--file-name` / `--output-dir` metadata without calling live APIs. - TestDrive_PullDryRun / TestDrive_PullDryRunAcceptsDuplicateRemoteStrategies: dry-run coverage for `drive +pull`; asserts the list-files request shape, Validate-stage safety guards, and acceptance of `--on-duplicate-remote=rename|newest|oldest` by the real CLI binary. - TestDrive_PushDryRun / TestDrive_PushDryRunAcceptsDuplicateRemoteStrategies: dry-run coverage for `drive +push`; asserts the list-files request shape, Validate-stage safety guards, conditional delete preflight, and acceptance of `--on-duplicate-remote=newest|oldest` by the real CLI binary. @@ -32,6 +33,7 @@ | ✓ | drive +export | shortcut | drive_export_dryrun_test.go::TestDriveExportDryRun_FileNameMetadata | `--token`; `--doc-type`; `--file-extension`; `--file-name`; `--output-dir` | dry-run only; no live export workflow yet | | ✕ | drive +export-download | shortcut | | none | no export-download workflow yet | | ✕ | drive +import | shortcut | | none | no import workflow yet | +| ✓ | drive +list-comments | shortcut | drive_list_comments_dryrun_test.go::TestDriveListCommentsDryRun_RequestsRelationAndBlockIDs + drive_list_comments_workflow_test.go::TestDriveListCommentsWorkflow | `--doc` docx URL; `need_relation=true`; docs fetch `export_block_id=true`; opt-in live docx comment round-trip | dry-run runs in CI; live workflow gated by `LARK_DRIVE_LIST_COMMENTS_E2E=1`; unit/httpmock E2E cover relation/content_deleted sorting and filtering | | ✕ | drive +move | shortcut | | none | no move workflow yet | | ✓ | drive +pull | shortcut | drive_pull_dryrun_test.go::TestDrive_PullDryRun + drive_duplicate_sync_workflow_test.go::TestDrive_DuplicateRemoteWorkflow | `--local-dir`; `--folder-token`; `--on-duplicate-remote=rename\|newest\|oldest`; `--delete-local --yes` guard | dry-run locks flag/validate shape; live workflow proves duplicate fail-fast and rename recovery | | ✓ | drive +push | shortcut | drive_push_dryrun_test.go::TestDrive_PushDryRun + drive_duplicate_sync_workflow_test.go::TestDrive_DuplicateRemoteWorkflow | `--local-dir`; `--folder-token`; `--if-exists`; `--on-duplicate-remote=newest\|oldest`; `--delete-remote --yes` | dry-run locks flag/validate shape; live workflow proves overwrite + duplicate cleanup converges status | diff --git a/tests/cli_e2e/drive/drive_list_comments_dryrun_test.go b/tests/cli_e2e/drive/drive_list_comments_dryrun_test.go new file mode 100644 index 0000000000..b3df0a1875 --- /dev/null +++ b/tests/cli_e2e/drive/drive_list_comments_dryrun_test.go @@ -0,0 +1,42 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package drive + +import ( + "context" + "testing" + "time" + + clie2e "github.com/larksuite/cli/tests/cli_e2e" + "github.com/stretchr/testify/require" + "github.com/tidwall/gjson" +) + +func TestDriveListCommentsDryRun_RequestsRelationAndBlockIDs(t *testing.T) { + setDriveDryRunConfigEnv(t) + + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + t.Cleanup(cancel) + + result, err := clie2e.RunCmd(ctx, clie2e.Request{ + Args: []string{ + "drive", "+list-comments", + "--doc", "https://example.larksuite.com/docx/doxDryRunComments", + "--dry-run", + }, + DefaultAs: "bot", + }) + require.NoError(t, err) + result.AssertExitCode(t, 0) + + out := result.Stdout + require.Equal(t, "/open-apis/drive/v1/files/doxDryRunComments/comments", gjson.Get(out, "api.0.url").String(), "stdout:\n%s", out) + require.True(t, gjson.Get(out, "api.0.params.need_relation").Bool(), "stdout:\n%s", out) + require.True(t, gjson.Get(out, "api.0.params.need_reaction").Bool(), "stdout:\n%s", out) + require.False(t, gjson.Get(out, "api.0.params.is_solved").Bool(), "stdout:\n%s", out) + + require.Equal(t, "/open-apis/docs_ai/v1/documents/doxDryRunComments/fetch", gjson.Get(out, "api.1.url").String(), "stdout:\n%s", out) + require.Equal(t, "xml", gjson.Get(out, "api.1.body.format").String(), "stdout:\n%s", out) + require.True(t, gjson.Get(out, "api.1.body.export_option.export_block_id").Bool(), "stdout:\n%s", out) +} diff --git a/tests/cli_e2e/drive/drive_list_comments_workflow_test.go b/tests/cli_e2e/drive/drive_list_comments_workflow_test.go new file mode 100644 index 0000000000..9a56b4fec3 --- /dev/null +++ b/tests/cli_e2e/drive/drive_list_comments_workflow_test.go @@ -0,0 +1,200 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package drive + +import ( + "context" + "encoding/json" + "encoding/xml" + "fmt" + "os" + "strings" + "testing" + "time" + + clie2e "github.com/larksuite/cli/tests/cli_e2e" + "github.com/stretchr/testify/require" + "github.com/tidwall/gjson" +) + +func TestDriveListCommentsWorkflow(t *testing.T) { + if os.Getenv("LARK_DRIVE_LIST_COMMENTS_E2E") == "" { + t.Skip("set LARK_DRIVE_LIST_COMMENTS_E2E=1 to run the docx list-comments live workflow") + } + clie2e.SkipWithoutUserToken(t) + + parentT := t + ctx, cancel := context.WithTimeout(context.Background(), 3*time.Minute) + t.Cleanup(cancel) + + suffix := clie2e.GenerateSuffix() + anchorText := "lark-cli-e2e-list-comments-anchor-" + suffix + commentText := "please review " + suffix + + createResult, err := clie2e.RunCmdWithRetry(ctx, clie2e.Request{ + Args: []string{ + "docs", "+create", + "--api-version", "v2", + "--doc-format", "xml", + "--content", fmt.Sprintf("%s", anchorText), + }, + DefaultAs: "user", + }, clie2e.RetryOptions{}) + require.NoError(t, err) + createResult.AssertExitCode(t, 0) + + docToken := firstGJSON(createResult.Stdout, + "data.document.document_id", + "document.document_id", + "data.document_id", + ) + require.NotEmpty(t, docToken, "stdout:\n%s", createResult.Stdout) + + parentT.Cleanup(func() { + cleanupCtx, cleanupCancel := clie2e.CleanupContext() + defer cleanupCancel() + + deleteResult, deleteErr := clie2e.RunCmd(cleanupCtx, clie2e.Request{ + Args: []string{ + "drive", "+delete", + "--file-token", docToken, + "--type", "docx", + }, + DefaultAs: "user", + Yes: true, + }) + clie2e.ReportCleanupFailure(parentT, "delete list-comments docx "+docToken, deleteResult, deleteErr) + }) + + fetchResult, err := clie2e.RunCmdWithRetry(ctx, clie2e.Request{ + Args: []string{ + "docs", "+fetch", + "--api-version", "v2", + "--doc", docToken, + "--doc-format", "xml", + "--detail", "with-ids", + }, + DefaultAs: "user", + }, clie2e.RetryOptions{}) + require.NoError(t, err) + fetchResult.AssertExitCode(t, 0) + + docXML := gjson.Get(fetchResult.Stdout, "data.document.content").String() + require.NotEmpty(t, docXML, "stdout:\n%s", fetchResult.Stdout) + blockID := firstCommentableBlockID(t, docXML, docToken) + require.NotEmpty(t, blockID, "doc XML:\n%s", docXML) + + commentContent := mustCommentContentJSON(t, commentText) + commentResult, err := clie2e.RunCmdWithRetry(ctx, clie2e.Request{ + Args: []string{ + "drive", "+add-comment", + "--doc", docToken, + "--type", "docx", + "--block-id", blockID, + "--content", commentContent, + }, + DefaultAs: "user", + }, clie2e.RetryOptions{}) + require.NoError(t, err) + commentResult.AssertExitCode(t, 0) + commentResult.AssertStdoutStatus(t, true) + + commentID := gjson.Get(commentResult.Stdout, "data.comment_id").String() + require.NotEmpty(t, commentID, "stdout:\n%s", commentResult.Stdout) + + listResult, err := clie2e.RunCmdWithRetry(ctx, clie2e.Request{ + Args: []string{ + "drive", "+list-comments", + "--doc", docToken, + "--type", "docx", + "--include-resolved", + "--include-orphaned", + }, + DefaultAs: "user", + }, clie2e.RetryOptions{ + ShouldRetry: func(result *clie2e.Result) bool { + return result == nil || result.ExitCode != 0 || !strings.Contains(result.Stdout, commentID) + }, + }) + require.NoError(t, err) + listResult.AssertExitCode(t, 0) + + item := findCommentItem(t, listResult.Stdout, commentID) + require.Equal(t, "valid", item.Get("anchor_state").String(), "stdout:\n%s", listResult.Stdout) + require.Equal(t, "relation_exact", item.Get("location_accuracy").String(), "stdout:\n%s", listResult.Stdout) + require.Equal(t, blockID, item.Get("anchor_block_id").String(), "stdout:\n%s", listResult.Stdout) + require.False(t, item.Get("content_deleted").Bool(), "stdout:\n%s", listResult.Stdout) + require.True(t, item.Get("relation.relation").Exists(), "stdout:\n%s", listResult.Stdout) +} + +func firstGJSON(raw string, paths ...string) string { + for _, path := range paths { + if value := strings.TrimSpace(gjson.Get(raw, path).String()); value != "" { + return value + } + } + return "" +} + +func firstCommentableBlockID(t *testing.T, content string, docToken string) string { + t.Helper() + + decoder := xml.NewDecoder(strings.NewReader(content)) + var fallback string + for { + token, err := decoder.Token() + if err != nil { + break + } + start, ok := token.(xml.StartElement) + if !ok { + continue + } + blockID := firstXMLAttrValue(start, "id", "block_id") + if blockID == "" { + continue + } + if fallback == "" { + fallback = blockID + } + if blockID != docToken { + return blockID + } + } + return fallback +} + +func firstXMLAttrValue(start xml.StartElement, names ...string) string { + for _, name := range names { + for _, attr := range start.Attr { + if attr.Name.Local == name { + return attr.Value + } + } + } + return "" +} + +func mustCommentContentJSON(t *testing.T, text string) string { + t.Helper() + + payload := []map[string]string{{"type": "text", "text": text}} + raw, err := json.Marshal(payload) + require.NoError(t, err) + return string(raw) +} + +func findCommentItem(t *testing.T, stdout string, commentID string) gjson.Result { + t.Helper() + + items := gjson.Get(stdout, "data.items").Array() + require.NotEmpty(t, items, "stdout:\n%s", stdout) + for _, item := range items { + if item.Get("comment_id").String() == commentID { + return item + } + } + t.Fatalf("comment %s not found in list output:\n%s", commentID, stdout) + return gjson.Result{} +}