feature(lark-mail): add data integrity and write-confirmation rules#749
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:
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 (2)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughAdds a "数据真实性与操作合规" section to mail domain docs that requires truthful reporting when referenced mailbox objects are missing and mandates action previews plus user confirmation for non-send irreversible write operations; includes authorization rules and a bulk-delete example. ChangesMail Operations Compliance
Sequence Diagram(s)sequenceDiagram
participant User
participant Assistant
participant MailService
User->>Assistant: Request action referencing mailbox objects
Assistant->>MailService: Query targets
MailService-->>Assistant: Found / NotFound
Assistant->>User: Present triage + action preview (key fields, affected count)
User->>Assistant: Confirm
Assistant->>MailService: Execute write operation (e.g., *.batch_trash)
MailService-->>Assistant: Execution result
Assistant->>User: Report outcome
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #749 +/- ##
==========================================
+ Coverage 64.96% 65.40% +0.43%
==========================================
Files 502 508 +6
Lines 46224 46795 +571
==========================================
+ Hits 30030 30605 +575
+ Misses 13583 13548 -35
- Partials 2611 2642 +31 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@1c462507cc50aaad2ecfe4a98c7ca8181d112a42🧩 Skill updatenpx skills add xzcong0820/larksuite-cli#feature/lark-mail-data-integrity-rules -y -g |
6d1b6cc to
30ab418
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
skills/lark-mail/SKILL.md (1)
485-486: ⚡ Quick winConsider explicitly listing both add and remove label operations for clarity.
Line 486 mentions "加标签" (add label) as exempt but doesn't explicitly list "移除标签" (remove label), even though line 68 shows both
*.add_labeland*.remove_labelare marked as reversible and exempt from confirmation.While the "等可逆
modify" (etc. reversible modify) phrasing should cover remove labels, explicitly listing both would prevent confusion—especially since a previous review flagged inconsistency in how label operations were documented.📝 Suggested wording for improved clarity
-> **写操作前回顶部规则**:调用 `*.delete` / `*.trash` / `*.batch_trash` / `*.cancel_scheduled_send` / `rules.*` 删改前,先回顶部 [`## 数据真实性与操作合规`](`#数据真实性与操作合规`) 的"写操作前显式确认"。标记已读、加标签、改文件夹等可逆 `modify` 不需要回顶部。 +> **写操作前回顶部规则**:调用 `*.delete` / `*.trash` / `*.batch_trash` / `*.cancel_scheduled_send` / `rules.*` 删改前,先回顶部 [`## 数据真实性与操作合规`](`#数据真实性与操作合规`) 的"写操作前显式确认"。标记已读、加/移除标签、改文件夹等可逆 `modify` 不需要回顶部。🤖 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 485 - 486, The doc line about exempt reversible "modify" operations is ambiguous for label changes—explicitly mention both add and remove label operations to match the actual API and avoid confusion: update the sentence that currently lists "加标签" to list "加标签/移除标签 (e.g., *.add_label, *.remove_label)" and ensure the notation matches other examples like the documented `*.add_label` and `*.remove_label` entries earlier so readers clearly see both operations are exempt from the pre-write confirmation rule.
🤖 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.
Nitpick comments:
In `@skills/lark-mail/SKILL.md`:
- Around line 485-486: The doc line about exempt reversible "modify" operations
is ambiguous for label changes—explicitly mention both add and remove label
operations to match the actual API and avoid confusion: update the sentence that
currently lists "加标签" to list "加标签/移除标签 (e.g., *.add_label, *.remove_label)" and
ensure the notation matches other examples like the documented `*.add_label` and
`*.remove_label` entries earlier so readers clearly see both operations are
exempt from the pre-write confirmation rule.
chanthuang
left a comment
There was a problem hiding this comment.
LGTM. 规则清晰,表格化的确认/免确认分类比上一版好很多,流程示例也有助于 agent 理解预期行为。
30ab418 to
0536ce2
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
skill-template/domains/mail.md (1)
44-60: ⚡ Quick winConsider clarifying "动作预览" field requirements.
Line 46 states that the action preview must include "操作类型 + 关键字段:发件人 / 主题 / 文件夹 / 受影响数量", but the workflow example at line 67 only shows sender, subject, and count (no folder). While this is likely intentional since not all fields apply to all operations, consider adding "适用时" or "as applicable" to prevent implementers from trying to force all fields into every preview.
📝 Suggested clarification
-下列操作(除发送类外)执行前,必须展示**动作预览**(操作类型 + 关键字段:发件人 / 主题 / 文件夹 / 受影响数量)并取得确认: +下列操作(除发送类外)执行前,必须展示**动作预览**(操作类型 + 适用的关键字段:发件人 / 主题 / 文件夹 / 受影响数量)并取得确认:🤖 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 `@skill-template/domains/mail.md` around lines 44 - 60, Clarify that the "动作预览" requirement applies fields conditionally by adding wording such as "适用时" or "仅在相关操作适用时展示" next to the listed fields (发件人 / 主题 / 文件夹 / 受影响数量) so implementers know not all previews must include every field; update the section that defines 动作预览 and the 批量操作说明 to reference this conditional rule and ensure the workflow example (which currently shows only 发件人 / 主题 / 数量) is annotated or commented as an "适用时示例".
🤖 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 `@skill-template/domains/mail.md`:
- Around line 29-69: Add a cross-reference at the start of the "原生 API 调用规则"
section that points to and enforces the "数据真实性与操作合规" rules (the section titled
exactly "数据真实性与操作合规"), instructing agents that all native API calls must still
follow those data authenticity and operational compliance requirements (e.g., no
fabricating IDs, explicit confirmation rules for destructive/batch ops); place
this note as the first paragraph under "原生 API 调用规则" so agents invoking native
APIs are explicitly reminded to follow the compliance rules.
---
Nitpick comments:
In `@skill-template/domains/mail.md`:
- Around line 44-60: Clarify that the "动作预览" requirement applies fields
conditionally by adding wording such as "适用时" or "仅在相关操作适用时展示" next to the
listed fields (发件人 / 主题 / 文件夹 / 受影响数量) so implementers know not all previews
must include every field; update the section that defines 动作预览 and the 批量操作说明 to
reference this conditional rule and ensure the workflow example (which currently
shows only 发件人 / 主题 / 数量) is annotated or commented as an "适用时示例".
🪄 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: f02814a3-58d4-4386-b3fb-23ada9c44bbe
📒 Files selected for processing (1)
skill-template/domains/mail.md
0536ce2 to
14e7f06
Compare
Adds a new top-level safety section "数据真实性与操作合规" to the
lark-mail skill via the canonical generation pipeline:
- skill-template/domains/mail.md (source) — adds the section to the
domain introduction file that gen-skills.py renders into SKILL.md.
- skills/lark-mail/SKILL.md (regenerated product) — produced by
`make gen-skills project=mail` from larksuite-cli-registry against
the modified mail.md source.
Why both files: skills/lark-mail/SKILL.md is auto-generated from
skill-template/domains/mail.md + registry-conf/skill-meta.yaml +
output/from_meta/mail.json. Editing only SKILL.md would be reverted on
the next `make gen-skills` run because SKILL.md has no AUTO-GENERATED
markers and falls into the "no markers -> overwrite whole file" branch
in scripts/gen-skills.py.
The section adds 3 hard constraints on agent behavior:
- empty result is a valid answer; do not fabricate IDs or placeholders
- explicit action preview before destructive write operations
(delete / trash / batch_trash / cancel_scheduled_send / rules.*)
- reversible modifications (label / read state / folder move) are
exempt from the preview requirement
Addresses recurring evaluation failures (c03/c04/c06/c09/c14/c19~c24/c40)
where the agent fabricated IDs or auto-executed destructive operations.
14e7f06 to
1c46250
Compare
Add a new top-level safety section "数据真实性与操作合规" with 3 hard constraints on agent behavior: empty result is a valid answer, do not fabricate target objects, and explicitly confirm before destructive write operations. Cross-reference the rule from API Resources section to catch agents that jump directly to native API lookup.
Summary
Changes
Test Plan
lark xxxcommand works as expectedRelated Issues
Summary by CodeRabbit