feat(drive): add drive preview and cover shortcuts and document quota details#1259
Merged
Merged
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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds two Drive shortcuts: ChangesDrive Preview and Cover Shortcuts
Sequence Diagram(s)sequenceDiagram
participant CLI
participant DriveAPI
participant LocalFS
CLI->>DriveAPI: POST /open-apis/drive/v1/medias/:file_token/preview_result (list)
DriveAPI-->>CLI: preview candidates (types/status/downloadable)
CLI->>CLI: select candidate / build preview_download params
CLI->>DriveAPI: GET /open-apis/drive/v1/medias/:file_token/preview_download?...
DriveAPI-->>CLI: stream artifact (Content-Type/Disposition)
CLI->>LocalFS: write file (apply if-exists policy, extension inference)
Estimated code review effort 🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
github-actions
Bot
added
domain/ccm
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1259 +/- ##
==========================================
+ Coverage 69.90% 69.96% +0.05%
==========================================
Files 645 669 +24
Lines 60233 64967 +4734
==========================================
+ Hits 42108 45454 +3346
- Misses 14823 15860 +1037
- Partials 3302 3653 +351 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@52abe1f7ea078ed27faa54966836a45a6922756d🧩 Skill updatenpx skills add larksuite/cli#feat/drive-add-preview-cover-and-quota -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: 4
🧹 Nitpick comments (1)
shortcuts/drive/drive_preview_test.go (1)
290-317: ⚡ Quick winMount the real shortcut instead of re-declaring its flags.
Lines 295-302 duplicate the preview/cover flag surface in test code, so these DryRun tests can keep passing after a production flag rename, default change, or registration tweak. Building the runtime from the mounted
DrivePreview/DriveCovercommand would keep the tests coupled to the real CLI contract.🤖 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_preview_test.go` around lines 290 - 317, newDrivePreviewRuntime is re-declaring the preview/cover CLI flags in the test instead of mounting the real shortcut command, which breaks tests if production flags change; update newDrivePreviewRuntime to instantiate or mount the actual DrivePreview (and DriveCover if relevant) cobra.Command (instead of creating cmd := &cobra.Command{Use: use}) to inherit the real flag set, then apply stringFlags and boolFlags via cmd.Flags().Set as before and return the runtime from common.TestNewRuntimeContextWithCtx(context.Background(), cmd, driveTestConfig()) so tests track the real CLI registration.
🤖 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_preview_common.go`:
- Around line 501-506: The current switch branch for ifExists (case "" and
drivePreviewIfExistsError) incorrectly treats any non-nil runtime.FileIO().Stat
error as "path does not exist"; instead, change the logic in that branch to
explicitly check for fs.ErrNotExist and treat that as the non-existent path case
(return finalPath, resolution, nil), but propagate any other Stat error (return
it up) so permission/I/O errors are preserved; locate the check using
runtime.FileIO().Stat(finalPath), the drivePreviewIfExistsError constant, and
the errs.NewValidationError(...) code that currently returns the validation
error, and mirror the explicit fs.ErrNotExist handling already used in
nextAvailableDrivePreviewPath.
- Around line 501-519: The switch uses the raw ifExists string, which can
include surrounding whitespace and mismatch validation; normalize it first
(e.g., normalized := strings.TrimSpace(ifExists) or reassign ifExists =
strings.TrimSpace(ifExists)) and switch on that normalized value so cases like
drivePreviewIfExistsRename match; update the switch expression and keep
references to ifExists,
drivePreviewIfExistsError/drivePreviewIfExistsOverwrite/drivePreviewIfExistsRename,
nextAvailableDrivePreviewPath, and runtime.ResolveSavePath unchanged.
In `@skills/lark-drive/references/lark-drive-preview.md`:
- Line 45: The docs show `--type` example values (`png`/`jpg`/`source_file`)
that don't match the flag contract in shortcuts/drive/drive_preview.go (which
expects `pdf | html | text | image | source`); update the lark-drive-preview.md
`--type` examples and description to use the exact supported values (`pdf`,
`html`, `text`, `image`, `source`) and remove/replace `png`/`jpg`/`source_file`
so examples and text match the implementation and won't trigger the
unavailable-branch in DrivePreview flag parsing.
In `@skills/lark-drive/SKILL.md`:
- Line 272: Update the `+cover` SKILL.md entry so its 404 semantics match the
behavior in drive_preview_workflow_test.go: replace the phrase that says
"download-time HTTP 404 means the file has no artifact for that cover spec" with
text stating that a download-time HTTP 404 may indicate the artifact is not yet
ready or unavailable and that callers/agents should retry (or list artifacts
first then retry) rather than treat 404 as a final, non-retriable error.
---
Nitpick comments:
In `@shortcuts/drive/drive_preview_test.go`:
- Around line 290-317: newDrivePreviewRuntime is re-declaring the preview/cover
CLI flags in the test instead of mounting the real shortcut command, which
breaks tests if production flags change; update newDrivePreviewRuntime to
instantiate or mount the actual DrivePreview (and DriveCover if relevant)
cobra.Command (instead of creating cmd := &cobra.Command{Use: use}) to inherit
the real flag set, then apply stringFlags and boolFlags via cmd.Flags().Set as
before and return the runtime from
common.TestNewRuntimeContextWithCtx(context.Background(), cmd,
driveTestConfig()) so tests track the real CLI registration.
🪄 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: 8125166d-2326-4f48-bb2c-7c8108788966
📒 Files selected for processing (11)
shortcuts/drive/drive_cover.goshortcuts/drive/drive_preview.goshortcuts/drive/drive_preview_common.goshortcuts/drive/drive_preview_test.goshortcuts/drive/shortcuts.goshortcuts/drive/shortcuts_test.goskills/lark-drive/SKILL.mdskills/lark-drive/references/lark-drive-cover.mdskills/lark-drive/references/lark-drive-preview.mdtests/cli_e2e/drive/drive_preview_dryrun_test.gotests/cli_e2e/drive/drive_preview_workflow_test.go
- add `drive +preview` and `drive +cover` shortcuts - wrap `preview_result` output with stable preview item fields - support cover download via `preview_download` with validated preset mappings - update lark-drive skill references for preview and cover usage
zhoujunteng-max
force-pushed
the
feat/drive-add-preview-cover-and-quota
branch
from
0505142 to
f3f06cd
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
shortcuts/drive/drive_preview_common.go (1)
501-521:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winTrim
--if-existsbefore switching to keep Validate/Execute behavior consistent.Line 501 switches on the raw
ifExists, while Line 254 validatesstrings.TrimSpace(policy). A value like--if-exists=" rename "passes validation but falls into the default error path at execution.Suggested fix
func resolveDrivePreviewOutputPath(runtime *common.RuntimeContext, outputPath string, header http.Header, fallbackExt, ifExists string) (string, *driveExtensionResolution, error) { finalPath, resolution := autoAppendDrivePreviewExtension(outputPath, header, fallbackExt) + ifExists = strings.TrimSpace(ifExists) if _, err := runtime.ResolveSavePath(finalPath); err != nil { return "", nil, errs.NewValidationError(errs.SubtypeInvalidArgument, "unsafe output path: %s", err).WithParam("--output") }🤖 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_preview_common.go` around lines 501 - 521, The switch on the raw ifExists value can mis-handle values with surrounding whitespace; trim the value before switching so execution matches earlier validation: apply strings.TrimSpace to the ifExists value (or reuse the already-trimmed policy variable) before the switch that checks drivePreviewIfExistsError, drivePreviewIfExistsOverwrite, and drivePreviewIfExistsRename so values like " rename " follow the same code paths (keep downstream calls like nextAvailableDrivePreviewPath and runtime.ResolveSavePath unchanged).
🤖 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.
Duplicate comments:
In `@shortcuts/drive/drive_preview_common.go`:
- Around line 501-521: The switch on the raw ifExists value can mis-handle
values with surrounding whitespace; trim the value before switching so execution
matches earlier validation: apply strings.TrimSpace to the ifExists value (or
reuse the already-trimmed policy variable) before the switch that checks
drivePreviewIfExistsError, drivePreviewIfExistsOverwrite, and
drivePreviewIfExistsRename so values like " rename " follow the same code paths
(keep downstream calls like nextAvailableDrivePreviewPath and
runtime.ResolveSavePath unchanged).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 82e202ce-8c98-4cdb-9082-5f905b02c8b7
📒 Files selected for processing (11)
shortcuts/drive/drive_cover.goshortcuts/drive/drive_preview.goshortcuts/drive/drive_preview_common.goshortcuts/drive/drive_preview_test.goshortcuts/drive/shortcuts.goshortcuts/drive/shortcuts_test.goskills/lark-drive/SKILL.mdskills/lark-drive/references/lark-drive-cover.mdskills/lark-drive/references/lark-drive-preview.mdtests/cli_e2e/drive/drive_preview_dryrun_test.gotests/cli_e2e/drive/drive_preview_workflow_test.go
✅ Files skipped from review due to trivial changes (1)
- skills/lark-drive/SKILL.md
🚧 Files skipped from review as they are similar to previous changes (7)
- shortcuts/drive/shortcuts.go
- shortcuts/drive/shortcuts_test.go
- tests/cli_e2e/drive/drive_preview_dryrun_test.go
- shortcuts/drive/drive_preview.go
- tests/cli_e2e/drive/drive_preview_workflow_test.go
- shortcuts/drive/drive_cover.go
- shortcuts/drive/drive_preview_test.go
wittam-01
previously approved these changes
Jun 4, 2026
fangshuyu-768
approved these changes
Jun 4, 2026
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
3 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
This PR adds
drive +previewanddrive +covershortcuts for Drive file preview artifact and cover downloads, and updates thelark-driveskill metadata to expose quota detail capability and the new shortcuts to AI agents.Changes
drive +previewto list and download available preview artifactsdrive +coverto list and download built-in cover presetsskills/lark-drive/SKILL.mdand add preview/cover referencesquota_details.getinskills/lark-drive/SKILL.mdTest Plan
lark-cli drive +previewandlark-cli drive +covercommands work as expectedAdditional verification:
make unit-testgo test ./shortcuts/drive -run 'TestDrivePreview|TestDriveCover|TestShortcutsIncludesExpectedCommands'go test ./tests/cli_e2e/drive -run 'TestDrive(Preview|Cover)DryRun'go test ./tests/cli_e2e/drive -run 'TestDrive_PreviewAndCoverWorkflow'Related Issues
Summary by CodeRabbit
New Features
Bug Fixes / Behavior
Documentation
Tests