feat: support files in drive +add-comment#975
Open
wittam-01 wants to merge 3 commits into
Open
Conversation
Change-Id: Id9a87706a1e43756d8142637be9ec1e0748d4ddf
📝 WalkthroughWalkthroughAdds Drive Markdown ChangesEndpoint Configuration
Drive Markdown File Comment Support
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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)
Warning Review ran into problems🔥 ProblemsStopped waiting for pipeline failures after 30000ms. One of your pipelines takes longer than our 30000ms fetch window to run, so review may not consider pipeline-failure results for inline comments if any failures occurred after the fetch window. Increase the timeout if you want to wait longer or run a 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
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/core/types.go (1)
27-28:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate stale endpoint examples in comments.
The comments still reference the old
*.feishu.cndomain format, but the actual default values now use*-pre.cndomains. The examples should reflect the current implementation.📝 Proposed fix to update comment examples
type Endpoints struct { - Open string // e.g. "https://open.feishu.cn" - Accounts string // e.g. "https://accounts.feishu.cn" + Open string // e.g. "https://open.feishu-pre.cn" + Accounts string // e.g. "https://accounts.feishu-pre.cn" MCP string // e.g. "https://mcp.feishu.cn" AppLink string // e.g. "https://applink.feishu.cn" }🤖 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 `@internal/core/types.go` around lines 27 - 28, Update the stale example comments for the Open and Accounts fields to match the current default domain format: change the examples from the old "*.feishu.cn" to the new "*-pre.cn" style and ensure the comment text alongside the Open and Accounts fields (the Open and Accounts fields in the types.go diff) reflect the exact current default hostnames used by the implementation.
🤖 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 `@internal/core/types.go`:
- Around line 45-48: Restore consistent production domains or align tests:
revert the two changed endpoints in internal/core/types.go so Open and Accounts
use "https://open.feishu.cn" and "https://accounts.feishu.cn" (to match MCP and
AppLink and the TestResolveEndpoints_Feishu expectation), or if the intent was
to switch to pre-production, change all four endpoints (Open, Accounts, MCP,
AppLink) to the -pre variants and update TestResolveEndpoints_Feishu to assert
the new values; ensure the code symbols ep.Open, ep.Accounts, MCP and AppLink
are updated together and the test TestResolveEndpoints_Feishu is updated
accordingly.
---
Outside diff comments:
In `@internal/core/types.go`:
- Around line 27-28: Update the stale example comments for the Open and Accounts
fields to match the current default domain format: change the examples from the
old "*.feishu.cn" to the new "*-pre.cn" style and ensure the comment text
alongside the Open and Accounts fields (the Open and Accounts fields in the
types.go diff) reflect the exact current default hostnames used by the
implementation.
🪄 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: 89e09742-fe49-47bf-9742-be4063d8159f
📒 Files selected for processing (7)
internal/core/types.goshortcuts/drive/drive_add_comment.goshortcuts/drive/drive_add_comment_test.goskills/lark-drive/references/lark-drive-add-comment.mdtests/cli_e2e/drive/coverage.mdtests/cli_e2e/drive/drive_add_comment_dryrun_test.gotests/cli_e2e/drive/drive_add_comment_workflow_test.go
Comment on lines
+45
to
48
| Open: "https://open.feishu-pre.cn", | ||
| Accounts: "https://accounts.feishu-pre.cn", | ||
| MCP: "https://mcp.feishu.cn", | ||
| AppLink: "https://applink.feishu.cn", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the new -pre domains are reachable and check for related config
echo "=== Testing endpoint reachability ==="
for url in "https://open.feishu-pre.cn" "https://accounts.feishu-pre.cn"; do
echo "Testing $url"
curl -s -o /dev/null -w "HTTP %{http_code}\n" --connect-timeout 5 "$url" || echo "Failed to connect"
done
echo -e "\n=== Searching for endpoint configuration documentation ==="
rg -n -C3 'feishu-pre' --type=md
echo -e "\n=== Searching for other references to -pre domains ==="
rg -n 'feishu-pre'
echo -e "\n=== Checking for environment-specific configuration ==="
rg -n -C3 'ResolveEndpoints|BrandFeishu' --type=goRepository: larksuite/cli
Length of output: 50370
Endpoint changes break existing unit tests.
The code change to use -pre domains (https://open.feishu-pre.cn, https://accounts.feishu-pre.cn) directly conflicts with existing test expectations in internal/core/types_test.go. The test TestResolveEndpoints_Feishu (line 10) explicitly asserts that ep.Open should equal "https://open.feishu.cn", which will fail with this change.
Additionally:
- Only 2 of 4 endpoints were migrated to
-predomains (Open and Accounts remain changed; MCP and AppLink remain on regular.feishu.cn). This inconsistency may cause runtime issues. -predomain suffix typically indicates pre-production/staging environments, not production-ready endpoints. Verify this is intentional for the default Feishu brand.- No PR description or comments explain why this endpoint migration is needed or related to the Markdown file support feature.
Either update the tests to match the new endpoints, or revert to .feishu.cn domains and explain the rationale if a domain change is truly required.
🤖 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 `@internal/core/types.go` around lines 45 - 48, Restore consistent production
domains or align tests: revert the two changed endpoints in
internal/core/types.go so Open and Accounts use "https://open.feishu.cn" and
"https://accounts.feishu.cn" (to match MCP and AppLink and the
TestResolveEndpoints_Feishu expectation), or if the intent was to switch to
pre-production, change all four endpoints (Open, Accounts, MCP, AppLink) to the
-pre variants and update TestResolveEndpoints_Feishu to assert the new values;
ensure the code symbols ep.Open, ep.Accounts, MCP and AppLink are updated
together and the test TestResolveEndpoints_Feishu is updated accordingly.
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@3f3bd1301c6fdc2eca3c670d412042491f716935🧩 Skill updatenpx skills add larksuite/cli#feat/drive-add-comment-support-md-files -y -g |
Change-Id: Ifffc4cdd963c13e53f4cad154aebe11ae309df9e
Change-Id: Ie6c7f38dbbea1f87a81600da71180627b53a2355
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@skills/lark-drive/SKILL.md`:
- Around line 142-143: Update the SKILL.md text that currently says "仅支持 `docx`"
to accurately describe supported partial-comment scopes: explain that the CLI
flag `--block-id` enables partial comments; `docx` supports both text- and
block-level定位 (定位/块定位), `sheet` and `slides` support only block-level 定位 via
their respective `--block-id` formats, and generic Drive `file` types do not
support 局部评论; also adjust the whitelist paragraph to keep full-file comment
limitations unchanged for the Drive file types list (e.g., `.pdf`, `.docx`,
`.xlsx` remain unsupported for partial comments).
🪄 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: f89521f5-04ce-4539-b0ff-3882f4c40e56
📒 Files selected for processing (7)
shortcuts/drive/drive_add_comment.goshortcuts/drive/drive_add_comment_test.goskills/lark-drive/SKILL.mdskills/lark-drive/references/lark-drive-add-comment.mdtests/cli_e2e/drive/coverage.mdtests/cli_e2e/drive/drive_add_comment_dryrun_test.gotests/cli_e2e/drive/drive_add_comment_workflow_test.go
✅ Files skipped from review due to trivial changes (1)
- tests/cli_e2e/drive/coverage.md
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/cli_e2e/drive/drive_add_comment_workflow_test.go
Comment on lines
+142
to
+143
| - 局部评论：传 `--block-id` 时启用；仅支持 `docx`，以及最终解析为 `docx` 的 wiki URL。block ID 可通过 `docs +fetch --api-version v2 --detail with-ids` 获取。Drive file 本次只支持全文评论，不支持局部评论。 | ||
| - Drive file 评论仅支持白名单扩展名：`.md`、`.txt`、`.json`、`.csv`、`.go`、`.js`、`.py`、`.pptx`、`.png`、`.jpg`、`.zip`、`.mp3`、`.mp4`。`.pdf`、`.docx`、`.xlsx` 等未在白名单内的普通文件暂不支持，CLI 会直接报错提示当前还不支持这种类型的评论。 |
There was a problem hiding this comment.
局部评论支持范围描述与当前能力不一致
这里写成“仅支持 docx”，但该命令同时支持 sheet 和 slides 的定位评论（通过各自格式的 --block-id）。建议改成区分：docx 支持文本定位/块定位，sheet/slides 仅支持块定位，file 不支持局部评论。
🤖 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 `@skills/lark-drive/SKILL.md` around lines 142 - 143, Update the SKILL.md text
that currently says "仅支持 `docx`" to accurately describe supported
partial-comment scopes: explain that the CLI flag `--block-id` enables partial
comments; `docx` supports both text- and block-level定位 (定位/块定位), `sheet` and
`slides` support only block-level 定位 via their respective `--block-id` formats,
and generic Drive `file` types do not support 局部评论; also adjust the whitelist
paragraph to keep full-file comment limitations unchanged for the Drive file
types list (e.g., `.pdf`, `.docx`, `.xlsx` remain unsupported for partial
comments).
wittam-01
changed the title
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
Extend Drive file support in
drive +add-comment. File targets are limited to full comments on supported Drive file extensions, with metadata validation before calling the comment API.Changes
--type fileindrive +add-comment..md,.txt,.json,.csv,.go,.js,.py,.pptx,.png,.jpg,.zip,.mp3,.mp4.metas.batch_querypreflight to read the target file title and verify its extension is supported..pdf,.docx, and.xlsx, before calling the comment API because those page previews currently display comments incorrectly.--block-idor--selection-with-ellipsis, since Drive file targets only support full comments.file_type=fileand a fixed placeholderanchor.block_idvalue oftest.file_extensionin the shortcut output.lark-driveskill documentation with file comment examples, supported-extension constraints, and the full-comment-only limitation.Test Plan
make buildgo test ./shortcuts/drive -count=1go test ./tests/cli_e2e/drive -run TestDriveAddCommentDryRun_File -count=1Related Issues
Summary by CodeRabbit
New Features
drive +add-commentnow accepts Drive file targets (file URLs/tokens) for supported extensions and only allows full-document comments; block-level or selection comments are rejected.Documentation
Tests