Open
Conversation
Change-Id: Iced88525deb10b014b755ec68bd9a8ae6a935143
github-actions
Bot
added
the
size/L
📝 WalkthroughWalkthroughThis PR introduces a new Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant CLI as CLI Handler
participant DriveAPI as Drive API
participant Remote as Remote Storage
Client->>CLI: markdown +create/+overwrite<br/>--content/--file
CLI->>CLI: Validate spec & get payload bytes
CLI->>DriveAPI: upload_prepare(file_name, parent_type, size)
DriveAPI-->>CLI: block_num, upload_id
loop For each chunk
CLI->>DriveAPI: upload_part(upload_id, part_seq, chunk_bytes)
DriveAPI-->>CLI: Part acknowledged
end
CLI->>DriveAPI: upload_finish(upload_id, block_num)
DriveAPI->>Remote: Save markdown file
DriveAPI-->>CLI: file_token, version
CLI-->>Client: Success (file_token, file_name, version, size_bytes)
sequenceDiagram
actor Client
participant CLI as CLI Handler
participant DriveAPI as Drive API
Client->>CLI: markdown +fetch --file-token TOKEN<br/>[--output PATH] [--overwrite]
CLI->>DriveAPI: GET /drive/v1/media/spaces/...<br/>/files/{file_token}
DriveAPI-->>CLI: Download stream + Content-Disposition header
alt --output not set
CLI->>CLI: Parse response body as UTF-8
CLI-->>Client: JSON (file_name, content, size_bytes)
else --output set
CLI->>CLI: Derive output path (append filename if directory)
CLI->>Client: Save stream to disk
CLI-->>Client: JSON (file_name, saved_path, size_bytes)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (3 warnings)
✅ 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)
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 |
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@14be4abe71d9720ba6c9271b3762f11bb4e4e2ab🧩 Skill updatenpx skills add larksuite/cli#feat/drive-version-shortcuts -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: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
shortcuts/register.go (1)
23-46:⚠️ Potential issue | 🟠 MajorThis PR registers markdown shortcuts but omits the stated drive version shortcuts.
The PR objective is adding
drive +version-history,+version-get,+version-revert, and+version-delete, yet this change registers markdown shortcuts (+create,+fetch,+overwrite) with no trace of the drive version shortcuts anywhere in the codebase. Either the PR description is incorrect, or the drive version implementation is missing from this commit.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/register.go` around lines 23 - 46, The PR claims to add drive version commands (+version-history, +version-get, +version-revert, +version-delete) but those shortcuts are not registered; update the code so the drive version shortcuts are actually included in the aggregated list by either (A) adding the version shortcuts into the existing drive.Shortcuts() implementation or (B) importing the package that defines them (e.g., driveversion or drive/version) and appending its Shortcuts() result to allShortcuts inside init() (alongside the existing drive.Shortcuts() call); ensure the functions that produce those shortcuts expose a Shortcuts() slice and that init() appends it to allShortcuts so the +version-* commands are registered.
🧹 Nitpick comments (1)
shortcuts/register_markdown_test.go (1)
13-15: Usecmdutil.TestFactoryhere instead of a zero-value factory.Line 15 makes this test depend on none of the mounted shortcuts touching factory-backed collaborators during registration. Using the standard test factory keeps it aligned with the rest of the repo’s shortcut tests and avoids nil-coupled regressions.
As per coding guidelines,
**/*_test.go: Usecmdutil.TestFactory(t, config)for test factories.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/register_markdown_test.go` around lines 13 - 15, The test TestRegisterShortcutsMountsMarkdownCommands should use the standard test factory instead of a zero-value factory; replace the &cmdutil.Factory{} usage in the RegisterShortcuts call with a factory created via cmdutil.TestFactory(t, config) (or cmdutil.TestFactory(t, nil) if no config is needed) so the test uses the same test-backed collaborators as other shortcut tests; update the TestRegisterShortcutsMountsMarkdownCommands function to obtain factory := cmdutil.TestFactory(t, config) and pass that factory into RegisterShortcuts.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/core/types.go`:
- Around line 43-44: The default Feishu endpoints were changed to pre-production
hosts (Open: "https://open.feishu-pre.cn", Accounts:
"https://accounts.feishu-pre.cn") which causes ResolveEndpoints() to route
normal `--brand feishu` traffic to preprod; revert these Open and Accounts
values back to the production Feishu endpoints (restore the prod hostnames) in
the types/endpoint mapping and ensure ResolveEndpoints() continues to select the
prod mapping for the default `feishu` brand so MCP and other services remain
consistent.
In `@README.md`:
- Line 9: Update the README references that still say "23" to "24": change the
phrase "23 AI Agent [Skills]" in the introductory sentence to "24 AI Agent
[Skills]" and update any other literal "23" counters elsewhere in the file
(e.g., table summaries or badges that count skills) to "24" so they match the
added lark-markdown entry; search for the string "23" and replace the
occurrences that refer to skill counts, leaving unrelated "23" values untouched.
In `@README.zh.md`:
- Line 9: The README Chinese intro text still states "23 AI Agent Skills" but
the skills list includes the new lark-markdown for a total of 24; update the
numeric references in README.zh.md (the introductory sentence containing "23 AI
Agent Skills" and any other occurrences of "23" used as counters) to "24" so
they match the skills table and the added lark-markdown entry.
In `@shortcuts/markdown/helpers.go`:
- Around line 499-510: The function fileNameFromDownloadHeader currently returns
the original fallback when the sanitized candidate is empty or "."/"..", which
can reintroduce path elements; change it to sanitize the fallback the same way
(trim, replace backslashes, take path.Base) and use that sanitized fallback
instead, and if that result is still invalid, return a fixed safe name such as
"download" (or another chosen constant) to avoid returning path traversal
components.
In `@skills/lark-drive/references/lark-drive-upload.md`:
- Line 9: Update the broken relative link `../lark-markdown/SKILL.md` in the
line mentioning lark-markdown so it points to the correct path
`../../lark-markdown/SKILL.md`; locate the link string
`../lark-markdown/SKILL.md` in skills/lark-drive/references/lark-drive-upload.md
and replace it with `../../lark-markdown/SKILL.md`.
In `@tests/cli_e2e/markdown/markdown_workflow_test.go`:
- Around line 18-19: The skip message in the test's environment gate is
misleading; update the t.Skip call that checks os.Getenv("LARK_MARKDOWN_E2E") to
use a clear, accurate reason mentioning that setting LARK_MARKDOWN_E2E=1 runs
the markdown live workflow (e.g., "set LARK_MARKDOWN_E2E=1 to run markdown live
workflow") so users enabling the env var aren't confused by the unrelated
"backend version support" wording.
---
Outside diff comments:
In `@shortcuts/register.go`:
- Around line 23-46: The PR claims to add drive version commands
(+version-history, +version-get, +version-revert, +version-delete) but those
shortcuts are not registered; update the code so the drive version shortcuts are
actually included in the aggregated list by either (A) adding the version
shortcuts into the existing drive.Shortcuts() implementation or (B) importing
the package that defines them (e.g., driveversion or drive/version) and
appending its Shortcuts() result to allShortcuts inside init() (alongside the
existing drive.Shortcuts() call); ensure the functions that produce those
shortcuts expose a Shortcuts() slice and that init() appends it to allShortcuts
so the +version-* commands are registered.
---
Nitpick comments:
In `@shortcuts/register_markdown_test.go`:
- Around line 13-15: The test TestRegisterShortcutsMountsMarkdownCommands should
use the standard test factory instead of a zero-value factory; replace the
&cmdutil.Factory{} usage in the RegisterShortcuts call with a factory created
via cmdutil.TestFactory(t, config) (or cmdutil.TestFactory(t, nil) if no config
is needed) so the test uses the same test-backed collaborators as other shortcut
tests; update the TestRegisterShortcutsMountsMarkdownCommands function to obtain
factory := cmdutil.TestFactory(t, config) and pass that factory into
RegisterShortcuts.
🪄 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: 221484ae-915e-4a2c-995f-3ac5f0958834
📒 Files selected for processing (19)
README.mdREADME.zh.mdinternal/core/types.goshortcuts/markdown/helpers.goshortcuts/markdown/markdown_create.goshortcuts/markdown/markdown_fetch.goshortcuts/markdown/markdown_overwrite.goshortcuts/markdown/markdown_test.goshortcuts/markdown/shortcuts.goshortcuts/register.goshortcuts/register_markdown_test.goskills/lark-drive/SKILL.mdskills/lark-drive/references/lark-drive-upload.mdskills/lark-markdown/SKILL.mdskills/lark-markdown/references/lark-markdown-create.mdskills/lark-markdown/references/lark-markdown-fetch.mdskills/lark-markdown/references/lark-markdown-overwrite.mdtests/cli_e2e/markdown/markdown_dryrun_test.gotests/cli_e2e/markdown/markdown_workflow_test.go
Comment on lines
+43
to
+44
| Open: "https://open.feishu-pre.cn", | ||
| Accounts: "https://accounts.feishu-pre.cn", |
There was a problem hiding this comment.
Do not switch the default Feishu brand to pre-production endpoints.
ResolveEndpoints() feeds every normal Feishu request URL, so this reroutes standard --brand feishu API/auth traffic to open.feishu-pre.cn / accounts.feishu-pre.cn. That is a global behavior break, and it also leaves MCP on prod, so the default environment becomes inconsistent.
Suggested fix
return Endpoints{
- Open: "https://open.feishu-pre.cn",
- Accounts: "https://accounts.feishu-pre.cn",
+ Open: "https://open.feishu.cn",
+ Accounts: "https://accounts.feishu.cn",
MCP: "https://mcp.feishu.cn",
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| Open: "https://open.feishu-pre.cn", | |
| Accounts: "https://accounts.feishu-pre.cn", | |
| return Endpoints{ | |
| Open: "https://open.feishu.cn", | |
| Accounts: "https://accounts.feishu.cn", | |
| MCP: "https://mcp.feishu.cn", | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/core/types.go` around lines 43 - 44, The default Feishu endpoints
were changed to pre-production hosts (Open: "https://open.feishu-pre.cn",
Accounts: "https://accounts.feishu-pre.cn") which causes ResolveEndpoints() to
route normal `--brand feishu` traffic to preprod; revert these Open and Accounts
values back to the production Feishu endpoints (restore the prod hostnames) in
the types/endpoint mapping and ensure ResolveEndpoints() continues to select the
prod mapping for the default `feishu` brand so MCP and other services remain
consistent.
| [中文版](./README.zh.md) | [English](./README.md) | ||
|
|
||
| The official [Lark/Feishu](https://www.larksuite.com/) CLI tool, maintained by the [larksuite](https://github.com/larksuite) team — built for humans and AI Agents. Covers core business domains including Messenger, Docs, Base, Sheets, Slides, Calendar, Mail, Tasks, Meetings, and more, with 200+ commands and 23 AI Agent [Skills](./skills/). | ||
| The official [Lark/Feishu](https://www.larksuite.com/) CLI tool, maintained by the [larksuite](https://github.com/larksuite) team — built for humans and AI Agents. Covers core business domains including Messenger, Docs, Base, Sheets, Slides, Calendar, Mail, Tasks, Meetings, Markdown, and more, with 200+ commands and 23 AI Agent [Skills](./skills/). |
There was a problem hiding this comment.
The skill count needs to be updated.
This line still says 23 AI Agent Skills, but the table below now lists 24 after adding lark-markdown. Please update the numeric references here and the matching 23 counters later in the file.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@README.md` at line 9, Update the README references that still say "23" to
"24": change the phrase "23 AI Agent [Skills]" in the introductory sentence to
"24 AI Agent [Skills]" and update any other literal "23" counters elsewhere in
the file (e.g., table summaries or badges that count skills) to "24" so they
match the added lark-markdown entry; search for the string "23" and replace the
occurrences that refer to skill counts, leaving unrelated "23" values untouched.
| [中文版](./README.zh.md) | [English](./README.md) | ||
|
|
||
| 飞书官方 CLI 工具，由 [larksuite](https://github.com/larksuite) 团队维护 — 让人类和 AI Agent 都能在终端中操作飞书。覆盖消息、文档、多维表格、电子表格、幻灯片、日历、邮箱、任务、会议等核心业务域，提供 200+ 命令及 23 个 AI Agent [Skills](./skills/)。 | ||
| 飞书官方 CLI 工具，由 [larksuite](https://github.com/larksuite) 团队维护 — 让人类和 AI Agent 都能在终端中操作飞书。覆盖消息、文档、多维表格、电子表格、幻灯片、日历、邮箱、任务、会议、Markdown 等核心业务域，提供 200+ 命令及 23 个 AI Agent [Skills](./skills/)。 |
There was a problem hiding this comment.
The skill count is stale after adding lark-markdown.
This intro still says 23 AI Agent Skills, but the table below now lists 24. Please sync the numeric references here and the matching 23 counters later in the file.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~9-~9: 数量词修饰并列短语，可能产生歧义
Context: ...团队维护 — 让人类和 AI Agent 都能在终端中操作飞书。覆盖消息、文档、多维表格、电子表格、幻灯片、日历、邮箱、任务、会议、Markdown 等核心业务域，提供 ...
(s5)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@README.zh.md` at line 9, The README Chinese intro text still states "23 AI
Agent Skills" but the skills list includes the new lark-markdown for a total of
24; update the numeric references in README.zh.md (the introductory sentence
containing "23 AI Agent Skills" and any other occurrences of "23" used as
counters) to "24" so they match the skills table and the added lark-markdown
entry.
Comment on lines
+499
to
+510
| func fileNameFromDownloadHeader(header http.Header, fallback string) string { | ||
| name := fallback | ||
| if header != nil { | ||
| if headerName := larkcore.FileNameByHeader(header); strings.TrimSpace(headerName) != "" { | ||
| name = headerName | ||
| } | ||
| } | ||
| name = strings.ReplaceAll(strings.TrimSpace(name), "\\", "/") | ||
| name = path.Base(name) | ||
| if name == "" || name == "." || name == ".." { | ||
| return fallback | ||
| } |
There was a problem hiding this comment.
Don't return the unsanitized fallback for invalid download names.
This helper sanitizes the candidate filename, but if the sanitized result becomes empty, ".", or "..", it returns the original fallback unchanged. That reintroduces the path elements you just stripped and can escape or target the output directory unexpectedly when the caller joins it with a user-selected folder.
Suggested fix
func fileNameFromDownloadHeader(header http.Header, fallback string) string {
- name := fallback
+ safeFallback := path.Base(strings.ReplaceAll(strings.TrimSpace(fallback), "\\", "/"))
+ if safeFallback == "" || safeFallback == "." || safeFallback == ".." {
+ safeFallback = "download.md"
+ }
+ name := safeFallback
if header != nil {
if headerName := larkcore.FileNameByHeader(header); strings.TrimSpace(headerName) != "" {
name = headerName
}
}
name = strings.ReplaceAll(strings.TrimSpace(name), "\\", "/")
name = path.Base(name)
if name == "" || name == "." || name == ".." {
- return fallback
+ return safeFallback
}
return name
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@shortcuts/markdown/helpers.go` around lines 499 - 510, The function
fileNameFromDownloadHeader currently returns the original fallback when the
sanitized candidate is empty or "."/"..", which can reintroduce path elements;
change it to sanitize the fallback the same way (trim, replace backslashes, take
path.Base) and use that sanitized fallback instead, and if that result is still
invalid, return a fixed safe name such as "download" (or another chosen
constant) to avoid returning path traversal components.
| 上传本地文件到飞书云空间。目标位置可以是 Drive 文件夹，也可以是 wiki 节点。 | ||
|
|
||
| ## 快速决策 | ||
| - 用户要在 Drive 里上传、创建、读取、覆盖更新**原生 `.md` 文件**（不是导入成 docx），切到 [`lark-markdown`](../lark-markdown/SKILL.md)。 |
There was a problem hiding this comment.
Fix the relative link to lark-markdown.
From skills/lark-drive/references/, ../lark-markdown/SKILL.md resolves to the wrong location. This should point to ../../lark-markdown/SKILL.md.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@skills/lark-drive/references/lark-drive-upload.md` at line 9, Update the
broken relative link `../lark-markdown/SKILL.md` in the line mentioning
lark-markdown so it points to the correct path `../../lark-markdown/SKILL.md`;
locate the link string `../lark-markdown/SKILL.md` in
skills/lark-drive/references/lark-drive-upload.md and replace it with
`../../lark-markdown/SKILL.md`.
Comment on lines
+18
to
+19
| if os.Getenv("LARK_MARKDOWN_E2E") == "" { | ||
| t.Skip("set LARK_MARKDOWN_E2E=1 to run markdown live workflow after backend version support is deployed") |
There was a problem hiding this comment.
Fix the stale skip reason.
Line 19 mentions “backend version support,” which is unrelated to this markdown workflow test and will confuse anyone trying to enable LARK_MARKDOWN_E2E.
Suggested wording
- t.Skip("set LARK_MARKDOWN_E2E=1 to run markdown live workflow after backend version support is deployed")
+ t.Skip("set LARK_MARKDOWN_E2E=1 to run the markdown live workflow test")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if os.Getenv("LARK_MARKDOWN_E2E") == "" { | |
| t.Skip("set LARK_MARKDOWN_E2E=1 to run markdown live workflow after backend version support is deployed") | |
| if os.Getenv("LARK_MARKDOWN_E2E") == "" { | |
| t.Skip("set LARK_MARKDOWN_E2E=1 to run the markdown live workflow test") |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/cli_e2e/markdown/markdown_workflow_test.go` around lines 18 - 19, The
skip message in the test's environment gate is misleading; update the t.Skip
call that checks os.Getenv("LARK_MARKDOWN_E2E") to use a clear, accurate reason
mentioning that setting LARK_MARKDOWN_E2E=1 runs the markdown live workflow
(e.g., "set LARK_MARKDOWN_E2E=1 to run markdown live workflow") so users
enabling the env var aren't confused by the unrelated "backend version support"
wording.
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
1 participant
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
Add a new set of Drive version shortcuts for file version history, version download, revert, and delete. This PR also adds the corresponding Drive skill references so AI agents can use the new version flows with bot identity and clearer parameter guidance.
Changes
Test Plan
Note: targeted tests for the new Drive version shortcuts and dry-run E2E coverage passed. Repository-wide make unit-test still reports unrelated pre-existing brand/endpoint expectation failures in other packages.
Related Issues
Summary by CodeRabbit
Release Notes
New Features
.mdfiles.Documentation
Tests