feat(docs): upload local image and source resources - #1864
feat(docs): upload local image and source resources#1864SunPeiYang996 wants to merge 10 commits into
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:
📝 WalkthroughWalkthroughChangesThe PR adds local image and attachment handling for Docs V2 create/update flows, including parsing, validation, upload, token binding, cleanup, dry-run support, pacing, scope enforcement, tests, and documentation. Local document resources
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant DocsCLI
participant DocsOpenAPI
participant MediaAPI
DocsCLI->>DocsCLI: parse and rewrite local resources
DocsCLI->>DocsOpenAPI: create or update document with placeholders
DocsOpenAPI-->>DocsCLI: return placeholder blocks
DocsCLI->>MediaAPI: upload local files and images
MediaAPI-->>DocsCLI: return media tokens
DocsCLI->>DocsOpenAPI: bind tokens and clean placeholders
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
github-actions
Bot
added
domain/ccm
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@1a79483ac529b9aac97ebbcc43929646e80ea40d🧩 Skill updatenpx skills add larksuite/cli#sun/lark-cli-local-resources -y -g |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1864 +/- ##
==========================================
- Coverage 75.19% 75.08% -0.11%
==========================================
Files 914 915 +1
Lines 96789 98367 +1578
==========================================
+ Hits 72778 73858 +1080
- Misses 18413 18785 +372
- Partials 5598 5724 +126 ☔ View full report in Codecov by Harness. 🚀 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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
shortcuts/doc/doc_media_insert.go (1)
524-561: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winMove recovery guidance into
.WithHint(...)for the"doc"case.Line 534 bakes recovery guidance ("use a docx token/URL or a wiki URL that resolves to docx") into the main error message instead of
.WithHint(...), while the sibling wiki/default branches keep the message fact-only. As per coding guidelines: "recovery guidance goes in.WithHint(...)".♻️ Proposed fix
case "doc": - return "", errs.NewValidationError(errs.SubtypeInvalidArgument, "this document operation only supports docx documents; use a docx token/URL or a wiki URL that resolves to docx").WithParam("--doc") + return "", errs.NewValidationError(errs.SubtypeInvalidArgument, "this document operation only supports docx documents"). + WithParam("--doc"). + WithHint("use a docx token/URL or a wiki URL that resolves to docx")🤖 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/doc_media_insert.go` around lines 524 - 561, Update the "doc" branch of resolveDocxDocumentID so the main validation error remains fact-focused, and move the existing recovery guidance into a chained WithHint call. Keep the existing error subtype and --doc parameter unchanged.Source: Coding guidelines
🤖 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/drive_media_upload.go`:
- Around line 8-12: Add tests around waitDriveMediaMultipartRequest covering
both MinRequestInterval pauses: prepare→part and part→finish. Verify each pause
enforces the configured interval, and add a cancellation case confirming the
wait exits promptly when the context is canceled.
In `@skills/lark-doc/references/lark-doc-update.md`:
- Around line 59-60: Keep the warning text beginning with “本地图片和附件” as one
continuous blockquote by removing the blank line inside it or adding the
blockquote marker to that line, preventing the warning from being split into
separate blocks.
In `@tests/cli_e2e/docs/coverage.md`:
- Around line 26-34: Update the command descriptions in the coverage table cells
for docs +fetch and docs +update to escape or replace the alternative separators
in “markdown|xml” and “overwrite|append”. Preserve the documented command
options while ensuring each row remains exactly six Markdown table columns and
satisfies MD056.
---
Outside diff comments:
In `@shortcuts/doc/doc_media_insert.go`:
- Around line 524-561: Update the "doc" branch of resolveDocxDocumentID so the
main validation error remains fact-focused, and move the existing recovery
guidance into a chained WithHint call. Keep the existing error subtype and --doc
parameter unchanged.
🪄 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: ea3862b6-8026-45fd-872d-1e380e5b465f
📒 Files selected for processing (19)
cmd/root_test.goshortcuts/common/drive_media_upload.goshortcuts/doc/doc_media_insert.goshortcuts/doc/doc_media_upload.goshortcuts/doc/docs_create.goshortcuts/doc/docs_create_v2.goshortcuts/doc/docs_update.goshortcuts/doc/docs_update_v2.goshortcuts/doc/helpers.goshortcuts/doc/html5_block_resources.goshortcuts/doc/local_doc_resources.goshortcuts/doc/local_doc_resources_test.goskills/lark-doc/references/lark-doc-create.mdskills/lark-doc/references/lark-doc-md.mdskills/lark-doc/references/lark-doc-update.mdskills/lark-doc/references/lark-doc-xml.mdtests/cli_e2e/docs/coverage.mdtests/cli_e2e/docs/docs_local_resources_dryrun_test.gotests/cli_e2e/docs/docs_local_resources_workflow_test.go
| "context" | ||
| "fmt" | ||
| "io" | ||
| "net/http" | ||
| "time" |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for tests covering the multipart pacing / MinRequestInterval behavior
rg -n 'MinRequestInterval\|waitDriveMediaMultipartRequest\|MinRequestInterval' --type go -g '*_test.go'Repository: larksuite/cli
Length of output: 151
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the target file and surrounding tests.
git ls-files 'shortcuts/common/drive_media_upload.go' 'shortcuts/common/*_test.go' 'shortcuts/**/*_test.go' | sed -n '1,200p'
echo '--- outline ---'
ast-grep outline shortcuts/common/drive_media_upload.go --view expanded
echo '--- search for pacing-related tests and symbols ---'
rg -n 'MinRequestInterval|waitDriveMediaMultipartRequest|multipart.*request|drive media upload|drive_media_upload' shortcuts -g '*_test.go' -g '*.go'Repository: larksuite/cli
Length of output: 11172
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- drive_media_upload_test.go outline ---'
ast-grep outline shortcuts/common/drive_media_upload_test.go --view expanded
echo '--- drive_media_upload_typed_test.go outline ---'
ast-grep outline shortcuts/common/drive_media_upload_typed_test.go --view expanded
echo '--- relevant test snippets ---'
sed -n '1,260p' shortcuts/common/drive_media_upload_test.go
echo '--- typed tests ---'
sed -n '1,260p' shortcuts/common/drive_media_upload_typed_test.goRepository: larksuite/cli
Length of output: 17589
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- all references to MinRequestInterval / waitDriveMediaMultipartRequest ---'
rg -n 'MinRequestInterval|waitDriveMediaMultipartRequest' shortcuts -g '*.go'
echo '--- continue drive_media_upload_typed_test.go ---'
sed -n '260,360p' shortcuts/common/drive_media_upload_typed_test.goRepository: larksuite/cli
Length of output: 2716
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- tests that exercise doc media upload flow ---'
rg -n 'upload_prepare|upload_part|upload_finish|MinRequestInterval|localDocResourceUploadInterval|wait.*Request' shortcuts/doc -g '*_test.go'
echo '--- test files in shortcuts/doc ---'
git ls-files 'shortcuts/doc/*_test.go' | sed -n '1,200p'Repository: larksuite/cli
Length of output: 4190
Add tests for multipart pacing. Existing multipart tests cover the request bodies, but not the new MinRequestInterval wait path in waitDriveMediaMultipartRequest or its context-cancellation behavior. Add coverage for the prepare→part and part→finish pauses.
🤖 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/drive_media_upload.go` around lines 8 - 12, Add tests around
waitDriveMediaMultipartRequest covering both MinRequestInterval pauses:
prepare→part and part→finish. Verify each pause enforces the configured
interval, and add a cancellation case confirming the wait exits promptly when
the context is canceled.
Source: Coding guidelines
| > 本地图片和附件只允许用于 `append` 或 `block_insert_after`。`str_replace` 不会创建资源 block，而 `block_replace` / `overwrite` 一旦在后续上传绑定失败会先破坏旧内容，因此 CLI 会在写文档前拒绝这些组合。 | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Keep the warning in one blockquote.
Line [60] is a blank line inside the blockquote, triggering MD028 and breaking the warning into separate blockquotes. Remove the blank line or prefix it with >.
Proposed fix
> 本地图片和附件只允许用于 `append` 或 `block_insert_after`。`str_replace` 不会创建资源 block，而 `block_replace` / `overwrite` 一旦在后续上传绑定失败会先破坏旧内容，因此 CLI 会在写文档前拒绝这些组合。
-
+>
> **匹配范围：**🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 60-60: Blank line inside blockquote
(MD028, no-blanks-blockquote)
🤖 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-doc/references/lark-doc-update.md` around lines 59 - 60, Keep the
warning text beginning with “本地图片和附件” as one continuous blockquote by removing
the blank line inside it or adding the blockquote marker to that line,
preventing the warning from being split into separate blocks.
Source: Linters/SAST tools
d960f15 to
f7401bc
Compare
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 `@shortcuts/doc/html5_block_resources.go`:
- Around line 110-112: Update the reference-map handling in the PUT body
construction to check runtime.Changed("reference-map") rather than
len(input.ReferenceMap) > 0, and include input.ReferenceMap even when it is an
explicitly parsed empty map so the update clears existing mappings.
🪄 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 Plus
Run ID: 06e7bd6c-47e3-4827-a3f5-a1b6500f1e03
📒 Files selected for processing (12)
cmd/root_test.goshortcuts/common/drive_media_upload.goshortcuts/doc/doc_media_insert.goshortcuts/doc/doc_media_upload.goshortcuts/doc/docs_create.goshortcuts/doc/docs_create_v2.goshortcuts/doc/docs_update.goshortcuts/doc/docs_update_v2.goshortcuts/doc/helpers.goshortcuts/doc/html5_block_resources.goshortcuts/doc/local_doc_resources.goshortcuts/doc/local_doc_resources_test.go
🚧 Files skipped from review as they are similar to previous changes (11)
- shortcuts/doc/docs_create.go
- shortcuts/doc/doc_media_upload.go
- cmd/root_test.go
- shortcuts/doc/helpers.go
- shortcuts/doc/doc_media_insert.go
- shortcuts/common/drive_media_upload.go
- shortcuts/doc/docs_update.go
- shortcuts/doc/docs_update_v2.go
- shortcuts/doc/docs_create_v2.go
- shortcuts/doc/local_doc_resources.go
- shortcuts/doc/local_doc_resources_test.go
| if len(input.ReferenceMap) > 0 { | ||
| body["reference_map"] = input.ReferenceMap | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Preserve explicitly empty reference maps on update.
An explicit {} should clear existing mappings, but len(input.ReferenceMap) > 0 omits it from the PUT body. Use the changed-state (runtime.Changed("reference-map")) and preserve the empty map parsed by the resolver.
As per coding guidelines, requested values must not be silently discarded.
Proposed fix
- if len(input.ReferenceMap) > 0 {
+ if runtime.Changed("reference-map") {
body["reference_map"] = input.ReferenceMap
}📝 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.
| if len(input.ReferenceMap) > 0 { | |
| body["reference_map"] = input.ReferenceMap | |
| } | |
| if runtime.Changed("reference-map") { | |
| body["reference_map"] = input.ReferenceMap | |
| } |
🤖 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/html5_block_resources.go` around lines 110 - 112, Update the
reference-map handling in the PUT body construction to check
runtime.Changed("reference-map") rather than len(input.ReferenceMap) > 0, and
include input.ReferenceMap even when it is an explicitly parsed empty map so the
update clears existing mappings.
Source: Coding guidelines
47b2009 to
1a79483
Compare
1 participant
Summary
Add first-class local resource support to
lark-cli docsv2:<img path="@relative/path.png">and<source path="@relative/file.pdf">plus<source path="@relative/file.pdf">docs +createanddocs +update(append/block_insert_after)The CLI validates local files, sends only opaque placeholder markers to Docs AI, uploads media after receiving distinct placeholder block IDs, and binds the uploaded tokens. Real local paths never leave the CLI.
What changed
<figure><source/></figure>blocks.lark-docskill documentation and CLI docs.Review regression coverage
Permanent live E2E assertions cover:
width/heightvalues of zero, negative and non-numeric;sizevalues of zero, negative and non-numeric;Safety and compatibility
Cross-repository dependency chain
Current exact heads:
c89354123e6c52e2c9bd6a888bf4ce3c1982980f4fddd72e615de96ccba729c47bfbd7a15484aa54d7e292caac118815332ffeaefd35128941dcc2e0bc22c5f0aab8891de00ef157699ff889d2e63811Verification
Pre-PPE:
make unit-test: pass.go vet ./...: pass.make build: pass (v1.0.77-23-gc8935412).go mod tidy -diff,go mod verify, remote build and Codebase CI: pass.PPE:
ppe_sun_ai_test1.0.0.5992081961573783420928373113883E2E report:
https://bytedance.larkoffice.com/docx/LFg3db8YZo5jh3xqSVhcnanHnYb
Rollback
Revert this PR to remove CLI-side local resource detection/upload/bind behavior. Existing documents contain normal image/file blocks and remain readable. If the server chain is also rolled back, revert in reverse order: ai_edit, SDK, then Engine.