feat(doc-skill): document upload-then-move workflow for inserting images into container blocks#966
Closed
fangshuyu-768 wants to merge 4 commits into
Closed
feat(doc-skill): document upload-then-move workflow for inserting images into container blocks#966fangshuyu-768 wants to merge 4 commits into
fangshuyu-768 wants to merge 4 commits into
Conversation
Add regression tests for drive +sync and related status/push helpers so the rebased sync workflow keeps its conflict handling, parent-folder errors, and non-interactive stdin preflight covered.
Move the nested overwrite parent-token change out of feat/drive-sync so this PR stays focused on sync behavior and coverage.
…ges into container blocks The +media-insert command only inserts images at the document top level. This PR documents the two-step "upload then move" workflow that enables inserting local images into container blocks (table cells, callouts, grid columns) using block_move_after, which supports cross-parent moves. Closes #503
📝 WalkthroughWalkthroughThis PR introduces a Drive ChangesDrive +sync implementation and testing
Documentation updates for drive and doc features
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
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)
⚔️ Resolve merge conflicts
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: 4
🤖 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-doc/references/lark-doc-media-insert.md`:
- Around line 109-112: The fenced code block containing the two-step
instructions ("Step 1: +media-insert ..." and "Step 2: +update block_move_after
...") is missing a language tag which triggers markdownlint MD040; update that
fenced block to include a language identifier such as "text" (e.g., change the
opening ``` to ```text) so the block is language-labelled and the linter warning
is resolved while preserving the existing content referencing +media-insert and
+update block_move_after.
In `@skills/lark-doc/references/lark-doc-xml.md`:
- Line 16: Update the example text so it matches the capabilities listed for the
`<callout>` component: change the example/note that currently lists
"text/title/list/checkbox/quote" to include "image" (or reorder to
"text/title/list/checkbox/quote/image") so the example section and the table row
for `<callout>` are consistent; locate the example/note referencing `<callout>`
in the example section and edit that sentence to include `image` among the
supported child block types.
In `@skills/lark-drive/references/lark-drive-push.md`:
- Line 70: Remove the accidental duplicate table rows and repeated paragraphs
that describe the `--if-exists` option (the duplicated `--if-exists` enum row
and its adjacent explanatory paragraphs); keep a single canonical row and one
explanatory paragraph, delete the redundant duplicates elsewhere in the
document, and scan the rest of the file for other repeated instances of the same
`--if-exists` content to remove so only one authoritative description remains.
In `@skills/lark-drive/SKILL.md`:
- Line 247: The table row for the `+sync` command contains unescaped pipe
characters inside inline code (e.g.,
`--on-conflict=remote-wins|local-wins|keep-both|ask` and `--on-duplicate-remote`
options), which breaks Markdown table parsing; edit the SKILL.md table cell for
`+sync` so pipes inside inline code are escaped (e.g.,
`--on-conflict=remote-wins\|local-wins\|keep-both\|ask` and
`--on-duplicate-remote=fail\|newest\|oldest`) or otherwise ensure the entire
option strings are wrapped/escaped to prevent `|` from being treated as column
separators (target the `+sync` table row and the inline code spans for
`--on-conflict` and `--on-duplicate-remote`).
🪄 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: 26d0f0cf-f3f4-4f99-bb12-39366ee99516
📒 Files selected for processing (11)
shortcuts/drive/drive_pull_test.goshortcuts/drive/drive_status_test.goshortcuts/drive/drive_sync.goshortcuts/drive/drive_sync_test.goshortcuts/drive/shortcuts.goshortcuts/drive/shortcuts_test.goskills/lark-doc/SKILL.mdskills/lark-doc/references/lark-doc-media-insert.mdskills/lark-doc/references/lark-doc-xml.mdskills/lark-drive/SKILL.mdskills/lark-drive/references/lark-drive-push.md
💤 Files with no reviewable changes (1)
- shortcuts/drive/drive_pull_test.go
Comment on lines
+109
to
+112
| ``` | ||
| Step 1: +media-insert 上传图片到文档末尾 → 得到 img block_id | ||
| Step 2: +update block_move_after 将图片移入目标容器 | ||
| ``` |
There was a problem hiding this comment.
Add a language to the fenced block to satisfy markdown lint.
Line 109 uses a fenced code block without language, which triggers MD040 from markdownlint.
Suggested diff
-```
+```text
Step 1: +media-insert 上传图片到文档末尾 → 得到 img block_id
Step 2: +update block_move_after 将图片移入目标容器</details>
<details>
<summary>🧰 Tools</summary>
<details>
<summary>🪛 markdownlint-cli2 (0.22.1)</summary>
[warning] 109-109: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
</details>
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
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-media-insert.md around lines 109 - 112,
The fenced code block containing the two-step instructions ("Step 1:
+media-insert ..." and "Step 2: +update block_move_after ...") is missing a
language tag which triggers markdownlint MD040; update that fenced block to
include a language identifier such as "text" (e.g., change the opening ``` to
while preserving the existing content referencing +media-insert and +update
block_move_after.
| |标签|说明|关键属性| | ||
| |-|-|-| | ||
| | `<callout>` | 高亮框，子块仅支持文本、标题、列表、待办、引用 | `emoji`(默认 bulb), `background-color`, `border-color`, `text-color` | | ||
| | `<callout>` | 高亮框，子块支持文本、标题、列表、待办、引用、图片 | `emoji`(默认 bulb), `background-color`, `border-color`, `text-color` | |
There was a problem hiding this comment.
Synchronize callout capability text with the example section.
Line 16 now states <callout> supports image child blocks, but the example note at Line 123 still says it only supports text/title/list/checkbox/quote. This contradiction can mislead users about allowed content.
Suggested diff
- <p>高亮框内容，子块仅支持文本/标题/列表/待办/引用</p>
+ <p>高亮框内容，子块支持文本/标题/列表/待办/引用/图片</p>🤖 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-xml.md` at line 16, Update the example
text so it matches the capabilities listed for the `<callout>` component: change
the example/note that currently lists "text/title/list/checkbox/quote" to
include "image" (or reorder to "text/title/list/checkbox/quote/image") so the
example section and the table row for `<callout>` are consistent; locate the
example/note referencing `<callout>` in the example section and edit that
sentence to include `image` among the supported child block types.
| | `--local-dir` | 是 | path | 本地根目录（**必须是 cwd 的相对路径**；绝对路径或逃出 cwd 的相对路径会被 CLI 直接拒绝） | | ||
| | `--folder-token` | 是 | string | 目标 Drive 文件夹 token | | ||
| | `--if-exists` | 否 | enum | 远端文件已存在时的策略：`skip`（**默认**，安全）/ `smart`（用于重复增量同步；当远端 `modified_time` 已匹配或更新时跳过上传，否则继续走覆盖路径）/ `overwrite`（依赖灰度后端协议，详见"覆盖语义"） | | ||
| | `--if-exists` | 否 | enum | 远端文件已存在时的策略：`skip`（**默认**，安全）/ `smart`（用于重复增量同步；当远端 `modified_time` 已匹配或更新时跳过上传，否则继续走覆盖路径）/ `overwrite`（依赖灰度后端协议，详见"覆盖语义"） | |
There was a problem hiding this comment.
Remove accidental duplicate lines introduced in this edit.
These lines repeat content already present immediately next to them (same --if-exists row and same paragraphs), which adds noise and can mislead readers.
Suggested patch
-| `--if-exists` | 否 | enum | 远端文件已存在时的策略：`skip`（**默认**，安全）/ `smart`（用于重复增量同步；当远端 `modified_time` 已匹配或更新时跳过上传，否则继续走覆盖路径）/ `overwrite`（依赖灰度后端协议，详见"覆盖语义"） |
@@
-- 已存在的远端文件按 `--if-exists` 决定 `overwrite` / `smart` / `skip`。其中 `smart` 是**增量优化模式**：只要远端 `modified_time` 在同等时间精度下已经等于或晚于本地 mtime，就跳过上传；时间戳缺失/非法时会退回安全路径继续上传，不会盲跳。**但如果远端更旧，`smart` 会继续走和 `overwrite` 相同的覆盖路径，因此也继承同样的 rollout / version 返回 caveat。** 想做 `keep-both` 这类的仍需自行改名再 push。
@@
-`--if-exists=smart` 是给“重复跑同步”的场景增加的增量优化：当远端 `modified_time` 在同等时间精度下已经等于或晚于本地 mtime 时，命令会把该文件计为 `skipped`；时间戳缺失、非法或更旧时，则继续走正常上传/覆盖路径。**也就是说，只要 smart 判定“远端不够新”，它就会进入与 `--if-exists=overwrite` 相同的覆盖实现，因此在未 rollout version 字段的 tenant 上仍可能非零失败。**
@@
-- 默认 `skip` 下，已存在的远端文件一律不碰；`overwrite` 下，重复跑会重传所有命中的同名文件；`smart` 下会按 `modified_time` 跳过已对齐的远端文件，但对“远端更旧”的文件仍会进入覆盖路径，因此它减少的是**不必要的重传**，不是把覆盖风险完全拿掉。Also applies to: 80-80, 87-87, 134-134
🤖 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-push.md` at line 70, Remove the
accidental duplicate table rows and repeated paragraphs that describe the
`--if-exists` option (the duplicated `--if-exists` enum row and its adjacent
explanatory paragraphs); keep a single canonical row and one explanatory
paragraph, delete the redundant duplicates elsewhere in the document, and scan
the rest of the file for other repeated instances of the same `--if-exists`
content to remove so only one authoritative description remains.
| | [`+download`](references/lark-drive-download.md) | Download a file from Drive to local | | ||
| | [`+status`](references/lark-drive-status.md) | Compare a local directory with a Drive folder by exact SHA-256 hash by default, or use `--quick` for a best-effort modified-time diff that skips remote downloads; reports `new_local` / `new_remote` / `modified` / `unchanged` plus `detection=exact` or `detection=quick`. Duplicate remote `rel_path` conflicts fail fast with `error.type=duplicate_remote_path` and list every conflicting entry; do not proceed as if one was chosen. `--local-dir` 必须是 cwd 内的相对路径，越界路径 CLI 会直接拒绝；目标在 cwd 外时引导用户切换 agent 工作目录，不要私自 `cd` 绕过。 | | ||
| | [`+pull`](references/lark-drive-pull.md) | File-level Drive → local mirror. Duplicate remote `rel_path` conflicts fail by default; for duplicate files, `rename` downloads all copies with stable hashed suffixes, while `newest` / `oldest` pick one. `--if-exists` supports `overwrite` / `smart` / `skip` (`smart` is a best-effort modified-time incremental mode for repeat syncs). `--delete-local` requires `--yes`, only removes regular files, and is skipped after item failures. `--local-dir` must stay inside cwd. | | ||
| | `+sync` | Two-way local ↔ Drive sync. Reuses `+status` diff buckets, pulls `new_remote`, pushes `new_local`, and resolves `modified` via `--on-conflict=remote-wins|local-wins|keep-both|ask`. `--quick` enables best-effort modified-time diffing (timestamp mismatches can still trigger real pull/push actions), `--on-duplicate-remote` supports `fail|newest|oldest`, and the command is intentionally non-destructive (no delete on either side). | |
There was a problem hiding this comment.
Fix table cell parsing by escaping pipes in inline code.
Line 247 puts | inside inline code in a table cell, which breaks column parsing and truncates this row in Markdown renderers/linting.
Suggested patch
-| `+sync` | Two-way local ↔ Drive sync. Reuses `+status` diff buckets, pulls `new_remote`, pushes `new_local`, and resolves `modified` via `--on-conflict=remote-wins|local-wins|keep-both|ask`. `--quick` enables best-effort modified-time diffing (timestamp mismatches can still trigger real pull/push actions), `--on-duplicate-remote` supports `fail|newest|oldest`, and the command is intentionally non-destructive (no delete on either side). |
+| `+sync` | Two-way local ↔ Drive sync. Reuses `+status` diff buckets, pulls `new_remote`, pushes `new_local`, and resolves `modified` via `--on-conflict=remote-wins\|local-wins\|keep-both\|ask`. `--quick` enables best-effort modified-time diffing (timestamp mismatches can still trigger real pull/push actions), `--on-duplicate-remote` supports `fail\|newest\|oldest`, and the command is intentionally non-destructive (no delete on either side). |🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 247-247: Spaces inside code span elements
(MD038, no-space-in-code)
[warning] 247-247: Table column count
Expected: 2; Actual: 7; Too many cells, extra data will be missing
(MD056, table-column-count)
🤖 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` at line 247, The table row for the `+sync`
command contains unescaped pipe characters inside inline code (e.g.,
`--on-conflict=remote-wins|local-wins|keep-both|ask` and `--on-duplicate-remote`
options), which breaks Markdown table parsing; edit the SKILL.md table cell for
`+sync` so pipes inside inline code are escaped (e.g.,
`--on-conflict=remote-wins\|local-wins\|keep-both\|ask` and
`--on-duplicate-remote=fail\|newest\|oldest`) or otherwise ensure the entire
option strings are wrapped/escaped to prevent `|` from being treated as column
separators (target the `+sync` table row and the inline code spans for
`--on-conflict` and `--on-duplicate-remote`).
Collaborator
Author
|
Superseded by #970 which uses a clean branch from main. |
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
docs +media-insertcurrently only inserts images at the document top level (end of document or near a top-level block matched by--selection-with-ellipsis). It cannot place images inside container blocks like table cells, callouts, or grid columns. This PR documents the "upload then move" two-step workflow that enables inserting local images into container blocks usingblock_move_after, which supports cross-parent moves.Changes
skills/lark-doc/references/lark-doc-media-insert.md: Updated title and description to reflect broader capability; added new "插入图片到容器 block" section documenting the upload-then-move workflow with step-by-step instructions, examples (table cell, callout, grid column), and caveatsskills/lark-doc/SKILL.md: Updated+media-insertshortcut description to mention the upload-then-move workflow for container blocksskills/lark-doc/references/lark-doc-xml.md: Updated<callout>child block description to include images; updated<img>description to mention container placementTest Plan
+media-insert, then usedblock_move_afterto move it into a<td>— confirmed the image appeared inside the table cellblock_move_aftersupports cross-parent moves (from document root into container block)Related Issues
Summary by CodeRabbit
New Features
drive +syncshortcut for bidirectional synchronization between local directories and Drive folders, with configurable conflict resolution strategies (remote-wins,local-wins,keep-both,ask) and a--quickmode for faster comparisons.Documentation
drive +pushdocumentation for duplicate handling and conflict semantics.