feat(im): add +chat-members-add shortcut#1988
Conversation
…check chat.members.create raw meta lists ["im:chat", "im:chat.members:write_only"] as OR alternatives, but the local scope precheck (MissingScopes) treats every entry in Scopes as required (AND), so declaring both wrongly rejected tokens holding only the narrower write_only scope. Declare only that narrow scope, matching the chat-members-list shortcut pattern, and update the doc troubleshooting row/login example to match.
zhangheng023
added
enhancement
github-actions
Bot
added
size/L
📝 WalkthroughWalkthroughAdds the ChangesIM chat member addition
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant ImChatMembersAdd
participant ChatMembersAPI
CLI->>ImChatMembersAdd: Execute chat and member flags
ImChatMembersAdd->>ChatMembersAPI: POST user members with open_id
ImChatMembersAdd->>ChatMembersAPI: POST bot members with app_id
ChatMembersAPI-->>ImChatMembersAdd: Return categorized member results
ImChatMembersAdd-->>CLI: Return merged ledger or typed authorization error
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1988 +/- ##
==========================================
+ Coverage 75.05% 75.07% +0.02%
==========================================
Files 901 902 +1
Lines 95391 95541 +150
==========================================
+ Hits 71591 71724 +133
- Misses 18307 18317 +10
- Partials 5493 5500 +7 ☔ 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@cdb5343bd435a29c390d90c60f13e1bfa7accd63🧩 Skill updatenpx skills add larksuite/cli#feat/im-chat-add-members -y -g |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@shortcuts/im/im_chat_members_add.go`:
- Around line 65-72: Add adjacent table-driven tests covering invalid --users
and --bots prefixes through both validateChatMembersAdd and
executeChatMembersAdd, asserting the collectMemberAddIDs errors are propagated.
Add direct allAuthClassified tests for an empty slice and a mixed-category
slice, asserting the empty guard and non-auth mismatch results.
- Around line 84-91: Replace the loose map-based API and call-error handling in
addChatMembersBatch with typed structures: introduce typed response and
call-error structs, decode the response into the typed API response once, and
change chatMembersAddResult.callErrors to []chatMembersAddCallError. Update the
affected ledger construction and consumption sites to use typed fields instead
of map indexing and type assertions, preserving the existing invalid,
nonexistent, pending-approval, and error values.
- Line 178: Remove the stray “user/bot;” fragment from the command Description
string, leaving the surrounding wording and batching behavior unchanged so the
CLI help text reads clearly.
🪄 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: b02903e2-e762-4fed-9529-8d3cadc6f949
📒 Files selected for processing (6)
shortcuts/im/helpers_test.goshortcuts/im/im_chat_members_add.goshortcuts/im/im_chat_members_add_test.goshortcuts/im/shortcuts.goskills/lark-im/SKILL.mdskills/lark-im/references/lark-im-chat-members-add.md
| users, err := collectMemberAddIDs(runtime, "users", "ou_", chatMembersAddMaxUsers) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| bots, err := collectMemberAddIDs(runtime, "bots", "cli_", chatMembersAddMaxBots) | ||
| if err != nil { | ||
| return err | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add test coverage for these error-propagation and classification branches.
Codecov flags Lines 67, 71 (validateChatMembersAdd returning collectMemberAddIDs errors for --users/--bots), Lines 235, 239 (same pattern in executeChatMembersAdd), and Lines 296, 301 (allAuthClassified's empty-slice guard and non-auth mismatch branch) as uncovered. None of the current table-driven tests exercise an invalid --users/--bots prefix through the full validateChatMembersAdd/executeChatMembersAdd wiring, or exercise allAuthClassified with an empty or mixed-category slice directly.
As per coding guidelines, "Every behavior change must include an adjacent test, and the test must directly assert the changed field or behavior so reverting the implementation causes the test to fail."
Also applies to: 233-240, 294-305
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 67-67: shortcuts/im/im_chat_members_add.go#L67
Added line #L67 was not covered by tests
[warning] 71-71: shortcuts/im/im_chat_members_add.go#L71
Added line #L71 was not covered by tests
🤖 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/im/im_chat_members_add.go` around lines 65 - 72, Add adjacent
table-driven tests covering invalid --users and --bots prefixes through both
validateChatMembersAdd and executeChatMembersAdd, asserting the
collectMemberAddIDs errors are propagated. Add direct allAuthClassified tests
for an empty slice and a mixed-category slice, asserting the empty guard and
non-auth mismatch results.
Sources: Coding guidelines, Linters/SAST tools
| type chatMembersAddResult struct { | ||
| succeeded []string | ||
| invalid []string | ||
| notExisted []string | ||
| pendingApproval []string | ||
| callErrors []map[string]interface{} | ||
| rawCallErrors []error | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
Loose-map usage for API response and ledger entries.
addChatMembersBatch indexes the decoded response via data["invalid_id_list"], data["not_existed_id_list"], etc., and callErrors is stored as []map[string]interface{} with call sites later doing ce["id_list"].([]string) type assertions (Line 264). This is new loose-map code at an API boundary rather than a parsed typed struct.
♻️ Suggested typed-struct approach
type chatMembersAddAPIResponse struct {
InvalidIDList []string `json:"invalid_id_list"`
NotExistedIDList []string `json:"not_existed_id_list"`
PendingApprovalIDList []string `json:"pending_approval_id_list"`
}
type chatMembersAddCallError struct {
MemberType string `json:"member_type"`
IDList []string `json:"id_list"`
Error string `json:"error"`
}Decode data into chatMembersAddAPIResponse once, and change chatMembersAddResult.callErrors to []chatMembersAddCallError, replacing the map indexing/assertions accordingly.
As per coding guidelines, "Parse map[string]interface{} into typed structs at boundaries, use projection functions per shape... Do not introduce new loose-map code."
Also applies to: 107-148, 263-267, 270-280
🤖 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/im/im_chat_members_add.go` around lines 84 - 91, Replace the loose
map-based API and call-error handling in addChatMembersBatch with typed
structures: introduce typed response and call-error structs, decode the response
into the typed API response once, and change chatMembersAddResult.callErrors to
[]chatMembersAddCallError. Update the affected ledger construction and
consumption sites to use typed fields instead of map indexing and type
assertions, preserving the existing invalid, nonexistent, pending-approval, and
error values.
Source: Coding guidelines
| var ImChatMembersAdd = common.Shortcut{ | ||
| Service: "im", | ||
| Command: "+chat-members-add", | ||
| Description: "Add users and/or bots to a group chat; user/bot; batches --users (open_id) and --bots (app_id) into up to 2 API calls under best-effort semantics; returns a merged succeeded/invalid/not_existed/pending_approval ledger", |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Stray fragment in the Description string.
"...to a group chat; user/bot; batches --users..." — the ; user/bot; clause reads as leftover/misplaced text and disrupts an otherwise clear help string shown to CLI users.
✏️ Suggested cleanup
- Description: "Add users and/or bots to a group chat; user/bot; batches --users (open_id) and --bots (app_id) into up to 2 API calls under best-effort semantics; returns a merged succeeded/invalid/not_existed/pending_approval ledger",
+ Description: "Add users and/or bots to a group chat; batches --users (open_id) and --bots (app_id) into up to 2 API calls under best-effort semantics; returns a merged succeeded/invalid/not_existed/pending_approval ledger",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Description: "Add users and/or bots to a group chat; user/bot; batches --users (open_id) and --bots (app_id) into up to 2 API calls under best-effort semantics; returns a merged succeeded/invalid/not_existed/pending_approval ledger", | |
| Description: "Add users and/or bots to a group chat; batches --users (open_id) and --bots (app_id) into up to 2 API calls under best-effort semantics; returns a merged succeeded/invalid/not_existed/pending_approval ledger", |
🤖 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/im/im_chat_members_add.go` at line 178, Remove the stray
“user/bot;” fragment from the command Description string, leaving the
surrounding wording and batching behavior unchanged so the CLI help text reads
clearly.
1 participant
Summary
Add
im +chat-members-add, a shortcut that adds users and/or bots to a group chat. The existing Meta API commandchat.members.createonly accepts onemember_id_typeper request (open_idfor users,app_idfor bots), so adding both requires two separate calls whose results must be merged by hand. This shortcut orchestrates that internally under best-effort semantics and returns one merged ledger.Changes
im +chat-members-addshortcut:--users/--botsflags with local validation (ID prefix + count limits),--dry-runpreview of up to two underlying API calls, and a merged output ledger (succeeded_id_list/invalid_id_list/not_existed_id_list/pending_approval_id_list/call_errors/success_count/failure_count/total)--usersand--botsare supplied and both underlying calls fail with an auth/permission-classified error, the command returns that typed error directly instead of building a ledgerTest Plan
go test ./shortcuts/im/...) — covers validation rules, batch-merge logic (all-succeed / partial failure across all three failure buckets / call-level failure), the auth-both-fail short-circuit, and dry-run outputgo build ./...andgo vet ./...passlark-cli im +chat-members-addflow works as expectedRelated Issues
Summary by CodeRabbit
New Features
+chat-members-addshortcut for batch-adding users and bots to chats.Documentation
Tests