feat(slides): add +update-slide to apply a page of XML by diffing it

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

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="@<path>"` or `src='@<path>'` inside <img> tags.
 // The "@" prefix is the magic marker for "this is a local file path; upload it and
 // replace with file_token".

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 <id>` 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)
 		})

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", "<slide/>"})
+	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
+}

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)
 		}