Skip to content

feat(slides): add +update-slide to replace a page with the XML you hand it - #2143

Open
tianyouskrrr wants to merge 1 commit into
mainfrom
feat/slides-update-slide-via-page-id
Open

feat(slides): add +update-slide to replace a page with the XML you hand it#2143
tianyouskrrr wants to merge 1 commit into
mainfrom
feat/slides-update-slide-via-page-id

Conversation

@tianyouskrrr

@tianyouskrrr tianyouskrrr commented Aug 2, 2026

Copy link
Copy Markdown
Collaborator

What this changes

Adds slides +update-slide, which applies one complete <slide> XML document to an existing page in a single block_replace request. The request uses the page slide_id as block_id, so the page is updated in place and keeps both its slide_id and its position in the deck.

Command surface

  • Adds the canonical slides +update-slide shortcut.
  • Adds hidden +update and singular slide aliases for common agent-generated spellings.
  • Adds scoped --content aliases: --xml, --slide-xml, --slide-content, and --content-xml.
  • Deprecates slides +replace-pages while keeping it fully working: the shortcut carries the standard Go Deprecated: marker, its --help description leads with the replacement, and every output shape (dry-run, validate-only, real runs) carries a deprecated field naming it. The skill no longer routes to it — decision tables, command list, and its reference page are removed — so nothing steers a new caller there, but existing callers keep working for a deprecation window. An old skill copy on a new binary therefore still works, and learns about +update-slide from the output of its very first call.
  • Aligns +add-slide / +delete-slide with the shared presentation-flag contract (requiredPresentationRefFlag()), so their --presentation accepts the same alias spellings (--token, --url, …) as every other Slides shortcut; a contract test now pins this across all of them.

+replace-pages rebuilt each page with create-then-delete, changing the page ID and every element ID while exposing a non-atomic partial-failure flow. For multi-page work, agents now call +update-slide once per page. The XML/revision helpers it shared with +add-slide / +delete-slide moved to a shared file, so removing the command after the window cannot break them.

Behavior worth reviewing

  • --content describes the final page state, not a patch. Elements omitted from it are deleted.
  • Elements included with their original IDs retain those IDs; elements without IDs are inserted as new elements.
  • <style> and <note> follow the supplied XML, which allows page background and speaker-note changes.
  • The input must contain exactly one <slide> root. Malformed XML, a bare element, trailing content, and a root ID that names a different page are rejected before any API request is built.
  • Editing one element remains cheaper and safer with slides +replace-slide.
  • Wiki presentation URLs are resolved consistently with the other Slides shortcuts.

Tests and documentation

  • Adds unit coverage for request shape, validation, typed errors, error-cause preservation, aliases, revision and transaction parameters, and backend failure handling.
  • Adds built-binary dry-run E2E coverage for the canonical command, aliases, and refusal paths, including typed stderr metadata.
  • Updates the opt-in Slides history workflow to perform a live +update-slide round trip and assert that slide_id is preserved.
  • Updates the Slides skill and editing workflow documentation, including the warning that IDs are preserved only when the original IDs are kept in --content.
  • Restructures the +update-slide reference doc to the house skeleton shared by the sibling shortcut docs (命令 / 参数 / 成功输出 / 常见错误); scope requirements now live in the 403 row of 常见错误 like everywhere else, instead of a dedicated permissions section.
  • Adds a regression test pinning the deprecation contract: +replace-pages stays executable and each of its output shapes carries the full deprecation note.

Local checks completed:

  • make unit-test
  • go vet ./...
  • gofmt -l .
  • go mod tidy with no module-file changes
  • golangci-lint with 0 issues
  • go test ./tests/cli_e2e/slides -count=1

Backend dependency

This command relies on the server accepting the page ID as block_id for a whole-page replacement. The rollout is tracked separately. Please do not merge before that capability is available in the target environment. Until then, the default CI proof is dry-run coverage; the real API round trip remains opt-in through LARK_SLIDES_HISTORY_E2E=1.

Summary by CodeRabbit

  • New Features

    • Added +update-slide for whole-slide XML replacement while preserving slide IDs and page order.
    • Added slide as an alias for the slides service.
    • Added content aliases, dry-run support, validation, and clearer error reporting.
  • Documentation

    • Updated slide editing workflows and guidance for whole-page updates.
  • Removed

    • Removed +replace-pages; use per-slide +update-slide operations instead.

@coderabbitai

coderabbitai Bot commented Aug 2, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

The PR adds slides +update-slide for complete XML slide replacement. It adds service and content aliases, validates page XML, sends one block_replace request, shares endpoint construction with +replace-slide, removes +replace-pages, and adds unit, dry-run, E2E, and documentation coverage.

Changes

Whole-slide update

Layer / File(s) Summary
Alias wiring
shortcuts/register.go, shortcuts/slides/shortcuts.go, shortcuts/slides/shortcuts_alias_test.go
Adds the slide service alias and scoped XML/content flag aliases. Replaces +replace-pages registration with +update-slide and hidden +update aliases.
Update-slide command
shortcuts/slides/slides_update_slide.go, shortcuts/slides/slides_replace_slide.go
Adds full-slide XML validation, dry-run and execution flows, query propagation, API failure handling, error hints, and shared replacement-path construction.
Command validation tests
shortcuts/slides/slides_update_slide_test.go
Tests scopes, aliases, request payloads, XML validation, query parameters, endpoint encoding, backend failures, error hints, and idempotent error enrichment.
Documentation and E2E coverage
skills/lark-slides/*, tests/cli_e2e/slides/*
Documents +update-slide, replaces the page-list workflow, updates coverage records, and adds canonical, alias, and refusal dry-run tests.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant CLI
  participant SlidesUpdateSlide
  participant XMLValidator
  participant SlideAPI
  CLI->>SlidesUpdateSlide: submit slide ID and XML content
  SlidesUpdateSlide->>XMLValidator: validate and normalize slide XML
  XMLValidator-->>SlidesUpdateSlide: validated page payload
  SlidesUpdateSlide->>SlideAPI: send one block_replace request
  SlideAPI-->>SlidesUpdateSlide: return result or failure reason
  SlidesUpdateSlide-->>CLI: output metadata or error
Loading

Possibly related PRs

  • larksuite/cli#516: Provides the existing slide replacement shortcut and shared helpers extended by this PR.
  • larksuite/cli#2131: Modifies the same slide-update command, aliases, endpoint helper, tests, and documentation.

Suggested labels: domain/ccm

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 72.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly identifies the main change: adding +update-slide to replace an existing slide with supplied XML.
Description check ✅ Passed The description clearly covers scope, behavior, compatibility, tests, documentation, and backend dependency, despite using different section headings.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/slides-update-slide-via-page-id

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added the size/L Large or sensitive change across domains or core paths label Aug 2, 2026
@github-actions

github-actions Bot commented Aug 2, 2026

Copy link
Copy Markdown

🚀 PR Preview Install Guide

🧰 CLI update

npm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@81f21368907719acf0b2792971399467c1f319ba

🧩 Skill update

npx skills add larksuite/cli#feat/slides-update-slide-via-page-id -y -g

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🧹 Nitpick comments (1)
shortcuts/slides/slides_update_slide.go (1)

262-270: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Preserve the ensureXMLRootID cause in updateSlideContent.

When ensureXMLRootID fails, return the command-specific ValidationError wrapped around the original error instead of discarding err:

return "", errs.NewValidationError(errs.SubtypeInvalidArgument, "--content root <slide> is written in a form the page id cannot be attached to\...").WithCause(err).WithParam("--content")

This keeps errors.Is/errors.As pathability while keeping the user-facing message.

🤖 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/slides/slides_update_slide.go` around lines 262 - 270, Update the
ensureXMLRootID error path in updateSlideContent to preserve err by wrapping it
in the command-specific ValidationError with subtype invalid argument and
parameter --content. Keep the existing user-facing message unchanged while
attaching err as the cause so errors.Is/errors.As remain pathable.

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/slides/slides_update_slide_test.go`:
- Around line 313-316: The error-path tests lack complete typed metadata
assertions. In shortcuts/slides/slides_update_slide_test.go lines 313-316,
require errors.As to succeed, remove the tt.name exclusion, and assert category,
subtype, and param via errs.ProblemOf, including param --content; in lines
177-186, retain the failed_reason message check and add errs.ProblemOf
assertions for category and subtype, while verifying the underlying cause is
preserved.
- Around line 177-186: Enhance the error assertions in the failed_reason test
around updateSlideExecute by extracting typed metadata with errs.ProblemOf(err)
and asserting the expected category, subtype, and param values, including the
failed_reason hint. Keep the existing backend-message and no-success-envelope
assertions.

In `@skills/lark-slides/SKILL.md`:
- Line 113: Update the layout lint requirement near the complete <slide> XML
submission guidance to include +update-slide alongside +create --slides,
xml_presentation.slide create, and +replace-pages. Keep the existing lint
command and other workflow references unchanged.

In `@tests/cli_e2e/slides/slides_update_slide_dryrun_test.go`:
- Around line 155-163: Update errorEnvelope to require the typed error envelope
on result.Stderr and assert result.Stdout is empty for validate-stage failures;
remove the fallback that accepts stdout. At the two call sites around the
data.api assertions, replace those ineffective envelope checks with explicit
stdout-empty assertions, and correct the nearby comment to state that the
envelope is written to stderr.

---

Nitpick comments:
In `@shortcuts/slides/slides_update_slide.go`:
- Around line 262-270: Update the ensureXMLRootID error path in
updateSlideContent to preserve err by wrapping it in the command-specific
ValidationError with subtype invalid argument and parameter --content. Keep the
existing user-facing message unchanged while attaching err as the cause so
errors.Is/errors.As remain pathable.
🪄 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: 8fc121c6-f2f1-468e-ab47-c4d196b44047

📥 Commits

Reviewing files that changed from the base of the PR and between 40a0a9d and 6e88409.

📒 Files selected for processing (10)
  • shortcuts/register.go
  • shortcuts/slides/shortcuts.go
  • shortcuts/slides/shortcuts_alias_test.go
  • shortcuts/slides/slides_replace_slide.go
  • shortcuts/slides/slides_update_slide.go
  • shortcuts/slides/slides_update_slide_test.go
  • skills/lark-slides/SKILL.md
  • skills/lark-slides/references/lark-slides-update-slide.md
  • tests/cli_e2e/slides/coverage.md
  • tests/cli_e2e/slides/slides_update_slide_dryrun_test.go

Comment thread shortcuts/slides/slides_update_slide_test.go
Comment thread shortcuts/slides/slides_update_slide_test.go
Comment thread skills/lark-slides/SKILL.md Outdated
Comment thread tests/cli_e2e/slides/slides_update_slide_dryrun_test.go Outdated
@codecov

codecov Bot commented Aug 2, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 72.95918% with 53 lines in your changes missing coverage. Please review.
✅ Project coverage is 75.69%. Comparing base (2a16134) to head (81f2136).
⚠️ Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
shortcuts/slides/slides_update_slide.go 71.33% 35 Missing and 8 partials ⚠️
shortcuts/slides/slides_shared.go 71.42% 5 Missing and 5 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2143      +/-   ##
==========================================
+ Coverage   75.65%   75.69%   +0.03%     
==========================================
  Files         940      944       +4     
  Lines       99876   100230     +354     
==========================================
+ Hits        75564    75867     +303     
- Misses      18530    18564      +34     
- Partials     5782     5799      +17     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.


## 依赖的后端能力

底层是 `xml_presentation.slide.replace`,发一条 `block_replace`,`block_id` 传的是**页面自己的 id**。服务端据此替换整个 `<slide>`。

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个底层细节需要描述吗

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

不需要,整节已删掉。
里面三段只有一段有用(@文件 只接受相对路径),已挪到 --content 的说明里——它本来就是 CLI 约束,归在「依赖的后端能力」下面是错的。

Comment thread skills/lark-slides/SKILL.md Outdated
@@ -82,6 +82,7 @@ metadata:
| 新建 PPT | 先规划 `slide_plan.json`,再按复杂度选择一步或两步创建 | `planning-layer.md`、`visual-planning.md`、`asset-planning.md`、`lark-slides-create.md`、`slides +create` |
| 用户要求使用模板,或提供 PPTX 文件要求修改、美化 | 将模板导入为 Slides 再编辑 | `lark-slides-pptx-template-workflows.md` |
| 编辑单个标题、文本块、图片或局部元素 | 优先块级替换/插入,不改页序 | `slides +replace-slide`、`lark-slides-replace-slide.md` |

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

旧的+replace-slide能不能删掉?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个我保留了,两者不是新旧关系,是爆炸半径不同:+replace-slide 只动点名的 block,+update-slide 是整页覆盖,没写进 --content 的元素会被删除。改一个标题用 +update-slide 得把整页每个元素抄对,抄漏一个那个元素就没了。

Comment thread skills/lark-slides/SKILL.md Outdated
**CRITICAL — 创建前自检或失败排障时,MUST 按 [troubleshooting.md](references/troubleshooting.md) 检查 XML 转义、结构、shell 截断、图片 token、3350001 和布局风险。**

**编辑已有幻灯片页面**:单个标题、文本块、图片或局部元素优先用 [`+replace-slide`](references/lark-slides-replace-slide.md)(块级替换/插入,不动页序);已有 Slides 的多页大改优先用 [`+replace-pages`](references/lark-slides-replace-pages.md) 在原 presentation 内批量重建页面,避免 `slides +create` 生成新链接。选择 action 和完整读-改-写流程见 [`lark-slides-edit-workflows.md`](references/lark-slides-edit-workflows.md)。
**编辑已有幻灯片页面**:单个标题、文本块、图片或局部元素优先用 [`+replace-slide`](references/lark-slides-replace-slide.md)(块级替换/插入,不动页序);一页里改动很多(例如批量换字体)、要改背景、或要删掉若干元素时用 [`+update-slide`](references/lark-slides-update-slide.md) 整页覆盖(`slide_id` 和页序不变,但没写进 `--content` 的元素会被删除);已有 Slides 的多页大改优先用 [`+replace-pages`](references/lark-slides-replace-pages.md) 在原 presentation 内批量重建页面,避免 `slides +create` 生成新链接。选择 action 和完整读-改-写流程见 [`lark-slides-edit-workflows.md`](references/lark-slides-edit-workflows.md)。

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+replace-pages 是不是可以删掉?只留+update-slide

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

已按这个做了。+replace-pages 靠「建新页再删旧页」实现,会换掉 slide_id、重建该页所有元素 id(评论和直达链接随之失效)、删旧页不可逆、且非原子;+update-slide 原地覆盖,这些代价一个都没有。多页就每页各跑一次。

@tianyouskrrr
tianyouskrrr force-pushed the feat/slides-update-slide-via-page-id branch from 6e88409 to 1585195 Compare August 3, 2026 08:51

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 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/slides/slides_replace_pages.go`:
- Around line 26-34: The documentation in
shortcuts/slides/slides_replace_pages.go lines 26-34 should claim preservation
only for slide_id and page order, and state that elements omitted from --content
are removed while elements without IDs are inserted. In
skills/lark-slides/references/lark-slides-edit-workflows.md lines 3-14, remove
the claim that all element IDs remain unchanged and link readers to the
whole-slide replacement behavior and deletion warning; do not imply comments or
deep links anchored to element IDs are preserved.

In `@skills/lark-slides/SKILL.md`:
- Around line 84-85: Update the wiki URL guidance sentences near the core
concepts section to include +update-slide alongside the existing +replace-slide
and +media-upload shortcuts, keeping the documented URL-resolution behavior
consistent with the new command guidance.

In `@tests/cli_e2e/slides/slides_update_slide_dryrun_test.go`:
- Around line 148-150: Update the cross-page validation assertions in the slide
update dry-run test to verify the error envelope has ok=false, type validation,
subtype invalid_argument, and param --content, matching
TestSlidesUpdateSlideRefusesElementRootDryRunE2E. Keep the existing message
assertion and rely on errorEnvelope’s stdout validation.
🪄 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: dd3acafe-b76d-4f61-b1f7-d7985919a0c0

📥 Commits

Reviewing files that changed from the base of the PR and between 6e88409 and 1585195.

📒 Files selected for processing (13)
  • shortcuts/register.go
  • shortcuts/slides/shortcuts.go
  • shortcuts/slides/shortcuts_alias_test.go
  • shortcuts/slides/slides_replace_pages.go
  • shortcuts/slides/slides_replace_slide.go
  • shortcuts/slides/slides_update_slide.go
  • shortcuts/slides/slides_update_slide_test.go
  • skills/lark-slides/SKILL.md
  • skills/lark-slides/references/lark-slides-edit-workflows.md
  • skills/lark-slides/references/lark-slides-replace-pages.md
  • skills/lark-slides/references/lark-slides-update-slide.md
  • tests/cli_e2e/slides/coverage.md
  • tests/cli_e2e/slides/slides_update_slide_dryrun_test.go
💤 Files with no reviewable changes (1)
  • skills/lark-slides/references/lark-slides-replace-pages.md
🚧 Files skipped from review as they are similar to previous changes (6)
  • shortcuts/slides/shortcuts_alias_test.go
  • shortcuts/register.go
  • shortcuts/slides/slides_replace_slide.go
  • shortcuts/slides/shortcuts.go
  • shortcuts/slides/slides_update_slide_test.go
  • shortcuts/slides/slides_update_slide.go

Comment thread shortcuts/slides/slides_replace_pages.go Outdated
Comment thread tests/cli_e2e/slides/slides_update_slide_dryrun_test.go
@tianyouskrrr
tianyouskrrr force-pushed the feat/slides-update-slide-via-page-id branch from 1585195 to 2154d0b Compare August 3, 2026 09:19

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
tests/cli_e2e/slides/slides_history_workflow_test.go (1)

87-100: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win

Add direct coverage for deck-position preservation.

This workflow creates one slide, so it cannot detect a replacement that moves the target slide. Create a second slide and assert that both slide IDs remain in their original order after the update.

As per coding guidelines, contract tests must assert the changed field or behavior directly.

🤖 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/slides/slides_history_workflow_test.go` around lines 87 - 100,
Extend the slides history workflow test around the update command to create a
second slide before updating the first, then assert the resulting deck lists
both slide IDs in their original order. Keep the existing slide_id assertion and
add a direct assertion against the deck-order field returned by the relevant CLI
response, using the created IDs.

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 `@skills/lark-slides/SKILL.md`:
- Around line 84-85: Update the “整页覆盖” guidance in SKILL.md to clarify that only
elements in --content with their original IDs retain those IDs; elements without
IDs are inserted as new elements and receive new IDs, while preserving the
existing slide_id and page-order behavior.

---

Nitpick comments:
In `@tests/cli_e2e/slides/slides_history_workflow_test.go`:
- Around line 87-100: Extend the slides history workflow test around the update
command to create a second slide before updating the first, then assert the
resulting deck lists both slide IDs in their original order. Keep the existing
slide_id assertion and add a direct assertion against the deck-order field
returned by the relevant CLI response, using the created IDs.
🪄 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: cf4f901c-5045-4b72-afc0-e67d8d388f02

📥 Commits

Reviewing files that changed from the base of the PR and between 1585195 and 2154d0b.

📒 Files selected for processing (15)
  • shortcuts/register.go
  • shortcuts/slides/shortcuts.go
  • shortcuts/slides/shortcuts_alias_test.go
  • shortcuts/slides/slides_replace_pages.go
  • shortcuts/slides/slides_replace_pages_test.go
  • shortcuts/slides/slides_replace_slide.go
  • shortcuts/slides/slides_update_slide.go
  • shortcuts/slides/slides_update_slide_test.go
  • skills/lark-slides/SKILL.md
  • skills/lark-slides/references/lark-slides-edit-workflows.md
  • skills/lark-slides/references/lark-slides-replace-pages.md
  • skills/lark-slides/references/lark-slides-update-slide.md
  • tests/cli_e2e/slides/coverage.md
  • tests/cli_e2e/slides/slides_history_workflow_test.go
  • tests/cli_e2e/slides/slides_update_slide_dryrun_test.go
💤 Files with no reviewable changes (3)
  • skills/lark-slides/references/lark-slides-replace-pages.md
  • shortcuts/slides/slides_replace_pages_test.go
  • shortcuts/slides/slides_replace_pages.go
🚧 Files skipped from review as they are similar to previous changes (9)
  • shortcuts/slides/slides_replace_slide.go
  • tests/cli_e2e/slides/coverage.md
  • shortcuts/register.go
  • shortcuts/slides/shortcuts_alias_test.go
  • shortcuts/slides/shortcuts.go
  • tests/cli_e2e/slides/slides_update_slide_dryrun_test.go
  • shortcuts/slides/slides_update_slide.go
  • skills/lark-slides/references/lark-slides-edit-workflows.md
  • shortcuts/slides/slides_update_slide_test.go

Comment thread skills/lark-slides/SKILL.md Outdated
@tianyouskrrr
tianyouskrrr force-pushed the feat/slides-update-slide-via-page-id branch from 2154d0b to 0cde957 Compare August 3, 2026 09:48
@tianyouskrrr
tianyouskrrr force-pushed the feat/slides-update-slide-via-page-id branch 2 times, most recently from b1fa292 to 72f1fff Compare August 3, 2026 11:42

## 权限

`slides:presentation:update` + `slides:presentation:write_only`;`--presentation` 传 `/wiki/` 链接时还需要 `wiki:node:read`。

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

权限描述是不是可以不进这个文件,其他的shortcut也很少描述权限的事情

@tianyouskrrr
tianyouskrrr force-pushed the feat/slides-update-slide-via-page-id branch 4 times, most recently from 97c309f to 5c0fc24 Compare August 3, 2026 12:55
@tianyouskrrr
tianyouskrrr force-pushed the feat/slides-update-slide-via-page-id branch 2 times, most recently from 7e6b8c8 to 22ae5b8 Compare August 3, 2026 13:16

// replacePagesDeprecationNote is surfaced in every +replace-pages output —
// dry-run, validate-only and real runs — so callers that never read --help
// still see the deprecation and the replacement.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Do not promise preservation of all element IDs

This deprecation notice is emitted on every +replace-pages run and currently says that +update-slide keeps element IDs. In fact, only elements carried over in --content with their original IDs retain them; omitted elements are deleted and ID-less elements receive new IDs. Callers relying on this wording may lose comments or deep links anchored to those IDs. Please limit the unconditional guarantee to slide_id and page order, and state the condition for preserving element IDs. The Deprecated: comment below should be corrected as well.

require.NoError(t, err)
updateResult.AssertExitCode(t, 0)
updateResult.AssertStdoutStatus(t, true)
require.Equal(t, slideID, gjson.Get(updateResult.Stdout, "data.slide_id").String(),

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Verify element-ID preservation in the live workflow

The command publicly promises that elements supplied with their original IDs retain those IDs, but this workflow replaces the original shape with ID-less XML and only checks the page ID. A backend that regenerates every element ID would therefore pass this test while breaking comments and deep links—the same integrity risk used to justify deprecating +replace-pages. Please capture an original element ID, carry it into the updated XML, and assert that the fetched page still contains that ID after the update.

Add an in-place whole-page slide update shortcut with validation, aliases, docs, unit tests, and dry-run E2E coverage.

Deprecate the superseded +replace-pages: the binary keeps the command working for a deprecation window, with the replacement named in its --help description and in a `deprecated` field on every output (dry-run, validate-only and real runs), while the skill no longer routes to it. Multi-page updates now call +update-slide once per page. The XML/revision helpers it shared with +add-slide / +delete-slide move to slides_shared.go so its eventual removal cannot break them.
@tianyouskrrr
tianyouskrrr force-pushed the feat/slides-update-slide-via-page-id branch from 22ae5b8 to 81f2136 Compare August 3, 2026 14:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/L Large or sensitive change across domains or core paths

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants