feat: add chat-members-add shortcut - #2214
Conversation
…-members-add ledger
github-actions
Bot
added
domain/im
📝 WalkthroughWalkthroughChangesThe PR adds the IM chat member addition
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant ImChatMembersAdd
participant ChatMembersAPI
CLI->>ImChatMembersAdd: provide chat, user, bot, and succeed_type flags
ImChatMembersAdd->>ImChatMembersAdd: validate inputs and build request
ImChatMembersAdd->>ChatMembersAPI: POST chat members with open_id identifiers
ChatMembersAPI-->>ImChatMembersAdd: return addition results
ImChatMembersAdd-->>CLI: render success and failure output
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 #2214 +/- ##
========================================
Coverage 76.40% 76.41%
========================================
Files 1012 1013 +1
Lines 111565 111669 +104
========================================
+ Hits 85241 85330 +89
- Misses 19832 19840 +8
- Partials 6492 6499 +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@689595e7bed4fd886fe232ed3616b048b1162692🧩 Skill updatenpx skills add larksuite/cli#feat/im-add-members-shortcut -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: 1
🧹 Nitpick comments (5)
shortcuts/im/im_chat_members_add_test.go (3)
521-539: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the dry-run payload; the test name promises body coverage.
TestImChatMembersAddDryRunRendersBodyonly checks thatDryRunreturns a non-nil value. The test passes even if the method, path,member_id_type,succeed_type, orid_listchange. Assert those fields so a regression inDryRunfails the test.As per coding guidelines: "contract tests must assert the changed field or behavior directly so reverting the implementation causes failure".
🤖 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_test.go` around lines 521 - 539, Strengthen TestImChatMembersAddDryRunRendersBody by asserting the returned dry-run request payload, including the HTTP method, path, member_id_type, succeed_type, and id_list fields. Keep the existing no-API-call setup, and make each expected value explicit so reverting ImChatMembersAdd.DryRun causes the test to fail.Source: Coding guidelines
349-398: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueParse the JSON output instead of scanning substrings.
extractArrayField,extractIntField, and thestrings.Containsassertions depend on key order, spacing, and formatting of the emitted JSON. Decode the captured stdout withencoding/jsonand assert on typed values. Also,joinCommaduplicatesstrings.Join.🤖 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_test.go` around lines 349 - 398, Replace the substring-based helpers extractArrayField and extractIntField, along with related strings.Contains assertions, by decoding captured stdout through encoding/json into typed values and asserting against those values independent of JSON formatting or key order. Remove joinComma and use strings.Join for comma-separated IDs.
96-119: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse distinct IDs in the over-limit fixture.
"ou_"+string(rune('a'+i%26))produces duplicate IDs fori >= 26. The test then depends oncommon.SplitCSVnot deduplicating. Generate 51 unique IDs, for example withfmt.Sprintf("ou_%d", i), so the fixture tests the cap and not the split behavior.🤖 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_test.go` around lines 96 - 119, Update the fixture in TestCollectChatMembersToAddRejectsTooManyUsers to generate 51 distinct user IDs, replacing the rune-based ID construction with a numeric format such as fmt.Sprintf("ou_%d", i); add or reuse the required fmt import. Keep the test focused on rejecting the user-count limit.shortcuts/im/im_chat_members_add.go (2)
85-119: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueConfirm the intent of the stderr failure count.
failedIDsincludes IDs returned by the server that were never requested.failure_countcounts only requested IDs. When the server echoes an unrequested ID, the stderr warning count and the JSONfailure_countdiffer. The testTestEmitChatMembersAddResultInvariantHoldsUnderServerAnomaliescovers this case for the JSON invariant but not for the warning text. Consider filteringfailedIDsto requested IDs, or keep the current behavior deliberately.🤖 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 85 - 119, Align the stderr warning count with the requested-member failure count in the result-building flow around failedSet, failedIDs, and outData. Filter server-reported failures to IDs present in requested before calculating the warning count, while preserving deduplication and the existing failure lists and JSON counts.
46-57: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a shared bot-ID validator for shortcut flags.
--userscallscommon.ValidateUserIDTyped, while both+chat-createand this shortcut use inlinestrings.HasPrefix(id, "cli_")checks. Introduce a sharedcommon.ValidateBotIDTypedhelper and use it here so bot-ID validation, normalization, and error text stay consistent.🤖 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 46 - 57, Add a shared common.ValidateBotIDTyped helper for bot-ID validation, normalization, and consistent error text, then replace the inline strings.HasPrefix check in the bots parsing block with that helper. Preserve the existing --bots count limit and append each validated/normalized bot ID to bots.
🤖 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-im/references/lark-im-chat-create.md`:
- Around line 89-93: Update the recovery guidance near the default
`--succeed-type 1` behavior to name all failure lists: `invalid_id_list`,
`not_existed_id_list`, and `pending_approval_ids`. State that any non-empty list
represents users not successfully added and causes `+chat-members-add` to exit
non-zero, matching the rule documented in `lark-im-chat-members-add.md`.
---
Nitpick comments:
In `@shortcuts/im/im_chat_members_add_test.go`:
- Around line 521-539: Strengthen TestImChatMembersAddDryRunRendersBody by
asserting the returned dry-run request payload, including the HTTP method, path,
member_id_type, succeed_type, and id_list fields. Keep the existing no-API-call
setup, and make each expected value explicit so reverting
ImChatMembersAdd.DryRun causes the test to fail.
- Around line 349-398: Replace the substring-based helpers extractArrayField and
extractIntField, along with related strings.Contains assertions, by decoding
captured stdout through encoding/json into typed values and asserting against
those values independent of JSON formatting or key order. Remove joinComma and
use strings.Join for comma-separated IDs.
- Around line 96-119: Update the fixture in
TestCollectChatMembersToAddRejectsTooManyUsers to generate 51 distinct user IDs,
replacing the rune-based ID construction with a numeric format such as
fmt.Sprintf("ou_%d", i); add or reuse the required fmt import. Keep the test
focused on rejecting the user-count limit.
In `@shortcuts/im/im_chat_members_add.go`:
- Around line 85-119: Align the stderr warning count with the requested-member
failure count in the result-building flow around failedSet, failedIDs, and
outData. Filter server-reported failures to IDs present in requested before
calculating the warning count, while preserving deduplication and the existing
failure lists and JSON counts.
- Around line 46-57: Add a shared common.ValidateBotIDTyped helper for bot-ID
validation, normalization, and consistent error text, then replace the inline
strings.HasPrefix check in the bots parsing block with that helper. Preserve the
existing --bots count limit and append each validated/normalized bot ID to bots.
🪄 Autofix
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 Plus
Run ID: d91f20e9-2f48-4657-a542-e12a85dde6b9
📒 Files selected for processing (10)
affordance/im.mdinternal/affordance/im_source_test.goshortcuts/im/affordance_migration_test.goshortcuts/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-create.mdskills/lark-im/references/lark-im-chat-members-add.md
| lark-cli im +chat-members-add --chat-id <chat_id from step 2> \ | ||
| --users ou_aaa,ou_bbb --as user | ||
| ``` | ||
|
|
||
| `succeed_type=1` ensures reachable users are added successfully; unreachable ones are returned in `invalid_id_list` instead of failing the whole request. | ||
| By default (`--succeed-type 1`), reachable users are added successfully; unreachable ones are returned in `invalid_id_list` instead of failing the whole request. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Name all three failure lists in the recovery step.
Line 93 mentions only invalid_id_list. +chat-members-add also reports not_existed_id_list and pending_approval_ids, and it exits non-zero when any of the three is non-empty. skills/lark-im/references/lark-im-chat-members-add.md states this rule. Update the text so the two documents agree.
📝 Proposed doc fix
- By default (`--succeed-type 1`), reachable users are added successfully; unreachable ones are returned in `invalid_id_list` instead of failing the whole request.
+ By default (`--succeed-type 1`), reachable users are added successfully; unreachable ones are returned in `invalid_id_list`, `not_existed_id_list`, or `pending_approval_ids` instead of failing the whole request. The command exits non-zero with `ok:false` when any of the three lists is non-empty.📝 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.
| lark-cli im +chat-members-add --chat-id <chat_id from step 2> \ | |
| --users ou_aaa,ou_bbb --as user | |
| ``` | |
| `succeed_type=1` ensures reachable users are added successfully; unreachable ones are returned in `invalid_id_list` instead of failing the whole request. | |
| By default (`--succeed-type 1`), reachable users are added successfully; unreachable ones are returned in `invalid_id_list` instead of failing the whole request. | |
| lark-cli im +chat-members-add --chat-id <chat_id from step 2> \ | |
| --users ou_aaa,ou_bbb --as user |
🤖 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-im/references/lark-im-chat-create.md` around lines 89 - 93,
Update the recovery guidance near the default `--succeed-type 1` behavior to
name all failure lists: `invalid_id_list`, `not_existed_id_list`, and
`pending_approval_ids`. State that any non-empty list represents users not
successfully added and causes `+chat-members-add` to exit non-zero, matching the
rule documented in `lark-im-chat-members-add.md`.
1 participant
Summary
Adds a new
im +chat-members-addshortcut that adds users and/or bots to an existing group chat. It replaces the rawim chat.members createmeta API call — which requires two separate JSON blobs (--paramsforchat_id/member_id_type/succeed_type,--dataforid_list) and an implicit rule that a user ID's format must matchmember_id_typewhile bot IDs are alwaysapp_id— with two plain flags (--users,--bots) and a fixedmember_id_type=open_id, removing that footgun entirely.Changes
shortcuts/im/im_chat_members_add.go:--chat-id(required),--users(comma-separatedou_xxx, max 50),--bots(comma-separatedcli_xxx, max 5),--succeed-type(0 = strict / 1 = partial-success, default 1)ok:falseledger envelope (mirrors the existing+feed-shortcut-createconvention), covering all three server response buckets (invalid_id_list,not_existed_id_list,pending_approval_id_list) so a member that failed to join is never miscounted as succeededshortcuts/im/shortcuts.goskills/lark-im/references/lark-im-chat-members-add.md;SKILL.mdShortcuts/permission tables updatedlark-im-chat-create.md,affordance/im.md) updated to point their two-step invite examples at the new shortcut instead of the raw meta command+messages-search(its affordance example still pinned--as userafter a recent PR made it dual-identity) was also corrected, since it was blocking a cleango testsignal for this branchTest Plan
go test ./shortcuts/im/...,go test ./internal/affordance/...) — covers flag validation, dry-run, the full ledger (all-success / partial-failure across all three failure buckets), and the--succeed-typestrict/partial modeslark-cli im +chat-members-add --chat-id oc_xxx --users ou_xxx --bots cli_xxxagainst a real Feishu tenant — confirmed the partial-failure ledger output and the empty--users/--botsvalidation errorRelated Issues
Summary by CodeRabbit
New Features
Documentation