feat(drive): add comment batch-query/resolve/restore, reply CRUD and reaction shortcuts#1898
feat(drive): add comment batch-query/resolve/restore, reply CRUD and reaction shortcuts#1898zzzchao123 wants to merge 17 commits into
Conversation
|
zchao.007 seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
📝 WalkthroughWalkthroughAdds eight Drive comment and reply shortcuts for batch querying, resolving/restoring, adding/listing/updating/deleting replies, and managing reactions. Shared handling supports URL/token/type resolution, Wiki unwrapping, validation, dry-run planning, tests, registry integration, and documentation. ChangesDrive comment workflows
Estimated code review effort: 5 (Critical) | ~90 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
81c45ba to
e1e08b2
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/cli_e2e/drive/coverage.md`:
- Around line 32-35: Add a self-contained live E2E workflow test file covering
the shortcut flows in TestDrive_CommentOpsDryRun: +batch-query-comments,
+resolve-comment, +add-reply, and +delete-reply. Use the live API to create
required comments and replies, exercise each operation including both resolve
and restore behavior, verify responses and state changes, then clean up all
created resources even when assertions fail.
In `@tests/cli_e2e/drive/drive_comment_ops_dryrun_test.go`:
- Around line 18-181: Add a self-contained live E2E test alongside
TestDrive_CommentOpsDryRun covering +batch-query-comments, +resolve-comment,
+add-reply, and +delete-reply. Create the required document/comment fixture,
exercise each shortcut against the live service with assertions on the
round-trip results, and ensure cleanup runs reliably via test cleanup even when
assertions fail.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c98e4543-2937-4a11-9d83-f1381cd3423b
📒 Files selected for processing (23)
internal/qualitygate/diff/diff.gointernal/qualitygate/diff/diff_test.goshortcuts/base/base_resolve.goshortcuts/drive/drive_add_reply.goshortcuts/drive/drive_add_reply_test.goshortcuts/drive/drive_batch_query_comments.goshortcuts/drive/drive_batch_query_comments_test.goshortcuts/drive/drive_comment_common.goshortcuts/drive/drive_comment_common_test.goshortcuts/drive/drive_delete_reply.goshortcuts/drive/drive_delete_reply_test.goshortcuts/drive/drive_export.goshortcuts/drive/drive_import.goshortcuts/drive/drive_inspect.goshortcuts/drive/drive_list_comments.goshortcuts/drive/drive_resolve_comment.goshortcuts/drive/drive_resolve_comment_test.goshortcuts/drive/shortcuts.goshortcuts/drive/shortcuts_test.goskills/lark-drive/SKILL.mdskills/lark-drive/references/lark-drive-comments-guide.mdtests/cli_e2e/drive/coverage.mdtests/cli_e2e/drive/drive_comment_ops_dryrun_test.go
e1e08b2 to
3ea7e26
Compare
3ea7e26 to
11e1595
Compare
|
Current PR head is still For the apps regression check, I used the provided page URL: ./lark-cli drive +list-comments --url "https://bytedance.feishu.cn/page/N1BWmMrqndT5ZcamAIBcnvDLnOf/" --solved-status all --page-size 100 --as bot --format jsonThis succeeds as bot and returns |
11e1595 to
41a7462
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@skills/lark-drive/SKILL.md`:
- Around line 104-105: 更新 SKILL.md 中关于评论文本转义的说明,明确使用 drive +add-comment 或 drive
+add-reply shortcut 时传入原始文本,由 shortcut 自动转义;直接调用 drive file.comments create_v2 或
drive file.comment.replys update 时,调用方必须先将文本中的 < 和 > 转义后再提交,避免 shortcut 调用方重复转义。
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7ec789d4-d494-456d-87e5-e38253e05dac
📒 Files selected for processing (16)
shortcuts/drive/drive_add_reply.goshortcuts/drive/drive_add_reply_test.goshortcuts/drive/drive_batch_query_comments.goshortcuts/drive/drive_batch_query_comments_test.goshortcuts/drive/drive_comment_common.goshortcuts/drive/drive_comment_common_test.goshortcuts/drive/drive_delete_reply.goshortcuts/drive/drive_delete_reply_test.goshortcuts/drive/drive_resolve_comment.goshortcuts/drive/drive_resolve_comment_test.goshortcuts/drive/shortcuts.goshortcuts/drive/shortcuts_test.goskills/lark-drive/SKILL.mdskills/lark-drive/references/lark-drive-comments-guide.mdtests/cli_e2e/drive/coverage.mdtests/cli_e2e/drive/drive_comment_ops_dryrun_test.go
🚧 Files skipped from review as they are similar to previous changes (15)
- shortcuts/drive/drive_delete_reply_test.go
- shortcuts/drive/shortcuts.go
- shortcuts/drive/drive_batch_query_comments.go
- shortcuts/drive/drive_delete_reply.go
- shortcuts/drive/drive_batch_query_comments_test.go
- shortcuts/drive/drive_resolve_comment_test.go
- shortcuts/drive/shortcuts_test.go
- shortcuts/drive/drive_comment_common.go
- tests/cli_e2e/drive/drive_comment_ops_dryrun_test.go
- shortcuts/drive/drive_resolve_comment.go
- tests/cli_e2e/drive/coverage.md
- shortcuts/drive/drive_comment_common_test.go
- shortcuts/drive/drive_add_reply.go
- shortcuts/drive/drive_add_reply_test.go
- skills/lark-drive/references/lark-drive-comments-guide.md
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1898 +/- ##
==========================================
+ Coverage 74.96% 75.23% +0.27%
==========================================
Files 892 909 +17
Lines 94058 96309 +2251
==========================================
+ Hits 70506 72458 +1952
- Misses 18139 18334 +195
- Partials 5413 5517 +104 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@0555bec4e5e3159d84c1a9a4bf7223acd865487c🧩 Skill updatenpx skills add larksuite/cli#feat/drive-comment-shortcuts -y -g |
41a7462 to
b7aea0b
Compare
b7aea0b to
a5e2d29
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
shortcuts/drive/drive_batch_query_comments_test.go (1)
1-1: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAPI-error propagation tests only assert on message substrings, not on typed error classification.
All four "PropagatesAPIError" tests share the same gap: they check
strings.Contains(err.Error(), "...")but never verify the error remains a typed API error (e.g., viaerrs.ProblemOf(err)→Category == errs.CategoryAPI). A regression whereruntime.CallAPITyped's typed error gets rewrapped as a plain error would pass all four tests silently.
shortcuts/drive/drive_batch_query_comments_test.go#L353-373: after the substring check, addif p, ok := errs.ProblemOf(err); !ok || p.Category != errs.CategoryAPI { t.Fatalf(...) }.shortcuts/drive/drive_add_reply_test.go#L261-282: add the sameerrs.ProblemOfCategory assertion after the substring check.shortcuts/drive/drive_resolve_comment_test.go#L266-287: add the sameerrs.ProblemOfCategory assertion after the substring check.shortcuts/drive/drive_delete_reply_test.go#L176-198: add the sameerrs.ProblemOfCategory assertion after the substring check.Based on learnings, in larksuite/cli Go test suites for shortcuts domain packages, when exercising error paths, tests should "assert
Category == errs.CategoryAPIand verifySubtypeis non-empty/populated" rather than relying only on message text. This is also consistent with the path instruction that "**/*_test.go... contract tests must assert the new field or behavior directly rather than relying only on happy-path substrings."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@shortcuts/drive/drive_batch_query_comments_test.go` at line 1, Update all four PropagatesAPIError tests in the drive shortcuts test suite to classify the returned error with errs.ProblemOf(err) after the existing message assertion. Fail unless extraction succeeds, Category equals errs.CategoryAPI, and Subtype is non-empty, preserving the current message checks while directly validating typed API-error propagation.Sources: Path instructions, Learnings
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@shortcuts/drive/drive_add_reply_test.go`:
- Around line 308-320: Update TestDriveAddReplyInvalidContent to call
assertDriveCommentValidationError(t, err, "--content") in addition to verifying
the invalid-JSON message, ensuring the returned error is typed and attributed to
the --content flag.
---
Nitpick comments:
In `@shortcuts/drive/drive_batch_query_comments_test.go`:
- Line 1: Update all four PropagatesAPIError tests in the drive shortcuts test
suite to classify the returned error with errs.ProblemOf(err) after the existing
message assertion. Fail unless extraction succeeds, Category equals
errs.CategoryAPI, and Subtype is non-empty, preserving the current message
checks while directly validating typed API-error propagation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e8d2da51-2003-4953-8cf2-384b1db7f06a
📒 Files selected for processing (17)
shortcuts/drive/drive_add_reply.goshortcuts/drive/drive_add_reply_test.goshortcuts/drive/drive_batch_query_comments.goshortcuts/drive/drive_batch_query_comments_test.goshortcuts/drive/drive_comment_common.goshortcuts/drive/drive_comment_common_test.goshortcuts/drive/drive_delete_reply.goshortcuts/drive/drive_delete_reply_test.goshortcuts/drive/drive_resolve_comment.goshortcuts/drive/drive_resolve_comment_test.goshortcuts/drive/shortcuts.goshortcuts/drive/shortcuts_test.goskills/lark-drive/SKILL.mdskills/lark-drive/references/lark-drive-comments-guide.mdtests/cli_e2e/drive/coverage.mdtests/cli_e2e/drive/drive_comment_ops_dryrun_test.gotests/cli_e2e/drive/drive_comment_ops_workflow_test.go
🚧 Files skipped from review as they are similar to previous changes (12)
- shortcuts/drive/shortcuts.go
- shortcuts/drive/drive_delete_reply.go
- tests/cli_e2e/drive/drive_comment_ops_dryrun_test.go
- shortcuts/drive/drive_batch_query_comments.go
- shortcuts/drive/drive_comment_common_test.go
- shortcuts/drive/drive_resolve_comment.go
- tests/cli_e2e/drive/drive_comment_ops_workflow_test.go
- shortcuts/drive/drive_comment_common.go
- shortcuts/drive/drive_add_reply.go
- shortcuts/drive/shortcuts_test.go
- tests/cli_e2e/drive/coverage.md
- skills/lark-drive/references/lark-drive-comments-guide.md
| - `is_solved=true` 的已解决评论不支持回复;遇到时提示“该评论已被解决,无法回复”。 | ||
| - 当目标评论不能回复时,只提示限制,不要自动替用户寻找其他可回复评论。 | ||
|
|
||
| ## 获取回复 |
There was a problem hiding this comment.
这些看是不是这期就抽成单独的ref skill,只在这里面引导就好,长期也是要抽出去的
| - 遍历评论卡片并顺带拿 reaction:使用 `drive +list-comments --need-reaction`。 | ||
| - 已知评论 ID,批量查看 reaction:使用 `drive +batch-query-comments --need-reaction`。 | ||
| - 某张评论卡片下继续翻页拉 reply reaction:使用 `drive +list-replies --need-reaction`,每一页都要持续带。 | ||
| - 返回形状:`items[].reactions[]` 为 `{reaction_key, count, ahead_users[]}`;**`count=0` 的条目是已删除 reaction 的残留(已实测确认),统计与判断是否存在都要按 `count>0` 过滤**。 |
…ands
+resolve-comment's name promised only the solving direction while
--action restore silently covered reopening — ambiguous for both humans
and agent routing. Replace the action flag with two direction-pure
commands over the same PATCH endpoint:
- +resolve-comment --comment-id <id> -> is_solved=true
- +restore-comment --comment-id <id> -> is_solved=false
Both are built by one shared constructor (config carries direction,
wording, and the counterpart-pointer tip), so the endpoint wrapper,
input resolution, wiki unwrapping, and output shape stay single-source.
Intent phrases now map one-to-one in the skill docs ("标记已处理" ->
resolve, "重新打开/恢复" -> restore) without an --action detour.
The command surface is PR-only (unreleased), so no compatibility shim
is kept. Unit tests, dry-run e2e cases, the live workflow helper, skill
docs, and coverage.md (21/40 covered) follow the split.
The batch_query endpoint honors need_relation in the request body even though the platform metadata does not list it (live-verified: a docx comment came back with the relation payload carrying the anchor blockID) — the same undocumented parameter +list-comments already sends as a query param. Mirror the +list-comments handling: a --need-relation bool flag that is sent for docx targets only and silently ignored otherwise, so invalid parameters never reach the API for other file types. Unit tests pin both the docx body value and the non-docx omission; the docx and base dry-run e2e cases assert presence and absence respectively; skill docs (comment-location guide incl. the query-vs-body placement difference, comments guide, SKILL.md routing/table) follow.
…rding
- remove the user_id value from +list-replies --user-id-type: it demands
the extra contact:user.employee_id:readonly scope and fails with 99991679
for most identities; open_id (default) and union_id remain
- reword the comment-family target support sentences: targets take --url
(wiki URLs included) or --token + --type, and wiki targets are resolved
to the real resource type/token internally
- drop verification-provenance asides ("live-verified" style notes) from
skill docs and code comments; state platform behavior as fact and keep
the verification story in coverage.md and the PR description
…s reference - rewrite lark-drive-comments-guide.md as a guidance-only doc in the permission-guide style: when to read, an intent routing table, the cross-command target rules, and the comment-card model with counting/sorting conventions; per-command details now live in the dedicated reference docs - add references/lark-drive-comment-ops.md carrying the usage and product constraints of +batch-query-comments, +resolve-comment, +restore-comment, +add-reply, +list-replies, +update-reply and +delete-reply (previously inlined in the guide) - retarget SKILL.md entry bullets and command-table rows and the list-comments references to the new doc
The wiki dry-run called RequestBody with the "<obj_type from step 1>" placeholder, which never equals "docx", so the docx-gated need_relation was dropped from the preview even though a real run that resolves to docx would send it. Mirror +list-comments: surface it as the "<sent only when obj_type is docx>" placeholder. Add a unit test and extend the wiki dry-run E2E case to assert the placeholder.
620239e to
e05ffe1
Compare
…nt ref Restructure the comment documentation so the main skill only routes and each shortcut owns its own reference, preventing knowledge drift: - SKILL.md keeps a single comment entry pointing at the comments guide; drop the inlined comment params, limits, Base/escaping notes and raw API examples, and make the command-table rows terse one-liners. - lark-drive-comments-guide.md becomes a second-level router: intent table linking to each shortcut ref, plus cross-command knowledge (target resolution, comment-card model, root reply, counting/sorting). - Add one reference per shortcut: batch-query-comments, resolve-comment, restore-comment, add-reply, list-replies, update-reply, delete-reply, react-reply. Remove the merged lark-drive-comment-ops.md. - Add lark-drive-comment-content.md as the shared --content reference (schema, text/mention_user/link, escaping, 10000-rune cap, shortcut -> raw body conversion); write-shortcut refs keep a plain-text example and link there for the rest. - Spell out the reply preconditions as steps: check is_whole/is_solved via +batch-query-comments by id, or +list-comments with --solved-status all (default hides solved comments). - Drop stale raw-API routing from list-comments, comment-location and reactions guides; raw commands are now framed only as the fallback for fields the shortcuts do not expose.
e05ffe1 to
6232a9c
Compare
…y/delete constraints Live testing surfaced that the "deleting the root reply deletes the whole comment card" claim only holds when the root is the card's sole reply. With other replies present, deleting the root removes only that reply and the card lives on with the rest — verified on a real docx. - comments-guide.md / delete-reply.md: reword to "deletion is per-reply; the card disappears when its last reply is removed; to delete a whole comment, delete all its replies" and note there is no dedicated delete-comment command. - comments-guide.md 通用原则: surface two cross-command constraints that were only implicit — repliability (only local, unsolved comments accept replies; whole/solved return 1069302, so create a local comment when you intend to reply) and delete granularity.
8fa3d12 to
c04efbb
Compare
…mmand guidance - comment-content.md: lead the --content schema with dedicated fields (mention_user carries the open_id, link the URL) instead of framing the open_id-in-text convenience as primary; keep it only as a de-emphasized note. Trim the escaping paragraph to one line and drop the raw-API caveat. Remove the "shortcut -> native body" wire-form conversion section, which encouraged hand-writing raw comment bodies. - add-comment.md: collapse the --content pointer to a single link instead of re-enumerating schema/escaping/limit topics owned by comment-content. - 8 command refs (resolve/restore/delete-reply/batch-query/list-replies/ add-reply/update-reply/react-reply): drop the circular "read comments-guide for routing/target-location" preamble and point to lark-shared for auth, matching the existing list-comments pattern; the target-location pointer already lives in each param table. - restore-comment.md: drop the --solved-status note on the --comment-id row (covered by comments-guide 通用原则). - add-reply.md / update-reply.md: fix the native-fallback note that referenced the removed wire-form table. Escaping and pagination claims were re-verified live: the server does NOT auto-escape <> (it stores raw), so shortcut-side escaping is real; and list-replies page_token paging works with the root reply only appearing as items[0] on the first page.
…eleton Give the eight comment-operation refs the same section skeleton as lark-drive-list-comments.md: 命令 → 参数 → 行为说明 → 输出 → 参考. - add a ## 输出 section to each with the real returned fields (all go through driveCommentTargetOutput, so every payload carries file_token / file_type, plus wiki_token for wiki input): batch-query items/count; resolve/restore comment_id + action + is_solved + updated; add-reply comment_id + created + reply_id; list-replies comment_id + items + has_more + page_token + count; update/delete-reply comment_id + reply_id + updated/deleted; react-reply reply_id + reaction_type + action + updated. - drop the standalone ## 原生兜底 section and fold the native-fallback note into a 行为说明 bullet, matching how +list-comments carries it.
|
|
||
| > **前置条件:** 先阅读 [`../../lark-shared/SKILL.md`](../../lark-shared/SKILL.md) 了解认证、全局参数和权限处理;`--content` 完整格式见 [`lark-drive-comment-content.md`](lark-drive-comment-content.md)。 | ||
|
|
||
| 给已有评论添加一条回复,走 `POST .../comments/:comment_id/replies` 回复端点。 |
There was a problem hiding this comment.
走 POST .../comments/:comment_id/replies 回复端点。 --去掉,不要包含内部实现
|
|
||
| 给已有评论添加一条回复,走 `POST .../comments/:comment_id/replies` 回复端点。 | ||
|
|
||
| ## 命令 |
There was a problem hiding this comment.
添加 --token、--type 命令demo,对齐list-comment
|
|
||
| | 参数 | 必填 | 说明 | | ||
| |---|---|---| | ||
| | `--url` / `--token` + `--type` | 是(二选一) | 目标定位,见 [`lark-drive-comments-guide.md`](lark-drive-comments-guide.md);wiki 自动解包 | |
There was a problem hiding this comment.
见 lark-drive-comments-guide.md。-- 不确定要引导走什么,可以对齐list-comment,说明下type 枚举、wiki场景type和token对齐
|
|
||
| | 参数 | 必填 | 说明 | | ||
| |---|---|---| | ||
| | `--url` / `--token` + `--type` | 是(二选一) | 目标定位,见 [`lark-drive-comments-guide.md`](lark-drive-comments-guide.md);wiki 自动解包 | |
|
|
||
| | 参数 | 必填 | 说明 | | ||
| |---|---|---| | ||
| | `--url` / `--token` + `--type` | 是(二选一) | 目标定位,见 [`lark-drive-comments-guide.md`](lark-drive-comments-guide.md);wiki 自动解包 | |
There was a problem hiding this comment.
还有支持wiki url、wiki token+wiki type这种,参考list-comment
|
|
||
| `is_whole=true` 的全文评论、`is_solved=true` 的已解决评论都不能回复。回复前先确认目标可回复: | ||
|
|
||
| - 已知 comment_id:`drive +batch-query-comments --url '<DOC_URL>' --comment-ids '<id>'`,看返回项的 `is_whole` / `is_solved`。 |
There was a problem hiding this comment.
这里可能会引导再查询batch-query,但用户场景一般这个时候已经通过list-comment拿到whole、solved信息了,不需要再查多一次,看是否简单引导“is_whole=true 的全文评论、is_solved=true 的已解决评论都不能回复”即可,由agent自行判断当前是否有足够信息
| `is_whole=true` 的全文评论、`is_solved=true` 的已解决评论都不能回复。回复前先确认目标可回复: | ||
|
|
||
| - 已知 comment_id:`drive +batch-query-comments --url '<DOC_URL>' --comment-ids '<id>'`,看返回项的 `is_whole` / `is_solved`。 | ||
| - 用 `drive +list-comments` 查找目标:**必须带 `--solved-status all`**,否则默认只查未解决评论,会漏掉已解决的目标;再核对 `is_whole` / `is_solved`。 |
|
|
||
| 命中限制时的提示口径:全文评论 → “全文评论不支持回复”;已解决评论 → “该评论已被解决,无法回复”。当目标评论不能回复时,只提示限制,不要自动替用户改回复到别的评论。 | ||
|
|
||
| ## 为什么不用“添加评论”接口回复 |
|
|
||
| ## 行为说明 | ||
|
|
||
| - 需要 shortcut 未暴露的字段时才用原生 `drive file.comment.replys create` 兜底(先 `lark-cli schema drive.file.comment.replys.create` 查契约,按 schema 拼 body);直接调原生时需自行转义文本,Base 的 `file_type` 传 `bitable`。 |
There was a problem hiding this comment.
不需要再暴露老metaapi,正常shortcut已经能满足所有场景
Remove the flag entirely: its user_id value needed the extra contact:user.employee_id:readonly scope and it added little over the open_id default. items[].user_id now follows the server default (open_id). - drive_list_replies.go: remove the flag, spec field and param wiring - tests: drop the union_id assertion/arg and the obsolete enum-rejection case; assert user_id_type is omitted - coverage.md: drop --user-id-type from the +list-replies row
…comments Give each comment shortcut ref the same concrete target-form examples as lark-drive-list-comments.md — real docx/sheet/apps (/page/)/wiki URLs plus bare wiki-token and token+type forms — instead of an abstract <DOC_URL>. Also drop the removed --user-id-type from the +list-replies and +update-reply docs.
link maps to the wire type docs_link (云文档链接). Reply endpoints (+add-reply / +update-reply) reject external URLs with 1069302 and accept only Lark cloud-doc URLs; +add-comment is lax but external links are not the intended use. Document this and use a cloud-doc URL in the example.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@shortcuts/drive/drive_batch_query_comments.go`:
- Around line 43-45: After resolving the target in the drive batch query flow,
reject --need-relation whenever the resolved file type is not docx, including
Wiki targets resolving to non-docx, by returning a typed validation error with
Param("--need-relation") instead of silently omitting need_relation. Update the
existing non-docx handling message accordingly, and revise
tests/cli_e2e/drive/coverage.md at line 34 to document rejection; add direct and
Wiki rejection coverage as requested.
In `@tests/cli_e2e/drive/coverage.md`:
- Line 16: Correct the manual-verification count in TestDrive_CommentOpsDryRun
to eight, matching the eight listed drive shortcuts including +restore-comment;
leave the rest of the coverage description unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 87f15e63-83d2-4f31-be59-aa6b42ae3902
📒 Files selected for processing (30)
shortcuts/drive/drive_batch_query_comments.goshortcuts/drive/drive_batch_query_comments_test.goshortcuts/drive/drive_comment_common_test.goshortcuts/drive/drive_list_replies.goshortcuts/drive/drive_list_replies_test.goshortcuts/drive/drive_react_reply.goshortcuts/drive/drive_react_reply_test.goshortcuts/drive/drive_resolve_comment.goshortcuts/drive/drive_resolve_comment_test.goshortcuts/drive/shortcuts.goshortcuts/drive/shortcuts_test.goskills/lark-drive/SKILL.mdskills/lark-drive/references/lark-drive-add-comment.mdskills/lark-drive/references/lark-drive-add-reply.mdskills/lark-drive/references/lark-drive-batch-query-comments.mdskills/lark-drive/references/lark-drive-comment-content.mdskills/lark-drive/references/lark-drive-comment-location.mdskills/lark-drive/references/lark-drive-comments-guide.mdskills/lark-drive/references/lark-drive-delete-reply.mdskills/lark-drive/references/lark-drive-list-comments.mdskills/lark-drive/references/lark-drive-list-replies.mdskills/lark-drive/references/lark-drive-react-reply.mdskills/lark-drive/references/lark-drive-reactions.mdskills/lark-drive/references/lark-drive-resolve-comment.mdskills/lark-drive/references/lark-drive-restore-comment.mdskills/lark-drive/references/lark-drive-update-reply.mdskills/lark-slides/references/lark-slides-media-upload.mdtests/cli_e2e/drive/coverage.mdtests/cli_e2e/drive/drive_comment_ops_dryrun_test.gotests/cli_e2e/drive/drive_comment_ops_workflow_test.go
💤 Files with no reviewable changes (1)
- shortcuts/drive/drive_list_replies.go
🚧 Files skipped from review as they are similar to previous changes (9)
- shortcuts/drive/shortcuts.go
- shortcuts/drive/shortcuts_test.go
- shortcuts/drive/drive_react_reply.go
- tests/cli_e2e/drive/drive_comment_ops_dryrun_test.go
- shortcuts/drive/drive_react_reply_test.go
- skills/lark-drive/references/lark-drive-reactions.md
- tests/cli_e2e/drive/drive_comment_ops_workflow_test.go
- shortcuts/drive/drive_list_replies_test.go
- shortcuts/drive/drive_batch_query_comments_test.go
| if s.NeedRelation && fileType == "docx" { | ||
| body["need_relation"] = true | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Reject --need-relation when the resolved target is not docx.
The command currently returns success while silently dropping a requested field. Return a typed validation error with Param("--need-relation") after target resolution, including Wiki targets resolved to non-docx, and add direct and Wiki rejection coverage.
shortcuts/drive/drive_batch_query_comments.go#L43-L45: validate the resolved target type instead of omitting the requested field.shortcuts/drive/drive_batch_query_comments.go#L63-L63: replace “ignored for non-docx targets” with rejection behavior.tests/cli_e2e/drive/coverage.md#L34-L34: document non-docx rejection rather than omission.
As per coding guidelines, “return a typed validation error when an option cannot be honored.”
📍 Affects 2 files
shortcuts/drive/drive_batch_query_comments.go#L43-L45(this comment)shortcuts/drive/drive_batch_query_comments.go#L63-L63tests/cli_e2e/drive/coverage.md#L34-L34
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@shortcuts/drive/drive_batch_query_comments.go` around lines 43 - 45, After
resolving the target in the drive batch query flow, reject --need-relation
whenever the resolved file type is not docx, including Wiki targets resolving to
non-docx, by returning a typed validation error with Param("--need-relation")
instead of silently omitting need_relation. Update the existing non-docx
handling message accordingly, and revise tests/cli_e2e/drive/coverage.md at line
34 to document rejection; add direct and Wiki rejection coverage as requested.
Source: Coding guidelines
| - TestDrive_ApplyPermissionDryRun / TestDrive_ApplyPermissionDryRunRejectsFullAccess: dry-run coverage for `drive +apply-permission`; asserts URL→type inference for docx/sheet/slides, explicit `--type` overriding URL inference when both a recognized URL and `--type` are supplied, bare-token + explicit `--type` path, request method/URL/type-query/perm/remark body shape, optional `remark` omission when unset, and client-side rejection of `--perm full_access`. Runs without hitting the live API. | ||
| - TestDriveAddCommentDryRun_File / TestDriveAddCommentDryRun_Base: dry-run coverage for `drive +add-comment` on supported Drive file and Base targets; pins the `metas.batch_query -> files/:token/new_comments` file chain, Base `file_type=bitable`, and Base anchor fields. | ||
| - TestDriveListCommentsDryRun_DocxDefaults / TestDriveListCommentsDryRun_AppsPageURL / TestDriveListCommentsDryRun_WikiToken: dry-run coverage for `drive +list-comments`; asserts URL parsing to `files/:token/comments`, apps `/page/<token>` URL parsing with `file_type=apps`, default `is_solved=false`, default omitted `is_whole` and `user_id_type`, and Wiki token orchestration (`get_node -> comments.list`) without live API calls. | ||
| - TestDrive_CommentOpsDryRun: dry-run coverage for `drive +batch-query-comments`, `drive +resolve-comment`, `drive +restore-comment`, `drive +add-reply`, `drive +list-replies`, `drive +update-reply`, `drive +delete-reply`, and `drive +react-reply`; asserts URL→type inference (incl. Miaoda apps `/page/<token>` → `file_type=apps` and Base `/base/` → `file_type=bitable`), `file_type`/`page_size`/`need_reaction`/`need_relation` (docx-gated) query wiring, `comment_ids` / `is_solved` / reply `content.elements[]` (text_run) / reaction `action`+`reaction_type`+`reply_id` body shapes, resolved `:comment_id`/`:reply_id` path segments, the Wiki `get_node -> batch_query` / `get_node -> replies list` / `get_node -> v2 reaction` orchestration plans, and the batch_query wiki dry-run surfacing `need_relation` as the `<sent only when obj_type is docx>` placeholder, without live API calls. All seven verified manually against live documents (list → batch-query → add-reply → list-replies → update-reply → react add/delete → resolve/restore → delete-reply round trip; root-reply update rewriting the comment body, the `1069303 forbidden` cross-identity update rejection, the server persisting arbitrary `reaction_type` strings, and count=0 reaction tombstones were probed live as well). |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Correct the manual-verification count.
The sentence lists eight shortcuts, including +restore-comment, not seven.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/cli_e2e/drive/coverage.md` at line 16, Correct the manual-verification
count in TestDrive_CommentOpsDryRun to eight, matching the eight listed drive
shortcuts including +restore-comment; leave the rest of the coverage description
unchanged.
Summary
Add eight comment-operation shortcuts to the
drivedomain, covering the remaining comment APIs —+batch-query-comments,+resolve-comment/+restore-comment, the full reply family+add-reply/+list-replies/+update-reply/+delete-reply(batch_query / patch / reply create / list / update / delete), and+react-replyfor reply emoji reactions (update_reaction) — with the same URL-parsing + wiki-unwrapping UX as+list-comments.Changes
+batch-query-commentsPOST /drive/v1/files/:token/comments/batch_query—--comment-ids(CSV/repeated, max 100),--need-reaction,--need-relation(docx-gated)+resolve-commentPATCH /drive/v1/files/:token/comments/:comment_id— sendsis_solved=true+restore-commentPATCHendpoint — sendsis_solved=false(reopen a solved comment)+add-replyPOST /drive/v1/files/:token/comments/:comment_id/replies—--contentreuses the+add-commentreply_elements JSON (same escaping and 10k-rune preflight), mapped totext_run/person/docs_link+list-repliesGET .../comments/:comment_id/replies—--page-size/--page-tokenpagination,--need-reaction,--user-id-type open_id|union_id+update-replyPUT .../comments/:comment_id/replies/:reply_id— whole-content replacement, same--contentJSON as+add-reply+delete-replyDELETE .../comments/:comment_id/replies/:reply_id— requires--yes+react-replyPOST /drive/v2/files/:token/comments/reaction—--reply-id+--emoji+--action add|delete→ bodyreply_id/reaction_type/action--urlor--token+--typeand unwrap wiki nodes automatically; shared input resolution lives indrive_comment_common.go, parameterized by each endpoint's supportedfile_typeset. All eight accept Base (/base/URLs,bitable/basetypes) and Miaoda apps targets (/page/<token>URLs,file_type=apps), matching the platform API metadata.+resolve-comment/+restore-commentare two commands sharing one constructor over the same patch endpoint (the original single+resolve-comment --action resolve|restoreread ambiguously — a "resolve" command that can also reopen); intent mapping is documented in the skill guide.+batch-query-comments --need-relationmirrors+list-comments: docx-only (silently omitted for other targets) and sent in the request body, whereas the list endpoint takes it as a query param. The parameter is absent from the platform metadata for batch_query but honored by the server — verified live, returningrelationwith the anchoring block position. Its wiki dry-run surfaces the docx gate as aneed_relation: "<sent only when obj_type is docx>"placeholder, since the resolved type is unknown until step 1.+add-replydeclaresdocs:document.comment:create— the replies create endpoint does not acceptdocs:document.comment:write_only, so declaring both would false-reject valid create-only tokens; resolve/restore/update/delete/react keepwrite_only, batch-query/list keepcomment:read.+add-replydeliberately does not use the documented "POST .../comments withcomment_idin the body" form: verified live, that variant silently creates a new standalone comment instead of replying. The dedicated replies endpoint is used instead (noted in code and in the skill guide).items[0]of the first page only — after--page-tokenpaging,items[0]is a regular reply, so position must not be used to identify the root when paging;1069303 forbidden(verified in both user→bot and bot→user directions);--user-id-typeacceptsopen_id(default) andunion_id, so callers can matchitems[].user_idagainst the ID form they hold when checking ownership before update/delete. Theuser_idvalue was dropped: it additionally requires thecontact:user.employee_id:readonlyscope and fails with99991679for most identities.reaction_type— arbitrary strings are accepted and persisted as broken reactions, so--emojiis validated locally against the 149-value platform enum from the API metadata (case-sensitive:THUMBSUPvsThumbsDown);count=0entries inneed_reactionreads, so presence checks filter bycount>0;+list-comments/+batch-query-comments/+list-repliesnow always emititemsas a JSON array (shareddriveCommentItemshelper) — previously a server response omittingitemswould surface"items": nulland break jq consumers iterating.data.items[].wiki:node:readas the wiki-unwrap conditional scope: live verification with both identities showedget_nodeacceptswiki:wiki/wiki:wiki:readonly/wiki:node:read(a token carrying onlywiki:node:retrieveis rejected with 99991679), and the sheets/slides domains already declarewiki:node:readfor the same unwrap step. Existing drive/base commands that still declarewiki:node:retrieveare intentionally left unchanged in this PR; aligning them is a candidate follow-up.SKILL.mdkeeps a single comment entry intoreferences/lark-drive-comments-guide.mdand terse command-table rows, dropping the inlined params/limits/Base/escaping/raw-API detail.lark-drive-comments-guide.mdis a second-level router (intent→command table + cross-command knowledge: target resolution, comment-card model, root reply, counting/sorting). Each shortcut gets its own ref (lark-drive-{batch-query-comments,resolve-comment,restore-comment,add-reply,list-replies,update-reply,delete-reply,react-reply}.md), and--contentschema/escaping/10k-cap/raw-body conversion is centralized in the newlark-drive-comment-content.md.lark-drive-reactions.mdandlark-drive-comment-location.mdremain cross-cutting refs. Reply preconditions are written as executable steps (checkis_whole/is_solvedvia+batch-query-comments, or+list-comments --solved-status all), and stale raw-API routing in the list-comments/location/reactions guides is reframed as fallback-only. Caveats above are retained (whole/solved comments reject replies; rapid resolve/restore flips can hit HTTP 429).Test Plan
make unit-test(-race) green; new-file statement coverage 94%+ (remaining lines are the CLI-unreachable defensive branches in DryRun/Execute closures); contract tests assert payload values directly (returned reply IDs and nested content on+list-replies, convertedtext_run.text/person.user_idin+add-reply/+update-replyrequest bodies)lark-cli drive +<command>flows work as expected — full live round trips on a real wiki→docx document with both--as userand--as bot: list → batch-query → add-reply → list-replies (incl.--page-size 1two-page paging and--user-id-type union_id) → update-reply → react add/delete → resolve/restore → delete-reply, document restored to its original state afterwards; cross-identity update rejection probed in both directions, and the unvalidated-reaction_type/count=0tombstone behaviors probed livetests/cli_e2e/drive/drive_comment_ops_dryrun_test.go) pin method/URL/params/body shapes for all eight commands incl. base/apps targets, the wiki 2-step plans, the resolve/restore split, and theneed_relationbody gating (present for docx, omitted for base);tests/cli_e2e/drive/coverage.mdupdated (40 leaf commands, 21 covered, 52.5%)tests/cli_e2e/drive/drive_comment_ops_workflow_test.go, opt-in viaLARK_DRIVE_MD_COMMENT_E2E=1, same gate as the existing file-comment workflow): creates a Markdown file + file comment fixture, batch-queries it by ID, attaches a reply, lists it back, rewrites it via+update-reply(polling+list-repliesuntil the new text lands), attaches and removes a THUMBSUP reaction (polling+list-replies --need-reactionwith count>0 presence checks), resolves/restores with polling reads between state flips, deletes the reply, and cleans up — passed live in 19.8sgo vet ./...,gofmt -l .clean,go mod tidyno-op,golangci-lint run --new-from-rev=origin/main0 issuesRelated Issues
Summary by CodeRabbit
+batch-query-comments,+resolve-comment,+restore-comment,+add-reply,+list-replies,+update-reply,+delete-reply,+react-reply.--dry-runAPI call plans for both direct and Wiki flows.nullitemsto[]andcountto0for comment/reply list outputs.