feat(drive): add +copy shortcut - #2129
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:
📝 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@f3d0a65a847a3932a614d044247ead780a9d17f2🧩 Skill updatenpx skills add larksuite/cli#feat/drive-copy-shortcut -y -g |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 75.39% 75.43% +0.04%
==========================================
Files 924 925 +1
Lines 98005 98181 +176
==========================================
+ Hits 73895 74067 +172
- Misses 18478 18480 +2
- Partials 5632 5634 +2 ☔ 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
61fc386 to
098f3f3
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). Reject wiki URLs/tokens with a typed validation error whose hint carries a ready-to-adapt wiki +node-copy command, 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.
098f3f3 to
4582331
Compare
wittam-01
left a comment
wittam-01
left a comment
There was a problem hiding this comment.
4 个已验证的 P1 finding，待作者处理。
|
|
||
| data, err := runtime.CallAPITyped( | ||
| "POST", | ||
| fmt.Sprintf("/open-apis/drive/v1/files/%s/copy", validate.EncodePathSegment(spec.Ref.Token)), |
There was a problem hiding this comment.
[P1] 校验来源 token，避免改变请求路径。 --token .. --type docx 会通过 resolveDriveCopyInput；validate.EncodePathSegment("..") 仍是 ..，这里最终构造 /open-apis/drive/v1/files/../copy，不再保持 files/:file_token/copy 的路径结构。Fix：在 URL 与裸 token 汇合后、返回 driveCopyRef 前调用 validate.ResourceName(token, sourceFlag)，并补充 ..、%2e%2e、控制字符和危险 Unicode 的契约测试。
| "wiki node %q cannot be copied with drive +copy; use wiki +node-copy instead", | ||
| nodeToken, | ||
| ).WithParam(param).WithHint( | ||
| "run: lark-cli wiki +node-copy --space-id <space-id> --node-token %s --target-space-id <target-space-id> (or --target-parent-node-token); resolve <space-id> with: lark-cli wiki +node-get --token %s", |
There was a problem hiding this comment.
[P1] 不要把原始 token 拼进可执行命令 hint。 nodeToken 来自用户输入，当前使用裸 %s 写进恢复命令；例如包含分号的 token 会在 Agent 或用户照抄 hint 时形成额外 shell 命令。Fix：两处都改用固定 <node-token> 占位符，或统一使用可靠的 shell quoting，并补充分号、空格、反引号等输入的测试。
| ## 输出 | ||
|
|
||
| ```json | ||
| { |
There was a problem hiding this comment.
[P1] 输出示例缺少 CLI envelope 的 data 层。 实现通过 runtime.Out 输出，新增 live E2E 也读取 data.file_token、data.name 和 data.folder_token；这里把 copied、file_token 等字段写在根节点，会让 Agent 按文档解析时拿不到副本 token。Fix：示例改成 {"ok":true,"identity":"bot","data":{"copied":true,...,"permission_grant":{...}}}，并将上文的字段名同步改为 data.permission_grant。
| | `+sync` | 双向同步本地目录与 Drive 文件夹：拉取 `new_remote`、推送 `new_local`，`modified` 按 `--on-conflict=remote-wins\|local-wins\|keep-both\|ask` 处理；`--quick` 用修改时间近似比较；`--on-duplicate-remote` 支持 `fail` / `newest` / `oldest`；只同步 `type=file`，跳过在线文档和 shortcut，且不会删除两端多余文件。 | | ||
| | [`+push`](references/lark-drive-push.md) | 将本地目录推送到 Drive 文件夹，支持 skip / smart / overwrite 与确认后删除远端。 | | ||
| | [`+create-shortcut`](references/lark-drive-create-shortcut.md) | 在另一个文件夹里创建现有 Drive 文件的快捷方式。 | | ||
| | [`+copy`](references/lark-drive-copy.md) | 复制 doc/docx/sheet/file/mindnote/slides/base(bitable) 到目标文件夹；支持 URL 传参，wiki 输入会引导改用 `wiki +node-copy`。 | |
There was a problem hiding this comment.
[P1] Wiki 来源应按目标位置分流，不能一律引导到 wiki +node-copy。 wiki +node-copy 只接收目标 Wiki space/parent，无法表达复制到 Drive 文件夹或 my_space；本 PR 的 copy reference 已说明后一种场景需要先 drive +inspect，再把底层 token/type 交给 drive +copy。Fix：目标是 Wiki 时使用 wiki +node-copy；目标是 Drive/my_space 时使用 drive +inspect → drive +copy，并同步修正快速决策中的无条件路由。
|
|
||
| `drive +copy` 只复制云盘（Drive）文件，不接受 wiki URL / token；传入时返回校验错误，错误 hint 会给出替代命令。知识库内复制节点用 [`lark-wiki`](../../lark-wiki/SKILL.md) 的 `wiki +node-copy`；要把 wiki 文档复制成 Drive 空间里的独立副本（脱离知识库），先用 `drive +inspect` 解包拿到底层 `token` 和 `type`，再对底层 token 执行 `drive +copy`。 | ||
|
|
||
| ## 行为说明 |
There was a problem hiding this comment.
结合下错误率监控，之前的一些场景错误也加下引导提示
2 participants
Summary
Wrap the Drive file-copy endpoint as drive +copy. Accept a document URL (recommended) or bare token + --type for the source, a folder token or folder URL for the target, and repeatable --extra key=value pairs forwarded verbatim for special copy semantics (e.g. target_type=docx to convert a legacy doc during copy). Reject wiki URLs/tokens with a typed validation error whose hint carries a ready-to-adapt wiki +node-copy command, because a Drive copy of a wiki-backed document would land in Drive space instead of the wiki tree.
Declare docs:document:copy (the narrowest scope in the endpoint's any-of set). Cover the shortcut with unit tests, dry-run e2e and a self-contained live workflow (upload -> copy -> download-verify -> 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.
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