feat(mail): add +rule-reorder shortcut with slot-replacement auto-fill#979
Open
bubbmon233 wants to merge 2 commits into
Open
feat(mail): add +rule-reorder shortcut with slot-replacement auto-fill#979bubbmon233 wants to merge 2 commits into
bubbmon233 wants to merge 2 commits into
Conversation
|
|
📝 WalkthroughWalkthroughAdds a new ChangesMail Rule Reorder Feature
Skill command and wiring
Sequence DiagramsequenceDiagram
participant User
participant CLI as lark-cli
participant API as MailboxRulesAPI
User->>CLI: run `mail +rule-reorder --rule-ids X,Y,Z`
CLI->>API: GET /mail/v1/mailboxes/{mailbox}/rules (list current)
API->>CLI: current rule list [A,B,C,D,E]
CLI->>CLI: validate X,Y,Z exist in current
CLI->>CLI: slot-replace -> merged order [X,Y,Z,B,E]
CLI->>API: POST /mail/v1/mailboxes/{mailbox}/rules/reorder with merged IDs
API->>CLI: success { rule_ids: [...], meta.count: 5 }
CLI->>User: print reordered list and count
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 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)
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: 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/mail/mail_rule_reorder.go`:
- Around line 43-49: DryRun currently only records the GET of rules; update the
DryRun lambda (the DryRun function that returns common.NewDryRunAPI) to also
record the intended reorder write by chaining a POST to mailboxPath(mailboxID,
"rules/reorder") so the dry-run shows the write path and payload; include the
same merged/user rule ids from runtime (e.g., runtime.Str("rule-ids") or the
appropriate merged id key) in the POST recording (using the DryRun API's
Set/Body method) so users can preview the reorder request alongside the GET.
In `@skills/lark-mail/references/lark-mail-rule-reorder.md`:
- Line 120: The document contains a typo: "竟态窗口" should be corrected to "竞态窗口";
update the text in skills/lark-mail/references/lark-mail-rule-reorder.md where
the sentence contains "List-Reorder 竟态窗口" (or the exact phrase "不要假设返回的
`rule_ids` 顺序就是\"最终服务端顺序\" — 极小概率存在 List-Reorder 竟态窗口（建议重跑确认）") to replace "竟态"
with "竞态" so the sentence reads "List-Reorder 竞态窗口".
🪄 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: 37012e67-c570-4a74-8f2f-04adbc5dcbd7
📒 Files selected for processing (3)
shortcuts/mail/mail_rule_reorder.goshortcuts/mail/shortcuts.goskills/lark-mail/references/lark-mail-rule-reorder.md
Comment on lines
+43
to
+49
| DryRun: func(ctx context.Context, runtime *common.RuntimeContext) *common.DryRunAPI { | ||
| mailboxID := resolveMailboxID(runtime) | ||
| return common.NewDryRunAPI(). | ||
| Desc("Step 1: list rules; Step 2: apply slot-replacement fill; Step 3: reorder with merged IDs"). | ||
| GET(mailboxPath(mailboxID, "rules")). | ||
| Set("user_rule_ids", runtime.Str("rule-ids")) | ||
| }, |
There was a problem hiding this comment.
Dry-run does not preview the reorder write request.
DryRun only records the list (GET) step. It currently misses the reorder (POST .../rules/reorder) intent, so users cannot preview the write operation path/payload in dry-run mode.
💡 Suggested fix
DryRun: func(ctx context.Context, runtime *common.RuntimeContext) *common.DryRunAPI {
mailboxID := resolveMailboxID(runtime)
return common.NewDryRunAPI().
Desc("Step 1: list rules; Step 2: apply slot-replacement fill; Step 3: reorder with merged IDs").
GET(mailboxPath(mailboxID, "rules")).
- Set("user_rule_ids", runtime.Str("rule-ids"))
+ Set("user_rule_ids", runtime.Str("rule-ids")).
+ POST(mailboxPath(mailboxID, "rules", "reorder")).
+ Set("rule_ids", []string{"<merged IDs from step 2>"})
},🤖 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/mail/mail_rule_reorder.go` around lines 43 - 49, DryRun currently
only records the GET of rules; update the DryRun lambda (the DryRun function
that returns common.NewDryRunAPI) to also record the intended reorder write by
chaining a POST to mailboxPath(mailboxID, "rules/reorder") so the dry-run shows
the write path and payload; include the same merged/user rule ids from runtime
(e.g., runtime.Str("rule-ids") or the appropriate merged id key) in the POST
recording (using the DryRun API's Set/Body method) so users can preview the
reorder request alongside the GET.
| - 不要传入邮箱中不存在的规则 ID — CLI 会在 Execute 阶段报 validation 错误 | ||
| - 不要传入重复的规则 ID — Validate 阶段即报错 | ||
| - 不要省略 `--rule-ids` — 该参数为必填，缺少时返回 validation 错误 | ||
| - 不要假设返回的 `rule_ids` 顺序就是"最终服务端顺序" — 极小概率存在 List-Reorder 竟态窗口（建议重跑确认） |
There was a problem hiding this comment.
修正文案错别字（竟态 → 竞态）。
这里应为“竞态窗口”，当前“竟态”是错别字。
✏️ Suggested fix
-- 不要假设返回的 `rule_ids` 顺序就是"最终服务端顺序" — 极小概率存在 List-Reorder 竟态窗口（建议重跑确认）
+- 不要假设返回的 `rule_ids` 顺序就是"最终服务端顺序" — 极小概率存在 List-Reorder 竞态窗口（建议重跑确认）🤖 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/references/lark-mail-rule-reorder.md` at line 120, The
document contains a typo: "竟态窗口" should be corrected to "竞态窗口"; update the text
in skills/lark-mail/references/lark-mail-rule-reorder.md where the sentence
contains "List-Reorder 竟态窗口" (or the exact phrase "不要假设返回的 `rule_ids`
顺序就是\"最终服务端顺序\" — 极小概率存在 List-Reorder 竟态窗口（建议重跑确认）") to replace "竟态" with "竞态"
so the sentence reads "List-Reorder 竞态窗口".
Codecov Report❌ Patch coverage is
❌ Your patch check has failed because the patch coverage (14.58%) is below the target coverage (60.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #979 +/- ##
==========================================
+ Coverage 65.91% 67.44% +1.52%
==========================================
Files 520 577 +57
Lines 49277 54413 +5136
==========================================
+ Hits 32483 36701 +4218
- Misses 14022 14669 +647
- Partials 2772 3043 +271 ☔ 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@03252373cc9a4e270a9c3f70073b661707d0bcb1🧩 Skill updatenpx skills add bubbmon233/cli#feat/648e851 -y -g |
…nce command
BUG-1: extractRuleIDs in shortcuts/mail/mail_rule_reorder.go was accessing
resp["data"]["items"] but CallAPI/HandleApiResult already unwraps the "data"
envelope, so resp is directly {"items": [...]}. Remove the extra "data" layer
so the function correctly reads resp["items"].
BUG-2: add `lark-cli skill reference <skill-name> --name <doc-name>` command
that prints the contents of skills/<skill-name>/references/<doc-name>.md to
stdout. Binary location is used to resolve the skills directory (checks
<binary_dir>/skills/ and <binary_dir>/../skills/ in order). Supports
LARKSUITE_CLI_SKILLS_DIR env override for test environments.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
github-actions
Bot
added
size/L
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 `@cmd/skill/skill.go`:
- Around line 41-45: Replace the bare fmt.Errorf returned from the RunE function
in skill.go with the command error wrapper used across CLI commands (use
output.Errorf or output.ErrWithHint) so stderr stays machine-parseable;
specifically, in the RunE closure where you check `if name == ""` return
output.Errorf("--name is required") (or output.ErrWithHint with an additional
hint string), and ensure the output package is imported/used consistently in
this file and any callers of the RunE error behavior.
- Around line 69-74: relPath and fullPath are built from untrusted CLI inputs
and then read with os.ReadFile; replace this with path validation and the repo
VFS: call validate.SafeInputPath on the user-derived path (e.g.,
validate.SafeInputPath(relPath) or on skillName/name before building relPath)
and skip the candidate when validation fails, then use the project's virtual FS
API (vfs.ReadFile or the repo's vfs.* equivalent) instead of os.ReadFile to
open/read fullPath; keep the loop over candidateDirs() and use the same
relPath/fullPath symbols so the change is localized to validation and replacing
os.ReadFile with vfs.ReadFile.
🪄 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: 40d67347-2de7-4b0a-b402-765b6f14d1db
📒 Files selected for processing (3)
cmd/build.gocmd/skill/skill.goshortcuts/mail/mail_rule_reorder.go
Comment on lines
+41
to
+45
| RunE: func(cmd *cobra.Command, args []string) error { | ||
| skillName := args[0] | ||
| if name == "" { | ||
| return fmt.Errorf("--name is required") | ||
| } |
There was a problem hiding this comment.
Use command error wrappers in RunE instead of fmt.Errorf.
Line 44 should return output.Errorf (or output.ErrWithHint) so stderr remains machine-parseable for AI agents.
As per coding guidelines: "cmd/**/*.go: RunE functions in commands must return output.Errorf / output.ErrWithHint — never bare fmt.Errorf, because AI agents parse stderr as JSON".
🤖 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 `@cmd/skill/skill.go` around lines 41 - 45, Replace the bare fmt.Errorf
returned from the RunE function in skill.go with the command error wrapper used
across CLI commands (use output.Errorf or output.ErrWithHint) so stderr stays
machine-parseable; specifically, in the RunE closure where you check `if name ==
""` return output.Errorf("--name is required") (or output.ErrWithHint with an
additional hint string), and ensure the output package is imported/used
consistently in this file and any callers of the RunE error behavior.
Comment on lines
+69
to
+74
| relPath := filepath.Join("skills", skillName, "references", name+".md") | ||
|
|
||
| for _, dir := range candidateDirs() { | ||
| fullPath := filepath.Join(dir, relPath) | ||
| data, err := os.ReadFile(fullPath) | ||
| if err == nil { |
There was a problem hiding this comment.
Validate and read user-derived paths via validate.SafeInputPath + vfs.*.
fullPath is built from untrusted CLI args (skillName, name) and then read with os.ReadFile. This bypasses required path validation and the repo’s filesystem abstraction.
As per coding guidelines: "Use vfs.* instead of os.* for all filesystem access" and "Validate paths before reading with validate.SafeInputPath because CLI arguments are untrusted (they come from AI agents)".
🤖 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 `@cmd/skill/skill.go` around lines 69 - 74, relPath and fullPath are built from
untrusted CLI inputs and then read with os.ReadFile; replace this with path
validation and the repo VFS: call validate.SafeInputPath on the user-derived
path (e.g., validate.SafeInputPath(relPath) or on skillName/name before building
relPath) and skip the candidate when validation fails, then use the project's
virtual FS API (vfs.ReadFile or the repo's vfs.* equivalent) instead of
os.ReadFile to open/read fullPath; keep the loop over candidateDirs() and use
the same relPath/fullPath symbols so the change is localized to validation and
replacing os.ReadFile with vfs.ReadFile.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a new
lark-cli mail +rule-reordershortcut that lets users specify only the rules they want to reorder. The CLI auto-fills the remaining rules from the current server-side order using a slot-replacement algorithm, then calls the Reorder API with the complete merged list.Problem:
ReorderUserMailboxRuleAPI requires all rule IDs. Providing a partial list causes an API error.Solution: The CLI fetches the current rule order first (List API), merges the user-supplied partial order via slot-replacement, then submits the full merged list.
Slot-Replacement Algorithm
Given current order
[D,A,G,C,B,E]and user inputE,A:[D,E,G,C,B,A]✓Changes
shortcuts/mail/mail_rule_reorder.goMailRuleReorderShortcut + helpersshortcuts/mail/shortcuts.goMailRuleReorderinShortcuts()skills/lark-mail/references/lark-mail-rule-reorder.md+rule-reorderFlags
--mailbox"me""me"resolves to the caller's own mailbox--rule-ids*Validated in the
Validatecallback (notRequired: true), so--dry-runworks without providing real IDs.Auth & Scope
user,botmail:user_mailbox.rule:writeTesting
Summary by CodeRabbit
New Features
mail +rule-reorderCLI shortcut to reorder inbox rules using a comma-separated list (validates IDs, rejects duplicates, supports dry-run preview).skillcommand with areferencesubcommand to print skill reference documents.Documentation