feat: support file-name for drive export#685
Conversation
📝 WalkthroughWalkthroughAdds a new Changes
Sequence Diagram(s)sequenceDiagram
participant CLI
participant Orchestrator
participant DriveAPI
participant Filesystem
CLI->>Orchestrator: drive +export --token ... --file-name custom-report --dry-run?
Orchestrator->>DriveAPI: POST /open-apis/drive/v1/export_tasks (no file_name in body)
Orchestrator-->>CLI: stdout JSON (includes file_name: custom-report.pdf, output_dir)
CLI->>Orchestrator: (sync markdown) fetch export content
Orchestrator->>DriveAPI: GET export content
Orchestrator->>Filesystem: write file using computed file_name
Note right of Orchestrator: For async task when Ready(), prefer provided file_name over remote status.FileName
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.Comment |
github-actions
Bot
added
domain/ccm
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #685 +/- ##
==========================================
+ Coverage 64.49% 64.57% +0.07%
==========================================
Files 516 516
Lines 45732 45749 +17
==========================================
+ Hits 29496 29541 +45
+ Misses 13650 13623 -27
+ Partials 2586 2585 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@d2dfe47c95b3b4fbd2d039bb2581441790b5013f🧩 Skill updatenpx skills add larksuite/cli#feat/drive-export-file-name -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 (4)
shortcuts/drive/drive_export.go (3)
36-36: Optional: enrich--file-nameflag description for AI agents.The current description (
preferred output filename (optional)) does not communicate the two non-obvious behaviors that are documented inskills/lark-drive/references/lark-drive-export.md: (1) the value is sanitized, and (2) the--file-extensionsuffix is auto-appended when missing. Since CLI flags here are designed for AI-agent consumption, surfacing this in--helpreduces the chance of agents passing redundant or invalid extensions.Proposed tweak
- {Name: "file-name", Desc: "preferred output filename (optional)"}, + {Name: "file-name", Desc: "preferred local output filename; sanitized and auto-extended with --file-extension when missing"},🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/drive/drive_export.go` at line 36, Update the CLI flag help for the flag with Name: "file-name" in drive_export.go to explicitly state the two non-obvious behaviors: that the provided filename will be sanitized and that the value will have the --file-extension suffix auto-appended if the extension is missing; change the Desc from "preferred output filename (optional)" to a concise sentence that mentions sanitization and automatic extension appending (using the --file-extension flag) so AI agents see the constraints and avoid redundant extensions.
58-90: Recommended: align dry-runfile_namereporting with execute’s default-naming behavior.In
Execute, stdout always carries afile_name(preferred name, remote title, server status name, or token-fallback). InDryRun,file_nameis only emitted when--file-nameis supplied. As a result, agents using--dry-runto preview the plan will silently miss the default filename branch, which is the more common case. Consider always settingfile_namein dry-run, falling back to<token><suffix>(or, where applicable, a placeholder describing the title-lookup fallback) so the dry-run shape matches execute.If you’d rather keep dry-run light (no remote title fetch), at minimum populate
file_namefrom<sanitized token><.ext>whenever--file-nameis empty so the field is always present and parseable.Sketch
- if name := strings.TrimSpace(runtime.Str("file-name")); name != "" { - dr.Set("file_name", ensureExportFileExtension(sanitizeExportFileName(name, spec.Token), spec.FileExtension)) - } + name := strings.TrimSpace(runtime.Str("file-name")) + if name == "" { + name = spec.Token // dry-run cannot resolve remote title; surface a deterministic placeholder + } + dr.Set("file_name", ensureExportFileExtension(sanitizeExportFileName(name, spec.Token), spec.FileExtension))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/drive/drive_export.go` around lines 58 - 90, Dry-run currently only sets file_name when runtime.Str("file-name") is non-empty; update the DryRun builder logic in the Drive export branch (the code that constructs dr via common.NewDryRunAPI() in drive_export.go) to always populate the "file_name" field: if runtime.Str("file-name") is non-empty use the existing sanitizeExportFileName(...)/ensureExportFileExtension(...) path, otherwise set file_name to a sanitized token fallback (sanitizeExportFileName(spec.Token, spec.Token) or similar) with ensureExportFileExtension(..., spec.FileExtension) so the dry-run output shape matches Execute's default-naming behavior; keep the rest of the GET/POST dry-run flows and don't perform remote title lookup.
250-265: PropagatelastStatus.FileNamein the timeout result when available.The timeout block already extracts several fields from
lastStatus(failed,jobStatus,jobStatusLabel) to allow callers to resume from a known state. AddinglastStatus.FileNamewould be consistent: the success path (line 37–38) already usesstatus.FileNameas a fallback whenpreferredFileNameis empty, indicating the server populates it during polling, not only at job completion. Surfacing it on timeout would let downstream+export-downloadcalls inherit the same filename without re-prompting the user.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/drive/drive_export.go` around lines 250 - 265, The timeout result map should include lastStatus.FileName when available so callers can resume with the server-provided filename: in the timeout handling where result := map[string]interface{}{...} is constructed, use lastStatus.FileName as the fallback for preferredFileName (or set preferredFileName = lastStatus.FileName when non-empty) before the block that sets result["file_name"] and call ensureExportFileExtension(sanitizeExportFileName(preferredFileName, spec.Token), spec.FileExtension) as currently done; ensure you reference lastStatus.FileName, preferredFileName, ensureExportFileExtension, sanitizeExportFileName, spec.FileExtension, and runtime.Out when making the change so the timeout path mirrors the success path filename behavior.skills/lark-drive/references/lark-drive-export.md (1)
42-48: Optional: example does not actually exercise the auto-append behavior it describes.The comment promises
会按导出格式自动补扩展名, but the example passesweekly-report.pdf, which already has the extension and so won’t trigger the append. Consider showing the bare name to make the behavior obvious to readers/agents (and keep the resume workflow at lines 100-101 / 112 consistent with whichever form is canonical).Proposed example tweak
- --file-name "weekly-report.pdf" \ + --file-name "weekly-report" \ --output-dir ./exports🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@skills/lark-drive/references/lark-drive-export.md` around lines 42 - 48, The example claims the CLI will auto-append the export extension but uses "--file-name \"weekly-report.pdf\"" so the behavior isn't demonstrated; change the example to use a bare name (e.g., "--file-name \"weekly-report\"") so the auto-append for doc-type/docx is exercised, and update the matching resume workflow examples referenced at lines 100-101 and 112 to use the same canonical form (bare filename) for consistency; locate the flags in the shown lark-cli drive +export snippet and make the corresponding replacements.
🤖 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_export.go`:
- Line 36: Update the CLI flag help for the flag with Name: "file-name" in
drive_export.go to explicitly state the two non-obvious behaviors: that the
provided filename will be sanitized and that the value will have the
--file-extension suffix auto-appended if the extension is missing; change the
Desc from "preferred output filename (optional)" to a concise sentence that
mentions sanitization and automatic extension appending (using the
--file-extension flag) so AI agents see the constraints and avoid redundant
extensions.
- Around line 58-90: Dry-run currently only sets file_name when
runtime.Str("file-name") is non-empty; update the DryRun builder logic in the
Drive export branch (the code that constructs dr via common.NewDryRunAPI() in
drive_export.go) to always populate the "file_name" field: if
runtime.Str("file-name") is non-empty use the existing
sanitizeExportFileName(...)/ensureExportFileExtension(...) path, otherwise set
file_name to a sanitized token fallback (sanitizeExportFileName(spec.Token,
spec.Token) or similar) with ensureExportFileExtension(..., spec.FileExtension)
so the dry-run output shape matches Execute's default-naming behavior; keep the
rest of the GET/POST dry-run flows and don't perform remote title lookup.
- Around line 250-265: The timeout result map should include lastStatus.FileName
when available so callers can resume with the server-provided filename: in the
timeout handling where result := map[string]interface{}{...} is constructed, use
lastStatus.FileName as the fallback for preferredFileName (or set
preferredFileName = lastStatus.FileName when non-empty) before the block that
sets result["file_name"] and call
ensureExportFileExtension(sanitizeExportFileName(preferredFileName, spec.Token),
spec.FileExtension) as currently done; ensure you reference lastStatus.FileName,
preferredFileName, ensureExportFileExtension, sanitizeExportFileName,
spec.FileExtension, and runtime.Out when making the change so the timeout path
mirrors the success path filename behavior.
In `@skills/lark-drive/references/lark-drive-export.md`:
- Around line 42-48: The example claims the CLI will auto-append the export
extension but uses "--file-name \"weekly-report.pdf\"" so the behavior isn't
demonstrated; change the example to use a bare name (e.g., "--file-name
\"weekly-report\"") so the auto-append for doc-type/docx is exercised, and
update the matching resume workflow examples referenced at lines 100-101 and 112
to use the same canonical form (bare filename) for consistency; locate the flags
in the shown lark-cli drive +export snippet and make the corresponding
replacements.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ba71c11d-cc03-4834-bfd1-aac94be9afab
📒 Files selected for processing (6)
shortcuts/drive/drive_export.goshortcuts/drive/drive_export_test.goskills/lark-drive/SKILL.mdskills/lark-drive/references/lark-drive-export.mdtests/cli_e2e/drive/coverage.mdtests/cli_e2e/drive/drive_export_dryrun_test.go
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@shortcuts/drive/drive_export_test.go`:
- Around line 648-691: The test TestDriveExportTimeoutPreservesProvidedFileName
can pollute the process cwd if export timeout logic writes files; update that
test to isolate filesystem side effects by creating a temp working directory
with t.TempDir(), change the process cwd to that temp dir at the start of the
test (using os.Chdir) and register a t.Cleanup to restore the original cwd, so
the rest of the test (including calls to mountAndRunDrive and assertions on
stdout) runs in an isolated directory; modify only the
TestDriveExportTimeoutPreservesProvidedFileName function to add the temp dir
setup and cleanup around the existing logic.
🪄 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: 0ed16159-56b5-4d15-8fc5-ba49210291f3
📒 Files selected for processing (1)
shortcuts/drive/drive_export_test.go
2 participants
Summary
Adds local filename support to
drive +export, matching the existingdrive +export-download --file-namebehavior so agents can choose the primary export command without losing filename control. The provided name is sanitized and extended according to--file-extension.Changes
--file-nametodrive +exportfor markdown direct export, completed async export downloads, and timeout metadata.file_name/output_dirwhile keeping local-only filename data out of the export task request body.Test Plan
make unit-testlark xxxcommand works as expected:go test ./tests/cli_e2e/drive -run TestDriveExportDryRun_FileNameMetadata -count=1go vet ./...gofmt -l .go mod tidywith nogo.mod/go.sumdiffgo run github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.1.6 run --new-from-rev=origin/mainRelated Issues
Summary by CodeRabbit
New Features
Documentation
Tests