feat(drive): add +resolve shortcut for document URL resolution with wiki unwrapping - #946
Closed
fangshuyu-768 wants to merge 4 commits into
Closed
feat(drive): add +resolve shortcut for document URL resolution with wiki unwrapping#946fangshuyu-768 wants to merge 4 commits into
fangshuyu-768 wants to merge 4 commits into
Conversation
Two DryRun functions in the sheets shortcuts called json.Unmarshal without checking the return value. This looks like a bug, but Validate already parses and validates the same --style / --data JSON before DryRun runs, so the error is structurally impossible at this point. Use _ = assignment + comment to silence the unchecked-error lint warning and make the safety invariant explicit to future readers.
…rapping Implements Issue #662: `lark-cli docs +info --url '<url>'` resolves any Lark/Feishu document URL to {type, title, token} with wiki unwrapping support. Algorithm: 1. Parse URL path → {type, token} via new ParseResourceURL (inverse of BuildResourceURL) 2. If type is wiki: call get_node API to unwrap to underlying doc 3. Call drive.metas.batch_query to verify and get title 4. Output JSON with type, title, token, url, and optional wiki_node Also extracts FetchDriveMetaTitle to common package to avoid duplication.
github-actions
Bot
added
domain/ccm
|
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 (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThis PR adds resource-URL parsing and tests, a shared Drive metadata title helper, and a new ChangesDrive +info Command and Resource Metadata Utilities
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 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 |
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@a965b8dc9f4475528487db35c90690d1ce83a617🧩 Skill updatenpx skills add larksuite/cli#pr-935 -y -g |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #946 +/- ##
==========================================
+ Coverage 65.92% 66.62% +0.69%
==========================================
Files 523 565 +42
Lines 49692 52545 +2853
==========================================
+ Hits 32758 35006 +2248
- Misses 14134 14636 +502
- Partials 2800 2903 +103 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/common/resource_url.go`:
- Around line 106-113: ParseResourceURL currently accepts any host if the path
matches; restrict it to only real Lark/Feishu web URLs by validating the parsed
URL's scheme and hostname before checking path mappings. In ParseResourceURL
(using rawURL and the parsed u), ensure u.Scheme == "https" and that
u.Hostname() matches one of the allowed domains or suffixes for Lark/Feishu
(e.g., exact hosts or suffixes like "larksuite.com", "feishu.cn", and any known
subdomain patterns); if the host check fails return ResourceRef{}, false. Do
this check before iterating urlPathToType so only URLs from approved domains are
considered.
In `@shortcuts/doc/docs_info.go`:
- Around line 69-73: The dry-run setup for the wiki resolve endpoint currently
sets the query param as "wiki_token" via Set("wiki_token", ref.Token) on the
dry.GET("/open-apis/wiki/v2/spaces/get_node") call, but real execution uses
"token"; update the dry-run to use Set("token", ref.Token) so the dry-run
request shape matches Execute (modify the Set call associated with
dry.GET("/open-apis/wiki/v2/spaces/get_node")).
🪄 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: d161c02f-9051-4292-b671-a8b46b553078
📒 Files selected for processing (9)
shortcuts/common/drive_meta.goshortcuts/common/resource_url.goshortcuts/common/resource_url_test.goshortcuts/doc/docs_info.goshortcuts/doc/shortcuts.goshortcuts/drive/drive_export.goshortcuts/drive/drive_export_common.goshortcuts/sheets/lark_sheets_cell_style_and_merge.gotests/cli_e2e/docs/docs_info_dryrun_test.go
💤 Files with no reviewable changes (1)
- shortcuts/drive/drive_export_common.go
Comment on lines
+106
to
+113
| u, err := url.Parse(rawURL) | ||
| if err != nil { | ||
| return ResourceRef{}, false | ||
| } | ||
|
|
||
| path := u.Path | ||
|
|
||
| for _, mapping := range urlPathToType { |
There was a problem hiding this comment.
Restrict parsing to real Lark/Feishu web URLs.
ParseResourceURL currently accepts any host as long as the path matches (e.g., https://example.com/docx/...), which makes docs +info --url validation overly permissive.
💡 Proposed fix
func ParseResourceURL(rawURL string) (ResourceRef, bool) {
rawURL = strings.TrimSpace(rawURL)
if rawURL == "" {
return ResourceRef{}, false
}
u, err := url.Parse(rawURL)
if err != nil {
return ResourceRef{}, false
}
+ if u.Scheme != "https" && u.Scheme != "http" {
+ return ResourceRef{}, false
+ }
+ host := strings.ToLower(strings.TrimSpace(u.Hostname()))
+ isFeishu := host == "feishu.cn" || strings.HasSuffix(host, ".feishu.cn")
+ isLark := host == "larksuite.com" || strings.HasSuffix(host, ".larksuite.com")
+ if !isFeishu && !isLark {
+ return ResourceRef{}, false
+ }
path := u.Path📝 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
| u, err := url.Parse(rawURL) | |
| if err != nil { | |
| return ResourceRef{}, false | |
| } | |
| path := u.Path | |
| for _, mapping := range urlPathToType { | |
| u, err := url.Parse(rawURL) | |
| if err != nil { | |
| return ResourceRef{}, false | |
| } | |
| if u.Scheme != "https" && u.Scheme != "http" { | |
| return ResourceRef{}, false | |
| } | |
| host := strings.ToLower(strings.TrimSpace(u.Hostname())) | |
| isFeishu := host == "feishu.cn" || strings.HasSuffix(host, ".feishu.cn") | |
| isLark := host == "larksuite.com" || strings.HasSuffix(host, ".larksuite.com") | |
| if !isFeishu && !isLark { | |
| return ResourceRef{}, false | |
| } | |
| path := u.Path | |
| for _, mapping := range urlPathToType { |
🤖 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/common/resource_url.go` around lines 106 - 113, ParseResourceURL
currently accepts any host if the path matches; restrict it to only real
Lark/Feishu web URLs by validating the parsed URL's scheme and hostname before
checking path mappings. In ParseResourceURL (using rawURL and the parsed u),
ensure u.Scheme == "https" and that u.Hostname() matches one of the allowed
domains or suffixes for Lark/Feishu (e.g., exact hosts or suffixes like
"larksuite.com", "feishu.cn", and any known subdomain patterns); if the host
check fails return ResourceRef{}, false. Do this check before iterating
urlPathToType so only URLs from approved domains are considered.
Comment on lines
+69
to
+73
| dry.Desc("2-step: resolve wiki node, then batch query metadata") | ||
| dry.GET("/open-apis/wiki/v2/spaces/get_node"). | ||
| Desc("[1] Resolve wiki node to underlying document"). | ||
| Set("wiki_token", ref.Token) | ||
| dry.POST("/open-apis/drive/v1/metas/batch_query"). |
There was a problem hiding this comment.
Align DryRun wiki request params with Execute behavior.
Line 72 uses wiki_token, but Execute sends query param token for the same endpoint. Dry-run should mirror the actual request shape.
💡 Proposed fix
if ref.Type == "wiki" {
dry.Desc("2-step: resolve wiki node, then batch query metadata")
dry.GET("/open-apis/wiki/v2/spaces/get_node").
Desc("[1] Resolve wiki node to underlying document").
- Set("wiki_token", ref.Token)
+ Params(map[string]interface{}{"token": ref.Token})
dry.POST("/open-apis/drive/v1/metas/batch_query").
Desc("[2] Batch query document metadata (title)")
return dry
}📝 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
| dry.Desc("2-step: resolve wiki node, then batch query metadata") | |
| dry.GET("/open-apis/wiki/v2/spaces/get_node"). | |
| Desc("[1] Resolve wiki node to underlying document"). | |
| Set("wiki_token", ref.Token) | |
| dry.POST("/open-apis/drive/v1/metas/batch_query"). | |
| dry.Desc("2-step: resolve wiki node, then batch query metadata") | |
| dry.GET("/open-apis/wiki/v2/spaces/get_node"). | |
| Desc("[1] Resolve wiki node to underlying document"). | |
| Params(map[string]interface{}{"token": ref.Token}) | |
| dry.POST("/open-apis/drive/v1/metas/batch_query"). |
🤖 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/doc/docs_info.go` around lines 69 - 73, The dry-run setup for the
wiki resolve endpoint currently sets the query param as "wiki_token" via
Set("wiki_token", ref.Token) on the
dry.GET("/open-apis/wiki/v2/spaces/get_node") call, but real execution uses
"token"; update the dry-run to use Set("token", ref.Token) so the dry-run
request shape matches Execute (modify the Set call associated with
dry.GET("/open-apis/wiki/v2/spaces/get_node")).
URL resolution is a cross-document capability (docx, sheet, bitable, wiki, file, etc.) backed by drive.metas.batch_query, so it belongs under the drive service, not docs.
fangshuyu-768
changed the title
The command resolves a document URL to its type/title/token, so +resolve is more specific and actionable than the generic +info.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
shortcuts/drive/drive_info.go (1)
70-72:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse the same wiki query key in DryRun and Execute.
Line 72 sends
wiki_token, but Execute sendstoken(Line 107). Dry-run should mirror the real request shape.💡 Proposed fix
if ref.Type == "wiki" { dry.Desc("2-step: resolve wiki node, then batch query metadata") dry.GET("/open-apis/wiki/v2/spaces/get_node"). Desc("[1] Resolve wiki node to underlying document"). - Set("wiki_token", ref.Token) + Set("token", ref.Token) dry.POST("/open-apis/drive/v1/metas/batch_query"). Desc("[2] Batch query document metadata (title)") return dry }🤖 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_info.go` around lines 70 - 72, The dry-run request for the wiki node uses a different query key than the real execute call; change the dry GET setup so it mirrors the real request shape by replacing Set("wiki_token", ref.Token) with the same key used by Execute (i.e., Set("token", ref.Token)) for the route dry.GET("/open-apis/wiki/v2/spaces/get_node") so the DryRun and Execute use identical query names.
🤖 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_info_dryrun_test.go`:
- Around line 62-70: The test currently only asserts the first dry-run step URL
via gjson.Get(result.Stdout, "api.0.url") but doesn't verify request params;
update the test in drive_info_dryrun_test.go to assert the wiki get_node params
include the expected token key/value by adding an assertion against the dry-run
JSON (use gjson.Get on result.Stdout, e.g. "api.0.params.<tokenKey>" or the
appropriate params path) so the first step not only has
"/open-apis/wiki/v2/spaces/get_node" but also contains the expected wiki token
param/value, keeping this as a pure dry-run assertion (no real API calls).
---
Duplicate comments:
In `@shortcuts/drive/drive_info.go`:
- Around line 70-72: The dry-run request for the wiki node uses a different
query key than the real execute call; change the dry GET setup so it mirrors the
real request shape by replacing Set("wiki_token", ref.Token) with the same key
used by Execute (i.e., Set("token", ref.Token)) for the route
dry.GET("/open-apis/wiki/v2/spaces/get_node") so the DryRun and Execute use
identical query names.
🪄 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: 4f1433a4-434e-46d7-825a-b23565d9bace
📒 Files selected for processing (4)
shortcuts/drive/drive_info.goshortcuts/drive/shortcuts.goshortcuts/drive/shortcuts_test.gotests/cli_e2e/drive/drive_info_dryrun_test.go
Comment on lines
+62
to
+70
| // Should plan two steps: get_node + batch_query. | ||
| require.Equal(t, int64(2), gjson.Get(result.Stdout, "api.#").Int(), | ||
| "expected exactly 2 dry-run API steps for wiki URL, stdout:\n%s", result.Stdout) | ||
| require.Equal(t, "/open-apis/wiki/v2/spaces/get_node", | ||
| gjson.Get(result.Stdout, "api.0.url").String(), | ||
| "expected get_node as first step, stdout:\n%s", result.Stdout) | ||
| require.Equal(t, "/open-apis/drive/v1/metas/batch_query", | ||
| gjson.Get(result.Stdout, "api.1.url").String(), | ||
| "expected batch_query as second step, stdout:\n%s", result.Stdout) |
There was a problem hiding this comment.
Assert wiki get_node request params in dry-run output.
This test only checks URL order, so a wrong query key can still pass. Please also assert the first step includes the expected wiki token param key/value.
As per coding guidelines tests/cli_e2e/**/*_test.go: Dry-run E2E tests required for every shortcut change must validate request structure without calling real APIs.
🤖 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_info_dryrun_test.go` around lines 62 - 70, The test
currently only asserts the first dry-run step URL via gjson.Get(result.Stdout,
"api.0.url") but doesn't verify request params; update the test in
drive_info_dryrun_test.go to assert the wiki get_node params include the
expected token key/value by adding an assertion against the dry-run JSON (use
gjson.Get on result.Stdout, e.g. "api.0.params.<tokenKey>" or the appropriate
params path) so the first step not only has "/open-apis/wiki/v2/spaces/get_node"
but also contains the expected wiki token param/value, keeping this as a pure
dry-run assertion (no real API calls).
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
2 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
Implements #662:
lark-cli drive +resolve --url <url>resolves any Lark/Feishu document URL to{type, title, token}with wiki unwrapping support.Algorithm
{type, token}via newParseResourceURL(inverse of existingBuildResourceURL)wiki: callGET /open-apis/wiki/v2/spaces/get_nodeto unwrap to underlying documentPOST /open-apis/drive/v1/metas/batch_queryto verify and get title{input_url, type, title, token, url, wiki_node?}Usage
Why
drive +resolve?drivenotdocs: URL resolution is a cross-document capability (docx, sheet, bitable, wiki, file, folder, mindnote, slides) backed bydrive.metas.batch_query, not specific to document content operations.+resolvenot+info: The command resolves a URL to its canonical identity;+resolveis specific and actionable, while+infois too generic.Changes
shortcuts/common/resource_url.goResourceRefstruct +ParseResourceURLfunctionshortcuts/common/resource_url_test.goshortcuts/common/drive_meta.goFetchDriveMetaTitleas public helpershortcuts/drive/drive_info.godrive +resolveshortcut implementationshortcuts/drive/shortcuts.goDriveInfoshortcuts/drive/shortcuts_test.go+resolveto expected commandsshortcuts/drive/drive_export.gocommon.FetchDriveMetaTitleshortcuts/drive/drive_export_common.gofetchDriveMetaTitle(moved to common)tests/cli_e2e/drive/drive_info_dryrun_test.goTest Plan
go build ./...— cleango test -race ./shortcuts/...— all passgo vet ./...— cleangofmt -l .— cleango mod tidy— no changes