feat(drive): extend permission shortcuts for Miaoda - #2070
Merged
Conversation
github-actions
Bot
added
domain/ccm
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughDrive permission shortcuts now support ChangesDrive resource targeting
Estimated code review effort: 3 (Moderate) | ~30 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@a95b7bf9684c8e9f615711fc23b091b99f62d5c7🧩 Skill updatenpx skills add larksuite/cli#feat/miaoda-drive-permission-openrest -y -g |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2070 +/- ##
==========================================
+ Coverage 75.19% 75.41% +0.22%
==========================================
Files 914 924 +10
Lines 96783 98188 +1405
==========================================
+ Hits 72778 74051 +1273
- Misses 18407 18498 +91
- Partials 5598 5639 +41 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (1)
shortcuts/drive/drive_public_permission.go (1)
24-38: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftConsider extracting the URL→type resolver into a shared helper.
This file,
drive_member_list.go, and (per the stack description)drive_apply_permission.go/drive_member_add.go/drive_secure_label.goeach define their own{Prefix, Type}table + parse loop. The conflict-check divergence found above is a direct symptom of this duplication making it easy for one copy to drift from another. A shared, parameterized resolver (allowed prefixes/types passed per shortcut) would reduce this class of inconsistency.Also applies to: 142-206
🤖 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_public_permission.go` around lines 24 - 38, Extract the duplicated URL-to-type prefix matching logic into a shared, parameterized resolver used by drive_public_permission.go, drive_member_list.go, drive_apply_permission.go, drive_member_add.go, and drive_secure_label.go. Pass each shortcut’s allowed prefix/type mappings to the helper, preserve their existing supported mappings and parsing behavior, and remove the local tables and parse loops so conflict checks cannot drift between implementations.
🤖 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_apply_permission.go`:
- Line 97: Update the token flag’s Desc in the permission command to include
“bitable” alongside the other supported token URL types, matching the parser’s
accepted /bitable/ format while preserving the existing description.
In `@shortcuts/drive/drive_permission_get_setting.go`:
- Around line 137-144: Update the resource URL mappings used by BuildResourceURL
and ParseResourceURL to support type "apps" with the /page/<token> path. Ensure
drivePermissionGetSettingSpec.url returns this fallback URL for bare apps
targets instead of an empty string, while preserving existing mappings for other
types.
In `@shortcuts/drive/drive_public_permission.go`:
- Around line 142-187: Update resolveDrivePublicPermissionTarget to reject a URL
when its inferred urlType conflicts with a non-empty explicitType, returning the
same typed validation-error behavior used by resolveDriveMemberListTarget; only
allow explicitType when it matches or no type was inferred. Update
TestResolveDrivePublicPermissionTarget_ExplicitTypeOverridesURL to assert
rejection of the conflicting input instead of override behavior.
In `@shortcuts/drive/drive_secure_label_test.go`:
- Around line 162-199: Extend TestResolveSecureLabelTarget_URLAndBareToken with
an apps-page URL case that expects resolveSecureLabelTarget to reject the input,
assert the returned validation error metadata is typed as intended, and verify
Param equals "--token". Keep the existing accepted URL and bare-token cases
unchanged.
In `@tests/cli_e2e/drive/coverage.md`:
- Around line 4-6: Update the coverage Metrics section in the command table to
reflect the table’s actual counts: use 37 command rows as the Denominator and 17
checked rows as Covered, then recalculate the percentage accordingly.
In `@tests/cli_e2e/drive/drive_apply_permission_dryrun_test.go`:
- Around line 112-138: Add live E2E coverage for the new apps resource behavior
in tests/cli_e2e/drive/drive_apply_permission_dryrun_test.go:112-138 and
tests/cli_e2e/drive/drive_member_add_dryrun_test.go:87-121. For
apply-permission, create a disposable apps-page resource, apply the permission
through the relevant test flow, and clean it up; for member-add, grant the
member permission on a disposable apps-page resource and remove it afterward.
Use bot credentials where required and keep each workflow self-contained, while
retaining the existing dry-run cases.
In `@tests/cli_e2e/drive/drive_permission_get_setting_test.go`:
- Around line 106-108: Update the assertion in the dry-run test to inspect the
nested request-parameter path under data.api.0.params, matching the path used by
the preceding assertions. Keep the existing failure behavior and output
diagnostic, but ensure it detects folder_token when it leaks into the request
params.
In `@tests/cli_e2e/drive/drive_public_permission_dryrun_test.go`:
- Around line 157-175: The helper requireDrivePublicPermissionValidationEnvelope
currently parses validation errors only from result.Stderr. Update its
assertions to support the JSON error envelope from the appropriate combined or
stdout-inclusive output while preserving stdout in failure diagnostics, so
dry-run validation failures do not depend on stderr-only parsing.
---
Nitpick comments:
In `@shortcuts/drive/drive_public_permission.go`:
- Around line 24-38: Extract the duplicated URL-to-type prefix matching logic
into a shared, parameterized resolver used by drive_public_permission.go,
drive_member_list.go, drive_apply_permission.go, drive_member_add.go, and
drive_secure_label.go. Pass each shortcut’s allowed prefix/type mappings to the
helper, preserve their existing supported mappings and parsing behavior, and
remove the local tables and parse loops so conflict checks cannot drift between
implementations.
🪄 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 Plus
Run ID: 80812435-3aba-4ccd-aaa5-454851280543
📒 Files selected for processing (30)
shortcuts/drive/drive_apply_permission.goshortcuts/drive/drive_apply_permission_test.goshortcuts/drive/drive_member_add.goshortcuts/drive/drive_member_add_test.goshortcuts/drive/drive_member_list.goshortcuts/drive/drive_member_list_test.goshortcuts/drive/drive_permission_get_setting.goshortcuts/drive/drive_permission_get_setting_test.goshortcuts/drive/drive_public_permission.goshortcuts/drive/drive_public_permission_test.goshortcuts/drive/drive_secure_label.goshortcuts/drive/drive_secure_label_test.goshortcuts/drive/shortcuts.goshortcuts/drive/shortcuts_test.goskills/lark-drive/SKILL.mdskills/lark-drive/references/lark-drive-apply-permission.mdskills/lark-drive/references/lark-drive-member-add.mdskills/lark-drive/references/lark-drive-member-list.mdskills/lark-drive/references/lark-drive-permission-get-setting.mdskills/lark-drive/references/lark-drive-public-permission-update.mdskills/lark-drive/references/lark-drive-secure-label.mdskills/lark-drive/references/lark-drive-workflow-permission-governance-commands.mdskills/lark-drive/references/lark-drive-workflow-permission-governance-outputs.mdskills/lark-drive/references/lark-drive-workflow-permission-governance.mdtests/cli_e2e/drive/coverage.mdtests/cli_e2e/drive/drive_apply_permission_dryrun_test.gotests/cli_e2e/drive/drive_member_add_dryrun_test.gotests/cli_e2e/drive/drive_member_list_test.gotests/cli_e2e/drive/drive_permission_get_setting_test.gotests/cli_e2e/drive/drive_public_permission_dryrun_test.go
zhaojiaxing-coding
force-pushed
the
feat/miaoda-drive-permission-openrest
branch
from
July 29, 2026 11:29
c84bd8a to
3439d6f
Compare
github-actions
Bot
added
size/M
zhaojiaxing-coding
force-pushed
the
feat/miaoda-drive-permission-openrest
branch
from
July 29, 2026 11:31
3439d6f to
7988515
Compare
Extend Drive permission shortcuts to accept Miaoda page URLs and the apps resource type while keeping each endpoint's accepted resource contract explicit. Key features: - Infer apps from /page/ URLs and accept explicit --type=apps in +apply-permission, +member-add, +member-list, and +permission-get-setting - Decouple secure-label target parsing so expanding apply-permission does not widen secure-label support - Align skill guidance and unit/dry-run coverage with the new resource type
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
shortcuts/drive/drive_member_list.go (1)
277-326: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftDecode the members response into typed structs at the API boundary.
The execution and pretty-render paths type-assert
map[string]interface{}values and silently render missing or malformeditemsas “no members” or-. That can turn an API/schema regression into a successful but incorrect result. Decode a typed response once, return a typed invalid-response error on shape mismatches, and retain the raw map separately for JSON passthrough.As per coding guidelines, parse
map[string]interface{}into typed structs at the boundary and use a projection function per response shape.🤖 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_list.go` around lines 277 - 326, Update the Drive member list API execution around CallAPITyped and renderDriveMemberListPretty to decode the raw response into typed response/member structs at the boundary, returning a typed invalid-response error when items or member fields have an unexpected shape instead of silently defaulting. Retain the raw map for OutFormat JSON passthrough, and add a projection function from the typed response to the pretty-render input so rendering no longer performs map assertions or uses driveMemberListValue for malformed data.Source: Coding guidelines
skills/lark-drive/SKILL.md (1)
30-30: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winMention
appsexplicitly in the primary shortcut index.The new
+member-listand+permission-get-settingentries omitapps, even though both shortcuts now accept/page/URLs and--type apps. Update these descriptions so users scanningSKILL.mdcan discover Miaoda support.Also applies to: 168-169
🤖 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` at line 30, 更新 SKILL.md 主快捷入口索引中的 +member-list 和 +permission-get-setting 描述，明确标注支持 Miaoda apps，包括 /page/ URL 与 --type apps。保留现有功能说明，并确保用户无需查看后续内容即可发现 apps 支持。
🧹 Nitpick comments (1)
shortcuts/drive/drive_permission_get_setting_test.go (1)
209-215: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
errors.Asto extractValidationError.The direct
err.(*errs.ValidationError)assertion is brittle if the validation error is wrapped later. Useerrors.Asbefore readingParam.Based on learnings,
Paramshould be read from*errs.ValidationErrorobtained witherrors.As;errs.ProblemOfexposes only problem-level fields.🤖 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_permission_get_setting_test.go` around lines 209 - 215, Update the validation error handling in the test to use errors.As to extract a *errs.ValidationError, allowing wrapped errors to be recognized before comparing its Param field with tt.wantParam. Keep the existing failure messages and type-validation behavior aligned with the extracted error.Source: Learnings
🤖 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.
Outside diff comments:
In `@shortcuts/drive/drive_member_list.go`:
- Around line 277-326: Update the Drive member list API execution around
CallAPITyped and renderDriveMemberListPretty to decode the raw response into
typed response/member structs at the boundary, returning a typed
invalid-response error when items or member fields have an unexpected shape
instead of silently defaulting. Retain the raw map for OutFormat JSON
passthrough, and add a projection function from the typed response to the
pretty-render input so rendering no longer performs map assertions or uses
driveMemberListValue for malformed data.
In `@skills/lark-drive/SKILL.md`:
- Line 30: 更新 SKILL.md 主快捷入口索引中的 +member-list 和 +permission-get-setting
描述，明确标注支持 Miaoda apps，包括 /page/ URL 与 --type apps。保留现有功能说明，并确保用户无需查看后续内容即可发现
apps 支持。
---
Nitpick comments:
In `@shortcuts/drive/drive_permission_get_setting_test.go`:
- Around line 209-215: Update the validation error handling in the test to use
errors.As to extract a *errs.ValidationError, allowing wrapped errors to be
recognized before comparing its Param field with tt.wantParam. Keep the existing
failure messages and type-validation behavior aligned with the extracted error.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: aa689105-f465-4cd0-a89d-1f0288965333
📒 Files selected for processing (20)
shortcuts/drive/drive_apply_permission.goshortcuts/drive/drive_apply_permission_test.goshortcuts/drive/drive_member_add.goshortcuts/drive/drive_member_add_test.goshortcuts/drive/drive_member_list.goshortcuts/drive/drive_member_list_test.goshortcuts/drive/drive_permission_get_setting.goshortcuts/drive/drive_permission_get_setting_test.goshortcuts/drive/drive_secure_label.goshortcuts/drive/drive_secure_label_test.goskills/lark-drive/SKILL.mdskills/lark-drive/references/lark-drive-apply-permission.mdskills/lark-drive/references/lark-drive-member-add.mdskills/lark-drive/references/lark-drive-member-list.mdskills/lark-drive/references/lark-drive-permission-get-setting.mdskills/lark-drive/references/lark-drive-workflow-permission-governance.mdtests/cli_e2e/drive/drive_apply_permission_dryrun_test.gotests/cli_e2e/drive/drive_member_add_dryrun_test.gotests/cli_e2e/drive/drive_member_list_test.gotests/cli_e2e/drive/drive_permission_get_setting_test.go
🚧 Files skipped from review as they are similar to previous changes (12)
- shortcuts/drive/drive_secure_label_test.go
- skills/lark-drive/references/lark-drive-member-add.md
- shortcuts/drive/drive_apply_permission_test.go
- tests/cli_e2e/drive/drive_apply_permission_dryrun_test.go
- shortcuts/drive/drive_member_add.go
- shortcuts/drive/drive_member_add_test.go
- shortcuts/drive/drive_apply_permission.go
- shortcuts/drive/drive_secure_label.go
- tests/cli_e2e/drive/drive_member_list_test.go
- shortcuts/drive/drive_permission_get_setting.go
- shortcuts/drive/drive_member_list_test.go
- tests/cli_e2e/drive/drive_member_add_dryrun_test.go
Add focused coverage for Miaoda apps target handling across apply-permission and secure-label boundaries. Exercise malformed page URLs, explicit apps bare tokens, typed validation errors, and command-level rejection so future resource-type changes cannot silently widen unsupported secure-label behavior.
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_apply_permission_test.go`:
- Around line 82-90: Update TestResolvePermApplyTarget_RejectsMalformedPageURL
to extract the returned error with errors.As into *errs.ValidationError, assert
its category and subtype, and verify the --token parameter directly on the
validation error. Use errs.ProblemOf for the problem metadata and assert the
wrapped cause when applicable, replacing the current message-substring check.
In `@shortcuts/drive/drive_secure_label_test.go`:
- Around line 269-313: Update TestDriveSecureLabelUpdate_RejectsAppsTargets by
separating the apps page URL resolver case from the explicit --type apps parser
case. For the resolver case, assert errs.ProblemOf metadata for category and
subtype, verify the returned *errs.ValidationError has Param set to "--token",
and check that the underlying cause is preserved; retain the message assertion
as needed. Keep the explicit enum-rejection case as a separate parser-error
assertion if typed metadata is unavailable.
🪄 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 Plus
Run ID: 4260560c-34a6-40e1-959a-625a14d8e3f3
📒 Files selected for processing (2)
shortcuts/drive/drive_apply_permission_test.goshortcuts/drive/drive_secure_label_test.go
Keep drive +apply-permission resource inference aligned with URL component boundaries. Parse and validate URL inputs before extracting tokens so query strings and fragments cannot redirect permission requests to a different resource. Key fixes: - Match document and apps markers only against the parsed URL path - Reject malformed URLs with a typed --token validation error - Cover /page/ markers found only in query strings or fragments
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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_apply_permission.go`:
- Around line 53-60: Update the --token parsing flow around url.Parse to detect
URL-like inputs by scheme/colon before the bare-token fallback, so values such
as https:/tenant.feishu.cn/page/appTok return the typed malformed-URL --token
validation error. Preserve bare-token handling for inputs without a
scheme/colon, and add a typed --token regression case covering this malformed
value.
🪄 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 Plus
Run ID: 29a0ed2f-0ca7-41cf-a427-cde7ba08e9fe
📒 Files selected for processing (2)
shortcuts/drive/drive_apply_permission.goshortcuts/drive/drive_apply_permission_test.go
caojie0621
previously approved these changes
Jul 30, 2026
Replace the concrete Miaoda page token with a representative pagcn placeholder. This keeps the token shape recognizable while avoiding exposure of a real resource identifier in the skill documentation.
Make Drive shortcut targets unambiguous before they reach read or write API paths. URL inputs now bind to a recognized root path and a single validated token segment, preventing encoded separators, dot segments, and type conflicts from silently changing the addressed resource. Key fixes: - Reject non-root URLs, dot/traversal tokens, and URL/type conflicts for secure-label and permission-apply writes - Keep permission-setting URL parsing and pretty output reversible for every supported command-local resource kind - Add unit and dry-run E2E regressions plus aligned permission-apply guidance
github-actions
Bot
added
size/L
SunPeiYang996
approved these changes
Jul 31, 2026
caojie0621
approved these changes
Jul 31, 2026
sang-neo03
added a commit
that referenced
this pull request
Jul 31, 2026
…-phase2 main brought behaviour, this branch brought renames, so every resolution keeps both. Nothing had to be given up. Two textual conflicts: - lint/domaincontract/scan.go: the package comment. Took main's wording, which describes the domain policy it just added and names no package, so this branch's edit — replacing a stale internal/core reference — needs nothing carried over. The behaviour-bearing line in the same file, resolverPath pointing at brand/brand.go, sits outside the conflict and is untouched. - shortcuts/drive/drive_io_test.go: the import block, resolved as the union of main's credential and validate with this branch's config and identity. Four more the merge resolved silently and wrongly, because #2070 added files naming a package this branch deletes. They compile only after rewriting: core.LarkBrand to brandpkg.Brand, core.BrandLark to brandpkg.Lark and core.ResolveOpenBaseURL to brand.ResolveOpenBaseURL, across drive_permission_get_setting.go, its test, and drive_io_test.go. The production file needs the brandpkg alias because a local variable there is already named brand. Diffing those files against main afterwards shows the import line and the symbol names as the only differences, so #2070's host selection is intact. Worth recording for the next merge: main's new drive test imports internal/credential, which shortcuts-runtime-gate denies and which this branch now also checks in test files. It passes only because internal/credential is one of the rule's TestExempt entries. A denied import outside that list would have forced a real choice.
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
4 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
Add Drive permission shortcuts and Miaoda apps resource support across permission reads, grants, applications, and public-setting updates. Keep secure-label target parsing independent so the apps contract does not widen secure-label behavior.
Changes
drive +member-list,drive +permission-get-setting, anddrive +public-permission-updatewith typed validation and structured output./page/URLs andtype=appsin+apply-permissionand+member-add.+secure-label-updateparsing from apply-permission while preserving its original eight resource types.Test Plan
lark-cli drivecommand flow works as expected.make build+permission-get-settingand+member-listwith a Miaoda apps target inboe_miaoda_drive.Related Issues
Summary by CodeRabbit
/page/URLs and explicit selection via--type apps(including bare-token usage).--tokenURL parsing/validation and refined user-facing wording (e.g., “resource owner”).--type apps, and dry-run request correctness.