docs: update Lark Mail skill API resources#1646
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthrough
Changeslark-mail documentation and guidance update
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 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 |
github-actions
Bot
added
domain/mail
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
skills/lark-mail/SKILL.md (1)
140-148: 📐 Maintainability & Code Quality | 🔵 TrivialClarify whether slash-separated
typevalues are alternatives or compound values.The table uses
user / chatterandchat / groupwith slashes. Consider using "或" (or) to explicitly indicate these are alternative values for the same entity type, e.g.,user（或 chatter）.🤖 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 `@skills/lark-mail/SKILL.md` around lines 140 - 148, The entity type table in the search results section is ambiguous because slash-separated values like user/chatter and chat/group can be read as compound types; update the descriptions in SKILL.md to make it explicit that these are alternatives for the same entity type, using wording such as “或” or parentheses, and keep the rest of the table entries consistent with that convention.
🤖 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-mail/SKILL.md`:
- Around line 98-104: Update the mail command guidance in SKILL.md for the
`+reply`, `+reply-all`, `+forward`, and `+send` entries so `--confirm-send` is
described as sending only after explicit user confirmation, not as an automatic
bypass. Rephrase those steps to make the confirmation requirement clear and
consistent with the safety rule in the document, while keeping the behavior of
`send_status`, `cancel_scheduled_send`, and the draft workflow unchanged.
---
Nitpick comments:
In `@skills/lark-mail/SKILL.md`:
- Around line 140-148: The entity type table in the search results section is
ambiguous because slash-separated values like user/chatter and chat/group can be
read as compound types; update the descriptions in SKILL.md to make it explicit
that these are alternatives for the same entity type, using wording such as “或”
or parentheses, and keep the rest of the table entries consistent with that
convention.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@a0cecd4e628149093afdbb243496a8e0586a56c0🧩 Skill updatenpx skills add infeng/cli#feat/1e0aada -y -g |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
skills/lark-mail/SKILL.md (1)
100-102: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winClarify that
--confirm-sendrequires explicit user confirmation first.Steps 4–6 still say "加
--confirm-send则立即发送" which can be misread as skipping confirmation. The critical safety rule at Line 35 mandates pre-send recipient/content confirmation. Rephrase to "加--confirm-send并在用户确认后发送" to prevent ambiguity, consistent with the prior review feedback.🤖 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 `@skills/lark-mail/SKILL.md` around lines 100 - 102, The reply/forward usage text in SKILL.md is ambiguous about `--confirm-send`; update the `+reply`, `+reply-all`, and `+forward` descriptions to explicitly say sending happens only after explicit user confirmation, matching the safety rule in the document and avoiding the impression that the flag bypasses confirmation.
🧹 Nitpick comments (1)
internal/qualitygate/rules/mail_skill_sender_lists_test.go (1)
27-31: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrefer
t.Errorfovert.Fatalfto report all missing strings.Using
t.Fatalfstops at the first missing string.t.Errorfwould surface every missing routing hint in one run, reducing fix-and-retest cycles.if !strings.Contains(body, want) { - t.Fatalf("lark-mail skill is missing blocked sender routing hint %q", want) + t.Errorf("lark-mail skill is missing blocked sender routing hint %q", want) }(and similarly for the template test).
Also applies to: 46-50
🤖 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 `@internal/qualitygate/rules/mail_skill_sender_lists_test.go` around lines 27 - 31, The checks in the mail skill sender list tests stop at the first missing routing hint because they use t.Fatalf inside the required-string loop. Update the assertions in the relevant test helper and the template test to use t.Errorf instead, so all missing strings are reported in one run. Keep the loop structure in the test functions that validate body contents and required hints, but avoid terminating early on the first failure.
🤖 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 `@internal/qualitygate/rules/mail_skill_sender_lists_test.go`:
- Line 14: The file path used in mail skill sender list tests is resolving from
internal/ instead of the repository root, so the markdown fixture paths are off
by one directory. Update the vfs.ReadFile calls in
mail_skill_sender_lists_test.go to use an additional "../" so they correctly
reach the root-level skills/ and skill-template/ directories, and make the same
fix for the other affected ReadFile usage in this test file.
---
Outside diff comments:
In `@skills/lark-mail/SKILL.md`:
- Around line 100-102: The reply/forward usage text in SKILL.md is ambiguous
about `--confirm-send`; update the `+reply`, `+reply-all`, and `+forward`
descriptions to explicitly say sending happens only after explicit user
confirmation, matching the safety rule in the document and avoiding the
impression that the flag bypasses confirmation.
---
Nitpick comments:
In `@internal/qualitygate/rules/mail_skill_sender_lists_test.go`:
- Around line 27-31: The checks in the mail skill sender list tests stop at the
first missing routing hint because they use t.Fatalf inside the required-string
loop. Update the assertions in the relevant test helper and the template test to
use t.Errorf instead, so all missing strings are reported in one run. Keep the
loop structure in the test functions that validate body contents and required
hints, but avoid terminating early on the first failure.
🪄 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: 33f04b0a-0b4b-47c0-aeb2-78933f213797
📒 Files selected for processing (4)
internal/qualitygate/rules/mail_skill_sender_lists_test.goskill-template/domains/mail.mdskills/lark-mail/SKILL.mdskills/lark-mail/references/lark-mail-rules.md
✅ Files skipped from review due to trivial changes (1)
- skills/lark-mail/references/lark-mail-rules.md
Change-Type: ci-fix
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
skills/lark-mail/SKILL.md (1)
68-68: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winClarify the
batch_removeAPI path reference.The table entry lists
allow_senders.batch_create/blocked_senders.batch_create/batch_remove— the lastbatch_removelacks a resource prefix, making it ambiguous whether it applies to bothallow_sendersandblocked_senders. Consider writingallow_senders.batch_remove/blocked_senders.batch_removefor symmetry and clarity.🤖 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 `@skills/lark-mail/SKILL.md` at line 68, The API path list is ambiguous because `batch_remove` is missing the resource prefix. Update the table entry in `SKILL.md` so it explicitly mirrors the existing `allow_senders.batch_create` / `blocked_senders.batch_create` pattern by naming both `allow_senders.batch_remove` and `blocked_senders.batch_remove`, keeping the reference clear and symmetric.
🤖 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.
Outside diff comments:
In `@skills/lark-mail/SKILL.md`:
- Line 68: The API path list is ambiguous because `batch_remove` is missing the
resource prefix. Update the table entry in `SKILL.md` so it explicitly mirrors
the existing `allow_senders.batch_create` / `blocked_senders.batch_create`
pattern by naming both `allow_senders.batch_remove` and
`blocked_senders.batch_remove`, keeping the reference clear and symmetric.
Change-Type: ci-fix
1 participant
Updates the Lark Mail skill documentation with expanded command coverage and current usage guidance for mailbox workflows.
Summary by CodeRabbit
-hguidance, and an updated receiver selection flow.--template-id).