feat(drive): add +inspect shortcut for document URL inspection with wiki unwrapping#947
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:
📝 WalkthroughWalkthroughAdds shared resource parsing and metadata helpers, consolidates export title lookup, implements a new Drive "+inspect" shortcut (dry-run and execute), registers the command, adds E2E dry-run tests, and updates docs and skill references. ChangesDrive +inspect resource inspection feature
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
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 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
shortcuts/common/resource_url_test.go (1)
15-52: ⚡ Quick winAdd two negative cases to cover untested parser branches.
Please add table cases for an invalid URL parse failure (e.g.,
https://%zz) and a recognized prefix without token (e.g.,https://xxx.feishu.cn/docx/). This closes the currently untestedurl.Parseerror and empty-token return paths inParseResourceURL.🤖 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_test.go` around lines 15 - 52, Add two negative test entries to the existing tests slice in resource_url_test.go to exercise the url.Parse error and empty-token branches of ParseResourceURL: add one named like "invalid url parse" with rawURL "https://%zz" expecting wantType="" wantToken="" wantOK=false, and one named like "recognized prefix no token" with rawURL "https://xxx.feishu.cn/docx/" expecting wantType="" wantToken="" wantOK=false; this ensures ParseResourceURL's URL parse failure and the branch that returns empty token for known prefixes are covered.tests/cli_e2e/drive/drive_info_dryrun_test.go (1)
35-40: ⚡ Quick winStrengthen dry-run E2E assertions to validate params/body shape.
These tests only check API count and URL. Please also assert request structure (e.g., wiki
api.0.params.token, batch queryapi.*.body.request_docs.0.doc_token/doc_type) so the tests enforce dry-run contract fidelity.As per coding guidelines: “Dry-run E2E tests required for every shortcut change must validate request structure without calling real APIs...”.
Also applies to: 63-70, 93-97
🤖 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 35 - 40, The test currently only asserts API count and URL; update drive_info_dryrun_test.go to also validate the dry-run request shapes: add gjson assertions that api.0.params.token exists and is non-empty (for the wiki step), and for the batch_query step assert api.*.body.request_docs.0.doc_token and api.*.body.request_docs.0.doc_type exist and match expected values or are non-empty; reference the existing assertions around gjson.Get(result.Stdout, "api.#"), gjson.Get(result.Stdout, "api.0.url") and extend them similarly (e.g., gjson.Get(result.Stdout, "api.0.params.token"), gjson.Get(result.Stdout, "api.0.body.request_docs.0.doc_token") and gjson.Get(result.Stdout, "api.0.body.request_docs.0.doc_type")); apply equivalent additional assertions to the other two test blocks noted (the sections around lines 63-70 and 93-97) so all dry-run E2E checks validate request param/body shape without calling real APIs.
🤖 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_info.go`:
- Around line 68-75: The dry-run for wiki handling uses the wrong query key and
omits the POST payload shape: in the ref.Type == "wiki" branch update the
dry.GET("/open-apis/wiki/v2/spaces/get_node") call to set the real query key
"token" (not "wiki_token") and mirror the exact query param(s) used by the real
API; also expand dry.POST("/open-apis/drive/v1/metas/batch_query") to include
the expected request body shape used in production (for example the list of
node/file IDs and requested fields or whatever keys metas/batch_query actually
expects) so --dry-run validates request structure; apply the same fixes to the
other similar block around the 78-80 region.
---
Nitpick comments:
In `@shortcuts/common/resource_url_test.go`:
- Around line 15-52: Add two negative test entries to the existing tests slice
in resource_url_test.go to exercise the url.Parse error and empty-token branches
of ParseResourceURL: add one named like "invalid url parse" with rawURL
"https://%zz" expecting wantType="" wantToken="" wantOK=false, and one named
like "recognized prefix no token" with rawURL "https://xxx.feishu.cn/docx/"
expecting wantType="" wantToken="" wantOK=false; this ensures ParseResourceURL's
URL parse failure and the branch that returns empty token for known prefixes are
covered.
In `@tests/cli_e2e/drive/drive_info_dryrun_test.go`:
- Around line 35-40: The test currently only asserts API count and URL; update
drive_info_dryrun_test.go to also validate the dry-run request shapes: add gjson
assertions that api.0.params.token exists and is non-empty (for the wiki step),
and for the batch_query step assert api.*.body.request_docs.0.doc_token and
api.*.body.request_docs.0.doc_type exist and match expected values or are
non-empty; reference the existing assertions around gjson.Get(result.Stdout,
"api.#"), gjson.Get(result.Stdout, "api.0.url") and extend them similarly (e.g.,
gjson.Get(result.Stdout, "api.0.params.token"), gjson.Get(result.Stdout,
"api.0.body.request_docs.0.doc_token") and gjson.Get(result.Stdout,
"api.0.body.request_docs.0.doc_type")); apply equivalent additional assertions
to the other two test blocks noted (the sections around lines 63-70 and 93-97)
so all dry-run E2E checks validate request param/body shape without calling real
APIs.
🪄 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: 77babcd3-d967-4794-8cae-1cacfb419da6
📒 Files selected for processing (10)
shortcuts/common/drive_meta.goshortcuts/common/resource_url.goshortcuts/common/resource_url_test.goshortcuts/drive/drive_export.goshortcuts/drive/drive_export_common.goshortcuts/drive/drive_info.goshortcuts/drive/shortcuts.goshortcuts/drive/shortcuts_test.goshortcuts/sheets/lark_sheets_cell_style_and_merge.gotests/cli_e2e/drive/drive_info_dryrun_test.go
💤 Files with no reviewable changes (1)
- shortcuts/drive/drive_export_common.go
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@67e322c71a159867f7203d5a808b3334d027ceec🧩 Skill updatenpx skills add larksuite/cli#feat/drive-resolve-url -y -g |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #947 +/- ##
==========================================
+ Coverage 66.71% 67.01% +0.30%
==========================================
Files 563 570 +7
Lines 52287 53089 +802
==========================================
+ Hits 34884 35580 +696
- Misses 14509 14578 +69
- Partials 2894 2931 +37 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
♻️ Duplicate comments (2)
tests/cli_e2e/drive/drive_resolve_dryrun_test.go (1)
35-40:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDry-run E2E assertions are incomplete for request-structure validation.
These tests only assert API count and URL. Please also assert planned query/body fields (e.g., wiki
tokenquery key andmetas/batch_queryrequest_docspayload shape) so dry-run truly validates request structure.As per coding guidelines: “Dry-run E2E tests required for every shortcut change must validate request structure without calling real APIs.”
Also applies to: 63-70, 93-97
🤖 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_resolve_dryrun_test.go` around lines 35 - 40, The current dry-run assertions only check API count and URL; extend them to validate the planned request structure by asserting the query token and the batch_query payload shape: use gjson on result.Stdout to assert that "api.0.query.token" exists and equals the expected wiki token key, and that "api.0.body.request_docs" is the expected array/object shape (e.g., contains required fields like url/filetype/doc_id), and add equivalent assertions for the other ranges (use "api.1..." or appropriate indices referenced at lines 63-70 and 93-97) so the test validates both query keys and the request_docs payload structure rather than only count and URL.shortcuts/drive/drive_resolve.go (1)
68-80:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDry-run request shape does not mirror real API calls.
The dry-run plan uses
wiki_tokeninstead of the real query key (token) and omitsmetas/batch_queryrequest body shape in both branches, so--dry-runcannot validate request structure reliably.Suggested patch
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)") + Desc("[2] Batch query document metadata (title)"). + Body(map[string]interface{}{ + "request_docs": []map[string]interface{}{ + {"doc_token": "<resolved_obj_token>", "doc_type": "<resolved_obj_type>"}, + }, + }) return dry } dry.Desc("1-step: batch query document metadata") - dry.POST("/open-apis/drive/v1/metas/batch_query") + dry.POST("/open-apis/drive/v1/metas/batch_query"). + Body(map[string]interface{}{ + "request_docs": []map[string]interface{}{ + {"doc_token": ref.Token, "doc_type": ref.Type}, + }, + }) return dryAs per coding guidelines: “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 `@shortcuts/drive/drive_resolve.go` around lines 68 - 80, The dry-run uses the wrong query key and omits the POST body shape; update drive_resolve.go so the wiki branch sets the real query key (replace Set("wiki_token", ref.Token) with Set("token", ref.Token)) and add the expected request body shape for POST "/open-apis/drive/v1/metas/batch_query" in both branches (use the same body structure used by the real API call in the codebase—e.g., the JSON payload passed to the actual metas batch_query call) so the dry-run accurately mirrors the real requests and can validate request structure.
🤖 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_resolve.go`:
- Around line 68-80: The dry-run uses the wrong query key and omits the POST
body shape; update drive_resolve.go so the wiki branch sets the real query key
(replace Set("wiki_token", ref.Token) with Set("token", ref.Token)) and add the
expected request body shape for POST "/open-apis/drive/v1/metas/batch_query" in
both branches (use the same body structure used by the real API call in the
codebase—e.g., the JSON payload passed to the actual metas batch_query call) so
the dry-run accurately mirrors the real requests and can validate request
structure.
In `@tests/cli_e2e/drive/drive_resolve_dryrun_test.go`:
- Around line 35-40: The current dry-run assertions only check API count and
URL; extend them to validate the planned request structure by asserting the
query token and the batch_query payload shape: use gjson on result.Stdout to
assert that "api.0.query.token" exists and equals the expected wiki token key,
and that "api.0.body.request_docs" is the expected array/object shape (e.g.,
contains required fields like url/filetype/doc_id), and add equivalent
assertions for the other ranges (use "api.1..." or appropriate indices
referenced at lines 63-70 and 93-97) so the test validates both query keys and
the request_docs payload structure rather than only count and URL.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a443a73e-a4aa-4eb9-af18-0efe3563d9ce
📒 Files selected for processing (3)
shortcuts/drive/drive_resolve.goshortcuts/drive/shortcuts.gotests/cli_e2e/drive/drive_resolve_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 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_inspect_dryrun_test.go`:
- Around line 35-40: The test currently only asserts step count and URL via
gjson.Get(result.Stdout, "api.#") and gjson.Get(result.Stdout, "api.0.url");
extend the assertions to validate the dry-run request payloads: use gjson.Get on
result.Stdout to check api.0.params (or api.0.query) contains the wiki get_node
query key/name (e.g., "get_node") and expected query fields, and assert
api.0.body (or api.0.payload) for the /metas/batch_query step contains the
required keys/shape (e.g., expected "queries"/"targets"/"metas" array and
required subfields). Apply the same additional assertions pattern to the other
similar checks at the ranges noted (the blocks around lines 63-70 and 93-97) so
each dry-run step validates both URL and request structure.
🪄 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: de211b96-923e-4d1c-ae5b-01e9d4bdb111
📒 Files selected for processing (4)
shortcuts/drive/drive_inspect.goshortcuts/drive/shortcuts.goshortcuts/drive/shortcuts_test.gotests/cli_e2e/drive/drive_inspect_dryrun_test.go
There was a problem hiding this comment.
🧹 Nitpick comments (1)
skills/lark-drive/references/lark-drive-inspect.md (1)
20-21: ⚡ Quick winDocument the
--formatparameter in the notes section.The example shows
--format prettybut this parameter is not mentioned in the "注意事项" (notes) section. If--formatis specific to this command rather than a global CLI parameter, consider adding documentation about available format options and the default format.🤖 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/references/lark-drive-inspect.md` around lines 20 - 21, Add documentation for the --format option used by the lark-cli drive +inspect command: in the "注意事项" (notes) section of lark-drive-inspect.md, describe that --format controls output formatting, list the supported values (e.g., pretty, json, yaml or whatever options the command supports), and state the default format when the flag is omitted; reference the example command "lark-cli drive +inspect --url ... --format pretty" so readers know this flag is command-scoped rather than global.
🤖 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.
Nitpick comments:
In `@skills/lark-drive/references/lark-drive-inspect.md`:
- Around line 20-21: Add documentation for the --format option used by the
lark-cli drive +inspect command: in the "注意事项" (notes) section of
lark-drive-inspect.md, describe that --format controls output formatting, list
the supported values (e.g., pretty, json, yaml or whatever options the command
supports), and state the default format when the flag is omitted; reference the
example command "lark-cli drive +inspect --url ... --format pretty" so readers
know this flag is command-scoped rather than global.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 561dc100-24af-4ef3-bfcb-159274b43e22
📒 Files selected for processing (3)
skill-template/domains/drive.mdskills/lark-drive/SKILL.mdskills/lark-drive/references/lark-drive-inspect.md
✅ Files skipped from review due to trivial changes (2)
- skills/lark-drive/SKILL.md
- skill-template/domains/drive.md
4ba0de1 to
f8a8949
Compare
f8a8949 to
e8073af
Compare
eae8768 to
79de33a
Compare
…iki unwrapping Implements #662: `lark-cli drive +inspect --url <url>` inspects any Lark/Feishu document URL to get its type, title, and canonical token, with automatic wiki URL unwrapping via get_node API. - Add ParseResourceURL (inverse of BuildResourceURL) in common - Extract FetchDriveMetaTitle as public shared helper - Add drive +inspect shortcut with wiki unwrapping support - Add skill reference docs and update SKILL.md - Dry-run E2E tests for docx URL, wiki URL, and bare token
79de33a to
201e805
Compare
ParseResourceURL is a general-purpose URL parser that should not hardcode domain lists — future Lark domains would silently break. Move isLarkHost/larkHostSuffixes to drive_inspect.go where host validation is a business decision of the +inspect command. Add E2E test for non-Lark host with Lark-like path.
Lark supports custom enterprise domains, so a hardcoded suffix list can never be exhaustive and would falsely reject valid URLs. Path-based matching in ParseResourceURL is sufficient; invalid URLs will fail naturally at the API call stage.
Summary
Implements #662:
lark-cli drive +inspect --url <url>inspects any Lark/Feishu document URL to get its type, title, and canonical 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 +inspect?drivenotdocs: URL inspection is a cross-document capability (docx, sheet, bitable, wiki, file, folder, mindnote, slides) backed bydrive.metas.batch_query, not specific to document content operations.+inspectnot+search:+inspectexamines a specific URL, while+searchdoes full-text search — no ambiguity.Test Plan
go build ./...— cleango test -race ./shortcuts/...— all passgo vet ./...— cleangofmt -l .— cleango mod tidy— no changesSummary by CodeRabbit
New Features
Tests
Documentation