Document mail sender allow and block APIs#1900
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughExpanded ChangesMail operations and API behavior
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@205a03af027a48e69b40d4b9194cfa12334801f7🧩 Skill updatenpx skills add infeng/cli#feat/1a3e5a9 -y -g |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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`:
- Line 306: Update the scheduling example’s --event-start timestamp to use a
clearly future placeholder or dynamically generated future value, ensuring it
remains at least five minutes ahead of execution and passes --send-time
validation.
- Around line 421-429: Update the warning in the shortcut recipient-merge
documentation to match the no-deduplication behavior: remove the claim that
--to/--cc/--bcc override or prevent repeated template recipients, and direct
users to clear template addresses with +template-update instead.
- Around line 300-309: Update the “发送带日程邀请的新邮件(先保存草稿,确认后发送)” example to preserve
its draft-first behavior by removing the --confirm-send flag from the lark-cli
mail +send invocation. Keep the event-related options unchanged.
- Around line 721-726: Update the scope table entries for
user_mailbox.templates.get/list to include mail:user_mailbox.message:readonly
rather than relying only on the broader modify scope. Clarify whether the
readonly and modify scopes listed for allow_sender.list and blocked_sender.list
are alternatives or both required, and state that requirement explicitly in the
table or surrounding guidance.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@cmd/service/service_test.go`:
- Around line 206-319: Expand the classifier tests around
TestClassifyServiceAPIError_UserAllowBlockCachePreparing to cover the positive
user_mailbox.allow_sender.list keyword-search path and negative cases for a
non-456 code, a message without 15190000, and a message without ErrCacheEmpty.
Assert directly that only the matching code/message combination receives
SubtypeUserAllowBlockCachePreparing, while each negative case retains generic
classification.
In `@cmd/service/service.go`:
- Around line 488-522: The helpers serviceRequestHasKeyword and
serviceCachePreparingText currently accept unsupported values through
fmt.Sprint; update these boundary projections to use typed structs and type
assertions instead. Treat keyword as present only when it is a non-empty string,
and accept response message fields only when they are strings, preserving cache
classification for valid typed responses while ignoring nil, collections, and
other unsupported values.
- Around line 470-475: Update the error handling branch around
errs.SubtypeUserAllowBlockCachePreparing so it preserves any existing
Problem.Hint populated by errclass.BuildAPIError while adding
userAllowBlockCachePreparingHint as recovery guidance. Replace the direct p.Hint
assignment with append/merge behavior, retaining the existing hint and avoiding
duplicate guidance when already present.
🪄 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: abdd7554-1f4c-477c-abde-8f6aa4773f67
📒 Files selected for processing (3)
cmd/service/service.gocmd/service/service_test.goerrs/subtypes.go
Merge newer remote service snapshots over embedded metadata instead of replacing the whole service. This keeps commands compiled into a fresh binary visible when a cached remote registry has a newer version but lacks those resources, while still allowing cached methods to override matching embedded methods.
Change-Type: ci-fix
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-mail/SKILL.md`:
- Line 428: Update the duplicate-recipient warning in the shortcut documentation
to cover all five affected commands: +send, +draft-create, +forward, +reply, and
+reply-all. Keep the existing warning behavior and guidance unchanged, unless
the implementation explicitly demonstrates that any listed flow differs.
🪄 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: 8fdc73b8-a631-46d2-ab10-55c72e795e5d
📒 Files selected for processing (3)
cmd/service/service.gocmd/service/service_test.goskills/lark-mail/SKILL.md
🚧 Files skipped from review as they are similar to previous changes (2)
- cmd/service/service_test.go
- cmd/service/service.go
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 (2)
skills/lark-mail/SKILL.md (2)
115-117: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winApply the
-hrule to the canonical workflow.The first workflow step at Line 96 directly invokes
user_mailboxes profile, so copying the documented “first operation” still violates this new help-first requirement. Add the required help invocation before that command, or clarify that the workflow assumes prior command discovery.As per coding guidelines, CLI behavior documentation must accurately describe machine-consumed command usage.
🤖 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 115 - 117, Update the canonical workflow before the first user_mailboxes profile invocation to include the required -h help command, ensuring the documented first operation follows the help-first rule. Keep the existing profile workflow unchanged after command discovery.Source: Coding guidelines
229-231: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winReconcile the scheduled-send status documentation.
Line 231 introduces a “待发送” status, but Line 229 documents only numeric statuses 1–6. Document the actual pending status code/field, or avoid presenting it as the same
statusenum.🤖 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 229 - 231, Update the scheduled-send guidance near the status definitions to clarify whether “待发送” is represented by a separate field or a specific code. Document its actual field/code if it belongs to the status response; otherwise distinguish it from the numeric status enum and avoid implying it is one of statuses 1–6.
🤖 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`:
- Around line 115-117: Update the canonical workflow before the first
user_mailboxes profile invocation to include the required -h help command,
ensuring the documented first operation follows the help-first rule. Keep the
existing profile workflow unchanged after command discovery.
- Around line 229-231: Update the scheduled-send guidance near the status
definitions to clarify whether “待发送” is represented by a separate field or a
specific code. Document its actual field/code if it belongs to the status
response; otherwise distinguish it from the numeric status enum and avoid
implying it is one of statuses 1–6.
Updates the mail skill documentation with user sender allow/block API coverage.
Summary by CodeRabbit
+lint-html, invites/read receipts, templates, and sender allow/block management. Also refreshed API resources and permission mappings.