From 735e988e991657b4e1dfd1be781d7aede4a28719 Mon Sep 17 00:00:00 2001 From: "zhuyuanbin.gdut" Date: Thu, 30 Jul 2026 12:07:43 +0800 Subject: [PATCH] feat(slides): add +add-slide and +delete-slide shortcuts Add two single-page slide shortcuts on top of the raw xml_presentation.slide create/delete APIs. slides +add-slide appends or inserts one page into an existing presentation. It accepts --presentation as a token, a /slides/ URL or a /wiki/ URL (resolved via wiki.spaces.get_node and checked for obj_type=slides), takes the page XML through --slide as a literal, @file or stdin so the document never has to be escaped into JSON and then into the shell, and auto-uploads placeholders, replacing them with the returned file_token. Omitting --before-slide-id appends to the end; the field is dropped from the body rather than sent empty, which the backend rejects as an unknown slide. slides +delete-slide removes one page by slide_id with the same --presentation resolution. It is deliberately Risk "write" rather than the raw command's high-risk-write, so it does not require --yes: it targets a single explicit page and the deck keeps its version history. Both take one page at a time so that batching stays an explicit loop and every call has an unambiguous outcome. The image placeholder validation used by +create is extracted into a shared helper so both commands fail before any API call when a referenced file is missing, is not a regular file or exceeds the 20 MB upload limit. Covered by unit tests and by dry-run e2e tests through the built binary, which is the only layer that proves a full document survives flag parsing intact. Reference docs are added for both commands and the existing slides skill docs now route to them. --- shortcuts/slides/helpers.go | 29 + shortcuts/slides/helpers_test.go | 92 +++ shortcuts/slides/shortcuts.go | 2 + shortcuts/slides/slides_add_slide.go | 250 +++++++ shortcuts/slides/slides_add_slide_test.go | 646 ++++++++++++++++++ shortcuts/slides/slides_create.go | 20 +- shortcuts/slides/slides_delete_slide.go | 164 +++++ shortcuts/slides/slides_delete_slide_test.go | 366 ++++++++++ shortcuts/slides/slides_errors.go | 21 +- shortcuts/slides/slides_errors_test.go | 37 +- shortcuts/slides/slides_media_upload.go | 2 +- skills/lark-slides/SKILL.md | 21 +- .../references/lark-slides-add-slide.md | 93 +++ .../references/lark-slides-create.md | 41 +- .../references/lark-slides-delete-slide.md | 66 ++ .../references/lark-slides-media-upload.md | 29 +- ...rk-slides-xml-presentation-slide-create.md | 6 +- ...rk-slides-xml-presentation-slide-delete.md | 8 +- .../lark-slides/references/troubleshooting.md | 1 - .../references/xml-schema-quick-ref.md | 6 +- tests/cli_e2e/slides/coverage.md | 10 +- .../slides_slide_add_delete_dryrun_test.go | 149 ++++ .../slides_slide_add_delete_workflow_test.go | 191 ++++++ 23 files changed, 2149 insertions(+), 101 deletions(-) create mode 100644 shortcuts/slides/slides_add_slide.go create mode 100644 shortcuts/slides/slides_add_slide_test.go create mode 100644 shortcuts/slides/slides_delete_slide.go create mode 100644 shortcuts/slides/slides_delete_slide_test.go create mode 100644 skills/lark-slides/references/lark-slides-add-slide.md create mode 100644 skills/lark-slides/references/lark-slides-delete-slide.md create mode 100644 tests/cli_e2e/slides/slides_slide_add_delete_dryrun_test.go create mode 100644 tests/cli_e2e/slides/slides_slide_add_delete_workflow_test.go diff --git a/shortcuts/slides/helpers.go b/shortcuts/slides/helpers.go index 2c22122646..270df80dc5 100644 --- a/shortcuts/slides/helpers.go +++ b/shortcuts/slides/helpers.go @@ -152,6 +152,35 @@ func extractImagePlaceholderPaths(slideXMLs []string) []string { return paths } +// validateImagePlaceholderFiles checks every @-placeholder path up front: +// exists, is a regular file, and fits the 20 MB single-part upload ceiling. +// Callers run this during Validate so a bad path fails before any API call, +// rather than half-way through an upload sequence. +// +// param names the flag the paths came from (e.g. "--slides", "--slide") so the +// typed Param points at what the caller actually typed. The message quotes the +// element instead of writing "--slide @path", which reads as if the flag +// argument itself were the missing image; the paths come from placeholders +// nested inside the XML, and they resolve against the process CWD rather than +// the directory of an @file passed to the flag. +func validateImagePlaceholderFiles(runtime *common.RuntimeContext, param string, paths []string) error { + for _, path := range paths { + placeholder := fmt.Sprintf(`%s: resolved from the current directory`, param, path) + stat, err := runtime.FileIO().Stat(path) + if err != nil { + return slidesInputStatError(err, param, placeholder) + } + if !stat.Mode().IsRegular() { + return errs.NewValidationError(errs.SubtypeInvalidArgument, "%s: must be a regular file", placeholder).WithParam(param) + } + if stat.Size() > common.MaxDriveMediaUploadSinglePartSize { + return errs.NewValidationError(errs.SubtypeInvalidArgument, "%s: file size %s exceeds 20 MB limit for slides image upload", + placeholder, common.FormatSize(stat.Size())).WithParam(param) + } + } + return nil +} + // xmlRootOpenTagRegex matches the first opening tag of an XML fragment: // skipping leading whitespace, XML declaration (), and comments // (). diff --git a/shortcuts/slides/helpers_test.go b/shortcuts/slides/helpers_test.go index bd1a740b74..efe8a270de 100644 --- a/shortcuts/slides/helpers_test.go +++ b/shortcuts/slides/helpers_test.go @@ -4,9 +4,13 @@ package slides import ( + "bytes" + "errors" "reflect" "strings" "testing" + + "github.com/larksuite/cli/errs" ) func TestParsePresentationRef(t *testing.T) { @@ -413,3 +417,91 @@ func TestEnsureXMLRootID(t *testing.T) { }) } } + +// errAnyCause asks assertValidationProblem for "a cause was preserved" without +// naming it. Used where the wrapped error is an ad-hoc fmt.Errorf from a shared +// validator with no sentinel to match on; prefer a real sentinel wherever one +// exists, because that is what proves the *right* error survived. +var errAnyCause = errors.New("any preserved cause") + +// assertValidationProblem asserts the typed metadata every error path in this +// package owes its callers: the validation Category/Subtype pair agents route +// on, the flag that produced it, and a preserved cause. Param is read through +// errors.As because ProblemOf returns the shared Problem, which does not carry +// it. wantCause is nil for the checks that reject an input outright and have no +// underlying error to wrap — those must not invent one. +func assertValidationProblem(t *testing.T, err error, wantParam string, wantCause error) *errs.ValidationError { + t.Helper() + + var ve *errs.ValidationError + if !errors.As(err, &ve) { + t.Fatalf("err = %v (%T), want *errs.ValidationError", err, err) + } + problem, ok := errs.ProblemOf(err) + if !ok { + t.Fatalf("err = %v, want typed problem metadata", err) + } + if problem.Category != errs.CategoryValidation { + t.Fatalf("Category = %q, want %q", problem.Category, errs.CategoryValidation) + } + if problem.Subtype != errs.SubtypeInvalidArgument { + t.Fatalf("Subtype = %q, want %q", problem.Subtype, errs.SubtypeInvalidArgument) + } + if ve.Param != wantParam { + t.Fatalf("Param = %q, want %q", ve.Param, wantParam) + } + switch { + case wantCause == nil: + if ve.Cause != nil { + t.Fatalf("Cause = %v, want none for an outright-rejected input", ve.Cause) + } + case errors.Is(wantCause, errAnyCause): + if ve.Cause == nil { + t.Fatal("Cause = nil, want the underlying error preserved") + } + default: + if !errors.Is(ve.Cause, wantCause) { + t.Fatalf("Cause = %v, want it to wrap %v", ve.Cause, wantCause) + } + } + return ve +} + +// decodeShortcutDryRunAPI returns the API steps a --dry-run run planned, in +// order. Dry-run output is the only place the orchestration shape (how many +// calls, and in which order) is observable without making real requests. +func decodeShortcutDryRunAPI(t *testing.T, stdout *bytes.Buffer) []map[string]interface{} { + t.Helper() + + data := decodeShortcutData(t, stdout) + raw, _ := data["api"].([]interface{}) + if len(raw) == 0 { + t.Fatalf("dry-run planned no API calls: %#v", data) + } + steps := make([]map[string]interface{}, 0, len(raw)) + for i, item := range raw { + step, ok := item.(map[string]interface{}) + if !ok { + t.Fatalf("api[%d] = %#v, want an object", i, item) + } + steps = append(steps, step) + } + return steps +} + +// assertDryRunStep checks one planned call's method and URL, and returns it for +// the caller's params/body assertions. +func assertDryRunStep(t *testing.T, steps []map[string]interface{}, i int, wantMethod, wantURL string) map[string]interface{} { + t.Helper() + + if i >= len(steps) { + t.Fatalf("want at least %d planned call(s), got %d: %#v", i+1, len(steps), steps) + } + if steps[i]["method"] != wantMethod { + t.Fatalf("api[%d].method = %v, want %s", i, steps[i]["method"], wantMethod) + } + if steps[i]["url"] != wantURL { + t.Fatalf("api[%d].url = %v, want %s", i, steps[i]["url"], wantURL) + } + return steps[i] +} diff --git a/shortcuts/slides/shortcuts.go b/shortcuts/slides/shortcuts.go index 729e729e62..0928a63889 100644 --- a/shortcuts/slides/shortcuts.go +++ b/shortcuts/slides/shortcuts.go @@ -22,6 +22,8 @@ var presentationFlagAliases = []string{ func Shortcuts() []common.Shortcut { all := []common.Shortcut{ SlidesCreate, + SlidesAddSlide, + SlidesDeleteSlide, SlidesMediaUpload, SlidesReplaceSlide, SlidesReplacePages, diff --git a/shortcuts/slides/slides_add_slide.go b/shortcuts/slides/slides_add_slide.go new file mode 100644 index 0000000000..7224d8af17 --- /dev/null +++ b/shortcuts/slides/slides_add_slide.go @@ -0,0 +1,250 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package slides + +import ( + "context" + "fmt" + "strings" + + "github.com/larksuite/cli/errs" + "github.com/larksuite/cli/internal/validate" + "github.com/larksuite/cli/shortcuts/common" +) + +// SlidesAddSlide appends (or inserts) a single page into an existing +// presentation. It is the second half of the two-step creation flow: create a +// blank deck with +create, then add pages one at a time. +// +// Value-adds over the raw xml_presentation.slide.create command: +// +// 1. --presentation accepts a token / slides URL / wiki URL, like every other +// slides shortcut, instead of a hand-built --params JSON blob. +// 2. --slide takes the XML directly (and via @file / stdin), so callers stop +// nesting a fully escaped XML document inside a JSON string inside a shell +// argument — the escaping layer that produces most 3350001 reports. +// 3. placeholders are uploaded and rewritten to +// file_tokens, the same as +create --slides. Previously this combination +// had no CLI support at all: adding an image-bearing page to an existing +// deck meant calling +media-upload and splicing the token in by hand. +// +// Deliberately single-page: the backend endpoint creates one page per call, so +// a batch flag here would just be a client-side loop with partial-failure +// semantics to explain. Callers who want many pages loop the command, or use +// +create --slides when the deck does not exist yet. +var SlidesAddSlide = common.Shortcut{ + Service: "slides", + Command: "+add-slide", + Description: "Add one page to an existing presentation ( placeholders are auto-uploaded and replaced with file_token)", + Risk: "write", + Scopes: []string{"slides:presentation:update", "slides:presentation:write_only"}, + // Both extras are path-dependent, so they stay conditional rather than + // gating every call: wiki:node:read only when --presentation is a wiki URL, + // docs:document.media:upload only when the XML carries @-placeholders. + // Unlike +create there is no orphan risk to pre-empt here — the + // presentation already exists, so a late upload 403 leaves nothing behind. + ConditionalScopes: []string{"wiki:node:read", "docs:document.media:upload"}, + AuthTypes: []string{"user", "bot"}, + Flags: []common.Flag{ + {Name: "presentation", Desc: "xml_presentation_id, slides URL, or wiki URL that resolves to slides", Required: true}, + // The "(supports @file, - reads stdin ...)" suffix is appended from Input + // below, so spelling it out here too produced a doubled parenthetical. + {Name: "slide", Desc: "one complete XML document", Required: true, Input: []string{common.File, common.Stdin}}, + {Name: "before-slide-id", Desc: "insert before this slide_id (default: append after the last page)"}, + {Name: "revision-id", Type: "int", Default: "-1", Desc: "presentation revision (-1 = latest; pass a specific number for optimistic locking)"}, + {Name: "tid", Desc: "transaction id for concurrent-edit locking (usually empty)"}, + }, + Tips: []string{ + " placeholders resolve against the current directory, not the directory of the --slide file, and are deduplicated per call: a page-by-page loop re-uploads a shared image once per page, so upload it once with slides +media-upload and reuse the file_token instead.", + }, + Validate: func(ctx context.Context, runtime *common.RuntimeContext) error { + ref, err := parsePresentationRef(runtime.Str("presentation")) + if err != nil { + return err + } + if ref.Kind == "wiki" { + if err := runtime.EnsureScopes([]string{"wiki:node:read"}); err != nil { + return err + } + } + slideXML, err := addSlideXML(runtime) + if err != nil { + return err + } + // validateCompleteSlideXML is shared with +replace-pages and reports the + // structural problem alone ("root element is , want + // "). Re-tag it with the flag it came from so the caller sees + // which input to fix, and so agents can route on the typed Param. + if err := validateCompleteSlideXML(slideXML); err != nil { + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--slide is not a single complete document: %v", err).WithParam("--slide").WithCause(err) + } + // Check placeholder files before any API call so a typo in a path fails + // locally instead of after the page is half-built. + placeholders := extractImagePlaceholderPaths([]string{slideXML}) + if len(placeholders) > 0 { + if err := runtime.EnsureScopes([]string{"docs:document.media:upload"}); err != nil { + return err + } + if err := validateImagePlaceholderFiles(runtime, "--slide", placeholders); err != nil { + return err + } + } + return nil + }, + DryRun: func(ctx context.Context, runtime *common.RuntimeContext) *common.DryRunAPI { + ref, err := parsePresentationRef(runtime.Str("presentation")) + if err != nil { + return common.NewDryRunAPI().Set("error", err.Error()) + } + slideXML, err := addSlideXML(runtime) + if err != nil { + return common.NewDryRunAPI().Set("error", err.Error()) + } + + placeholders := extractImagePlaceholderPaths([]string{slideXML}) + dry := common.NewDryRunAPI() + + presentationID := ref.Token + step := 1 + total := 1 + len(placeholders) + if ref.Kind == "wiki" { + total++ + } + + if ref.Kind == "wiki" { + presentationID = "" + dry.Desc(fmt.Sprintf("%d-step orchestration: resolve wiki → add page", total)). + GET("/open-apis/wiki/v2/spaces/get_node"). + Desc(fmt.Sprintf("[%d/%d] Resolve wiki node to slides presentation", step, total)). + Params(map[string]interface{}{"token": ref.Token}) + step++ + } else if len(placeholders) > 0 { + dry.Desc(fmt.Sprintf("Upload %d image(s) + add 1 page", len(placeholders))) + } else { + dry.Desc("Add 1 page") + } + + for _, path := range placeholders { + appendSlidesUploadDryRun(dry, path, presentationID, step) + step++ + } + + descSuffix := "" + if len(placeholders) > 0 { + descSuffix = " (img placeholders auto-replaced)" + } + dry.POST(fmt.Sprintf( + "/open-apis/slides_ai/v1/xml_presentations/%s/slide", + validate.EncodePathSegment(presentationID), + )). + Desc(fmt.Sprintf("[%d/%d] Add page%s", step, total, descSuffix)). + Params(addSlideQuery(runtime)). + Body(addSlideBody(slideXML, runtime.Str("before-slide-id"))) + + return dry.Set("images_to_upload", len(placeholders)) + }, + Execute: func(ctx context.Context, runtime *common.RuntimeContext) error { + ref, err := parsePresentationRef(runtime.Str("presentation")) + if err != nil { + return err + } + presentationID, err := resolvePresentationID(runtime, ref) + if err != nil { + return err + } + slideXML, err := addSlideXML(runtime) + if err != nil { + return err + } + + result := map[string]interface{}{ + "xml_presentation_id": presentationID, + } + + // Uploads run against the target presentation, so they can only happen + // after a wiki ref has been resolved to a real presentation id. + placeholders := extractImagePlaceholderPaths([]string{slideXML}) + if len(placeholders) > 0 { + tokens, uploaded, err := uploadSlidesPlaceholders(runtime, presentationID, placeholders) + if err != nil { + return appendSlidesProgressHint(err, fmt.Sprintf("no page was added; %d of %d image(s) uploaded before failure", uploaded, len(placeholders))) + } + slideXML = replaceImagePlaceholders(slideXML, tokens) + result["images_uploaded"] = uploaded + } + + beforeSlideID := strings.TrimSpace(runtime.Str("before-slide-id")) + data, err := runtime.CallAPITyped( + "POST", + fmt.Sprintf( + "/open-apis/slides_ai/v1/xml_presentations/%s/slide", + validate.EncodePathSegment(presentationID), + ), + addSlideQuery(runtime), + addSlideBody(slideXML, beforeSlideID), + ) + if err != nil { + if len(placeholders) > 0 { + // The images are already in the deck's media store; say so, or + // a retry silently uploads a second copy of every file. + err = appendSlidesProgressHint(err, fmt.Sprintf("%d image(s) were uploaded before the page failed; re-running will upload them again", len(placeholders))) + } + return enrichSlidesReplaceError(err) + } + + slideID := common.GetString(data, "slide_id") + if slideID == "" { + return errs.NewInternalError(errs.SubtypeInvalidResponse, "slide.create returned no slide_id") + } + result["slide_id"] = slideID + if beforeSlideID != "" { + result["before_slide_id"] = beforeSlideID + } + if rev, ok := revisionFromData(data); ok { + result["revision_id"] = rev + } + // issues carries backend-side schema warnings for content that was + // accepted but altered; pass it through untouched so the caller can + // decide whether the page still says what they meant. + if issues, ok := data["issues"]; ok { + result["issues"] = issues + } + + runtime.Out(result, nil) + return nil + }, +} + +// addSlideXML returns the trimmed --slide value, rejecting an empty one. +// --slide is Required, so cobra already blocks a missing flag; this catches +// `--slide ""` and an @file / stdin source that turned out to be blank. +func addSlideXML(runtime *common.RuntimeContext) (string, error) { + xml := strings.TrimSpace(runtime.Str("slide")) + if xml == "" { + return "", errs.NewValidationError(errs.SubtypeInvalidArgument, "--slide cannot be empty").WithParam("--slide") + } + return xml, nil +} + +// addSlideQuery builds the query params shared by dry-run and execute. +func addSlideQuery(runtime *common.RuntimeContext) map[string]interface{} { + query := map[string]interface{}{"revision_id": runtime.Int("revision-id")} + if tid := strings.TrimSpace(runtime.Str("tid")); tid != "" { + query["tid"] = tid + } + return query +} + +// addSlideBody builds the request body shared by dry-run and execute. +// before_slide_id is omitted when empty: the backend appends to the end only +// if the key is absent, and an empty string is rejected as an unknown slide. +func addSlideBody(slideXML, beforeSlideID string) map[string]interface{} { + body := map[string]interface{}{ + "slide": map[string]interface{}{"content": slideXML}, + } + if id := strings.TrimSpace(beforeSlideID); id != "" { + body["before_slide_id"] = id + } + return body +} diff --git a/shortcuts/slides/slides_add_slide_test.go b/shortcuts/slides/slides_add_slide_test.go new file mode 100644 index 0000000000..c434dd2964 --- /dev/null +++ b/shortcuts/slides/slides_add_slide_test.go @@ -0,0 +1,646 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package slides + +import ( + "encoding/json" + "io/fs" + "net/http" + "net/url" + "os" + "path/filepath" + "reflect" + "strings" + "testing" + + "github.com/larksuite/cli/errs" + "github.com/larksuite/cli/internal/cmdutil" + "github.com/larksuite/cli/internal/httpmock" +) + +const testSlideXML = `

hi

` + +func TestAddSlideDeclaredScopes(t *testing.T) { + // Pre-flight must stay at the two slides scopes: an XML-only page needs + // neither wiki read nor media upload, and gating every call on them would + // force unrelated consent. + want := []string{"slides:presentation:update", "slides:presentation:write_only"} + for _, identity := range []string{"user", "bot"} { + if got := SlidesAddSlide.ScopesForIdentity(identity); !reflect.DeepEqual(got, want) { + t.Fatalf("%s preflight scopes = %#v, want %#v", identity, got, want) + } + } + got := SlidesAddSlide.DeclaredScopesForIdentity("user") + wantDeclared := []string{"slides:presentation:update", "slides:presentation:write_only", "wiki:node:read", "docs:document.media:upload"} + if !reflect.DeepEqual(got, wantDeclared) { + t.Fatalf("declared scopes = %#v, want %#v", got, wantDeclared) + } +} + +// TestAddSlideOmitsBeforeSlideIDWhenUnset guards the append case: the backend +// appends only when before_slide_id is absent, so sending "" would turn a +// plain append into a lookup of a slide that does not exist. +func TestAddSlideOmitsBeforeSlideIDWhenUnset(t *testing.T) { + t.Parallel() + + f, stdout, _, reg := cmdutil.TestFactory(t, slidesTestConfig(t, "")) + var gotQuery url.Values + stub := &httpmock.Stub{ + Method: "POST", + URL: "/open-apis/slides_ai/v1/xml_presentations/pres_abc/slide", + Body: map[string]interface{}{"code": 0, "data": map[string]interface{}{"slide_id": "slide_new", "revision_id": 9}}, + OnMatch: func(req *http.Request) { gotQuery = req.URL.Query() }, + } + reg.Register(stub) + + err := runSlidesShortcut(t, f, stdout, SlidesAddSlide, []string{ + "+add-slide", + "--presentation", "pres_abc", + "--slide", testSlideXML, + "--as", "user", + }) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + var body map[string]interface{} + if err := json.Unmarshal(stub.CapturedBody, &body); err != nil { + t.Fatalf("decode body: %v", err) + } + if _, ok := body["before_slide_id"]; ok { + t.Fatalf("before_slide_id must be absent when --before-slide-id is unset, got body %s", stub.CapturedBody) + } + slide, _ := body["slide"].(map[string]interface{}) + if slide["content"] != testSlideXML { + t.Fatalf("slide.content = %v, want the XML passed to --slide", slide["content"]) + } + if gotQuery.Get("revision_id") != "-1" { + t.Fatalf("revision_id query = %q, want -1", gotQuery.Get("revision_id")) + } + if _, ok := gotQuery["tid"]; ok { + t.Fatalf("tid must be absent when --tid is unset, got %v", gotQuery) + } + + data := decodeShortcutData(t, stdout) + if data["slide_id"] != "slide_new" { + t.Fatalf("slide_id = %v, want slide_new", data["slide_id"]) + } + if data["revision_id"] != float64(9) { + t.Fatalf("revision_id = %v, want 9", data["revision_id"]) + } + if data["xml_presentation_id"] != "pres_abc" { + t.Fatalf("xml_presentation_id = %v, want pres_abc", data["xml_presentation_id"]) + } + if _, ok := data["before_slide_id"]; ok { + t.Fatalf("output should not report before_slide_id when unset: %#v", data) + } +} + +func TestAddSlideSendsBeforeSlideIDAndTid(t *testing.T) { + t.Parallel() + + f, stdout, _, reg := cmdutil.TestFactory(t, slidesTestConfig(t, "")) + var gotQuery url.Values + stub := &httpmock.Stub{ + Method: "POST", + URL: "/open-apis/slides_ai/v1/xml_presentations/pres_abc/slide", + Body: map[string]interface{}{"code": 0, "data": map[string]interface{}{"slide_id": "slide_new", "revision_id": 3}}, + OnMatch: func(req *http.Request) { gotQuery = req.URL.Query() }, + } + reg.Register(stub) + + err := runSlidesShortcut(t, f, stdout, SlidesAddSlide, []string{ + "+add-slide", + "--presentation", "pres_abc", + "--slide", testSlideXML, + "--before-slide-id", "slide_target", + "--revision-id", "12", + "--tid", "tx_1", + "--as", "user", + }) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + var body map[string]interface{} + if err := json.Unmarshal(stub.CapturedBody, &body); err != nil { + t.Fatalf("decode body: %v", err) + } + if body["before_slide_id"] != "slide_target" { + t.Fatalf("before_slide_id = %v, want slide_target", body["before_slide_id"]) + } + if gotQuery.Get("revision_id") != "12" { + t.Fatalf("revision_id query = %q, want 12", gotQuery.Get("revision_id")) + } + if gotQuery.Get("tid") != "tx_1" { + t.Fatalf("tid query = %q, want tx_1", gotQuery.Get("tid")) + } + + data := decodeShortcutData(t, stdout) + if data["before_slide_id"] != "slide_target" { + t.Fatalf("output before_slide_id = %v, want slide_target", data["before_slide_id"]) + } +} + +// TestAddSlideUploadsImagePlaceholder is the reason this shortcut exists for +// image-bearing pages: before it, adding one to an existing deck meant calling +// +media-upload and splicing the token in by hand. +func TestAddSlideUploadsImagePlaceholder(t *testing.T) { + dir := t.TempDir() + if err := os.WriteFile(filepath.Join(dir, "chart.png"), []byte("png-bytes"), 0o600); err != nil { + t.Fatalf("write fixture: %v", err) + } + withSlidesTestWorkingDir(t, dir) + + f, stdout, _, reg := cmdutil.TestFactory(t, slidesTestConfig(t, "")) + uploadStub := &httpmock.Stub{ + Method: "POST", + URL: "/open-apis/drive/v1/medias/upload_all", + Body: map[string]interface{}{"code": 0, "data": map[string]interface{}{"file_token": "tok_chart"}}, + } + slideStub := &httpmock.Stub{ + Method: "POST", + URL: "/open-apis/slides_ai/v1/xml_presentations/pres_img/slide", + Body: map[string]interface{}{"code": 0, "data": map[string]interface{}{"slide_id": "s_img", "revision_id": 4}}, + } + reg.Register(uploadStub) + reg.Register(slideStub) + + // The same image referenced twice must still upload once. + slideXML := `` + + `` + + `` + + `` + + err := runSlidesShortcut(t, f, stdout, SlidesAddSlide, []string{ + "+add-slide", + "--presentation", "pres_img", + "--slide", slideXML, + "--as", "user", + }) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + var body map[string]interface{} + if err := json.Unmarshal(slideStub.CapturedBody, &body); err != nil { + t.Fatalf("decode body: %v", err) + } + slide, _ := body["slide"].(map[string]interface{}) + content, _ := slide["content"].(string) + if strings.Contains(content, "@./chart.png") { + t.Fatalf("placeholder was not rewritten: %s", content) + } + if strings.Count(content, `src="tok_chart"`) != 2 { + t.Fatalf("both references should carry the uploaded token, got: %s", content) + } + + data := decodeShortcutData(t, stdout) + if data["images_uploaded"] != float64(1) { + t.Fatalf("images_uploaded = %v, want 1 (deduped by path)", data["images_uploaded"]) + } +} + +func TestAddSlideRejectsNonSlideRoot(t *testing.T) { + t.Parallel() + + f, stdout, _, _ := cmdutil.TestFactory(t, slidesTestConfig(t, "")) + err := runSlidesShortcut(t, f, stdout, SlidesAddSlide, []string{ + "+add-slide", + "--presentation", "pres_abc", + "--slide", ``, + "--as", "user", + }) + if err == nil { + t.Fatal("expected a validation error for a root") + } + // The flag tag is what lets an agent know which input to fix; the shared + // XML validator alone reports only the structural problem, and it is kept as + // the cause so the structural detail survives the re-tagging. + assertValidationProblem(t, err, "--slide", errAnyCause) + if !strings.Contains(err.Error(), "want ") { + t.Fatalf("message should name the expected root element: %v", err) + } +} + +func TestAddSlideRejectsEmptySlide(t *testing.T) { + t.Parallel() + + f, stdout, _, _ := cmdutil.TestFactory(t, slidesTestConfig(t, "")) + err := runSlidesShortcut(t, f, stdout, SlidesAddSlide, []string{ + "+add-slide", + "--presentation", "pres_abc", + "--slide", " ", + "--as", "user", + }) + if err == nil { + t.Fatal("expected a validation error for a blank --slide") + } + // No cause: the flag value is rejected outright, nothing is wrapped. + assertValidationProblem(t, err, "--slide", nil) +} + +// TestAddSlideMissingImageFailsBeforeAnyCall proves the placeholder check runs +// in Validate: a bad path must not reach the API, or the caller is left +// guessing whether a page was created. +func TestAddSlideMissingImageFailsBeforeAnyCall(t *testing.T) { + withSlidesTestWorkingDir(t, t.TempDir()) + + // Register nothing on purpose: httpmock rejects any unstubbed request, so + // reaching the API would surface as "no stub for POST .../slide" rather + // than the ValidationError asserted below. That is the no-call assertion. + f, stdout, _, _ := cmdutil.TestFactory(t, slidesTestConfig(t, "")) + + err := runSlidesShortcut(t, f, stdout, SlidesAddSlide, []string{ + "+add-slide", + "--presentation", "pres_abc", + "--slide", ``, + "--as", "user", + }) + if err == nil { + t.Fatal("expected a validation error for a missing image file") + } + // The Stat failure is wrapped, so a caller can still tell a missing file + // apart from an unreadable one without parsing the message. + assertValidationProblem(t, err, "--slide", fs.ErrNotExist) + // The path came from an inside the XML, not from an @file passed to + // --slide. Naming the element keeps the caller from re-checking the flag + // argument, which is what "--slide @./missing.png" used to imply. + if !strings.Contains(err.Error(), ``) { + t.Fatalf("error should quote the placeholder, got %v", err) + } + if !strings.HasSuffix(err.Error(), "file not found") { + t.Fatalf("a missing file should be reported as such, got %v", err) + } +} + +func TestAddSlideResolvesWikiURL(t *testing.T) { + t.Parallel() + + f, stdout, _, reg := cmdutil.TestFactory(t, slidesTestConfig(t, "")) + reg.Register(&httpmock.Stub{ + Method: "GET", + URL: "/open-apis/wiki/v2/spaces/get_node", + Body: map[string]interface{}{ + "code": 0, + "data": map[string]interface{}{ + "node": map[string]interface{}{"obj_type": "slides", "obj_token": "pres_from_wiki"}, + }, + }, + }) + slideStub := &httpmock.Stub{ + Method: "POST", + URL: "/open-apis/slides_ai/v1/xml_presentations/pres_from_wiki/slide", + Body: map[string]interface{}{"code": 0, "data": map[string]interface{}{"slide_id": "s_wiki", "revision_id": 2}}, + } + reg.Register(slideStub) + + err := runSlidesShortcut(t, f, stdout, SlidesAddSlide, []string{ + "+add-slide", + "--presentation", "https://example.feishu.cn/wiki/wikcnTOKEN", + "--slide", testSlideXML, + "--as", "user", + }) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + data := decodeShortcutData(t, stdout) + if data["xml_presentation_id"] != "pres_from_wiki" { + t.Fatalf("xml_presentation_id = %v, want the wiki-resolved token", data["xml_presentation_id"]) + } +} + +// TestAddSlideDryRunPlansSinglePost covers the plain append plan: one POST, no +// upload step, and the same query/body builders Execute uses. Dry-run is what +// callers inspect before a write, so a plan that disagrees with the real +// request is worse than no plan. +func TestAddSlideDryRunPlansSinglePost(t *testing.T) { + t.Parallel() + + f, stdout, _, _ := cmdutil.TestFactory(t, slidesTestConfig(t, "")) + err := runSlidesShortcut(t, f, stdout, SlidesAddSlide, []string{ + "+add-slide", + "--presentation", "pres_abc", + "--slide", testSlideXML, + "--dry-run", + "--as", "user", + }) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + data := decodeShortcutData(t, stdout) + if data["images_to_upload"] != float64(0) { + t.Fatalf("images_to_upload = %v, want 0", data["images_to_upload"]) + } + steps := decodeShortcutDryRunAPI(t, stdout) + if len(steps) != 1 { + t.Fatalf("planned %d calls, want a single POST: %#v", len(steps), steps) + } + step := assertDryRunStep(t, steps, 0, "POST", "/open-apis/slides_ai/v1/xml_presentations/pres_abc/slide") + + params, _ := step["params"].(map[string]interface{}) + if params["revision_id"] != float64(-1) { + t.Fatalf("params.revision_id = %v, want -1", params["revision_id"]) + } + if _, ok := params["tid"]; ok { + t.Fatalf("tid must be absent when --tid is unset: %#v", params) + } + body, _ := step["body"].(map[string]interface{}) + slide, _ := body["slide"].(map[string]interface{}) + if slide["content"] != testSlideXML { + t.Fatalf("body.slide.content = %v, want the --slide XML", slide["content"]) + } + // Same rule as the live request: appending is expressed by omitting the key. + if _, ok := body["before_slide_id"]; ok { + t.Fatalf("before_slide_id must be absent when appending: %#v", body) + } +} + +// TestAddSlideDryRunPlansUploadsBeforePost proves the plan states the upload +// cost up front. The uploads are the irreversible half of the command — they +// land in the deck's media store even if the page later fails — so a caller who +// dry-runs first must be able to see them coming. +func TestAddSlideDryRunPlansUploadsBeforePost(t *testing.T) { + dir := t.TempDir() + if err := os.WriteFile(filepath.Join(dir, "chart.png"), []byte("png-bytes"), 0o600); err != nil { + t.Fatalf("write fixture: %v", err) + } + withSlidesTestWorkingDir(t, dir) + + f, stdout, _, _ := cmdutil.TestFactory(t, slidesTestConfig(t, "")) + slideXML := `` + + `` + + `` + + err := runSlidesShortcut(t, f, stdout, SlidesAddSlide, []string{ + "+add-slide", + "--presentation", "pres_img", + "--slide", slideXML, + "--dry-run", + "--as", "user", + }) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + data := decodeShortcutData(t, stdout) + if data["images_to_upload"] != float64(1) { + t.Fatalf("images_to_upload = %v, want 1", data["images_to_upload"]) + } + steps := decodeShortcutDryRunAPI(t, stdout) + if len(steps) != 2 { + t.Fatalf("planned %d calls, want upload then add: %#v", len(steps), steps) + } + upload := assertDryRunStep(t, steps, 0, "POST", "/open-apis/drive/v1/medias/upload_all") + uploadBody, _ := upload["body"].(map[string]interface{}) + if uploadBody["parent_type"] != slidesMediaParentType { + t.Fatalf("upload parent_type = %v, want %q", uploadBody["parent_type"], slidesMediaParentType) + } + // parent_node is the presentation itself, not a placeholder: without a wiki + // hop the token is already known at plan time. + if uploadBody["parent_node"] != "pres_img" { + t.Fatalf("upload parent_node = %v, want pres_img", uploadBody["parent_node"]) + } + assertDryRunStep(t, steps, 1, "POST", "/open-apis/slides_ai/v1/xml_presentations/pres_img/slide") +} + +// TestAddSlideDryRunPlansWikiResolution pins that a wiki ref is declared as its +// own first step. The presentation token is unknown until that call returns, so +// the plan must show a placeholder rather than sending the wiki token to the +// slides API. +func TestAddSlideDryRunPlansWikiResolution(t *testing.T) { + t.Parallel() + + f, stdout, _, _ := cmdutil.TestFactory(t, slidesTestConfig(t, "")) + err := runSlidesShortcut(t, f, stdout, SlidesAddSlide, []string{ + "+add-slide", + "--presentation", "https://example.feishu.cn/wiki/wikcnTOKEN", + "--slide", testSlideXML, + "--before-slide-id", "slide_target", + "--tid", "tx_dry", + "--dry-run", + "--as", "user", + }) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + steps := decodeShortcutDryRunAPI(t, stdout) + if len(steps) != 2 { + t.Fatalf("planned %d calls, want resolve then add: %#v", len(steps), steps) + } + resolve := assertDryRunStep(t, steps, 0, "GET", "/open-apis/wiki/v2/spaces/get_node") + resolveParams, _ := resolve["params"].(map[string]interface{}) + if resolveParams["token"] != "wikcnTOKEN" { + t.Fatalf("resolve token = %v, want wikcnTOKEN", resolveParams["token"]) + } + add := assertDryRunStep(t, steps, 1, "POST", "/open-apis/slides_ai/v1/xml_presentations/%3Cresolved_slides_token%3E/slide") + + addParams, _ := add["params"].(map[string]interface{}) + if addParams["tid"] != "tx_dry" { + t.Fatalf("params.tid = %v, want tx_dry", addParams["tid"]) + } + addBody, _ := add["body"].(map[string]interface{}) + if addBody["before_slide_id"] != "slide_target" { + t.Fatalf("body.before_slide_id = %v, want slide_target", addBody["before_slide_id"]) + } +} + +// TestAddSlideRejectsResponseWithoutSlideID guards against reporting success +// for a page nobody can address afterwards: without a slide_id the caller +// cannot delete, replace or position relative to what was just created. +func TestAddSlideRejectsResponseWithoutSlideID(t *testing.T) { + t.Parallel() + + f, stdout, _, reg := cmdutil.TestFactory(t, slidesTestConfig(t, "")) + reg.Register(&httpmock.Stub{ + Method: "POST", + URL: "/open-apis/slides_ai/v1/xml_presentations/pres_abc/slide", + Body: map[string]interface{}{"code": 0, "data": map[string]interface{}{"revision_id": 4}}, + }) + + err := runSlidesShortcut(t, f, stdout, SlidesAddSlide, []string{ + "+add-slide", + "--presentation", "pres_abc", + "--slide", testSlideXML, + "--as", "user", + }) + if err == nil { + t.Fatal("expected an error when the response carries no slide_id") + } + problem, ok := errs.ProblemOf(err) + if !ok || problem.Category != errs.CategoryInternal || problem.Subtype != errs.SubtypeInvalidResponse { + t.Fatalf("problem = %#v, ok = %v, want CategoryInternal/SubtypeInvalidResponse", problem, ok) + } +} + +// TestAddSlidePassesThroughIssues keeps backend schema warnings visible. The +// page is accepted but altered in this case, so swallowing issues would leave +// the caller believing the deck says what they wrote. +func TestAddSlidePassesThroughIssues(t *testing.T) { + t.Parallel() + + f, stdout, _, reg := cmdutil.TestFactory(t, slidesTestConfig(t, "")) + reg.Register(&httpmock.Stub{ + Method: "POST", + URL: "/open-apis/slides_ai/v1/xml_presentations/pres_abc/slide", + Body: map[string]interface{}{"code": 0, "data": map[string]interface{}{ + "slide_id": "slide_new", + "issues": []interface{}{map[string]interface{}{"message": "width clamped"}}, + }}, + }) + + err := runSlidesShortcut(t, f, stdout, SlidesAddSlide, []string{ + "+add-slide", + "--presentation", "pres_abc", + "--slide", testSlideXML, + "--as", "user", + }) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + data := decodeShortcutData(t, stdout) + issues, _ := data["issues"].([]interface{}) + if len(issues) != 1 { + t.Fatalf("issues = %#v, want the backend warning passed through", data["issues"]) + } + // No revision_id in the response: the output must omit the key rather than + // report a zero revision the caller could pass back for optimistic locking. + if _, ok := data["revision_id"]; ok { + t.Fatalf("revision_id should be omitted when absent upstream: %#v", data) + } +} + +// TestAddSlideReportsUploadedImagesWhenPageFails covers the partial-failure +// hint. The images are already in the deck's media store at that point, so a +// blind retry silently uploads a second copy of every file; the hint is the +// only thing that tells the caller to reuse the tokens instead. +func TestAddSlideReportsUploadedImagesWhenPageFails(t *testing.T) { + dir := t.TempDir() + if err := os.WriteFile(filepath.Join(dir, "chart.png"), []byte("png-bytes"), 0o600); err != nil { + t.Fatalf("write fixture: %v", err) + } + withSlidesTestWorkingDir(t, dir) + + f, stdout, _, reg := cmdutil.TestFactory(t, slidesTestConfig(t, "")) + reg.Register(&httpmock.Stub{ + Method: "POST", + URL: "/open-apis/drive/v1/medias/upload_all", + Body: map[string]interface{}{"code": 0, "data": map[string]interface{}{"file_token": "tok_chart"}}, + }) + reg.Register(&httpmock.Stub{ + Method: "POST", + URL: "/open-apis/slides_ai/v1/xml_presentations/pres_img/slide", + Body: map[string]interface{}{"code": 3350001, "msg": "invalid slide xml"}, + }) + + slideXML := `` + + `` + + `` + + err := runSlidesShortcut(t, f, stdout, SlidesAddSlide, []string{ + "+add-slide", + "--presentation", "pres_img", + "--slide", slideXML, + "--as", "user", + }) + if err == nil { + t.Fatal("expected the backend rejection to surface") + } + problem, ok := errs.ProblemOf(err) + if !ok { + t.Fatalf("err = %v, want typed problem metadata", err) + } + if !strings.Contains(problem.Hint, "1 image(s) were uploaded before the page failed") { + t.Fatalf("Hint = %q, want the already-uploaded count", problem.Hint) + } +} + +// TestAddSlideReportsProgressWhenUploadFails covers the other half of the +// partial-failure story: the page was never attempted, so the hint must say so +// rather than leave the caller checking whether a page appeared. +func TestAddSlideReportsProgressWhenUploadFails(t *testing.T) { + dir := t.TempDir() + if err := os.WriteFile(filepath.Join(dir, "chart.png"), []byte("png-bytes"), 0o600); err != nil { + t.Fatalf("write fixture: %v", err) + } + withSlidesTestWorkingDir(t, dir) + + f, stdout, _, reg := cmdutil.TestFactory(t, slidesTestConfig(t, "")) + reg.Register(&httpmock.Stub{ + Method: "POST", + URL: "/open-apis/drive/v1/medias/upload_all", + Body: map[string]interface{}{"code": 1061002, "msg": "params error"}, + }) + // The slide endpoint is deliberately unstubbed: reaching it would fail as + // "no stub for POST .../slide", which is the no-page-created assertion. + + slideXML := `` + + `` + + `` + + err := runSlidesShortcut(t, f, stdout, SlidesAddSlide, []string{ + "+add-slide", + "--presentation", "pres_img", + "--slide", slideXML, + "--as", "user", + }) + if err == nil { + t.Fatal("expected the upload failure to surface") + } + problem, ok := errs.ProblemOf(err) + if !ok { + t.Fatalf("err = %v, want typed problem metadata", err) + } + if !strings.Contains(problem.Hint, "no page was added") { + t.Fatalf("Hint = %q, want it to state no page was added", problem.Hint) + } +} + +// TestAddSlideRejectsUnsupportedPresentation keeps a docx URL from being sent +// to the slides API as if it were a presentation token. +func TestAddSlideRejectsUnsupportedPresentation(t *testing.T) { + t.Parallel() + + f, stdout, _, _ := cmdutil.TestFactory(t, slidesTestConfig(t, "")) + err := runSlidesShortcut(t, f, stdout, SlidesAddSlide, []string{ + "+add-slide", + "--presentation", "https://example.feishu.cn/docx/doccnTOKEN", + "--slide", testSlideXML, + "--as", "user", + }) + if err == nil { + t.Fatal("expected a validation error for a docx URL") + } + assertValidationProblem(t, err, "--presentation", nil) +} + +// TestAddSlideRejectsDirectoryImagePlaceholder covers the placeholder that +// exists but is not a file: Stat succeeds, so only the mode check stops a +// directory from being streamed at the upload endpoint. +func TestAddSlideRejectsDirectoryImagePlaceholder(t *testing.T) { + dir := t.TempDir() + if err := os.Mkdir(filepath.Join(dir, "chart.png"), 0o750); err != nil { + t.Fatalf("make fixture dir: %v", err) + } + withSlidesTestWorkingDir(t, dir) + + f, stdout, _, _ := cmdutil.TestFactory(t, slidesTestConfig(t, "")) + err := runSlidesShortcut(t, f, stdout, SlidesAddSlide, []string{ + "+add-slide", + "--presentation", "pres_abc", + "--slide", ``, + "--as", "user", + }) + if err == nil { + t.Fatal("expected a validation error for a directory placeholder") + } + // No cause: Stat succeeded, so there is no underlying error to wrap. + assertValidationProblem(t, err, "--slide", nil) + if !strings.Contains(err.Error(), "must be a regular file") { + t.Fatalf("err = %v, want the regular-file diagnosis", err) + } +} diff --git a/shortcuts/slides/slides_create.go b/shortcuts/slides/slides_create.go index 4a7cbf1adb..642919e6ee 100644 --- a/shortcuts/slides/slides_create.go +++ b/shortcuts/slides/slides_create.go @@ -39,7 +39,7 @@ var SlidesCreate = common.Shortcut{ Scopes: []string{"slides:presentation:create", "slides:presentation:write_only", "docs:document.media:upload"}, Flags: []common.Flag{ {Name: "title", Desc: "presentation title"}, - {Name: "slides", Desc: "slide content JSON array (each element is a XML string, max 10; for more pages, create first then add via xml_presentation.slide.create). placeholders are auto-uploaded and replaced with file_token."}, + {Name: "slides", Desc: "slide content JSON array (each element is a XML string, max 10; for more pages, create first then add them one at a time with slides +add-slide). placeholders are auto-uploaded and replaced with file_token."}, }, Validate: func(ctx context.Context, runtime *common.RuntimeContext) error { if slidesStr := runtime.Str("slides"); slidesStr != "" { @@ -48,22 +48,12 @@ var SlidesCreate = common.Shortcut{ return errs.NewValidationError(errs.SubtypeInvalidArgument, "--slides invalid JSON, must be an array of XML strings").WithParam("--slides") } if len(slides) > maxSlidesPerCreate { - return errs.NewValidationError(errs.SubtypeInvalidArgument, "--slides array exceeds maximum of %d slides; create the presentation first, then add slides via xml_presentation.slide.create", maxSlidesPerCreate).WithParam("--slides") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--slides array exceeds maximum of %d slides; create the presentation first, then add the remaining pages one at a time with slides +add-slide", maxSlidesPerCreate).WithParam("--slides") } // Validate placeholder paths up front so we don't create a presentation // only to fail mid-way on a missing local file. - for _, path := range extractImagePlaceholderPaths(slides) { - stat, err := runtime.FileIO().Stat(path) - if err != nil { - return slidesInputStatError(err, "--slides", fmt.Sprintf("--slides @%s: file not found", path)) - } - if !stat.Mode().IsRegular() { - return errs.NewValidationError(errs.SubtypeInvalidArgument, "--slides @%s: must be a regular file", path).WithParam("--slides") - } - if stat.Size() > common.MaxDriveMediaUploadSinglePartSize { - return errs.NewValidationError(errs.SubtypeInvalidArgument, "--slides @%s: file size %s exceeds 20 MB limit for slides image upload", - path, common.FormatSize(stat.Size())).WithParam("--slides") - } + if err := validateImagePlaceholderFiles(runtime, "--slides", extractImagePlaceholderPaths(slides)); err != nil { + return err } } return nil @@ -265,7 +255,7 @@ func uploadSlidesPlaceholders(runtime *common.RuntimeContext, presentationID str for i, path := range paths { stat, err := runtime.FileIO().Stat(path) if err != nil { - return tokens, i, slidesInputStatError(err, "--slides", fmt.Sprintf("@%s: file not found", path)) + return tokens, i, slidesInputStatError(err, "--slides", fmt.Sprintf("@%s", path)) } if !stat.Mode().IsRegular() { return tokens, i, errs.NewValidationError(errs.SubtypeInvalidArgument, "@%s: must be a regular file", path).WithParam("--slides") diff --git a/shortcuts/slides/slides_delete_slide.go b/shortcuts/slides/slides_delete_slide.go new file mode 100644 index 0000000000..9ede6dff90 --- /dev/null +++ b/shortcuts/slides/slides_delete_slide.go @@ -0,0 +1,164 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package slides + +import ( + "context" + "fmt" + "strings" + + "github.com/larksuite/cli/errs" + "github.com/larksuite/cli/internal/validate" + "github.com/larksuite/cli/shortcuts/common" +) + +// SlidesDeleteSlide removes a single page from a presentation. +// +// Value-adds over the raw xml_presentation.slide.delete command are the same +// two every slides shortcut provides: --presentation accepts a token / slides +// URL / wiki URL, and the identifiers are ordinary flags instead of a +// hand-escaped --params JSON blob. +// +// Deliberately single-page: deletion is destructive and slide_id lists invite +// a partial-failure story ("3 of 5 deleted, which 3?"). One page per call +// keeps the outcome unambiguous. +// +// Risk is "write", not "high-risk-write", so no --yes gate. This matches +// +replace-pages, which already deletes pages through the same endpoint, and a +// mis-deleted page is recoverable via +history-list / +history-revert. Note +// this is a deliberate divergence from the raw xml_presentation.slide.delete +// command, whose schema marks it high-risk-write and does demand --yes. +var SlidesDeleteSlide = common.Shortcut{ + Service: "slides", + Command: "+delete-slide", + Description: "Delete one page from a presentation by slide_id", + Risk: "write", + Scopes: []string{"slides:presentation:update", "slides:presentation:write_only"}, + // wiki:node:read is required only when --presentation is a wiki URL. + ConditionalScopes: []string{"wiki:node:read"}, + AuthTypes: []string{"user", "bot"}, + Flags: []common.Flag{ + {Name: "presentation", Desc: "xml_presentation_id, slides URL, or wiki URL that resolves to slides", Required: true}, + {Name: "slide-id", Desc: "slide page identifier (slide_id) to delete", Required: true}, + {Name: "revision-id", Type: "int", Default: "-1", Desc: "presentation revision (-1 = latest; pass a specific number for optimistic locking)"}, + {Name: "tid", Desc: "transaction id for concurrent-edit locking (usually empty)"}, + }, + Tips: []string{ + "Deletion is not undoable in place; recover a wrongly deleted page with slides +history-list then +history-revert.", + "Use --dry-run to confirm which presentation and slide_id will be hit before running.", + }, + Validate: func(ctx context.Context, runtime *common.RuntimeContext) error { + ref, err := parsePresentationRef(runtime.Str("presentation")) + if err != nil { + return err + } + if ref.Kind == "wiki" { + if err := runtime.EnsureScopes([]string{"wiki:node:read"}); err != nil { + return err + } + } + if _, err := deleteSlideID(runtime); err != nil { + return err + } + return nil + }, + DryRun: func(ctx context.Context, runtime *common.RuntimeContext) *common.DryRunAPI { + ref, err := parsePresentationRef(runtime.Str("presentation")) + if err != nil { + return common.NewDryRunAPI().Set("error", err.Error()) + } + slideID, err := deleteSlideID(runtime) + if err != nil { + return common.NewDryRunAPI().Set("error", err.Error()) + } + + dry := common.NewDryRunAPI() + presentationID := ref.Token + step := 1 + total := 1 + if ref.Kind == "wiki" { + total = 2 + presentationID = "" + dry.Desc("2-step orchestration: resolve wiki → delete page"). + GET("/open-apis/wiki/v2/spaces/get_node"). + Desc("[1/2] Resolve wiki node to slides presentation"). + Params(map[string]interface{}{"token": ref.Token}) + step = 2 + } else { + dry.Desc(fmt.Sprintf("Delete page %s", slideID)) + } + + dry.DELETE(fmt.Sprintf( + "/open-apis/slides_ai/v1/xml_presentations/%s/slide", + validate.EncodePathSegment(presentationID), + )). + Desc(fmt.Sprintf("[%d/%d] Delete page %s", step, total, slideID)). + Params(deleteSlideQuery(runtime, slideID)) + + return dry.Set("slide_id", slideID) + }, + Execute: func(ctx context.Context, runtime *common.RuntimeContext) error { + ref, err := parsePresentationRef(runtime.Str("presentation")) + if err != nil { + return err + } + presentationID, err := resolvePresentationID(runtime, ref) + if err != nil { + return err + } + slideID, err := deleteSlideID(runtime) + if err != nil { + return err + } + + data, err := runtime.CallAPITyped( + "DELETE", + fmt.Sprintf( + "/open-apis/slides_ai/v1/xml_presentations/%s/slide", + validate.EncodePathSegment(presentationID), + ), + deleteSlideQuery(runtime, slideID), + nil, + ) + if err != nil { + return err + } + + result := map[string]interface{}{ + "xml_presentation_id": presentationID, + "slide_id": slideID, + "deleted": true, + } + if rev, ok := revisionFromData(data); ok { + result["revision_id"] = rev + } + + runtime.Out(result, nil) + return nil + }, +} + +// deleteSlideID returns the trimmed --slide-id, rejecting an empty one. +// --slide-id is Required, so cobra already blocks a missing flag; this catches +// `--slide-id ""`, which the backend would otherwise reject as a 404-ish +// "slide not found" after a pointless round trip. +func deleteSlideID(runtime *common.RuntimeContext) (string, error) { + id := strings.TrimSpace(runtime.Str("slide-id")) + if id == "" { + return "", errs.NewValidationError(errs.SubtypeInvalidArgument, "--slide-id cannot be empty").WithParam("--slide-id") + } + return id, nil +} + +// deleteSlideQuery builds the query params shared by dry-run and execute. +func deleteSlideQuery(runtime *common.RuntimeContext, slideID string) map[string]interface{} { + query := map[string]interface{}{ + "slide_id": slideID, + "revision_id": runtime.Int("revision-id"), + } + if tid := strings.TrimSpace(runtime.Str("tid")); tid != "" { + query["tid"] = tid + } + return query +} diff --git a/shortcuts/slides/slides_delete_slide_test.go b/shortcuts/slides/slides_delete_slide_test.go new file mode 100644 index 0000000000..1499ad79b5 --- /dev/null +++ b/shortcuts/slides/slides_delete_slide_test.go @@ -0,0 +1,366 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package slides + +import ( + "net/http" + "net/url" + "reflect" + "strings" + "testing" + + "github.com/larksuite/cli/internal/cmdutil" + "github.com/larksuite/cli/internal/httpmock" +) + +func TestDeleteSlideDeclaredScopes(t *testing.T) { + want := []string{"slides:presentation:update", "slides:presentation:write_only"} + for _, identity := range []string{"user", "bot"} { + if got := SlidesDeleteSlide.ScopesForIdentity(identity); !reflect.DeepEqual(got, want) { + t.Fatalf("%s preflight scopes = %#v, want %#v", identity, got, want) + } + } + got := SlidesDeleteSlide.DeclaredScopesForIdentity("user") + wantDeclared := []string{"slides:presentation:update", "slides:presentation:write_only", "wiki:node:read"} + if !reflect.DeepEqual(got, wantDeclared) { + t.Fatalf("declared scopes = %#v, want %#v", got, wantDeclared) + } +} + +// TestDeleteSlideRiskIsWriteNotHighRisk pins a deliberate divergence: the raw +// xml_presentation.slide.delete command is marked high-risk-write in its schema +// and demands --yes, while this shortcut does not. The rationale is that +// +replace-pages already deletes through the same endpoint at Risk "write", and +// a wrongly deleted page is recoverable via +history-revert. Flipping this to +// high-risk-write would silently start rejecting every existing caller that +// does not pass --yes, so make that a conscious edit rather than a drive-by. +func TestDeleteSlideRiskIsWriteNotHighRisk(t *testing.T) { + if SlidesDeleteSlide.Risk != "write" { + t.Fatalf("Risk = %q, want write (see the comment on this test before changing)", SlidesDeleteSlide.Risk) + } +} + +func TestDeleteSlideSendsSlideIDAndRevision(t *testing.T) { + t.Parallel() + + f, stdout, _, reg := cmdutil.TestFactory(t, slidesTestConfig(t, "")) + var gotQuery url.Values + reg.Register(&httpmock.Stub{ + Method: "DELETE", + URL: "/open-apis/slides_ai/v1/xml_presentations/pres_abc/slide", + Body: map[string]interface{}{"code": 0, "data": map[string]interface{}{"revision_id": 17}}, + OnMatch: func(req *http.Request) { gotQuery = req.URL.Query() }, + }) + + err := runSlidesShortcut(t, f, stdout, SlidesDeleteSlide, []string{ + "+delete-slide", + "--presentation", "pres_abc", + "--slide-id", "slide_gone", + "--as", "user", + }) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if gotQuery.Get("slide_id") != "slide_gone" { + t.Fatalf("slide_id query = %q, want slide_gone", gotQuery.Get("slide_id")) + } + if gotQuery.Get("revision_id") != "-1" { + t.Fatalf("revision_id query = %q, want -1", gotQuery.Get("revision_id")) + } + if _, ok := gotQuery["tid"]; ok { + t.Fatalf("tid must be absent when --tid is unset, got %v", gotQuery) + } + + data := decodeShortcutData(t, stdout) + if data["deleted"] != true { + t.Fatalf("deleted = %v, want true", data["deleted"]) + } + if data["slide_id"] != "slide_gone" { + t.Fatalf("slide_id = %v, want slide_gone", data["slide_id"]) + } + if data["xml_presentation_id"] != "pres_abc" { + t.Fatalf("xml_presentation_id = %v, want pres_abc", data["xml_presentation_id"]) + } + if data["revision_id"] != float64(17) { + t.Fatalf("revision_id = %v, want 17", data["revision_id"]) + } +} + +func TestDeleteSlideSendsTid(t *testing.T) { + t.Parallel() + + f, stdout, _, reg := cmdutil.TestFactory(t, slidesTestConfig(t, "")) + var gotQuery url.Values + reg.Register(&httpmock.Stub{ + Method: "DELETE", + URL: "/open-apis/slides_ai/v1/xml_presentations/pres_abc/slide", + Body: map[string]interface{}{"code": 0, "data": map[string]interface{}{"revision_id": 2}}, + OnMatch: func(req *http.Request) { gotQuery = req.URL.Query() }, + }) + + err := runSlidesShortcut(t, f, stdout, SlidesDeleteSlide, []string{ + "+delete-slide", + "--presentation", "pres_abc", + "--slide-id", "slide_gone", + "--revision-id", "8", + "--tid", "tx_9", + "--as", "user", + }) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if gotQuery.Get("tid") != "tx_9" { + t.Fatalf("tid query = %q, want tx_9", gotQuery.Get("tid")) + } + if gotQuery.Get("revision_id") != "8" { + t.Fatalf("revision_id query = %q, want 8", gotQuery.Get("revision_id")) + } +} + +// TestDeleteSlideRejectsBlankSlideID keeps a whitespace-only id from becoming a +// pointless round trip that the backend answers with an opaque 3350001. +func TestDeleteSlideRejectsBlankSlideID(t *testing.T) { + t.Parallel() + + // Register nothing on purpose: httpmock rejects any unstubbed request, so + // issuing the DELETE would surface as "no stub for DELETE .../slide" + // rather than the ValidationError asserted below. + f, stdout, _, _ := cmdutil.TestFactory(t, slidesTestConfig(t, "")) + + err := runSlidesShortcut(t, f, stdout, SlidesDeleteSlide, []string{ + "+delete-slide", + "--presentation", "pres_abc", + "--slide-id", " ", + "--as", "user", + }) + if err == nil { + t.Fatal("expected a validation error for a blank --slide-id") + } + // No cause: the flag value is rejected outright, nothing is wrapped. + assertValidationProblem(t, err, "--slide-id", nil) +} + +func TestDeleteSlideResolvesWikiURL(t *testing.T) { + t.Parallel() + + f, stdout, _, reg := cmdutil.TestFactory(t, slidesTestConfig(t, "")) + reg.Register(&httpmock.Stub{ + Method: "GET", + URL: "/open-apis/wiki/v2/spaces/get_node", + Body: map[string]interface{}{ + "code": 0, + "data": map[string]interface{}{ + "node": map[string]interface{}{"obj_type": "slides", "obj_token": "pres_from_wiki"}, + }, + }, + }) + reg.Register(&httpmock.Stub{ + Method: "DELETE", + URL: "/open-apis/slides_ai/v1/xml_presentations/pres_from_wiki/slide", + Body: map[string]interface{}{"code": 0, "data": map[string]interface{}{"revision_id": 5}}, + }) + + err := runSlidesShortcut(t, f, stdout, SlidesDeleteSlide, []string{ + "+delete-slide", + "--presentation", "https://example.feishu.cn/wiki/wikcnTOKEN", + "--slide-id", "slide_gone", + "--as", "user", + }) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + data := decodeShortcutData(t, stdout) + if data["xml_presentation_id"] != "pres_from_wiki" { + t.Fatalf("xml_presentation_id = %v, want the wiki-resolved token", data["xml_presentation_id"]) + } +} + +// TestDeleteSlideRejectsNonSlidesWiki guards the cross-type case: a wiki node +// pointing at a docx must fail before the DELETE, not delete something else. +func TestDeleteSlideRejectsNonSlidesWiki(t *testing.T) { + t.Parallel() + + f, stdout, _, reg := cmdutil.TestFactory(t, slidesTestConfig(t, "")) + reg.Register(&httpmock.Stub{ + Method: "GET", + URL: "/open-apis/wiki/v2/spaces/get_node", + Body: map[string]interface{}{ + "code": 0, + "data": map[string]interface{}{ + "node": map[string]interface{}{"obj_type": "docx", "obj_token": "doc_abc"}, + }, + }, + }) + // Only the wiki stub is registered: an unstubbed DELETE fails with + // "no stub for DELETE .../slide", so asserting the error names the + // resolved obj_type also proves the DELETE never went out. + err := runSlidesShortcut(t, f, stdout, SlidesDeleteSlide, []string{ + "+delete-slide", + "--presentation", "https://example.feishu.cn/wiki/wikcnTOKEN", + "--slide-id", "slide_gone", + "--as", "user", + }) + if err == nil { + t.Fatal("expected an error when the wiki node is not a slides deck") + } + // Tagged --presentation, not --slide-id: the wrong input is the URL. No + // cause — the mismatch is read off a successful get_node response. + assertValidationProblem(t, err, "--presentation", nil) + if !strings.Contains(err.Error(), "docx") { + t.Fatalf("err = %v, want mention of the resolved obj_type", err) + } +} + +// TestDeleteSlideDryRunNamesTheTarget covers the plan callers are told to run +// before a destructive command: it must state which presentation and which page +// will be hit, since that is the whole point of dry-running a delete. +func TestDeleteSlideDryRunNamesTheTarget(t *testing.T) { + t.Parallel() + + f, stdout, _, _ := cmdutil.TestFactory(t, slidesTestConfig(t, "")) + err := runSlidesShortcut(t, f, stdout, SlidesDeleteSlide, []string{ + "+delete-slide", + "--presentation", "pres_abc", + "--slide-id", "slide_gone", + "--dry-run", + "--as", "user", + }) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + data := decodeShortcutData(t, stdout) + if data["slide_id"] != "slide_gone" { + t.Fatalf("slide_id = %v, want slide_gone", data["slide_id"]) + } + steps := decodeShortcutDryRunAPI(t, stdout) + if len(steps) != 1 { + t.Fatalf("planned %d calls, want a single DELETE: %#v", len(steps), steps) + } + step := assertDryRunStep(t, steps, 0, "DELETE", "/open-apis/slides_ai/v1/xml_presentations/pres_abc/slide") + + params, _ := step["params"].(map[string]interface{}) + if params["slide_id"] != "slide_gone" { + t.Fatalf("params.slide_id = %v, want slide_gone", params["slide_id"]) + } + if params["revision_id"] != float64(-1) { + t.Fatalf("params.revision_id = %v, want -1", params["revision_id"]) + } + if _, ok := params["tid"]; ok { + t.Fatalf("tid must be absent when --tid is unset: %#v", params) + } +} + +// TestDeleteSlideDryRunPlansWikiResolution pins that a wiki ref costs an extra +// declared step. Sending the wiki token straight to the slides API would delete +// from nothing — or, worse, from something else that happens to match. +func TestDeleteSlideDryRunPlansWikiResolution(t *testing.T) { + t.Parallel() + + f, stdout, _, _ := cmdutil.TestFactory(t, slidesTestConfig(t, "")) + err := runSlidesShortcut(t, f, stdout, SlidesDeleteSlide, []string{ + "+delete-slide", + "--presentation", "https://example.feishu.cn/wiki/wikcnTOKEN", + "--slide-id", "slide_gone", + "--tid", "tx_dry", + "--dry-run", + "--as", "user", + }) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + steps := decodeShortcutDryRunAPI(t, stdout) + if len(steps) != 2 { + t.Fatalf("planned %d calls, want resolve then delete: %#v", len(steps), steps) + } + resolve := assertDryRunStep(t, steps, 0, "GET", "/open-apis/wiki/v2/spaces/get_node") + resolveParams, _ := resolve["params"].(map[string]interface{}) + if resolveParams["token"] != "wikcnTOKEN" { + t.Fatalf("resolve token = %v, want wikcnTOKEN", resolveParams["token"]) + } + del := assertDryRunStep(t, steps, 1, "DELETE", "/open-apis/slides_ai/v1/xml_presentations/%3Cresolved_slides_token%3E/slide") + delParams, _ := del["params"].(map[string]interface{}) + if delParams["tid"] != "tx_dry" { + t.Fatalf("params.tid = %v, want tx_dry", delParams["tid"]) + } +} + +// TestDeleteSlideOmitsRevisionWhenAbsent keeps a missing upstream revision from +// being reported as revision 0, which a caller could then pass back as an +// optimistic lock and have rejected — or, worse, matched. +func TestDeleteSlideOmitsRevisionWhenAbsent(t *testing.T) { + t.Parallel() + + f, stdout, _, reg := cmdutil.TestFactory(t, slidesTestConfig(t, "")) + reg.Register(&httpmock.Stub{ + Method: "DELETE", + URL: "/open-apis/slides_ai/v1/xml_presentations/pres_abc/slide", + Body: map[string]interface{}{"code": 0, "data": map[string]interface{}{}}, + }) + + err := runSlidesShortcut(t, f, stdout, SlidesDeleteSlide, []string{ + "+delete-slide", + "--presentation", "pres_abc", + "--slide-id", "slide_gone", + "--as", "user", + }) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + data := decodeShortcutData(t, stdout) + if data["deleted"] != true { + t.Fatalf("deleted = %v, want true", data["deleted"]) + } + if _, ok := data["revision_id"]; ok { + t.Fatalf("revision_id should be omitted when absent upstream: %#v", data) + } +} + +// TestDeleteSlideSurfacesBackendFailure pins that a backend rejection is not +// reported as a successful delete. "deleted": true is written unconditionally +// once the call returns, so the error path has to stop before it. +func TestDeleteSlideSurfacesBackendFailure(t *testing.T) { + t.Parallel() + + f, stdout, _, reg := cmdutil.TestFactory(t, slidesTestConfig(t, "")) + reg.Register(&httpmock.Stub{ + Method: "DELETE", + URL: "/open-apis/slides_ai/v1/xml_presentations/pres_abc/slide", + Body: map[string]interface{}{"code": 3350001, "msg": "slide not found"}, + }) + + err := runSlidesShortcut(t, f, stdout, SlidesDeleteSlide, []string{ + "+delete-slide", + "--presentation", "pres_abc", + "--slide-id", "slide_missing", + "--as", "user", + }) + if err == nil { + t.Fatal("expected the backend rejection to surface") + } + if strings.Contains(stdout.String(), `"deleted"`) { + t.Fatalf("a failed delete must not print a deleted envelope: %s", stdout.String()) + } +} + +// TestDeleteSlideRejectsUnsupportedPresentation keeps a docx URL from reaching +// the slides API as if it were a presentation token. +func TestDeleteSlideRejectsUnsupportedPresentation(t *testing.T) { + t.Parallel() + + f, stdout, _, _ := cmdutil.TestFactory(t, slidesTestConfig(t, "")) + err := runSlidesShortcut(t, f, stdout, SlidesDeleteSlide, []string{ + "+delete-slide", + "--presentation", "https://example.feishu.cn/docx/doccnTOKEN", + "--slide-id", "slide_gone", + "--as", "user", + }) + if err == nil { + t.Fatal("expected a validation error for a docx URL") + } + assertValidationProblem(t, err, "--presentation", nil) +} diff --git a/shortcuts/slides/slides_errors.go b/shortcuts/slides/slides_errors.go index 9856cf1a13..8d84fb9a3a 100644 --- a/shortcuts/slides/slides_errors.go +++ b/shortcuts/slides/slides_errors.go @@ -5,6 +5,7 @@ package slides import ( "errors" + "io/fs" "github.com/larksuite/cli/errs" "github.com/larksuite/cli/extension/fileio" @@ -17,14 +18,26 @@ import ( // user-actionable input problems (exit code 2). Already-typed errors are not // expected here (Stat returns raw fs errors), so this always classifies as // validation. -func slidesInputStatError(err error, param, msg string) error { +// +// Why the "file not found" wording lives here and not at the call sites: Stat +// fails for a missing path, an unreadable parent directory and a rejected path +// shape alike, so a caller-supplied "file not found" suffix turns a permission +// error into the self-contradicting "file not found: permission denied". context +// names what was being read (the flag, the @-placeholder); this helper decides +// what actually went wrong. +func slidesInputStatError(err error, param, context string) error { if err == nil { return nil } - if errors.Is(err, fileio.ErrPathValidation) { - return errs.NewValidationError(errs.SubtypeInvalidArgument, "%s: unsafe file path: %s", msg, err).WithParam(param).WithCause(err) + switch { + case errors.Is(err, fileio.ErrPathValidation): + return errs.NewValidationError(errs.SubtypeInvalidArgument, "%s: unsafe file path: %s", context, err).WithParam(param).WithCause(err) + case errors.Is(err, fs.ErrNotExist): + // The raw error only restates the path, which context already names. + return errs.NewValidationError(errs.SubtypeInvalidArgument, "%s: file not found", context).WithParam(param).WithCause(err) + default: + return errs.NewValidationError(errs.SubtypeInvalidArgument, "%s: cannot read file: %s", context, err).WithParam(param).WithCause(err) } - return errs.NewValidationError(errs.SubtypeInvalidArgument, "%s: %s", msg, err).WithParam(param).WithCause(err) } // appendSlidesProgressHint preserves err's typed classification (per diff --git a/shortcuts/slides/slides_errors_test.go b/shortcuts/slides/slides_errors_test.go index f175402e0c..dacd16284a 100644 --- a/shortcuts/slides/slides_errors_test.go +++ b/shortcuts/slides/slides_errors_test.go @@ -5,6 +5,9 @@ package slides import ( "errors" + "fmt" + "io/fs" + "strings" "testing" "github.com/larksuite/cli/errs" @@ -15,6 +18,11 @@ import ( // offending flag via the typed Param — so callers route on the structured // field rather than parsing the message — and always classifies as a // validation error while preserving the underlying cause. +// +// The per-case wantMsg assertions exist because the helper, not the caller, +// decides which of the three stat failures happened: a caller that hard-coded +// "file not found" would report a permission error as "file not found: +// permission denied". func TestSlidesInputStatError(t *testing.T) { t.Parallel() @@ -23,27 +31,46 @@ func TestSlidesInputStatError(t *testing.T) { } tests := []struct { - name string - in error + name string + in error + wantMsg string }{ - {"path validation", fileio.ErrPathValidation}, - {"generic stat error", errors.New("permission denied")}, + {"path validation", fileio.ErrPathValidation, "ctx: unsafe file path:"}, + {"missing file", fs.ErrNotExist, "ctx: file not found"}, + {"wrapped missing file", fmt.Errorf("stat ./x.png: %w", fs.ErrNotExist), "ctx: file not found"}, + {"permission denied", fs.ErrPermission, "ctx: cannot read file:"}, + {"other stat error", errors.New("input/output error"), "ctx: cannot read file:"}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { t.Parallel() - err := slidesInputStatError(tt.in, "--file", "file not found") + err := slidesInputStatError(tt.in, "--file", "ctx") var ve *errs.ValidationError if !errors.As(err, &ve) { t.Fatalf("err = %v, want *errs.ValidationError", err) } + problem, ok := errs.ProblemOf(err) + if !ok || problem.Category != errs.CategoryValidation || problem.Subtype != errs.SubtypeInvalidArgument { + t.Fatalf("problem = %#v, ok = %v, want CategoryValidation/SubtypeInvalidArgument", problem, ok) + } if ve.Param != "--file" { t.Fatalf("Param = %q, want --file", ve.Param) } + if ve.Cause == nil { + t.Fatal("Cause must be preserved so callers can inspect the stat failure") + } if !errors.Is(err, tt.in) { t.Fatalf("err must wrap the underlying cause %v", tt.in) } + if !strings.HasPrefix(err.Error(), tt.wantMsg) { + t.Fatalf("err = %q, want prefix %q", err.Error(), tt.wantMsg) + } + // A missing file must not be reported with a second, contradicting + // diagnosis appended from the raw error. + if tt.wantMsg == "ctx: file not found" && strings.Contains(err.Error(), "cannot read file") { + t.Fatalf("err = %q, want a single diagnosis", err.Error()) + } }) } } diff --git a/shortcuts/slides/slides_media_upload.go b/shortcuts/slides/slides_media_upload.go index bd611f1776..8593499176 100644 --- a/shortcuts/slides/slides_media_upload.go +++ b/shortcuts/slides/slides_media_upload.go @@ -86,7 +86,7 @@ var SlidesMediaUpload = common.Shortcut{ stat, err := runtime.FileIO().Stat(filePath) if err != nil { - return slidesInputStatError(err, "--file", "file not found") + return slidesInputStatError(err, "--file", filePath) } if !stat.Mode().IsRegular() { return errs.NewValidationError(errs.SubtypeInvalidArgument, "file must be a regular file: %s", filePath).WithParam("--file") diff --git a/skills/lark-slides/SKILL.md b/skills/lark-slides/SKILL.md index 9232dd65bd..cff4a1fbac 100644 --- a/skills/lark-slides/SKILL.md +++ b/skills/lark-slides/SKILL.md @@ -79,9 +79,11 @@ metadata: | 用户需求 | 优先动作 | 关键文档 / 命令 | |----------|----------|-----------------| -| 新建 PPT | 先规划 `slide_plan.json`，再按复杂度选择一步或两步创建 | `planning-layer.md`、`visual-planning.md`、`asset-planning.md`、`lark-slides-create.md`、`slides +create` | +| 新建 PPT | 先规划 `slide_plan.json`，再按复杂度选择一步或两步创建 | `planning-layer.md`、`visual-planning.md`、`asset-planning.md`、`lark-slides-create.md`、`slides +create`、`lark-slides-add-slide.md`（两步创建逐页添加） | | 用户要求使用模板，或提供 PPTX 文件要求修改、美化 | 将模板导入为 Slides 再编辑 | `lark-slides-pptx-template-workflows.md` | | 编辑单个标题、文本块、图片或局部元素 | 优先块级替换/插入，不改页序 | `slides +replace-slide`、`lark-slides-replace-slide.md` | +| 给已有 PPT 追加或插入页面 | 一次一页，`--slide` 支持 `@file` 绕开 shell 转义 | `slides +add-slide`、`lark-slides-add-slide.md` | +| 删除页面 | 按 `slide_id` 单页删除，删前先回读确认 | `slides +delete-slide`、`lark-slides-delete-slide.md` | | 读取或分析已有 PPT | 解析 slides/wiki token，用 shortcut 回读全文 XML 或读取单页 XML，保存 `xml_presentation_id`、`slide_id`、`revision_id` | `slides +xml-get`、`xml_presentation.slide.get`、`lark-slides-xml-presentations-get.md` | | 查看或回滚历史版本 | 先用 `+history-list` 找 `history_version_id`，再 `+history-revert`，必要时 `+history-revert-status` 轮询 | [`lark-slides-history.md`](references/lark-slides-history.md) | | 获取幻灯片页面截图 | 用 `slide_id` 或页号指定页面，一次不超过 10 页 | `slides +screenshot`、`lark-slides-screenshot.md` | @@ -103,7 +105,7 @@ metadata: **CRITICAL — 新建演示文稿或大幅改写页面时，规划 `asset_need` MUST 遵循 [asset-planning.md](references/asset-planning.md)：只做元数据规划，必须有 `fallback_if_missing`，不得要求真实搜索、下载或上传素材。** -**CRITICAL — 将完整 `` XML 提交给 `slides +create --slides`、`xml_presentation.slide create` 或 `slides +replace-pages` 之前，MUST 先把待提交 XML 保存到本地文件并运行唯一版式准出入口 [`scripts/xml_text_overlap_lint.py`](scripts/xml_text_overlap_lint.py)；`summary.error_count` 必须为 0 才能调用接口，`summary.warning_count > 0` 时必须先做对应页面的截图复核。** +**CRITICAL — 将完整 `` XML 提交给 `slides +create --slides`、`slides +add-slide`、`xml_presentation.slide create` 或 `slides +replace-pages` 之前，MUST 先把待提交 XML 保存到本地文件并运行唯一版式准出入口 [`scripts/xml_text_overlap_lint.py`](scripts/xml_text_overlap_lint.py)；`summary.error_count` 必须为 0 才能调用接口。** **CRITICAL — 创建或大幅改写后，MUST 按 [validation-checklist.md](references/validation-checklist.md) 做显式验证：回读全文 XML、核对页数和关键元素，并使用 [`scripts/xml_text_overlap_lint.py`](scripts/xml_text_overlap_lint.py) 统一检查 XML、越界、重叠、空白页和内容稀疏风险。** @@ -145,7 +147,8 @@ lark-cli auth login --domain slides 调用相关命令前必须读取相关的文档以了解命令的使用方式： -- 创建：[`lark-slides-create.md`](references/lark-slides-create.md)、[`lark-slides-xml-presentation-slide-create.md`](references/lark-slides-xml-presentation-slide-create.md)（逐页添加） +- 创建：[`lark-slides-create.md`](references/lark-slides-create.md)、[`lark-slides-add-slide.md`](references/lark-slides-add-slide.md)（逐页添加 / 给已有 PPT 追加页面） +- 删除页面：[`lark-slides-delete-slide.md`](references/lark-slides-delete-slide.md) - 阅读：[`lark-slides-xml-presentations-get.md`](references/lark-slides-xml-presentations-get.md) - 编辑：[`lark-slides-edit-workflows.md`](references/lark-slides-edit-workflows.md)、[`lark-slides-replace-slide.md`](references/lark-slides-replace-slide.md)、[`lark-slides-replace-pages.md`](references/lark-slides-replace-pages.md) - 历史版本：[`lark-slides-history.md`](references/lark-slides-history.md) @@ -208,7 +211,7 @@ Step 2: 生成大纲 → 写入 slide_plan.json Step 3: 按 slide_plan.json 生成 XML → 创建 - 逐页消费 plan：key_message 定主结论，layout_type 定几何，visual_focus 定主视觉，text_density 定文本量 - 缺少真实素材时必须用 `fallback_if_missing` 生成替代图片，不要留空 - - 读 lark-slides-create.md 定一步创建还是两步创建，并据此构造 `slides +create`；两步创建再读 lark-slides-xml-presentation-slide-create.md 逐页添加 + - 读 lark-slides-create.md 定一步创建还是两步创建，并据此构造 `slides +create`；两步创建再读 lark-slides-add-slide.md 用 `+add-slide` 逐页添加 - 图片按 lark-slides-media-upload.md 处理；复杂 XML、转义和 3350001 排查按 troubleshooting.md 执行 Step 4: 审查 & 交付 @@ -270,7 +273,7 @@ N. 结尾页：[结尾文案] | `/slides/` | `https://example.larkoffice.com/slides/xxxxxxxxxxxxx` | `xml_presentation_id` | URL 路径中的 token 直接作为 `xml_presentation_id` 使用 | | `/wiki/` | `https://example.larkoffice.com/wiki/wikcnxxxxxxxxx` | `wiki_token` | ⚠️ **不能直接使用**，需要先查询获取真实的 `obj_token` | -> `+replace-slide` 和 `+media-upload` shortcut 会自动解析以上两种 URL；直接调用原生 API 时仍需手动解析 wiki 链接。 +> 带 `--presentation` 的 slides shortcut 都会自动解析以上两种 URL；直接调用原生 API 时仍需手动解析 wiki 链接。 ### Wiki 链接特殊处理（关键！） @@ -280,7 +283,7 @@ N. 结尾页：[结尾文案] lark-cli wiki spaces get_node --as user --params '{"token":"wiki_token"}' ``` -Shortcut `+replace-slide` 和 `+media-upload` 会自动解析 `/wiki/` URL；手动调用 `xml_presentations.*` / `xml_presentation.slide.*` 时才需要自己做这一步。 +带 `--presentation` 的 slides shortcut 都会自动解析 `/wiki/` URL 并校验 `obj_type`；手动调用 `xml_presentations.*` / `xml_presentation.slide.*` 时才需要自己做这一步。 ### 资源关系 @@ -303,6 +306,8 @@ Shortcut 是对常用操作的高级封装（`lark-cli slides + [flags]` | Shortcut | 说明 | |----------|------| | [`+create`](references/lark-slides-create.md) | 创建 PPT，可选一步添加页面 | +| [`+add-slide`](references/lark-slides-add-slide.md) | 向已有演示文稿追加或插入**一页**（`--before-slide-id` 控制位置），XML 支持 `@file` / stdin，`` 占位符自动上传 | +| [`+delete-slide`](references/lark-slides-delete-slide.md) | 按 `slide_id` 删除**一页**；不需要 `--yes`（原生 `xml_presentation.slide delete` 需要） | | [`+xml-get`](references/lark-slides-xml-presentations-get.md) | 读取全文 XML，用 `--presentation` 指定演示文稿的 `xml_presentation_id`，用 `--output` 把 XML 存到本地文件（必须是 CWD 内的相对路径，如 `.lark-slides/plan//readback.xml`） | | [`+screenshot`](references/lark-slides-screenshot.md) | 把幻灯片页面截图保存为本地图片，用 `--slide-number` 指定页号（从 1 开始，多页重复传入，一次最多 10 页），用 `--output-dir` 指定保存目录（必须是 CWD 内的相对路径，默认 `.lark-slides/screenshots`），失败时降级到 XML 回读等非截图检查 | | [`+media-upload`](references/lark-slides-media-upload.md) | 上传本地图片到指定演示文稿，返回 `file_token`（用作 ``），最大 20 MB | @@ -325,8 +330,8 @@ lark-cli slides [flags] # 调用 API 3. **`` 直接子元素只有 `