feat: auto-grant current user access for bot-created docs, sheets, imports, and uploads#360
Conversation
📝 WalkthroughWalkthroughThis PR adds a bot-only automatic Drive permission-granting flow and integrates it across shortcuts (docs create, drive upload/import, sheets create). It introduces Changes
Sequence Diagram(s)sequenceDiagram
actor CLI
participant Runtime as RuntimeContext
participant Shortcut as Shortcut (upload/import/create)
participant DriveAPI as Drive Permissions API
CLI->>Runtime: run shortcut (--as bot)
Shortcut->>Runtime: perform operation (upload/import/create)
Shortcut-->>Runtime: receive result with token/type
alt runtime.IsBot() && token != ""
Shortcut->>Runtime: Runtime.UserOpenId()
alt userOpenId present
Shortcut->>DriveAPI: POST /open-apis/drive/v1/permissions/{token}/members {member_type: "openid", member_id: userOpenId, perm: "full_access", type: "user", perm_type: ...}
DriveAPI-->>Shortcut: 200 OK / error
Shortcut-->>CLI: include permission_grant {status: "granted"/"failed", user_open_id, message, perm}
else missing open_id
Shortcut-->>CLI: include permission_grant {status: "skipped", perm: "full_access", message}
end
else
Shortcut-->>CLI: no permission_grant included
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Greptile SummaryThis PR adds auto-grant of Confidence Score: 5/5Safe to merge — all remaining findings are P2 style/clarity suggestions with no correctness impact. The shared helper correctly gates on IsBot(), handles all three outcome states, and never lets a grant failure bubble up to the caller. Tests cover granted, skipped, and failed paths across all affected commands. Both P2 findings (misleading skip message wording and missing DocType fallback comment in +task_result) are polish items that don't affect functional correctness. shortcuts/common/permission_grant.go (skip message wording) and shortcuts/drive/drive_task_result.go (missing DocType fallback note)
|
| Filename | Overview |
|---|---|
| shortcuts/common/permission_grant.go | New shared helper for auto-granting Drive permissions; handles all skipped/failed/granted outcomes, but the "missing token/type" skip message fires even when only resourceType is empty (token is present), which can be misleading in the +task_result scenario. |
| shortcuts/doc/docs_create.go | Adds augmentDocsCreateResult that selects the permission target from doc_url (with wiki/docx type detection) or falls back to doc_id with type "docx"; grant failure does not fail the create operation. |
| shortcuts/drive/drive_import.go | Auto-grant wired correctly after the polling loop resolves ready=true; uses spec.DocType fallback when status.DocType is empty, unlike the standalone +task_result path. |
| shortcuts/drive/drive_task_result.go | Renames queryImportTask to queryImportTaskAndAutoGrantPermission and appends grant result when status.Ready(); has no fallback for empty status.DocType unlike drive_import.go, causing a silent skip with a misleading message if the API omits the type. |
| shortcuts/drive/drive_upload.go | Auto-grant appended after successful upload with resourceType="file"; clean and consistent with other commands. |
| shortcuts/sheets/sheet_create.go | Auto-grant appended with resourceType="sheet" after successful spreadsheet creation; grant failure does not fail the create; missing a test for the grant-fails-but-create-succeeds scenario. |
| shortcuts/drive/drive_permission_grant_test.go | Covers granted, skipped (user mode), and import auto-grant success; missing a test for the sheet/upload grant-fails-but-command-succeeds scenario. |
| shortcuts/drive/drive_task_result_test.go | New test TestDriveTaskResultImportBotAutoGrantSuccess covers the ready-state grant path; no-grant assertions added to the existing processing-state test. |
| shortcuts/doc/docs_create_test.go | Comprehensive: covers granted (docx URL), skipped (no user open_id), user-mode skip, and failed-grant-does-not-fail-create (wiki with perm_type=container check). |
| shortcuts/sheets/sheet_create_test.go | Covers granted and user-mode skip; missing a failure scenario test analogous to TestDocsCreateBotAutoGrantFailureDoesNotFailCreate. |
| shortcuts/drive/drive_import_common_test.go | Adds permission_grant absence assertion to the existing TestDriveImportTimeoutReturnsFollowUpCommand test; no new failures introduced. |
Sequence Diagram
sequenceDiagram
participant CLI
participant Lark API
participant Drive Permissions API
Note over CLI: --as bot mode only
CLI->>Lark API: Create resource (doc/sheet/upload/import)
Lark API-->>CLI: resource token + type
alt token && type present
CLI->>CLI: AutoGrantCurrentUserDrivePermission(token, type)
CLI->>CLI: Fetch UserOpenId from config
alt UserOpenId empty
CLI-->>CLI: permission_grant = {status: skipped}
else UserOpenId present
CLI->>Drive Permissions API: POST /drive/v1/permissions/{token}/members
alt API success
Drive Permissions API-->>CLI: 200 OK
CLI-->>CLI: permission_grant = {status: granted}
else API error
Drive Permissions API-->>CLI: error
CLI-->>CLI: permission_grant = {status: failed}
end
end
else token or type missing
CLI-->>CLI: permission_grant = {status: skipped}
end
CLI-->>CLI: runtime.Out(result with permission_grant)
Reviews (2): Last reviewed commit: "feat: auto-grant current user access for..." | Re-trigger Greptile
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@3961e37d8c9f2849a7e444322cffecff5af14121🧩 Skill updatenpx skills add larksuite/cli#feat/bot-auto-grant-current-user-access -y -g |
There was a problem hiding this comment.
🧹 Nitpick comments (3)
shortcuts/drive/drive_permission_grant_test.go (2)
232-234: Dead code:registerDriveBotTokenStubis a no-op.This function accepts a registry but doesn't register any stubs. It's called in all three tests but has no effect. Either implement the intended stub registration or remove the function and its call sites.
🔧 Remove unused placeholder
-func registerDriveBotTokenStub(reg *httpmock.Registry) { - _ = reg -}And remove the calls from the test functions:
func TestDriveUploadBotAutoGrantSuccess(t *testing.T) { f, stdout, _, reg := cmdutil.TestFactory(t, drivePermissionGrantTestConfig(t, "ou_current_user")) - registerDriveBotTokenStub(reg) reg.Register(&httpmock.Stub{🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/drive/drive_permission_grant_test.go` around lines 232 - 234, The helper registerDriveBotTokenStub(reg *httpmock.Registry) is a no-op and should be removed or implemented; pick one: either implement the intended HTTP stub registration inside registerDriveBotTokenStub (use the reg to add the expected token endpoints used by the tests) or delete the function and remove all calls to registerDriveBotTokenStub from the three test functions so there are no unused placeholders; update any imports or test setup accordingly and ensure tests still register any needed stubs directly if you remove the helper.
19-76: Consider addingt.Parallel()for test isolation.The other test files in this PR (
docs_create_test.go,sheet_create_test.go) uset.Parallel(). This test useswithDriveWorkingDirto change the working directory, which may prevent parallelization. If these tests cannot run in parallel due to working directory changes, consider documenting why, or refactor to avoid global state mutation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/drive/drive_permission_grant_test.go` around lines 19 - 76, Test TestDriveUploadBotAutoGrantSuccess mutates global working directory via withDriveWorkingDir which prevents safe parallel execution; either mark the test parallel by adding t.Parallel() at the top of TestDriveUploadBotAutoGrantSuccess and ensure withDriveWorkingDir properly isolates/restores cwd, or refactor to avoid global state (e.g., change withDriveWorkingDir to return a scoped working directory helper that chdirs only for the test and restores on cleanup, or use a per-test mutex around os.Chdir). Update the TestDriveUploadBotAutoGrantSuccess function and the withDriveWorkingDir helper (or introduce a scoped helper) to guarantee cwd is restored so t.Parallel() can be safely added.shortcuts/sheets/sheet_create_test.go (1)
79-111: Consider adding tests for skipped and failed grant scenarios.The
docs_create_test.goincludes tests for:
TestDocsCreateBotAutoGrantSkippedWithoutCurrentUserTestDocsCreateBotAutoGrantFailureDoesNotFailCreateAdding similar tests for sheets would ensure consistent coverage across all resource types.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/sheets/sheet_create_test.go` around lines 79 - 111, Add two tests mirroring the docs tests: one to verify auto-grant is skipped when there is no current user and one to verify a permission-grant failure does not cause sheet creation to fail. Create tests named similarly to TestSheetCreateBotAutoGrantSkippedWithoutCurrentUser and TestSheetCreateBotAutoGrantFailureDoesNotFailCreate that reuse runSheetCreateShortcut, decodeSheetCreateEnvelope and the HTTP stubbing approach (reg.Register) used in TestSheetCreateUserSkipsPermissionGrantAugmentation; for the skipped case stub the grant endpoint to not be called or return an expected "skipped" response and assert permission_grant is absent, and for the failure case stub the grant endpoint to return an error payload but assert the create succeeds (no returned error) and the output still contains the new spreadsheet info.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@shortcuts/drive/drive_permission_grant_test.go`:
- Around line 232-234: The helper registerDriveBotTokenStub(reg
*httpmock.Registry) is a no-op and should be removed or implemented; pick one:
either implement the intended HTTP stub registration inside
registerDriveBotTokenStub (use the reg to add the expected token endpoints used
by the tests) or delete the function and remove all calls to
registerDriveBotTokenStub from the three test functions so there are no unused
placeholders; update any imports or test setup accordingly and ensure tests
still register any needed stubs directly if you remove the helper.
- Around line 19-76: Test TestDriveUploadBotAutoGrantSuccess mutates global
working directory via withDriveWorkingDir which prevents safe parallel
execution; either mark the test parallel by adding t.Parallel() at the top of
TestDriveUploadBotAutoGrantSuccess and ensure withDriveWorkingDir properly
isolates/restores cwd, or refactor to avoid global state (e.g., change
withDriveWorkingDir to return a scoped working directory helper that chdirs only
for the test and restores on cleanup, or use a per-test mutex around os.Chdir).
Update the TestDriveUploadBotAutoGrantSuccess function and the
withDriveWorkingDir helper (or introduce a scoped helper) to guarantee cwd is
restored so t.Parallel() can be safely added.
In `@shortcuts/sheets/sheet_create_test.go`:
- Around line 79-111: Add two tests mirroring the docs tests: one to verify
auto-grant is skipped when there is no current user and one to verify a
permission-grant failure does not cause sheet creation to fail. Create tests
named similarly to TestSheetCreateBotAutoGrantSkippedWithoutCurrentUser and
TestSheetCreateBotAutoGrantFailureDoesNotFailCreate that reuse
runSheetCreateShortcut, decodeSheetCreateEnvelope and the HTTP stubbing approach
(reg.Register) used in TestSheetCreateUserSkipsPermissionGrantAugmentation; for
the skipped case stub the grant endpoint to not be called or return an expected
"skipped" response and assert permission_grant is absent, and for the failure
case stub the grant endpoint to return an error payload but assert the create
succeeds (no returned error) and the output still contains the new spreadsheet
info.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0b8d76c4-d989-4041-b309-3bba8d810457
📒 Files selected for processing (16)
shortcuts/common/permission_grant.goshortcuts/doc/docs_create.goshortcuts/doc/docs_create_test.goshortcuts/drive/drive_import.goshortcuts/drive/drive_import_common_test.goshortcuts/drive/drive_permission_grant_test.goshortcuts/drive/drive_task_result.goshortcuts/drive/drive_task_result_test.goshortcuts/drive/drive_upload.goshortcuts/sheets/sheet_create.goshortcuts/sheets/sheet_create_test.goskills/lark-doc/references/lark-doc-create.mdskills/lark-drive/references/lark-drive-import.mdskills/lark-drive/references/lark-drive-task-result.mdskills/lark-drive/references/lark-drive-upload.mdskills/lark-sheets/references/lark-sheets-create.md
…ports, and uploads Change-Id: Idf5b35dbf77d72788895e0a3c34563281d658c88
3d86395 to
3961e37
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
shortcuts/doc/docs_create_test.go (1)
65-67: Prefer semantic assertion over exact full message match.This assertion is brittle to non-functional copy changes. Consider validating stable substrings (like you already do in the failure case).
♻️ Suggested assertion tweak
- if grant["message"] != "Granted the current CLI user full_access (可管理权限) on the new document." { - t.Fatalf("permission_grant.message = %#v", grant["message"]) - } + msg, _ := grant["message"].(string) + if !strings.Contains(msg, "Granted the current CLI user") || + !strings.Contains(msg, "full_access (可管理权限)") { + t.Fatalf("permission_grant.message = %q", msg) + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/doc/docs_create_test.go` around lines 65 - 67, The test currently asserts exact equality on grant["message"], which is brittle; instead check for stable substrings (e.g., that grant["message"] contains "Granted the current CLI user" and/or "full_access" or "可管理权限") using a contains/assert approach and adjust the t.Fatalf branch accordingly; update the test around grant["message"] (and add strings import if needed) so the assertion verifies semantic content rather than an exact full-message match.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@shortcuts/doc/docs_create_test.go`:
- Around line 65-67: The test currently asserts exact equality on
grant["message"], which is brittle; instead check for stable substrings (e.g.,
that grant["message"] contains "Granted the current CLI user" and/or
"full_access" or "可管理权限") using a contains/assert approach and adjust the
t.Fatalf branch accordingly; update the test around grant["message"] (and add
strings import if needed) so the assertion verifies semantic content rather than
an exact full-message match.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: dfc87bfd-7dd9-4ed0-9112-4acba5c5530a
📒 Files selected for processing (16)
shortcuts/common/permission_grant.goshortcuts/doc/docs_create.goshortcuts/doc/docs_create_test.goshortcuts/drive/drive_import.goshortcuts/drive/drive_import_common_test.goshortcuts/drive/drive_permission_grant_test.goshortcuts/drive/drive_task_result.goshortcuts/drive/drive_task_result_test.goshortcuts/drive/drive_upload.goshortcuts/sheets/sheet_create.goshortcuts/sheets/sheet_create_test.goskills/lark-doc/references/lark-doc-create.mdskills/lark-drive/references/lark-drive-import.mdskills/lark-drive/references/lark-drive-task-result.mdskills/lark-drive/references/lark-drive-upload.mdskills/lark-sheets/references/lark-sheets-create.md
✅ Files skipped from review due to trivial changes (1)
- shortcuts/sheets/sheet_create_test.go
🚧 Files skipped from review as they are similar to previous changes (5)
- shortcuts/drive/drive_import.go
- skills/lark-drive/references/lark-drive-task-result.md
- shortcuts/drive/drive_upload.go
- shortcuts/sheets/sheet_create.go
- shortcuts/drive/drive_permission_grant_test.go
Change-Id: Idf5b35dbf77d72788895e0a3c34563281d658c88
Summary
This PR improves the bot-identity resource creation flow by automatically granting the current CLI user
full_accesson newly created docs, sheets, uploaded files, and imported resources when possible. It also makes the grantresult explicit in command output and aligns the related skill/docs wording with the
full_access (可管理权限)terminology.Changes
permission_grantdocs +create,sheets +create,drive +upload,drive +import, anddrive +task_result --scenario importfull_accessas可管理权限Test Plan
lark xxxcommand works as expectedManual verification included:
./lark-cli docs +create --as bot./lark-cli sheets +create --as bot./lark-cli drive +import --as bot --file ./2.csv --type sheet./lark-cli drive +import --as bot --file ./big.xlsx --type bitableRelated Issues
Summary by CodeRabbit
New Features
Documentation
Tests