diff --git a/internal/qualitygate/config/allowlists/public-domains.txt b/internal/qualitygate/config/allowlists/public-domains.txt index 3bf6e32c4a..bc69db3783 100644 --- a/internal/qualitygate/config/allowlists/public-domains.txt +++ b/internal/qualitygate/config/allowlists/public-domains.txt @@ -15,4 +15,5 @@ registry.npmjs.org registry.npmmirror.com sf16-sg.tiktokcdn.com www.feishu.cn +www.larkoffice.com www.larksuite.com diff --git a/shortcuts/slides/helpers.go b/shortcuts/slides/helpers.go index 2c22122646..0a316defad 100644 --- a/shortcuts/slides/helpers.go +++ b/shortcuts/slides/helpers.go @@ -10,6 +10,7 @@ import ( "strings" "github.com/larksuite/cli/errs" + "github.com/larksuite/cli/internal/validate" "github.com/larksuite/cli/shortcuts/common" ) @@ -110,6 +111,16 @@ func resolvePresentationID(runtime *common.RuntimeContext, ref presentationRef) } } +// slideReplaceAPIPath builds the xml_presentation.slide.replace endpoint for a +// presentation. Shared by +replace-slide (element-level parts) and +// +update-slide (a single whole-page part) so the two cannot drift apart. +func slideReplaceAPIPath(presentationID string) string { + return fmt.Sprintf( + "/open-apis/slides_ai/v1/xml_presentations/%s/slide/replace", + validate.EncodePathSegment(presentationID), + ) +} + // imgSrcPlaceholderRegex matches `src="@"` or `src='@'` inside tags. // The "@" prefix is the magic marker for "this is a local file path; upload it and // replace with file_token". diff --git a/shortcuts/slides/shortcuts.go b/shortcuts/slides/shortcuts.go index 729e729e62..e7069b1d33 100644 --- a/shortcuts/slides/shortcuts.go +++ b/shortcuts/slides/shortcuts.go @@ -18,50 +18,102 @@ var presentationFlagAliases = []string{ "url", } +// contentFlagAliases are the spellings agents reach for instead of --content +// when handing a whole page of XML to +update-slide. +// +// Deliberately not "slide": several slides commands take a --slide-id, so +// `--slide ` is a likely typo for that, and resolving it to --content +// would turn the typo into a request carrying an id where page XML belongs. +var contentFlagAliases = []string{ + "xml", + "slide-xml", + "slide-content", + "content-xml", +} + +// presentationAliasMap resolves every --presentation spelling and is attached +// to every shortcut that declares that flag. +var presentationAliasMap = aliasMap(map[string][]string{"presentation": presentationFlagAliases}) + +// wholePageAliasMap additionally resolves the --content spellings. It is +// attached only to the whole-page overwrite commands: --content exists on +// other slides shortcuts too, and letting these aliases resolve there would +// rewrite a mistyped flag into one the caller never meant to use. +var wholePageAliasMap = aliasMap(map[string][]string{ + "presentation": presentationFlagAliases, + "content": contentFlagAliases, +}) + +// aliasMap inverts canonical→aliases into alias→canonical. +func aliasMap(byCanonical map[string][]string) map[string]string { + out := make(map[string]string) + for canonical, aliases := range byCanonical { + for _, alias := range aliases { + out[alias] = canonical + } + } + return out +} + // Shortcuts returns all slides shortcuts. func Shortcuts() []common.Shortcut { - all := []common.Shortcut{ - SlidesCreate, - SlidesMediaUpload, - SlidesReplaceSlide, - SlidesReplacePages, - SlidesScreenshot, - SlidesXMLGet, - SlidesHistoryList, - SlidesHistoryRevert, - SlidesHistoryRevertStatus, + all := []struct { + shortcut common.Shortcut + aliases map[string]string + }{ + {shortcut: SlidesCreate, aliases: presentationAliasMap}, + {shortcut: SlidesMediaUpload, aliases: presentationAliasMap}, + {shortcut: SlidesReplaceSlide, aliases: presentationAliasMap}, + {shortcut: SlidesReplacePages, aliases: presentationAliasMap}, + {shortcut: SlidesUpdateSlide, aliases: wholePageAliasMap}, + {shortcut: SlidesUpdate, aliases: wholePageAliasMap}, + {shortcut: SlidesScreenshot, aliases: presentationAliasMap}, + {shortcut: SlidesXMLGet, aliases: presentationAliasMap}, + {shortcut: SlidesHistoryList, aliases: presentationAliasMap}, + {shortcut: SlidesHistoryRevert, aliases: presentationAliasMap}, + {shortcut: SlidesHistoryRevertStatus, aliases: presentationAliasMap}, } - for i := range all { - if hasPresentationFlag(all[i].Flags) { - all[i].PostMount = withPresentationFlagAliases(all[i].PostMount) + out := make([]common.Shortcut, 0, len(all)) + for _, entry := range all { + if hasAliasableFlag(entry.shortcut.Flags, entry.aliases) { + entry.shortcut.PostMount = withFlagAliases(entry.aliases, entry.shortcut.PostMount) } + out = append(out, entry.shortcut) } - return all + return out } -func hasPresentationFlag(flags []common.Flag) bool { +// hasAliasableFlag reports whether the shortcut declares a flag that one of +// the aliases resolves to, i.e. whether attaching the normalizer can do +// anything. +func hasAliasableFlag(flags []common.Flag, aliases map[string]string) bool { for _, flag := range flags { - if flag.Name == "presentation" { - return true + for _, canonical := range aliases { + if flag.Name == canonical { + return true + } } } return false } -// withPresentationFlagAliases accepts common agent-generated spellings for -// --presentation without registering extra flags. The aliases therefore stay -// out of help and completion while resolving to the canonical flag at parse -// time, matching the zero-round-trip compatibility used by Sheets. -func withPresentationFlagAliases(prev func(cmd *cobra.Command)) func(cmd *cobra.Command) { +// withFlagAliases accepts common agent-generated spellings for canonical flags +// without registering extra flags. The aliases therefore stay out of help and +// completion while resolving to the canonical flag at parse time, matching the +// zero-round-trip compatibility used by Sheets. +func withFlagAliases(aliases map[string]string, prev func(cmd *cobra.Command)) func(cmd *cobra.Command) { return func(cmd *cobra.Command) { if prev != nil { prev(cmd) } - cmd.Flags().SetNormalizeFunc(func(_ *pflag.FlagSet, name string) pflag.NormalizedName { - for _, alias := range presentationFlagAliases { - if name == alias { - return pflag.NormalizedName("presentation") - } + cmd.Flags().SetNormalizeFunc(func(fs *pflag.FlagSet, name string) pflag.NormalizedName { + // fs.Lookup re-enters this func with the canonical name; that + // terminates because no canonical name is itself an alias key + // (asserted by TestFlagAliasesAreNotCanonicalNames). Looking the + // canonical name up keeps a mistyped alias reported as the flag + // the caller actually typed on commands that lack the target. + if canonical, ok := aliases[name]; ok && fs.Lookup(canonical) != nil { + return pflag.NormalizedName(canonical) } return pflag.NormalizedName(name) }) diff --git a/shortcuts/slides/shortcuts_alias_test.go b/shortcuts/slides/shortcuts_alias_test.go index 72ab60863e..d4d41dd0de 100644 --- a/shortcuts/slides/shortcuts_alias_test.go +++ b/shortcuts/slides/shortcuts_alias_test.go @@ -7,37 +7,128 @@ import ( "strings" "testing" + "github.com/larksuite/cli/shortcuts/common" "github.com/spf13/cobra" ) -func TestWithPresentationFlagAliases(t *testing.T) { - for _, alias := range presentationFlagAliases { - t.Run(alias, func(t *testing.T) { - cmd := &cobra.Command{Use: "test"} - cmd.Flags().String("presentation", "", "presentation reference") - withPresentationFlagAliases(nil)(cmd) +func TestWithFlagAliases(t *testing.T) { + cases := []struct { + canonical string + aliases []string + }{ + {canonical: "presentation", aliases: presentationFlagAliases}, + {canonical: "content", aliases: contentFlagAliases}, + } + for _, tc := range cases { + for _, alias := range tc.aliases { + t.Run(tc.canonical+"/"+alias, func(t *testing.T) { + cmd := &cobra.Command{Use: "test"} + cmd.Flags().String(tc.canonical, "", tc.canonical+" value") + withFlagAliases(wholePageAliasMap, nil)(cmd) + + if err := cmd.Flags().Parse([]string{"--" + alias, "valABC"}); err != nil { + t.Fatalf("--%s should resolve to --%s: %v", alias, tc.canonical, err) + } + got, err := cmd.Flags().GetString(tc.canonical) + if err != nil { + t.Fatalf("read --%s: %v", tc.canonical, err) + } + if got != "valABC" { + t.Fatalf("--%s set --%s to %q, want valABC", alias, tc.canonical, got) + } + if usage := cmd.Flags().FlagUsages(); strings.Contains(usage, "--"+alias) { + t.Fatalf("hidden compatibility alias --%s leaked into help:\n%s", alias, usage) + } + }) + } + } +} + +// TestFlagAliasesOnlyResolveDeclaredFlags pins the guard that keeps a mistyped +// alias reported as the flag the caller actually typed: when the command does +// not declare the canonical target, the alias must be left alone. +func TestFlagAliasesOnlyResolveDeclaredFlags(t *testing.T) { + cmd := &cobra.Command{Use: "test"} + cmd.Flags().String("presentation", "", "presentation reference") + withFlagAliases(wholePageAliasMap, nil)(cmd) + + err := cmd.Flags().Parse([]string{"--xml", ""}) + if err == nil { + t.Fatal("--xml resolved on a command without --content, want unknown-flag error") + } + if !strings.Contains(err.Error(), "xml") { + t.Fatalf("error should name the flag the user typed, got: %v", err) + } +} + +// TestContentAliasesStayOffOtherShortcuts is the regression guard for a +// package-wide alias table: --content exists on other slides shortcuts, so a +// shared table silently turned `--xml` / `--slide-xml` there into a --content +// value the caller never meant to pass. Only the whole-page commands may +// resolve them. +func TestContentAliasesStayOffOtherShortcuts(t *testing.T) { + wholePage := map[string]bool{"+update-slide": true, "+update": true} + for _, shortcut := range Shortcuts() { + if wholePage[shortcut.Command] || !declaresFlag(shortcut.Flags, "content") { + continue + } + if shortcut.PostMount == nil { + continue + } + cmd := &cobra.Command{Use: shortcut.Command} + cmd.Flags().String("content", "", "content") + cmd.Flags().String("presentation", "", "presentation reference") + shortcut.PostMount(cmd) - if err := cmd.Flags().Parse([]string{"--" + alias, "presABC"}); err != nil { - t.Fatalf("--%s should resolve to --presentation: %v", alias, err) + for _, alias := range contentFlagAliases { + if err := cmd.Flags().Parse([]string{"--" + alias, "x"}); err == nil { + t.Errorf("%s resolved --%s to --content; content aliases must be scoped to the whole-page commands", shortcut.Command, alias) } - got, err := cmd.Flags().GetString("presentation") - if err != nil { - t.Fatalf("read --presentation: %v", err) + } + } +} + +// TestFlagAliasesAreNotCanonicalNames guards the termination argument in +// withFlagAliases: fs.Lookup re-enters the normalizer with the canonical name, +// which must not itself be an alias key. +func TestFlagAliasesAreNotCanonicalNames(t *testing.T) { + for _, aliases := range []map[string]string{presentationAliasMap, wholePageAliasMap} { + canonical := map[string]bool{} + for _, name := range aliases { + canonical[name] = true + } + for alias, target := range aliases { + if canonical[alias] { + t.Errorf("alias %q is also a canonical flag name; normalization would recurse", alias) } - if got != "presABC" { - t.Fatalf("--%s set --presentation to %q, want presABC", alias, got) + if alias == target { + t.Errorf("alias %q maps to itself", alias) } - if usage := cmd.Flags().FlagUsages(); strings.Contains(usage, "--"+alias) { - t.Fatalf("hidden compatibility alias --%s leaked into help:\n%s", alias, usage) + } + } +} + +// TestFlagAliasesDoNotShadowRealFlags catches the dangerous direction of +// pflag.SetNormalizeFunc: it re-normalizes flags that are already registered, +// so if a shortcut ever declares a flag whose name is an alias key, that flag +// collapses into the canonical one — no panic, no error, just a missing flag. +func TestFlagAliasesDoNotShadowRealFlags(t *testing.T) { + for _, shortcut := range Shortcuts() { + if shortcut.PostMount == nil { + continue + } + for _, flag := range shortcut.Flags { + if canonical, ok := wholePageAliasMap[flag.Name]; ok { + t.Errorf("%s declares --%s, which is an alias of --%s; the normalizer would erase it", shortcut.Command, flag.Name, canonical) } - }) + } } } -func TestShortcutsAttachPresentationFlagAliases(t *testing.T) { +func TestShortcutsAttachFlagAliases(t *testing.T) { count := 0 for _, shortcut := range Shortcuts() { - if !hasPresentationFlag(shortcut.Flags) { + if !declaresFlag(shortcut.Flags, "presentation") { continue } count++ @@ -66,3 +157,12 @@ func TestShortcutsAttachPresentationFlagAliases(t *testing.T) { t.Fatal("expected at least one slides shortcut with --presentation") } } + +func declaresFlag(flags []common.Flag, name string) bool { + for _, flag := range flags { + if flag.Name == name { + return true + } + } + return false +} diff --git a/shortcuts/slides/slides_replace_slide.go b/shortcuts/slides/slides_replace_slide.go index b518b2c814..d1125eb4ac 100644 --- a/shortcuts/slides/slides_replace_slide.go +++ b/shortcuts/slides/slides_replace_slide.go @@ -10,7 +10,6 @@ import ( "strings" "github.com/larksuite/cli/errs" - "github.com/larksuite/cli/internal/validate" "github.com/larksuite/cli/shortcuts/common" ) @@ -116,10 +115,7 @@ var SlidesReplaceSlide = common.Shortcut{ } else { dry.Desc(fmt.Sprintf("Replace %d part(s) on slide %s", len(parts), slideID)) } - dry.POST(fmt.Sprintf( - "/open-apis/slides_ai/v1/xml_presentations/%s/slide/replace", - validate.EncodePathSegment(presentationID), - )). + dry.POST(slideReplaceAPIPath(presentationID)). Params(query). Body(body) return dry.Set("parts_count", len(parts)) @@ -156,11 +152,7 @@ var SlidesReplaceSlide = common.Shortcut{ } body := map[string]interface{}{"parts": injected} - url := fmt.Sprintf( - "/open-apis/slides_ai/v1/xml_presentations/%s/slide/replace", - validate.EncodePathSegment(presentationID), - ) - data, err := runtime.CallAPITyped("POST", url, query, body) + data, err := runtime.CallAPITyped("POST", slideReplaceAPIPath(presentationID), query, body) if err != nil { return enrichSlidesReplaceError(err) } diff --git a/shortcuts/slides/slides_update_slide.go b/shortcuts/slides/slides_update_slide.go new file mode 100644 index 0000000000..dfe5b74b95 --- /dev/null +++ b/shortcuts/slides/slides_update_slide.go @@ -0,0 +1,390 @@ +// 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" +) + +// SlidesUpdateSlide applies a whole page of XML to an existing slide, keeping +// its slide_id and its position in the deck. +// +// The caller hands over the page they want; the CLI reads the page that is +// there, diffs the two, and sends one element-level part per difference. That +// indirection is not a stylistic choice — a single part covering the whole page +// is impossible. ReplacePart.block_id is validated as a short ELEMENT id (it +// must start with "b"), so the page's own id ("p"-prefixed) and the background +// fill's id ("f"-prefixed) are both rejected with 3350001. Element ids are the +// only handles this endpoint offers. +// +// What that buys the caller is the part they actually found painful: they no +// longer enumerate parts or hand-write each element's full XML (coordinates, +// size, font size included) to restyle a page. What it costs is one capability +// the endpoint cannot express at all: +// +// - The page background lives in + + + +

BEFORE

+
+
+ + +

SECOND

+
+
+
+ + + +
` + +const currentStyleXML = `` + +// wantPage assembles a page in the caller's own style, so the tests exercise +// the canonical comparison rather than string equality. +func wantPage(style, data, note string) string { + return `` + style + `` + data + `` + note + `` +} + +const ( + elemOne = `

BEFORE

` + elemOneNew = `

BEFORE

` + elemTwo = `

SECOND

` + noteKept = `` +) + +// registerPageRead stubs the read half. The write stub must be registered with +// Method POST, since httpmock matches the URL by substring and "/slide" is a +// prefix of "/slide/replace". +func registerPageRead(t *testing.T, reg *httpmock.Registry, pageXML string) { + t.Helper() + reg.Register(&httpmock.Stub{ + Method: "GET", + URL: "/open-apis/slides_ai/v1/xml_presentations/pres_abc/slide", + Body: map[string]interface{}{ + "code": 0, + "data": map[string]interface{}{ + "slide": map[string]interface{}{"slide_id": "piy", "content": pageXML}, + "revision_id": 7, + }, + }, + }) +} + +func registerWriteStub(t *testing.T, reg *httpmock.Registry, revision int) *httpmock.Stub { + t.Helper() + stub := &httpmock.Stub{ + Method: "POST", + URL: "/slide/replace", + Body: map[string]interface{}{"code": 0, "data": map[string]interface{}{"revision_id": revision}}, + } + reg.Register(stub) + return stub +} + +// forbidWrite registers a POST stub that fails the test if it is ever hit. +func forbidWrite(t *testing.T, reg *httpmock.Registry, why string) { + t.Helper() + reg.Register(&httpmock.Stub{ + Method: "POST", + URL: "/slide/replace", + Optional: true, + OnMatch: func(*http.Request) { t.Error(why) }, + Body: map[string]interface{}{"code": 0, "data": map[string]interface{}{}}, + }) +} + +func runUpdateSlide(t *testing.T, f *cmdutil.Factory, content string, extra ...string) error { + t.Helper() + args := append([]string{ + "+update-slide", + "--presentation", "pres_abc", + "--slide-id", "piy", + "--content", content, + }, extra...) + return runSlidesShortcut(t, f, nil, SlidesUpdateSlide, append(args, "--as", "user")) +} + +func TestUpdateSlideDeclaredScopes(t *testing.T) { + // The read scope is ENFORCED, not merely declared: every execution reads + // the page before writing it, and ConditionalScopes would let a write-only + // token reach the GET before failing. + want := []string{"slides:presentation:read", "slides:presentation:update", "slides:presentation:write_only"} + for _, sc := range []common.Shortcut{SlidesUpdateSlide, SlidesUpdate} { + if got := sc.ScopesForIdentity("user"); !reflect.DeepEqual(got, want) { + t.Errorf("%s user preflight scopes = %#v, want %#v", sc.Command, got, want) + } + declared := sc.DeclaredScopesForIdentity("user") + found := false + for _, got := range declared { + if got == "wiki:node:read" { + found = true + } + } + if !found { + t.Errorf("%s declared scopes %#v missing wiki:node:read", sc.Command, declared) + } + } +} + +func TestUpdateSlideIsRegisteredWithAlias(t *testing.T) { + canonical := findSlidesShortcut(t, "+update-slide") + alias := findSlidesShortcut(t, "+update") + + if canonical.Hidden { + t.Error("+update-slide must be visible in --help") + } + if !alias.Hidden { + t.Error("+update alias must stay hidden so only the canonical name is advertised") + } + if alias.Service != canonical.Service || alias.Risk != canonical.Risk || + !reflect.DeepEqual(alias.AuthTypes, canonical.AuthTypes) || + !reflect.DeepEqual(alias.Scopes, canonical.Scopes) || + !reflect.DeepEqual(alias.ConditionalScopes, canonical.ConditionalScopes) || + !reflect.DeepEqual(alias.Flags, canonical.Flags) || + !reflect.DeepEqual(alias.Tips, canonical.Tips) || + alias.Description != canonical.Description { + t.Error("alias metadata drifted from +update-slide; it must be derived, not re-declared") + } +} + +// TestUpdateSlideSendsOnePartPerChangedElement is the core contract: only what +// differs is touched, the part addresses the element by its own id, and the +// replacement carries the caller's bytes rather than a re-serialized form. +func TestUpdateSlideSendsOnePartPerChangedElement(t *testing.T) { + t.Parallel() + + f, stdout, _, reg := cmdutil.TestFactory(t, slidesTestConfig(t, "")) + registerPageRead(t, reg, currentPageXML) + stub := registerWriteStub(t, reg, 8) + + err := runUpdateSlide(t, f, wantPage(currentStyleXML, elemOneNew+elemTwo, noteKept)) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + parts := decodeUpdateSlideParts(t, stub.CapturedBody) + if len(parts) != 1 { + t.Fatalf("parts = %d, want 1 (only the restyled element differs): %#v", len(parts), parts) + } + if parts[0].Action != "block_replace" || parts[0].BlockID != "bbD" { + t.Errorf("part = %+v, want block_replace on bbD", parts[0]) + } + if !strings.Contains(parts[0].Replacement, `fontFamily="楷体"`) { + t.Errorf("replacement lost the edit: %q", parts[0].Replacement) + } + if parts[0].Replacement != elemOneNew { + t.Errorf("replacement should be the caller's exact bytes:\n got %q\nwant %q", parts[0].Replacement, elemOneNew) + } + + data := decodeShortcutData(t, stdout) + if data["parts_count"] != float64(1) || data["replaced"] != float64(1) || + data["inserted"] != float64(0) || data["deleted"] != float64(0) { + t.Errorf("counters = %v", data) + } + if data["revision_id"] != float64(8) { + t.Errorf("revision_id = %v, want 8", data["revision_id"]) + } +} + +// TestUpdateSlideCanonicalComparison pins that the server's own formatting does +// not read as a change. The caller here reorders attributes, collapses the +// indentation and self-closes differently, but means the same page. +func TestUpdateSlideCanonicalComparison(t *testing.T) { + t.Parallel() + + f, stdout, _, reg := cmdutil.TestFactory(t, slidesTestConfig(t, "")) + registerPageRead(t, reg, currentPageXML) + forbidWrite(t, reg, "an unchanged page must not be written") + + err := runUpdateSlide(t, f, wantPage(currentStyleXML, elemOne+elemTwo, noteKept)) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + data := decodeShortcutData(t, stdout) + if data["unchanged"] != true || data["parts_count"] != float64(0) { + t.Fatalf("an identical page should report unchanged, got %v", data) + } +} + +func TestUpdateSlideDeletesDroppedElements(t *testing.T) { + t.Parallel() + + f, stdout, _, reg := cmdutil.TestFactory(t, slidesTestConfig(t, "")) + registerPageRead(t, reg, currentPageXML) + stub := registerWriteStub(t, reg, 9) + + // bbv is dropped from the target page. + err := runUpdateSlide(t, f, wantPage(currentStyleXML, elemOne, noteKept)) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + parts := decodeUpdateSlideParts(t, stub.CapturedBody) + if len(parts) != 1 { + t.Fatalf("parts = %#v, want a single delete", parts) + } + if parts[0].BlockID != "bbv" || parts[0].Replacement != "" { + t.Errorf("part = %+v, want an empty replacement on bbv (delete)", parts[0]) + } + if data := decodeShortcutData(t, stdout); data["deleted"] != float64(1) { + t.Errorf("deleted = %v, want 1", data["deleted"]) + } +} + +func TestUpdateSlideInsertsNewElementsInPlace(t *testing.T) { + t.Parallel() + + f, stdout, _, reg := cmdutil.TestFactory(t, slidesTestConfig(t, "")) + registerPageRead(t, reg, currentPageXML) + stub := registerWriteStub(t, reg, 10) + + // A new element without an id, written between the two existing ones. + fresh := `` + err := runUpdateSlide(t, f, wantPage(currentStyleXML, elemOne+fresh+elemTwo, noteKept)) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + parts := decodeUpdateSlideParts(t, stub.CapturedBody) + if len(parts) != 1 || parts[0].Action != "block_insert" { + t.Fatalf("parts = %#v, want a single block_insert", parts) + } + if parts[0].Insertion != fresh { + t.Errorf("insertion = %q, want the caller's bytes", parts[0].Insertion) + } + // Position is expressed by naming the element it precedes; without it the + // new element would land at the end of the page. + if parts[0].InsertBeforeBlockID != "bbv" { + t.Errorf("insert_before_block_id = %q, want bbv", parts[0].InsertBeforeBlockID) + } + if data := decodeShortcutData(t, stdout); data["inserted"] != float64(1) { + t.Errorf("inserted = %v, want 1", data["inserted"]) + } +} + +func TestUpdateSlideAppendsWhenNewElementIsLast(t *testing.T) { + t.Parallel() + + f, _, _, reg := cmdutil.TestFactory(t, slidesTestConfig(t, "")) + registerPageRead(t, reg, currentPageXML) + stub := registerWriteStub(t, reg, 11) + + fresh := `` + err := runUpdateSlide(t, f, wantPage(currentStyleXML, elemOne+elemTwo+fresh, noteKept)) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + parts := decodeUpdateSlideParts(t, stub.CapturedBody) + if len(parts) != 1 || parts[0].InsertBeforeBlockID != "" { + t.Fatalf("a trailing new element must be appended, got %#v", parts) + } +} + +// TestUpdateSlideRejectsBackgroundChange is the honest-failure case. The +// background has no addressable element id, so the only alternative to an error +// is returning success with the background untouched. +func TestUpdateSlideRejectsBackgroundChange(t *testing.T) { + t.Parallel() + + for _, tt := range []struct { + name string + style string + wantWord string + }{ + { + name: "changed_fill", + style: ``, + wantWord: "background", + }, + { + name: "style_dropped", + style: ``, + wantWord: "background", + }, + } { + t.Run(tt.name, func(t *testing.T) { + f, _, _, reg := cmdutil.TestFactory(t, slidesTestConfig(t, "")) + registerPageRead(t, reg, currentPageXML) + forbidWrite(t, reg, "a background change must be rejected before any write") + + err := runUpdateSlide(t, f, wantPage(tt.style, elemOne+elemTwo, noteKept)) + if err == nil { + t.Fatal("expected a validation error") + } + var ve *errs.ValidationError + if !errors.As(err, &ve) { + t.Fatalf("expected *errs.ValidationError, got %T: %v", err, err) + } + if ve.Param != "--content" || !strings.Contains(ve.Message, tt.wantWord) { + t.Fatalf("error should name --content and mention the background, got %q (param %q)", ve.Message, ve.Param) + } + }) + } +} + +func TestUpdateSlideHandlesNote(t *testing.T) { + t.Parallel() + + t.Run("replaced", func(t *testing.T) { + f, stdout, _, reg := cmdutil.TestFactory(t, slidesTestConfig(t, "")) + registerPageRead(t, reg, currentPageXML) + stub := registerWriteStub(t, reg, 12) + + newNote := `

talk track

` + err := runUpdateSlide(t, f, wantPage(currentStyleXML, elemOne+elemTwo, newNote)) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + parts := decodeUpdateSlideParts(t, stub.CapturedBody) + if len(parts) != 1 || parts[0].BlockID != "bbb" || parts[0].Replacement != newNote { + t.Fatalf("parts = %#v, want a block_replace on the note id", parts) + } + if data := decodeShortcutData(t, stdout); data["note_replaced"] != true { + t.Errorf("note_replaced = %v, want true", data["note_replaced"]) + } + }) + + t.Run("cleared_when_omitted", func(t *testing.T) { + f, stdout, _, reg := cmdutil.TestFactory(t, slidesTestConfig(t, "")) + // A page whose note has content, so omitting is a real change. + withNote := strings.Replace(currentPageXML, ` + + `, `

old note

`, 1) + registerPageRead(t, reg, withNote) + stub := registerWriteStub(t, reg, 13) + + err := runUpdateSlide(t, f, wantPage(currentStyleXML, elemOne+elemTwo, "")) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + parts := decodeUpdateSlideParts(t, stub.CapturedBody) + if len(parts) != 1 || parts[0].BlockID != "bbb" { + t.Fatalf("parts = %#v, want the note cleared", parts) + } + if !strings.Contains(parts[0].Replacement, "") { + t.Errorf("replacement = %q, want an empty note", parts[0].Replacement) + } + if data := decodeShortcutData(t, stdout); data["note_cleared"] != true { + t.Errorf("note_cleared = %v, want true", data["note_cleared"]) + } + }) +} + +// TestUpdateSlideRejectsUnexpressibleEdits covers the edits that element-level +// parts cannot describe. Each one must fail before any write rather than land +// partially. +func TestUpdateSlideRejectsUnexpressibleEdits(t *testing.T) { + t.Parallel() + + for _, tt := range []struct { + name string + content string + wantWord string + }{ + { + name: "reordered_elements", + content: wantPage(currentStyleXML, elemTwo+elemOne, noteKept), + wantWord: "reorders", + }, + { + name: "unknown_element_id", + content: wantPage(currentStyleXML, elemOne+elemTwo+``, noteKept), + wantWord: "does not exist", + }, + { + name: "duplicate_element_id", + content: wantPage(currentStyleXML, elemOne+elemOne+elemTwo, noteKept), + wantWord: "twice", + }, + } { + t.Run(tt.name, func(t *testing.T) { + f, _, _, reg := cmdutil.TestFactory(t, slidesTestConfig(t, "")) + registerPageRead(t, reg, currentPageXML) + forbidWrite(t, reg, "an unexpressible edit must be rejected before any write") + + err := runUpdateSlide(t, f, tt.content) + if err == nil { + t.Fatal("expected a validation error") + } + p, ok := errs.ProblemOf(err) + if !ok { + t.Fatalf("expected a typed errs.* error, got %T: %v", err, err) + } + if !strings.Contains(p.Message, tt.wantWord) { + t.Fatalf("error message = %q, want it to mention %q", p.Message, tt.wantWord) + } + }) + } +} + +// TestUpdateSlideRejectsBadContentBeforeReading pins that malformed input fails +// without spending an API call. +func TestUpdateSlideRejectsBadContentBeforeReading(t *testing.T) { + t.Parallel() + + for _, tt := range []struct { + name string + content string + wantWord string + }{ + {name: "element_root", content: ``, wantWord: "+replace-slide"}, + {name: "presentation_root", content: ``, wantWord: "+replace-pages"}, + {name: "second_page", content: ``, wantWord: "one page per call"}, + {name: "trailing_text", content: `oops`, wantWord: "text after"}, + {name: "not_xml", content: `not xml at all`, wantWord: "no root element"}, + {name: "unclosed", content: ``, wantWord: "well-formed"}, + {name: "blank", content: ` `, wantWord: "cannot be empty"}, + // Slide-level structure the diff cannot represent: accepting any of + // these would silently drop the edit — and could even answer + // `unchanged` for a change the caller asked for. + {name: "unknown_slide_child", content: ``, wantWord: "unknown "}, + {name: "duplicate_data", content: ``, wantWord: "second "}, + {name: "duplicate_style", content: `

Overview

` + +// TestSlidesUpdateSlideDryRunE2E pins the shape of the orchestration through the +// built CLI: the command reads the page before writing it, because the parts it +// sends are derived from the page's current state rather than from --content +// alone. Dry-run deliberately cannot show the parts — computing them would +// require the read it is not allowed to perform. +func TestSlidesUpdateSlideDryRunE2E(t *testing.T) { + setSlidesDryRunEnv(t) + + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + t.Cleanup(cancel) + + result, err := clie2e.RunCmd(ctx, clie2e.Request{ + Args: []string{ + "slides", "+update-slide", + "--presentation", "presUpdateSlideDryRun", + "--slide-id", "piy", + "--content", updateSlideDryRunPageXML, + "--revision-id", "17", + "--dry-run", + }, + DefaultAs: "bot", + }) + require.NoError(t, err) + result.AssertExitCode(t, 0) + + api := gjson.Get(result.Stdout, "data.api").Array() + require.Len(t, api, 2, "the command reads then writes\n%s", result.Stdout) + + require.Equal(t, "GET", api[0].Get("method").String(), result.Stdout) + require.Equal(t, + "/open-apis/slides_ai/v1/xml_presentations/presUpdateSlideDryRun/slide", + api[0].Get("url").String(), result.Stdout, + ) + require.Equal(t, "POST", api[1].Get("method").String(), result.Stdout) + require.Equal(t, + "/open-apis/slides_ai/v1/xml_presentations/presUpdateSlideDryRun/slide/replace", + api[1].Get("url").String(), result.Stdout, + ) + + // The same revision is used for both calls so the parts apply to the + // snapshot they were diffed against. + for i := range api { + require.Equal(t, "piy", api[i].Get("params.slide_id").String(), result.Stdout) + require.Equal(t, int64(17), api[i].Get("params.revision_id").Int(), result.Stdout) + require.False(t, api[i].Get("params.tid").Exists(), "tid must be omitted when empty\n%s", result.Stdout) + } + + require.Equal(t, int64(1), gjson.Get(result.Stdout, "data.wanted_element_count").Int(), result.Stdout) +} + +// TestSlidesUpdateAliasDryRunE2E proves the hidden `+update` spelling reaches +// the same logic through the real CLI, along with the hidden --token / --xml +// flag spellings. +func TestSlidesUpdateAliasDryRunE2E(t *testing.T) { + setSlidesDryRunEnv(t) + + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + t.Cleanup(cancel) + + result, err := clie2e.RunCmd(ctx, clie2e.Request{ + Args: []string{ + "slides", "+update", + "--token", "presUpdateSlideDryRun", + "--slide-id", "piy", + "--xml", updateSlideDryRunPageXML, + "--dry-run", + }, + DefaultAs: "bot", + }) + require.NoError(t, err) + result.AssertExitCode(t, 0) + + require.Equal(t, + "/open-apis/slides_ai/v1/xml_presentations/presUpdateSlideDryRun/slide/replace", + gjson.Get(result.Stdout, "data.api.1.url").String(), + result.Stdout, + ) + require.Equal(t, int64(-1), gjson.Get(result.Stdout, "data.api.1.params.revision_id").Int(), + "-1 is the default: apply against the latest revision\n%s", result.Stdout) +} + +// TestSlidesUpdateSlideRejectsBadContentDryRunE2E is the guardrail check +// through the built CLI: inputs whose failure mode is data loss must be +// refused before any request is built, with the full typed error contract — +// agents branch on type/subtype/param, not on prose. +func TestSlidesUpdateSlideRejectsBadContentDryRunE2E(t *testing.T) { + setSlidesDryRunEnv(t) + + ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second) + t.Cleanup(cancel) + + for _, tt := range []struct { + name string + content string + wantMessage string + }{ + { + // An element-level fragment would mean "the page should contain + // only this" and delete everything else. + name: "element_root", + content: `

oops

`, + wantMessage: "+replace-slide", + }, + { + // XML fetched for page A posted against page B. + name: "root_id_mismatch", + content: ``, + wantMessage: "read from a different page", + }, + { + // Slide-level structure the diff cannot represent would be + // silently dropped — possibly reported as `unchanged`. + name: "unknown_slide_child", + content: ``, + wantMessage: "unknown ", + }, + { + // Container attributes have no element-level part to travel in. + name: "root_attribute", + content: ``, + wantMessage: "unsupported attribute", + }, + { + // A namespace binding inherited from the root changes what every + // descendant name means; only the official SML declaration passes. + name: "wrong_default_xmlns", + content: ``, + wantMessage: "unsupported xmlns", + }, + } { + t.Run(tt.name, func(t *testing.T) { + result, err := clie2e.RunCmd(ctx, clie2e.Request{ + Args: []string{ + "slides", "+update-slide", + "--presentation", "presUpdateSlideDryRun", + "--slide-id", "piy", + "--content", tt.content, + "--dry-run", + }, + DefaultAs: "bot", + }) + // RunCmd errors only when the CLI could not be launched; a normal + // non-zero exit lands in result.ExitCode. Discarding this error + // would turn a broken harness into a nil-pointer panic. + require.NoError(t, err, "the CLI must launch") + // Validation errors have a fixed process contract: exit code 2, + // nothing on stdout, the typed envelope on stderr. + require.Equal(t, 2, result.ExitCode, + "stdout:\n%s\nstderr:\n%s", result.Stdout, result.Stderr) + require.Empty(t, result.Stdout, "a refused command must not emit a result") + + require.Equal(t, "validation", gjson.Get(result.Stderr, "error.type").String(), result.Stderr) + require.Equal(t, "invalid_argument", gjson.Get(result.Stderr, "error.subtype").String(), result.Stderr) + require.Equal(t, "--content", gjson.Get(result.Stderr, "error.param").String(), result.Stderr) + require.Contains(t, gjson.Get(result.Stderr, "error.message").String(), tt.wantMessage, result.Stderr) + }) + } +} diff --git a/tests/cli_e2e/slides/slides_update_slide_workflow_test.go b/tests/cli_e2e/slides/slides_update_slide_workflow_test.go new file mode 100644 index 0000000000..a541e6f45a --- /dev/null +++ b/tests/cli_e2e/slides/slides_update_slide_workflow_test.go @@ -0,0 +1,262 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package slides + +import ( + "context" + "os" + "regexp" + "strings" + "testing" + "time" + + clie2e "github.com/larksuite/cli/tests/cli_e2e" + "github.com/stretchr/testify/require" + "github.com/tidwall/gjson" +) + +// skipWithoutCleanupScopes refuses to create a deck this run cannot delete, +// when that is knowable. AGENTS.md wants live workflows self-contained +// (create → use → cleanup); a cleanup that fails on missing scopes both fails +// the package after the workflow already passed and leaks one presentation per +// run. The capability is probed up front with --dry-run, which runs the same +// scope pre-flight as the real cleanup without touching anything remote. +// +// The probe is authoritative only for stored credentials, whose scope grants +// the pre-flight can read. A token injected through the environment +// (TEST_USER_ACCESS_TOKEN / LARKSUITE_CLI_USER_ACCESS_TOKEN) carries no scope +// metadata, and the pre-flight deliberately skips when scopes are unknown — +// exit 0 from the probe proves nothing there. No API exposes a token's grants +// without exercising them, so for that path the run proceeds on the documented +// requirement that the CI identity is provisioned with the cleanup scopes +// (coverage.md), and a cleanup failure stays fatal and visible. +func skipWithoutCleanupScopes(ctx context.Context, t *testing.T) { + t.Helper() + if os.Getenv("TEST_USER_ACCESS_TOKEN") != "" || os.Getenv("LARKSUITE_CLI_USER_ACCESS_TOKEN") != "" { + t.Log("cleanup-scope probe skipped: environment tokens carry no scope metadata, so a dry-run pre-flight cannot prove anything; the CI identity must be provisioned with space:document:delete and drive:drive.metadata:readonly (see coverage.md)") + return + } + result, err := clie2e.RunCmd(ctx, clie2e.Request{ + Args: []string{"drive", "+delete", "--file-token", "cleanup_scope_probe", "--type", "slides", "--yes", "--dry-run"}, + DefaultAs: "user", + }) + require.NoError(t, err, "the CLI must launch for the scope probe") + switch { + case result.ExitCode == 0: + // Stored credential with the scopes present. + case strings.Contains(result.Stderr, "missing_scope"): + t.Skipf("user token lacks the cleanup scopes (space:document:delete, drive:drive.metadata:readonly); refusing to create a deck the run cannot delete\nstderr:\n%s", result.Stderr) + default: + // Anything else is a broken probe, not a known-good capability; + // proceeding would risk creating a deck under unknown conditions. + t.Fatalf("cleanup-scope probe failed unexpectedly (exit %d)\nstdout:\n%s\nstderr:\n%s", result.ExitCode, result.Stdout, result.Stderr) + } +} + +// TestSlides_UpdateSlideWorkflowAsUser is the only test that can prove this +// command works, and it exists because an earlier version of it did not: the +// whole design once rested on sending a single part covering the page, which +// unit stubs happily accepted and the real API rejects outright (block_id is +// validated as a short ELEMENT id, so a page id cannot be addressed). Stubs +// prove the request shape; only a live round trip proves the request is legal +// and does what it claims. +// +// It walks every operation the diff can emit — replace, insert, delete, and the +// no-op — then checks the two things element-level parts cannot express are +// refused rather than silently dropped. +func TestSlides_UpdateSlideWorkflowAsUser(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 4*time.Minute) + t.Cleanup(cancel) + + // Without a user token the load-bearing backend behavior goes unverified in + // this run; say so rather than skipping quietly. + clie2e.SkipWithoutUserToken(t) + skipWithoutCleanupScopes(ctx, t) + + parentT := t + suffix := clie2e.GenerateSuffix() + title := "slides-update-e2e-" + suffix + controlText := "Control " + suffix + originalText := "Original " + suffix + + page := func(body string) string { + return `` + + `` + + `

` + body + `

` + } + jsonArray := func(xmls ...string) string { + quoted := make([]string, 0, len(xmls)) + for _, xml := range xmls { + quoted = append(quoted, `"`+strings.ReplaceAll(xml, `"`, `\"`)+`"`) + } + return "[" + strings.Join(quoted, ",") + "]" + } + readPage := func(t *testing.T, presentationID, slideID string) string { + t.Helper() + result, err := clie2e.RunCmd(ctx, clie2e.Request{ + Args: []string{"slides", "+xml-get", "--presentation", presentationID, "--slide-id", slideID}, + DefaultAs: "user", + }) + require.NoError(t, err) + result.AssertExitCode(t, 0) + content := gjson.Get(result.Stdout, "data.slide.content").String() + require.NotEmpty(t, content, "stdout:\n%s", result.Stdout) + return content + } + update := func(t *testing.T, presentationID, slideID, content string) *clie2e.Result { + t.Helper() + result, err := clie2e.RunCmd(ctx, clie2e.Request{ + Args: []string{ + "slides", "+update-slide", + "--presentation", presentationID, + "--slide-id", slideID, + "--content", content, + }, + DefaultAs: "user", + }) + require.NoError(t, err) + return result + } + + var presentationID, targetSlideID string + + t.Run("create a two-page presentation as user", func(t *testing.T) { + result, err := clie2e.RunCmd(ctx, clie2e.Request{ + Args: []string{ + "slides", "+create", + "--title", title, + "--slides", jsonArray(page(controlText), page(originalText)), + }, + DefaultAs: "user", + }) + require.NoError(t, err) + result.AssertExitCode(t, 0) + result.AssertStdoutStatus(t, true) + + presentationID = gjson.Get(result.Stdout, "data.xml_presentation_id").String() + require.NotEmpty(t, presentationID, "stdout:\n%s", result.Stdout) + slideIDs := gjson.Get(result.Stdout, "data.slide_ids").Array() + require.Len(t, slideIDs, 2, "stdout:\n%s", result.Stdout) + targetSlideID = slideIDs[1].String() + + parentT.Cleanup(func() { + cleanupCtx, cancel := clie2e.CleanupContext() + defer cancel() + + deleteResult, deleteErr := clie2e.RunCmd(cleanupCtx, clie2e.Request{ + Args: []string{ + "drive", "+delete", + "--file-token", presentationID, + "--type", "slides", + "--yes", + }, + DefaultAs: "user", + }) + // Deleting needs space:document:delete, which a token scoped only + // for slides does not carry; report rather than fail so a scope gap + // does not mask the workflow result. The deck is named with the + // run suffix so a leftover is identifiable. + clie2e.ReportCleanupFailure(parentT, "delete presentation "+presentationID, deleteResult, deleteErr) + }) + }) + + t.Run("restyle an element: one replace part", func(t *testing.T) { + require.NotEmpty(t, targetSlideID, "presentation should be created first") + + // Change the font on every element, which is the edit the command + // exists for. Only is touched, so exactly one element differs. + current := readPage(t, presentationID, targetSlideID) + wanted := regexp.MustCompile(`fontFamily="[^"]*"`).ReplaceAllString(current, `fontFamily="楷体"`) + require.NotEqual(t, current, wanted, "the page should have had a fontFamily to change:\n%s", current) + + result := update(t, presentationID, targetSlideID, wanted) + result.AssertExitCode(t, 0) + result.AssertStdoutStatus(t, true) + require.Equal(t, int64(1), gjson.Get(result.Stdout, "data.replaced").Int(), result.Stdout) + require.Equal(t, int64(1), gjson.Get(result.Stdout, "data.parts_count").Int(), result.Stdout) + require.Equal(t, targetSlideID, gjson.Get(result.Stdout, "data.slide_id").String(), + "the page keeps its slide_id\n%s", result.Stdout) + + after := readPage(t, presentationID, targetSlideID) + require.Contains(t, after, `fontFamily="楷体"`, "the restyle must be live:\n%s", after) + require.Contains(t, after, originalText, "restyling must not change the text:\n%s", after) + }) + + t.Run("writing the same page again is a no-op", func(t *testing.T) { + current := readPage(t, presentationID, targetSlideID) + result := update(t, presentationID, targetSlideID, current) + result.AssertExitCode(t, 0) + require.True(t, gjson.Get(result.Stdout, "data.unchanged").Bool(), + "an identical page must not be written\n%s", result.Stdout) + require.Equal(t, int64(0), gjson.Get(result.Stdout, "data.parts_count").Int(), result.Stdout) + }) + + t.Run("add an element without an id: one insert part", func(t *testing.T) { + current := readPage(t, presentationID, targetSlideID) + added := `

Added ` + suffix + `

` + wanted := strings.Replace(current, "
", added+"
", 1) + + result := update(t, presentationID, targetSlideID, wanted) + result.AssertExitCode(t, 0) + require.Equal(t, int64(1), gjson.Get(result.Stdout, "data.inserted").Int(), result.Stdout) + + after := readPage(t, presentationID, targetSlideID) + require.Contains(t, after, "Added "+suffix, "the new element must be live:\n%s", after) + require.Contains(t, after, originalText, "the existing element must survive:\n%s", after) + }) + + t.Run("drop an element: one delete part", func(t *testing.T) { + current := readPage(t, presentationID, targetSlideID) + // Remove the original title shape, keeping the one added above. + wanted := regexp.MustCompile(`(?s)\s*]*>\s*]*>\s*

`+regexp.QuoteMeta(originalText)+`

.*?`). + ReplaceAllString(current, "") + require.NotEqual(t, current, wanted, "the title shape should have been removable:\n%s", current) + + result := update(t, presentationID, targetSlideID, wanted) + result.AssertExitCode(t, 0) + require.Equal(t, int64(1), gjson.Get(result.Stdout, "data.deleted").Int(), result.Stdout) + + after := readPage(t, presentationID, targetSlideID) + require.NotContains(t, after, originalText, "the dropped element must be gone:\n%s", after) + require.Contains(t, after, "Added "+suffix, "the kept element must remain:\n%s", after) + }) + + t.Run("a background change is refused, not dropped", func(t *testing.T) { + current := readPage(t, presentationID, targetSlideID) + // Matches both the self-closing . + styleBlock := regexp.MustCompile(`(?s)|]*>.*?`) + require.True(t, styleBlock.MatchString(current), "page should carry a `) + + result := update(t, presentationID, targetSlideID, wanted) + require.NotEqual(t, 0, result.ExitCode, + "the background cannot be expressed, so it must fail loudly\nstdout:\n%s\nstderr:\n%s", result.Stdout, result.Stderr) + require.Contains(t, result.Stderr, "background", "stderr:\n%s", result.Stderr) + + // And nothing may have been written: the page is still what it was. + require.Equal(t, current, readPage(t, presentationID, targetSlideID), + "a refused background change must leave the page untouched") + }) + + t.Run("the other page and the page order are untouched", func(t *testing.T) { + result, err := clie2e.RunCmd(ctx, clie2e.Request{ + Args: []string{"api", "get", "/open-apis/slides_ai/v1/xml_presentations/" + presentationID}, + DefaultAs: "user", + Params: map[string]any{"revision_id": -1}, + }) + require.NoError(t, err) + result.AssertExitCode(t, 0) + + content := gjson.Get(result.Stdout, "data.xml_presentation.content").String() + require.Contains(t, content, controlText, "the control page must be untouched\n%s", content) + + controlAt := strings.Index(content, controlText) + editedAt := strings.Index(content, "Added "+suffix) + require.GreaterOrEqual(t, controlAt, 0, content) + require.GreaterOrEqual(t, editedAt, 0, content) + require.Less(t, controlAt, editedAt, "the deck order must not change\n%s", content) + }) +}