feat(drive): add +member-add shortcut for Drive collaborator permissions#1204
Open
zhaojiaxing-coding wants to merge 1 commit into
Open
feat(drive): add +member-add shortcut for Drive collaborator permissions#1204zhaojiaxing-coding wants to merge 1 commit into
zhaojiaxing-coding wants to merge 1 commit into
Conversation
Add drive +member-add shortcut that grants view/edit/full_access permissions to collaborators on Drive documents, files, folders, and wiki nodes. Supports single and batch member add (up to 10). Key features: - Auto-infer resource type from Feishu/Lark/Doubao URL or token prefix - Required --member-type with prefix conflict detection (appid aliased to userid) - Reject --perm-type for non-wiki resource types - Trusted host validation for URL inputs Also extends shortcuts/common/resource_url with minutes type support and InferResourceTypeFromToken for bare token prefix mapping.
|
|
📝 WalkthroughWalkthroughThis PR introduces the ChangesDrive +member-add shortcut and resource type inference
Sequence DiagramsequenceDiagram
participant User
participant CLI as drive +member-add
participant Parser as readDriveMemberAddSpec
participant Inference as Token/Type Inference
participant DriveAPI as Drive API
participant Output as driveMemberAddOutput
User->>CLI: --token URL/token --member-id ID --perm view
CLI->>Parser: parse flags
Parser->>Inference: infer resource type from token
Inference->>Parser: resource type (doc/wiki/etc)
Parser->>Parser: validate member-type, resolve perm defaults
CLI->>DriveAPI: single/batch collaborator.create
DriveAPI->>Output: API response
Output->>Output: backfill missing fields, alias types
Output->>User: flattened JSON + status
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes The PR introduces a substantial new feature with multiple interacting components: resource type inference, command-line parsing with cross-field validation, nested execution logic for single vs batch operations, API response reshaping, and comprehensive test coverage spanning unit, integration, and e2e scenarios. The changes are spread across multiple files with varied complexity, including permission/notification constraints, bot-specific behavior, and wiki-specific rules that require careful review. Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
github-actions
Bot
added
domain/ccm
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1204 +/- ##
==========================================
+ Coverage 68.61% 69.17% +0.56%
==========================================
Files 625 631 +6
Lines 58348 59571 +1223
==========================================
+ Hits 40035 41210 +1175
- Misses 15029 15040 +11
- Partials 3284 3321 +37 ☔ 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@737045a42620977aabec9b06eef8626eee2231cc🧩 Skill updatenpx skills add larksuite/cli#feat/drive-member-add-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: 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 `@shortcuts/drive/drive_member_add_test.go`:
- Around line 673-711: The test currently only asserts the POST body but must
also assert the bot request omitted the need_notification query param; update
the test that uses the httpmock.Stub (stub) in DriveMemberAdd/mountAndRunDrive
to capture the incoming request's URL.RawQuery (or use the stub's
CapturedRequest/URL field) after mountAndRunDrive runs, then assert that the raw
query string does not contain "need_notification" (or that parsed query has no
"need_notification" key); keep existing body assertions (captured["perm"] ==
"edit") and add a failing t.Fatalf message if the query contains
need_notification so the execute-path contract is enforced.
In `@shortcuts/drive/drive_member_add.go`:
- Around line 414-438: The loop currently only appends members when
rawMembers[i] is a valid map and falls back to backfilling only if results is
empty; change it to iterate over spec.MemberIDs (by index) and for each index
attempt to read rawMembers[i] as a map[string]interface{}—if present use
driveMemberAddOutput(spec, memberID, m), otherwise call
driveMemberAddOutput(spec, memberID, nil) so every requested member is
represented; ensure results is built in the same order as spec.MemberIDs and
that the final runtime.Out and the printed count reflect the full spec.MemberIDs
length.
In `@skills/lark-drive/SKILL.md`:
- Around line 259-261: The two adjacent blockquote paragraphs in SKILL.md need
to be merged into one contiguous blockquote to satisfy markdownlint MD028;
remove the blank line between the two blocks and combine their text into a
single blockquote so the note about member-type mapping and recommendation to
use bot open_id appears as one continuous block (look for the blockquote
starting with "注意：此方式仅适用于需要授权给**当前应用**的场景。" and the following "补充：" paragraph
and join them).
In `@tests/cli_e2e/drive/drive_member_add_dryrun_test.go`:
- Around line 319-320: The test currently asserts the dry-run validation error
only against result.Stderr; update the assertion so Validate-stage failures are
checked from the full CLI output by combining result.Stdout and result.Stderr.
Replace the require.Contains call that uses result.Stderr with one that checks
result.Stdout+result.Stderr (keeping the existing result.AssertExitCode(t, 2)
and tt.wantErr), so the test verifies the error message in the structured stdout
envelope as well as stderr.
🪄 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: e259d1a8-6b0f-4838-9192-0e4b19d3539b
📒 Files selected for processing (9)
shortcuts/common/resource_url.goshortcuts/common/resource_url_test.goshortcuts/drive/drive_member_add.goshortcuts/drive/drive_member_add_test.goshortcuts/drive/shortcuts.goshortcuts/drive/shortcuts_test.goskills/lark-drive/SKILL.mdskills/lark-drive/references/lark-drive-member-add.mdtests/cli_e2e/drive/drive_member_add_dryrun_test.go
Comment on lines
+673
to
+711
| stub := &httpmock.Stub{ | ||
| Method: "POST", | ||
| URL: "/open-apis/drive/v1/permissions/wikcnBotTok/members", | ||
| Body: map[string]interface{}{ | ||
| "code": 0, | ||
| "msg": "success", | ||
| "data": map[string]interface{}{ | ||
| "member": map[string]interface{}{ | ||
| "member_id": "ou_bot_target", | ||
| "member_type": "openid", | ||
| "perm": "edit", | ||
| "type": "user", | ||
| }, | ||
| }, | ||
| }, | ||
| } | ||
| reg.Register(stub) | ||
|
|
||
| err := mountAndRunDrive(t, DriveMemberAdd, []string{ | ||
| "+member-add", | ||
| "--token", "wikcnBotTok", | ||
| "--member-id", "ou_bot_target", | ||
| "--member-type", "openid", | ||
| "--perm", "edit", | ||
| "--as", "bot", | ||
| "--yes", | ||
| }, f, stdout) | ||
| if err != nil { | ||
| t.Fatalf("unexpected error: %v", err) | ||
| } | ||
|
|
||
| // Bot identity should NOT send need_notification in query. | ||
| var captured map[string]interface{} | ||
| if err := json.Unmarshal(stub.CapturedBody, &captured); err != nil { | ||
| t.Fatalf("decode captured body: %v", err) | ||
| } | ||
| if captured["perm"] != "edit" { | ||
| t.Fatalf("captured body perm = %v, want edit", captured["perm"]) | ||
| } |
There was a problem hiding this comment.
Assert the bot query omission you describe.
This test says bot execution must not send need_notification, but it only checks the captured body. Capture req.URL.RawQuery and assert the parameter is absent so the execute-path contract is actually pinned.
Suggested test update
func TestDriveMemberAdd_ExecuteSuccessAsBot(t *testing.T) {
t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir())
f, stdout, _, reg := cmdutil.TestFactory(t, driveTestConfig())
+ var capturedQuery string
stub := &httpmock.Stub{
Method: "POST",
URL: "/open-apis/drive/v1/permissions/wikcnBotTok/members",
+ OnMatch: func(req *http.Request) {
+ capturedQuery = req.URL.RawQuery
+ },
Body: map[string]interface{}{
"code": 0,
"msg": "success",
"data": map[string]interface{}{
@@
if captured["perm"] != "edit" {
t.Fatalf("captured body perm = %v, want edit", captured["perm"])
}
+ if strings.Contains(capturedQuery, "need_notification=") {
+ t.Fatalf("captured query = %q, want need_notification omitted", capturedQuery)
+ }
data := decodeDriveEnvelope(t, stdout)🤖 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/drive/drive_member_add_test.go` around lines 673 - 711, The test
currently only asserts the POST body but must also assert the bot request
omitted the need_notification query param; update the test that uses the
httpmock.Stub (stub) in DriveMemberAdd/mountAndRunDrive to capture the incoming
request's URL.RawQuery (or use the stub's CapturedRequest/URL field) after
mountAndRunDrive runs, then assert that the raw query string does not contain
"need_notification" (or that parsed query has no "need_notification" key); keep
existing body assertions (captured["perm"] == "edit") and add a failing t.Fatalf
message if the query contains need_notification so the execute-path contract is
enforced.
Comment on lines
+414
to
+438
| // Flatten batch response: data.members is an array of member objects. | ||
| rawMembers, _ := data["members"].([]interface{}) | ||
| var results []map[string]interface{} | ||
| for i, raw := range rawMembers { | ||
| if m, ok := raw.(map[string]interface{}); ok { | ||
| memberID := "" | ||
| if i < len(spec.MemberIDs) { | ||
| memberID = spec.MemberIDs[i] | ||
| } | ||
| out := driveMemberAddOutput(spec, memberID, m) | ||
| results = append(results, out) | ||
| } | ||
| } | ||
| if len(results) == 0 { | ||
| // Fallback: API returned success but no member details; backfill from spec. | ||
| for _, mid := range spec.MemberIDs { | ||
| results = append(results, driveMemberAddOutput(spec, mid, nil)) | ||
| } | ||
| } | ||
| fmt.Fprintf(runtime.IO().ErrOut, "Added %d Drive member(s)\n", len(results)) | ||
| runtime.Out(map[string]interface{}{ | ||
| "resource_token": spec.Token, | ||
| "resource_type": spec.ResourceType, | ||
| "members": results, | ||
| }, nil) |
There was a problem hiding this comment.
Backfill partially omitted batch results instead of only the all-or-nothing case.
If the API returns a shorter members array, or one element is missing/not an object, this loop silently drops that requested member and prints a smaller success count. For a permission-grant command, that makes the final envelope misleading even though you already have the original request list locally. Consider iterating over spec.MemberIDs and backfilling each slot individually when rawMembers[i] is absent or malformed, rather than only falling back when len(results) == 0.
💡 Suggested fix
- rawMembers, _ := data["members"].([]interface{})
- var results []map[string]interface{}
- for i, raw := range rawMembers {
- if m, ok := raw.(map[string]interface{}); ok {
- memberID := ""
- if i < len(spec.MemberIDs) {
- memberID = spec.MemberIDs[i]
- }
- out := driveMemberAddOutput(spec, memberID, m)
- results = append(results, out)
- }
- }
- if len(results) == 0 {
- // Fallback: API returned success but no member details; backfill from spec.
- for _, mid := range spec.MemberIDs {
- results = append(results, driveMemberAddOutput(spec, mid, nil))
- }
- }
+ rawMembers, _ := data["members"].([]interface{})
+ results := make([]map[string]interface{}, 0, len(spec.MemberIDs))
+ for i, mid := range spec.MemberIDs {
+ var rawMember map[string]interface{}
+ if i < len(rawMembers) {
+ if m, ok := rawMembers[i].(map[string]interface{}); ok {
+ rawMember = m
+ }
+ }
+ results = append(results, driveMemberAddOutput(spec, mid, rawMember))
+ }📝 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.
Suggested change
| // Flatten batch response: data.members is an array of member objects. | |
| rawMembers, _ := data["members"].([]interface{}) | |
| var results []map[string]interface{} | |
| for i, raw := range rawMembers { | |
| if m, ok := raw.(map[string]interface{}); ok { | |
| memberID := "" | |
| if i < len(spec.MemberIDs) { | |
| memberID = spec.MemberIDs[i] | |
| } | |
| out := driveMemberAddOutput(spec, memberID, m) | |
| results = append(results, out) | |
| } | |
| } | |
| if len(results) == 0 { | |
| // Fallback: API returned success but no member details; backfill from spec. | |
| for _, mid := range spec.MemberIDs { | |
| results = append(results, driveMemberAddOutput(spec, mid, nil)) | |
| } | |
| } | |
| fmt.Fprintf(runtime.IO().ErrOut, "Added %d Drive member(s)\n", len(results)) | |
| runtime.Out(map[string]interface{}{ | |
| "resource_token": spec.Token, | |
| "resource_type": spec.ResourceType, | |
| "members": results, | |
| }, nil) | |
| // Flatten batch response: data.members is an array of member objects. | |
| rawMembers, _ := data["members"].([]interface{}) | |
| results := make([]map[string]interface{}, 0, len(spec.MemberIDs)) | |
| for i, mid := range spec.MemberIDs { | |
| var rawMember map[string]interface{} | |
| if i < len(rawMembers) { | |
| if m, ok := rawMembers[i].(map[string]interface{}); ok { | |
| rawMember = m | |
| } | |
| } | |
| results = append(results, driveMemberAddOutput(spec, mid, rawMember)) | |
| } | |
| fmt.Fprintf(runtime.IO().ErrOut, "Added %d Drive member(s)\n", len(results)) | |
| runtime.Out(map[string]interface{}{ | |
| "resource_token": spec.Token, | |
| "resource_type": spec.ResourceType, | |
| "members": results, | |
| }, nil) |
🤖 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/drive/drive_member_add.go` around lines 414 - 438, The loop
currently only appends members when rawMembers[i] is a valid map and falls back
to backfilling only if results is empty; change it to iterate over
spec.MemberIDs (by index) and for each index attempt to read rawMembers[i] as a
map[string]interface{}—if present use driveMemberAddOutput(spec, memberID, m),
otherwise call driveMemberAddOutput(spec, memberID, nil) so every requested
member is represented; ensure results is built in the same order as
spec.MemberIDs and that the final runtime.Out and the printed count reflect the
full spec.MemberIDs length.
Comment on lines
+259
to
+261
| > **注意**：此方式仅适用于需要授权给**当前应用**的场景。授权给其他用户时，直接使用对方的 open_id 即可，无需调用 bot info 接口。`drive +member-add` 会从可信飞书/Lark/豆包 URL 或裸 token 前缀推断 resource type；`--member-type` 必须显式传入，如果 ID 前缀与 member-type 不一致会报错。 | ||
|
|
||
| `<resource_type>` 可选值：`doc`、`docx`、`sheet`、`bitable`、`file`、`folder`、`wiki`、`slides`。 | ||
| > **补充**：如果你手里只有 app id，也可以把 `--member-type` 设为 `appid`；CLI 会在请求层兼容映射为 Drive API 的 `member_type=userid`。但在 skill 文档和实际操作里，仍然优先推荐使用 bot `open_id` + `--member-type openid`，因为这条路径语义更直接、和仓库里其他实现也更一致。 |
There was a problem hiding this comment.
Keep this blockquote contiguous.
The blank line here triggers markdownlint MD028 (no-blanks-blockquote). Remove the empty line or merge the two notes into one blockquote block.
Suggested markdown fix
> **注意**：此方式仅适用于需要授权给**当前应用**的场景。授权给其他用户时，直接使用对方的 open_id 即可，无需调用 bot info 接口。`drive +member-add` 会从可信飞书/Lark/豆包 URL 或裸 token 前缀推断 resource type；`--member-type` 必须显式传入，如果 ID 前缀与 member-type 不一致会报错。
-
> **补充**：如果你手里只有 app id，也可以把 `--member-type` 设为 `appid`；CLI 会在请求层兼容映射为 Drive API 的 `member_type=userid`。但在 skill 文档和实际操作里，仍然优先推荐使用 bot `open_id` + `--member-type openid`，因为这条路径语义更直接、和仓库里其他实现也更一致。📝 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.
Suggested change
| > **注意**：此方式仅适用于需要授权给**当前应用**的场景。授权给其他用户时，直接使用对方的 open_id 即可，无需调用 bot info 接口。`drive +member-add` 会从可信飞书/Lark/豆包 URL 或裸 token 前缀推断 resource type；`--member-type` 必须显式传入，如果 ID 前缀与 member-type 不一致会报错。 | |
| `<resource_type>` 可选值：`doc`、`docx`、`sheet`、`bitable`、`file`、`folder`、`wiki`、`slides`。 | |
| > **补充**：如果你手里只有 app id，也可以把 `--member-type` 设为 `appid`；CLI 会在请求层兼容映射为 Drive API 的 `member_type=userid`。但在 skill 文档和实际操作里，仍然优先推荐使用 bot `open_id` + `--member-type openid`，因为这条路径语义更直接、和仓库里其他实现也更一致。 | |
| > **注意**：此方式仅适用于需要授权给**当前应用**的场景。授权给其他用户时，直接使用对方的 open_id 即可，无需调用 bot info 接口。`drive +member-add` 会从可信飞书/Lark/豆包 URL 或裸 token 前缀推断 resource type；`--member-type` 必须显式传入，如果 ID 前缀与 member-type 不一致会报错。 | |
| > **补充**：如果你手里只有 app id，也可以把 `--member-type` 设为 `appid`；CLI 会在请求层兼容映射为 Drive API 的 `member_type=userid`。但在 skill 文档和实际操作里，仍然优先推荐使用 bot `open_id` + `--member-type openid`，因为这条路径语义更直接、和仓库里其他实现也更一致。 |
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 260-260: Blank line inside blockquote
(MD028, no-blanks-blockquote)
🤖 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-drive/SKILL.md` around lines 259 - 261, The two adjacent
blockquote paragraphs in SKILL.md need to be merged into one contiguous
blockquote to satisfy markdownlint MD028; remove the blank line between the two
blocks and combine their text into a single blockquote so the note about
member-type mapping and recommendation to use bot open_id appears as one
continuous block (look for the blockquote starting with
"注意：此方式仅适用于需要授权给**当前应用**的场景。" and the following "补充：" paragraph and join them).
Comment on lines
+319
to
+320
| result.AssertExitCode(t, 2) | ||
| require.Contains(t, result.Stderr, tt.wantErr, "stderr:\n%s", result.Stderr) |
There was a problem hiding this comment.
Read validate-stage dry-run errors from the full CLI output.
These cases are exercising Validate, so the failure message can come from the structured stdout envelope rather than stderr alone. Asserting only result.Stderr makes the test miss the intended dry-run contract.
Suggested assertion change
require.NoError(t, err)
result.AssertExitCode(t, 2)
- require.Contains(t, result.Stderr, tt.wantErr, "stderr:\n%s", result.Stderr)
+ output := result.Stdout + result.Stderr
+ require.Contains(t, output, tt.wantErr, "stdout:\n%s\nstderr:\n%s", result.Stdout, result.Stderr)
})
}
}Based on learnings: in tests/cli_e2e dry-run tests, Validate-stage failures should exit with code 2 and be verified from result.Stdout + result.Stderr, not stderr alone.
📝 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.
Suggested change
| result.AssertExitCode(t, 2) | |
| require.Contains(t, result.Stderr, tt.wantErr, "stderr:\n%s", result.Stderr) | |
| result.AssertExitCode(t, 2) | |
| output := result.Stdout + result.Stderr | |
| require.Contains(t, output, tt.wantErr, "stdout:\n%s\nstderr:\n%s", result.Stdout, result.Stderr) | |
| }) | |
| } | |
| } |
🤖 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 `@tests/cli_e2e/drive/drive_member_add_dryrun_test.go` around lines 319 - 320,
The test currently asserts the dry-run validation error only against
result.Stderr; update the assertion so Validate-stage failures are checked from
the full CLI output by combining result.Stdout and result.Stderr. Replace the
require.Contains call that uses result.Stderr with one that checks
result.Stdout+result.Stderr (keeping the existing result.AssertExitCode(t, 2)
and tt.wantErr), so the test verifies the error message in the structured stdout
envelope as well as stderr.
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.
Add drive +member-add shortcut that grants view/edit/full_access permissions to collaborators on Drive documents, files, folders, and wiki nodes. Supports single and batch member add (up to 10).
Key features:
Also extends shortcuts/common/resource_url with minutes type support and InferResourceTypeFromToken for bare token prefix mapping.
Summary
Changes
Test Plan
lark-cli <domain> <command>flow works as expectedRelated Issues
Summary by CodeRabbit
New Features
drive +member-addshortcut to add collaborators to Drive resources (docs, folders, spaces, wikis) with batch support, member type inference, and notification options.minutesresource type.Documentation
drive +member-addcommand with usage examples and parameter specifications.Tests
drive +member-addcommand validation, behavior, and end-to-end CLI scenarios.