feat(drive): add +copy shortcut - #2129
Conversation
|
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:
📝 WalkthroughWalkthroughAdds the ChangesDrive copy workflow
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Operator
participant DriveCopy
participant DriveAPI
Operator->>DriveCopy: Provide source, name, and folder
DriveCopy->>DriveCopy: Resolve and validate inputs
DriveCopy->>DriveAPI: POST files copy request
DriveAPI-->>DriveCopy: Return copied-file metadata
DriveCopy-->>Operator: Print copy status and resource URL
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
⚔️ Resolve merge conflicts 💡
🧪 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@cd41c59a97bb89631eac180cd9033875a68bd3a2🧩 Skill updatenpx skills add larksuite/cli#feat/drive-copy-shortcut -y -g |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/cli_e2e/drive/drive_copy_dryrun_test.go (1)
105-108: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the exact exit code for the Validate-stage rejection.
The test only checks
result.ExitCode == 0fails. Assertresult.ExitCode == 2directly. Based on learnings, when a shortcut'sValidatecallback rejects input, the CLI exits with code 2 and prints a structured JSON error envelope to stdout. Asserting the exact code makes this a stronger contract test and matches the established pattern used elsewhere in this test suite (e.g.,TestDrive_PullDryRunRejectsAbsoluteLocalDir).♻️ Proposed tightening of the exit-code assertion
- if result.ExitCode == 0 { + if result.ExitCode != 2 { t.Fatalf("wiki URL should be rejected with a redirect error\nstdout:\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_copy_dryrun_test.go` around lines 105 - 108, Update the exit-code assertion in the affected dry-run rejection test to require result.ExitCode == 2 directly, preserving the existing failure diagnostics and stdout/stderr checks.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.
Nitpick comments:
In `@tests/cli_e2e/drive/drive_copy_dryrun_test.go`:
- Around line 105-108: Update the exit-code assertion in the affected dry-run
rejection test to require result.ExitCode == 2 directly, preserving the existing
failure diagnostics and stdout/stderr checks.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a0e2c344-7685-4d93-a2ca-c7cd425f8c29
📒 Files selected for processing (9)
shortcuts/drive/drive_copy.goshortcuts/drive/drive_copy_test.goshortcuts/drive/shortcuts.goshortcuts/drive/shortcuts_test.goskills/lark-drive/SKILL.mdskills/lark-drive/references/lark-drive-copy.mdtests/cli_e2e/drive/coverage.mdtests/cli_e2e/drive/drive_copy_dryrun_test.gotests/cli_e2e/drive/drive_copy_workflow_test.go
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2129 +/- ##
==========================================
+ Coverage 76.13% 76.15% +0.01%
==========================================
Files 984 985 +1
Lines 103731 104006 +275
==========================================
+ Hits 78977 79206 +229
- Misses 18763 18800 +37
- Partials 5991 6000 +9 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
85c093b to
07b9b3a
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
shortcuts/drive/drive_copy_test.go (1)
30-61: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd a case for a matching
--typewith a URL input.The table covers a conflicting
--type(lines 80-86) but not a matching one. The flag description states that--typeis optional for URLs and must match the URL type. A case such as--url .../docx/docxCopySourcewith--type docx, and a case with--type baseagainst a/base/URL, would lock in the normalized-comparison path at line 190.🤖 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_copy_test.go` around lines 30 - 61, Add table-driven test cases in the existing drive copy input tests for URL inputs whose --type matches the URL type, including a /docx/ URL with docx and a /base/ URL with base; assert the expected token and normalized bitable type to cover the matching normalized-comparison path.shortcuts/drive/drive_copy.go (1)
25-25: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDerive the allowed-type text from one source.
The supported-type list is repeated in four places:
driveCopyTypes(line 25), the two error messages (lines 204 and 219, 225), and thedriveCopyTypeSupportedswitch (line 294). A future type addition must be applied in each place, and a missed edit produces a misleading error message. Build the message text from the supported set instead.♻️ Example consolidation
+var driveCopySupportedTypes = []string{"doc", "docx", "sheet", "file", "mindnote", "slides", "bitable"} + +// driveCopySupportedTypesText lists the accepted values, including the base alias. +func driveCopySupportedTypesText() string { + return strings.Join(append(driveCopySupportedTypes, "base"), ", ") +} + func driveCopyTypeSupported(docType string) bool { - switch normalizeDriveCopyType(docType) { - case "doc", "docx", "sheet", "file", "mindnote", "slides", "bitable": - return true - default: - return false - } + normalized := normalizeDriveCopyType(docType) + for _, allowed := range driveCopySupportedTypes { + if normalized == allowed { + return true + } + } + return false }Also applies to: 201-226, 292-299
🤖 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_copy.go` at line 25, Consolidate supported drive-copy types around the single source `driveCopyTypes`: update `driveCopyTypeSupported` to derive membership from that set and build both error messages in the affected validation paths from the same list, rather than repeating type names. Preserve the existing validation behavior and keep the displayed allowed-type text synchronized automatically when `driveCopyTypes` changes.
🤖 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 `@tests/cli_e2e/drive/drive_copy_dryrun_test.go`:
- Around line 139-152: The wiki URL redirect assertions should validate the
typed error’s exact exit code and inspect combined process output. In the
relevant test, replace the nonzero check with result.AssertExitCode(t, 2) (or an
equivalent exact assertion), define combined output from result.Stdout and
result.Stderr, and use it for all validation and guidance string checks.
In `@tests/cli_e2e/drive/drive_copy_workflow_test.go`:
- Around line 18-24: Add clie2e.SkipWithoutTenantAccessToken(t) at the start of
TestDrive_CopyWorkflow, before the createDriveFolder setup, so the workflow is
skipped when tenant/BOT credentials are unavailable.
---
Nitpick comments:
In `@shortcuts/drive/drive_copy_test.go`:
- Around line 30-61: Add table-driven test cases in the existing drive copy
input tests for URL inputs whose --type matches the URL type, including a /docx/
URL with docx and a /base/ URL with base; assert the expected token and
normalized bitable type to cover the matching normalized-comparison path.
In `@shortcuts/drive/drive_copy.go`:
- Line 25: Consolidate supported drive-copy types around the single source
`driveCopyTypes`: update `driveCopyTypeSupported` to derive membership from that
set and build both error messages in the affected validation paths from the same
list, rather than repeating type names. Preserve the existing validation
behavior and keep the displayed allowed-type text synchronized automatically
when `driveCopyTypes` changes.
🪄 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: 8d1d5be5-4615-445c-ae18-efabb82185f2
📒 Files selected for processing (9)
shortcuts/drive/drive_copy.goshortcuts/drive/drive_copy_test.goshortcuts/drive/shortcuts.goshortcuts/drive/shortcuts_test.goskills/lark-drive/SKILL.mdskills/lark-drive/references/lark-drive-copy.mdtests/cli_e2e/drive/coverage.mdtests/cli_e2e/drive/drive_copy_dryrun_test.gotests/cli_e2e/drive/drive_copy_workflow_test.go
🚧 Files skipped from review as they are similar to previous changes (4)
- shortcuts/drive/shortcuts_test.go
- shortcuts/drive/shortcuts.go
- tests/cli_e2e/drive/coverage.md
- skills/lark-drive/references/lark-drive-copy.md
098f3f3 to
4582331
Compare
wittam-01
left a comment
There was a problem hiding this comment.
4 个已验证的 P1 finding,待作者处理。
4582331 to
8b9ce33
Compare
0bdd97e to
147b79f
Compare
147b79f to
4f6cbcb
Compare
f2369a2 to
ef5ea57
Compare
Wrap the Drive file-copy endpoint as drive +copy. Accept a document URL (recommended) or bare token + --type for the source; the target takes a folder token, a folder URL, or the my_space constant, which resolves the caller's My Space root folder via the root-folder-meta endpoint (absent from platform metadata, works for both user and bot). Repeatable --extra key=value pairs are forwarded verbatim for special copy semantics (e.g. target_type=docx to convert a legacy doc during copy). Source and folder tokens are validated with validate.ResourceName before path interpolation. Reject wiki URLs/tokens with a typed validation error whose hint carries a wiki +node-copy command template using a fixed <node-token> placeholder, because a Drive copy of a wiki-backed document would land in Drive space instead of the wiki tree. In bot mode the CLI auto-grants the current CLI user full_access on the new copy (same behavior as +upload/+import), reporting the outcome in the permission_grant output field without failing the copy. Declare docs:document:copy (the narrowest scope in the endpoint's any-of set) plus a conditional drive:drive.metadata:readonly for my_space resolution. Cover the shortcut with unit tests, dry-run e2e and a self-contained live workflow (upload -> copy -> download-verify -> my_space copy -> cleanup), and register it in tests/cli_e2e/drive/coverage.md. Route copy intents in the lark-drive skill to the shortcut instead of the raw files copy service command.
ef5ea57 to
cd41c59
Compare
Wrap the Drive file-copy endpoint as drive +copy. Accept a document URL (recommended) or bare token + --type for the source; the target takes a folder token, a folder URL, or the my_space constant, which resolves the caller's My Space root folder via the root-folder-meta endpoint (absent from platform metadata, works for both user and bot). Repeatable --extra key=value pairs are forwarded verbatim for special copy semantics (e.g. target_type=docx to convert a legacy doc during copy). Source and folder tokens are validated with validate.ResourceName before path interpolation. Reject wiki URLs/tokens with a typed validation error whose hint carries a wiki +node-copy command template using a fixed <node-token> placeholder, because a Drive copy of a wiki-backed document would land in Drive space instead of the wiki tree. In bot mode the CLI auto-grants the current CLI user full_access on the new copy (same behavior as +upload/+import), reporting the outcome in the permission_grant output field without failing the copy. Declare docs:document:copy (the narrowest scope in the endpoint's any-of set) plus a conditional drive:drive.metadata:readonly for my_space resolution. Cover the shortcut with unit tests, dry-run e2e and a self-contained live workflow (upload -> copy -> download-verify -> my_space copy -> cleanup), and register it in tests/cli_e2e/drive/coverage.md. Route copy intents in the lark-drive skill to the shortcut instead of the raw files copy service command. Co-authored-by: TRAE CLI <traecli@bytedance.com>
Summary
Wrap the Drive file-copy endpoint as drive +copy. Accept a document or Wiki URL, or a bare token with --type; Wiki sources are automatically resolved to their underlying Drive resource. Support folder tokens/URLs, my_space, all Drive-copy resource types, and repeatable --extra key=value parameters.
Declare the required copy, Wiki resolution, Drive metadata, and bot permission scopes. Add unit, dry-run E2E, and self-contained live workflow coverage, and route copy guidance in the lark-drive skill to the shortcut.
Test Plan
lark-cli <domain> <command>flow works as expectedRelated Issues
Summary by CodeRabbit
New Features
drive +copycommand for copying Drive documents and folders.Documentation
Tests