feat(drive): add +folder-permission-get shortcut#1738
Open
zhaojiaxing-coding wants to merge 1 commit into
Open
feat(drive): add +folder-permission-get shortcut#1738zhaojiaxing-coding wants to merge 1 commit into
zhaojiaxing-coding wants to merge 1 commit into
Conversation
Add a Drive shortcut for reading a folder's own public permission settings through the v2 permission endpoint. This gives agents a typed, folder-specific path when raw permission.public get does not accept folder targets, without turning folder permission checks into recursive governance scans. Key features: - Accept exactly one folder locator through --url or --folder-token and validate non-folder inputs before API calls - Return permission_public unchanged so callers can reason from server-provided fields - Register the shortcut and cover unit plus dry-run E2E behavior - Document when to use +folder-permission-get in lark-drive permission workflows
github-actions
Bot
added
domain/ccm
📝 WalkthroughWalkthroughAdds a new ChangesFolder Permission Get Shortcut
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant CLI as lark-cli
participant Spec as readDriveFolderPermissionGetSpec
participant API as Open API
User->>CLI: drive +folder-permission-get --url/--folder-token
CLI->>Spec: parse and validate input
Spec-->>CLI: FolderToken
alt dry-run
CLI-->>User: print GET request description
else execute
CLI->>API: GET folder permission_public
API-->>CLI: permission_public data
CLI-->>User: formatted output (masked token)
end
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
PR Quality SummaryCI did not complete successfully. Use the failed check links below to decide whether this PR needs a code change or a rerun. Failed checks
deterministic-gate
|
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@963d8b3c2494a88f83812bc7067680e7b1a2d771🧩 Skill updatenpx skills add larksuite/cli#feat/drive-folder-permission-get -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: 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/drive/drive_folder_permission_get.go`:
- Around line 21-64: The validation errors in readDriveFolderPermissionGetSpec
mix the failing input with recovery guidance in the message, which should be
moved to .WithHint(...). Update the checks around rawURL/rawToken, the
unsupported URL case, and the non-folder ref.Type branch so the error message
only states what failed and .WithHint(...) carries guidance like which flag to
pass or what folder URL format is accepted; keep the param on the actual failing
input such as --url or --folder-token.
- Around line 87-95: The driveFolderPermissionGetSpec.output method is falling
back to the entire data map when permission_public is missing, which reshapes
the output unexpectedly; update output to return only the nested
permission_public object (or an empty/absent value) instead of data, and keep
the behavior localized to driveFolderPermissionGetSpec.output and common.GetMap
handling. Add a test covering the missing permission_public case to verify the
response does not expose unrelated top-level fields.
🪄 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: 57f56f6c-bd78-4676-8c24-5648173b585e
📒 Files selected for processing (9)
shortcuts/drive/drive_folder_permission_get.goshortcuts/drive/drive_folder_permission_get_test.goshortcuts/drive/shortcuts.goshortcuts/drive/shortcuts_test.goskills/lark-drive/SKILL.mdskills/lark-drive/references/lark-drive-folder-permission-get.mdskills/lark-drive/references/lark-drive-workflow-permission-governance-commands.mdskills/lark-drive/references/lark-drive-workflow-permission-governance.mdtests/cli_e2e/drive/drive_folder_permission_get_dryrun_test.go
Comment on lines
+21
to
+64
| func readDriveFolderPermissionGetSpec(runtime *common.RuntimeContext) (driveFolderPermissionGetSpec, error) { | ||
| rawURL := strings.TrimSpace(runtime.Str("url")) | ||
| rawToken := strings.TrimSpace(runtime.Str("folder-token")) | ||
|
|
||
| if rawURL == "" && rawToken == "" { | ||
| return driveFolderPermissionGetSpec{}, errs.NewValidationError( | ||
| errs.SubtypeInvalidArgument, | ||
| "pass exactly one of --url or --folder-token", | ||
| ).WithParam("--url") | ||
| } | ||
| if rawURL != "" && rawToken != "" { | ||
| return driveFolderPermissionGetSpec{}, errs.NewValidationError( | ||
| errs.SubtypeInvalidArgument, | ||
| "--url and --folder-token are mutually exclusive; pass only one folder locator", | ||
| ).WithParam("--url") | ||
| } | ||
|
|
||
| if rawToken != "" { | ||
| if err := validate.ResourceName(rawToken, "--folder-token"); err != nil { | ||
| return driveFolderPermissionGetSpec{}, errs.NewValidationError(errs.SubtypeInvalidArgument, "%s", err).WithParam("--folder-token") | ||
| } | ||
| return driveFolderPermissionGetSpec{FolderToken: rawToken}, nil | ||
| } | ||
|
|
||
| ref, ok := common.ParseResourceURL(rawURL) | ||
| if !ok { | ||
| return driveFolderPermissionGetSpec{}, errs.NewValidationError( | ||
| errs.SubtypeInvalidArgument, | ||
| "unsupported --url %q: pass a recognized Lark Drive folder URL such as https://example.feishu.cn/drive/folder/<folder_token>", | ||
| rawURL, | ||
| ).WithParam("--url") | ||
| } | ||
| if ref.Type != "folder" { | ||
| return driveFolderPermissionGetSpec{}, errs.NewValidationError( | ||
| errs.SubtypeInvalidArgument, | ||
| "--url must point to a Drive folder; got resource type %q", | ||
| ref.Type, | ||
| ).WithParam("--url") | ||
| } | ||
| if err := validate.ResourceName(ref.Token, "--url"); err != nil { | ||
| return driveFolderPermissionGetSpec{}, errs.NewValidationError(errs.SubtypeInvalidArgument, "%s", err).WithParam("--url") | ||
| } | ||
| return driveFolderPermissionGetSpec{FolderToken: ref.Token}, nil | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Move recovery guidance into .WithHint(...).
Several validation errors embed recovery guidance directly in the message instead of using .WithHint(...) (e.g., "pass exactly one of --url or --folder-token", "pass a recognized Lark Drive folder URL such as..."). Per coding guidelines, the message/param should identify what failed, and actionable guidance belongs in .WithHint(...).
♻️ Example fix
if rawURL == "" && rawToken == "" {
return driveFolderPermissionGetSpec{}, errs.NewValidationError(
errs.SubtypeInvalidArgument,
- "pass exactly one of --url or --folder-token",
- ).WithParam("--url")
+ "missing folder locator",
+ ).WithParam("--url").WithHint("pass exactly one of --url or --folder-token")
}Apply similarly to the mutually-exclusive and unsupported/non-folder URL errors.
As per coding guidelines, "param field in errors should only name the user input that actually failed; recovery guidance goes in .WithHint(...)".
🤖 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_folder_permission_get.go` around lines 21 - 64, The
validation errors in readDriveFolderPermissionGetSpec mix the failing input with
recovery guidance in the message, which should be moved to .WithHint(...).
Update the checks around rawURL/rawToken, the unsupported URL case, and the
non-folder ref.Type branch so the error message only states what failed and
.WithHint(...) carries guidance like which flag to pass or what folder URL
format is accepted; keep the param on the actual failing input such as --url or
--folder-token.
Source: Coding guidelines
Comment on lines
+87
to
+95
| func (s driveFolderPermissionGetSpec) output(runtime *common.RuntimeContext, data map[string]interface{}) map[string]interface{} { | ||
| permissionPublic := interface{}(data) | ||
| if nestedPermissionPublic := common.GetMap(data, "permission_public"); nestedPermissionPublic != nil { | ||
| permissionPublic = nestedPermissionPublic | ||
| } | ||
| return map[string]interface{}{ | ||
| "permission_public": permissionPublic, | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- file: shortcuts/drive/drive_folder_permission_get.go ---'
cat -n shortcuts/drive/drive_folder_permission_get.go | sed -n '1,180p'
printf '\n%s\n' '--- search for permission_public usage ---'
rg -n "permission_public|driveFolderPermissionGetSpec|output\\(" shortcuts/drive -S
printf '\n%s\n' '--- relevant tests ---'
cat -n shortcuts/drive/drive_folder_permission_get_test.go | sed -n '1,260p'Repository: larksuite/cli
Length of output: 17452
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- common.GetMap ---'
rg -n "func GetMap|GetMap\\(" shortcuts internal -S
printf '\n%s\n' '--- similar output() patterns in shortcuts ---'
rg -n "permission_public|interface{}\\(data\\)|nested.*GetMap" shortcuts -S
printf '\n%s\n' '--- drive permission-related shortcuts ---'
fd -a 'drive_*permission*go' shortcuts
printf '\n%s\n' '--- read any closely related implementations ---'
for f in \
shortcuts/drive/drive_folder_permission_get.go \
shortcuts/drive/drive_folder_permission_list.go \
shortcuts/drive/drive_file_permission_get.go \
shortcuts/drive/drive_file_permission_list.go
do
[ -f "$f" ] && { echo "### $f"; cat -n "$f" | sed -n '1,220p'; }
doneRepository: larksuite/cli
Length of output: 7948
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- shortcuts/common/extract.go ---'
cat -n shortcuts/common/extract.go | sed -n '1,140p'
printf '\n%s\n' '--- drive/doc/app output patterns ---'
for f in \
shortcuts/doc/doc_resource_cover.go \
shortcuts/apps/apps_openapi_key_get.go \
shortcuts/apps/apps_openapi_key_create.go \
shortcuts/drive/drive_member_add.go
do
if [ -f "$f" ]; then
echo "### $f"
cat -n "$f" | sed -n '1,220p'
echo
fi
done
printf '\n%s\n' '--- search for "return map[string]interface{}{\"permission_public\": data}"-style fallbacks ---'
rg -n "return map\\[string\\]interface\\{\\}.*data|permission_public" shortcuts -U -SRepository: larksuite/cli
Length of output: 32891
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- search for drive permission public API docs / response shape ---'
rg -n "permission_public|permissions/.*/public|drive/v2/permissions" . -S
printf '\n%s\n' '--- inspect any docs or comments around the shortcut ---'
for f in \
shortcuts/drive/drive_folder_permission_get.go \
shortcuts/drive/drive_folder_permission_get_test.go \
README.md
do
if [ -f "$f" ]; then
echo "### $f"
cat -n "$f" | sed -n '1,260p'
echo
fi
doneRepository: larksuite/cli
Length of output: 33248
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- CallAPITyped implementation and callers ---'
rg -n "func .*CallAPITyped|CallAPITyped\\(" shortcuts internal errs -S
printf '\n%s\n' '--- RuntimeContext / typed API response handling ---'
for f in \
shortcuts/common/runtime.go \
shortcuts/common/runtime_test.go \
internal/httpmock/*.go
do
[ -f "$f" ] && { echo "### $f"; cat -n "$f" | sed -n '1,260p'; echo; }
doneRepository: larksuite/cli
Length of output: 47313
Avoid falling back to the full response map
If permission_public is missing, this should not return the entire data payload as its value. That reshapes the output, can surface unrelated top-level fields, and hides a schema mismatch; return only the nested object (or fail/leave it empty) instead. Add a test for the missing-key case.
🤖 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_folder_permission_get.go` around lines 87 - 95, The
driveFolderPermissionGetSpec.output method is falling back to the entire data
map when permission_public is missing, which reshapes the output unexpectedly;
update output to return only the nested permission_public object (or an
empty/absent value) instead of data, and keep the behavior localized to
driveFolderPermissionGetSpec.output and common.GetMap handling. Add a test
covering the missing permission_public case to verify the response does not
expose unrelated top-level fields.
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
1 participant
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 a Drive shortcut for reading a folder's own public permission settings through the v2 permission endpoint. This gives agents a typed, folder-specific path when raw permission.public get does not accept folder targets, without turning folder permission checks into recursive governance scans.
Key features:
Accept exactly one folder locator through --url or --folder-token and validate non-folder inputs before API calls
Return permission_public unchanged so callers can reason from server-provided fields
Register the shortcut and cover unit plus dry-run E2E behavior
Document when to use +folder-permission-get in lark-drive permission workflows
Summary
Changes
Test Plan
lark-cli <domain> <command>flow works as expectedRelated Issues
Summary by CodeRabbit
New Features
Bug Fixes
Documentation