feat: support separate and suite skill layouts (#2211)

Author: Tantanz20020918

cmd/notice_test.go

@@ -8,6 +8,7 @@ import (
 	"testing"
 
 	"github.com/larksuite/cli/internal/deprecation"
+	"github.com/larksuite/cli/internal/skillscheck"
 )
 
 // composePendingNotice must surface a deprecated-command alias under the
@@ -45,6 +46,27 @@ func TestComposePendingNoticeDeprecatedCommand(t *testing.T) {
 	}
 }
 
+func TestComposePendingNoticeOfficialSkillsUnknown(t *testing.T) {
+	t.Cleanup(func() { skillscheck.SetPending(nil) })
+	skillscheck.SetPending(&skillscheck.StaleNotice{
+		Current:         "1.0.21",
+		Target:          "1.0.21",
+		OfficialUnknown: true,
+	})
+
+	got := composePendingNotice(nil)
+	entry, ok := got["skills"].(map[string]interface{})
+	if !ok {
+		t.Fatalf("missing skills notice: %#v", got)
+	}
+	if entry["official_unknown"] != true {
+		t.Fatalf("skills notice = %#v, want official_unknown=true", entry)
+	}
+	if entry["command"] != "lark-cli update" {
+		t.Fatalf("skills notice command = %v, want lark-cli update", entry["command"])
+	}
+}
+
 // With nothing pending, the provider returns nil so no "_notice" field is
 // emitted on a clean run.
 func TestComposePendingNoticeEmpty(t *testing.T) {

cmd/root.go

@@ -194,12 +194,16 @@ func composePendingNotice(plan *surface.Plan) map[string]interface{} {
 			}
 		}
 		if stale := skillscheck.GetPending(); stale != nil {
-			notice["skills"] = map[string]interface{}{
+			entry := map[string]interface{}{
 				"current": stale.Current,
 				"target":  stale.Target,
 				"message": stale.Message(),
 				"command": "lark-cli update",
 			}
+			if stale.OfficialUnknown {
+				entry["official_unknown"] = true
+			}
+			notice["skills"] = entry
 		}
 	}
 	if dep := deprecation.GetPending(); dep != nil {

cmd/update/update.go

@@ -88,10 +88,11 @@ func symArrow() string {
 
 // UpdateOptions holds inputs for the update command.
 type UpdateOptions struct {
-	Factory *cmdutil.Factory
-	JSON    bool
-	Force   bool
-	Check   bool
+	Factory      *cmdutil.Factory
+	JSON         bool
+	Force        bool
+	Check        bool
+	SkillsLayout string
 }
 
 // NewCmdUpdate creates the update command.
@@ -109,7 +110,9 @@ Detects the installation method automatically:
   - manual/other: shows GitHub Releases download URL
 
 Use --json for structured output (for AI agents and scripts).
-Use --check to only check for updates without installing.`,
+Use --check to only check for updates without installing.
+
+The skill name "lark-suite" is reserved for CLI-managed suite layout.`,
 		RunE: func(cmd *cobra.Command, args []string) error {
 			return updateRun(opts)
 		},
@@ -118,13 +121,24 @@ Use --check to only check for updates without installing.`,
 	cmd.Flags().BoolVar(&opts.JSON, "json", false, "structured JSON output")
 	cmd.Flags().BoolVar(&opts.Force, "force", false, "force reinstall even if already up to date")
 	cmd.Flags().BoolVar(&opts.Check, "check", false, "only check for updates, do not install")
+	cmd.Flags().StringVar(&opts.SkillsLayout, "skills-layout", "", "skills layout: separate or suite")
 	cmdutil.SetRisk(cmd, "high-risk-write")
 
 	return cmd
 }
 
 func updateRun(opts *UpdateOptions) error {
 	io := opts.Factory.IOStreams
+	if _, err := skillscheck.ParseLayout(opts.SkillsLayout); err != nil {
+		return reportError(opts, io, "validation",
+			errs.NewValidationError(errs.SubtypeInvalidArgument, "--skills-layout must be one of separate or suite").WithParam("--skills-layout"))
+	}
+	if opts.Check && strings.TrimSpace(opts.SkillsLayout) != "" {
+		return reportError(opts, io, "validation",
+			errs.NewValidationError(errs.SubtypeInvalidArgument, "--skills-layout cannot be used with --check").
+				WithParam("--skills-layout").
+				WithHint("Remove --skills-layout when using --check."))
+	}
 	cur := currentVersion()
 	updater := newUpdater()
 	// Brand only steers skills sync. updateRun skips that resolution in --check,
@@ -152,7 +166,10 @@ func updateRun(opts *UpdateOptions) error {
 	if !opts.Force && !update.IsNewer(latest, cur) {
 		var skillsResult *skillscheck.SyncResult
 		if !opts.Check {
-			skillsResult = runSkillsAndState(updater, io, cur, opts.Force)
+			skillsResult = runSkillsAndState(updater, io, cur, opts.Force, opts.SkillsLayout)
+			if err := reportSkillsFailure(opts, io, skillsResult); err != nil {
+				return err
+			}
 		}
 		return reportAlreadyUpToDate(opts, io, cur, latest, skillsResult, opts.Check)
 	}
@@ -195,10 +212,18 @@ func resolveSkillsBrand(f *cmdutil.Factory, errOut stdio.Writer) core.LarkBrand
 // error's exit code bare; human mode returns the typed error for the
 // dispatcher to render.
 func reportError(opts *UpdateOptions, io *cmdutil.IOStreams, errType string, typedErr errs.TypedError) error {
+	return reportErrorWithFields(opts, io, errType, typedErr, nil)
+}
+
+func reportErrorWithFields(opts *UpdateOptions, io *cmdutil.IOStreams, errType string, typedErr errs.TypedError, fields map[string]interface{}) error {
 	if opts.JSON {
-		output.PrintJson(io.Out, map[string]interface{}{
-			"ok": false, "error": map[string]interface{}{"type": errType, "message": typedErr.ProblemDetail().Message},
-		})
+		out := make(map[string]interface{}, len(fields)+2)
+		for key, value := range fields {
+			out[key] = value
+		}
+		out["ok"] = false
+		out["error"] = map[string]interface{}{"type": errType, "message": typedErr.ProblemDetail().Message}
+		output.PrintJson(io.Out, out)
 		return output.ErrBare(output.ExitCodeOf(typedErr))
 	}
 	return typedErr
@@ -229,8 +254,7 @@ func reportCheckResult(opts *UpdateOptions, io *cmdutil.IOStreams, cur, latest s
 }
 
 func doManualUpdate(opts *UpdateOptions, io *cmdutil.IOStreams, cur, latest string, detect selfupdate.DetectResult, updater *selfupdate.Updater) error {
-	skillsResult := runSkillsAndState(updater, io, cur, opts.Force)
-
+	skillsResult := runSkillsAndState(updater, io, cur, opts.Force, opts.SkillsLayout)
 	reason := detect.ManualReason()
 	if opts.JSON {
 		out := map[string]interface{}{
@@ -240,6 +264,9 @@ func doManualUpdate(opts *UpdateOptions, io *cmdutil.IOStreams, cur, latest stri
 			"url":     releaseURL(latest), "changelog": changelogURL(),
 		}
 		applySkillsResult(out, skillsResult)
+		if err := reportSkillsFailureWithFields(opts, io, skillsResult, out); err != nil {
+			return err
+		}
 		output.PrintJson(io.Out, out)
 		return nil
 	}
@@ -252,6 +279,9 @@ func doManualUpdate(opts *UpdateOptions, io *cmdutil.IOStreams, cur, latest stri
 	} else {
 		fmt.Fprintf(io.ErrOut, "\nOr install via npm (note: skills will not be synced):\n  npm install -g %s@%s\n  npx skills add larksuite/cli -y -g   # sync skills separately\n", selfupdate.NpmPackage, latest)
 	}
+	if err := reportSkillsFailure(opts, io, skillsResult); err != nil {
+		return err
+	}
 	emitSkillsTextHints(io, skillsResult)
 	return nil
 }
@@ -319,7 +349,21 @@ func doAutoUpdate(opts *UpdateOptions, io *cmdutil.IOStreams, cur, latest string
 		return output.ErrBare(output.ExitAPI)
 	}
 
-	skillsResult := runSkillsAndState(updater, io, latest, opts.Force)
+	skillsResult := runSkillsAndState(updater, io, latest, opts.Force, opts.SkillsLayout)
+	if skillsResult != nil && skillsResult.Err != nil {
+		fields := map[string]interface{}{
+			"previous_version": cur, "current_version": latest,
+			"latest_version": latest, "action": "updated",
+			"message": fmt.Sprintf("lark-cli updated from %s to %s, but skills update failed", cur, latest),
+			"url":     releaseURL(latest), "changelog": changelogURL(),
+		}
+		applySkillsResult(fields, skillsResult)
+		if !opts.JSON {
+			fmt.Fprintf(io.ErrOut, "\n%s lark-cli binary updated from %s to %s\n", symOK(), cur, latest)
+			fmt.Fprintf(io.ErrOut, "  Changelog: %s\n", changelogURL())
+		}
+		return reportSkillsFailureWithFields(opts, io, skillsResult, fields)
+	}
 
 	if opts.JSON {
 		result := map[string]interface{}{
@@ -366,14 +410,18 @@ func verificationFailureHint(updater *selfupdate.Updater, latest, pm string) str
 	return fmt.Sprintf("automatic rollback is unavailable on this platform; reinstall manually (skills will not be synced): npm install -g %s@%s && npx skills add larksuite/cli -y -g, or download %s", selfupdate.NpmPackage, latest, releaseURL(latest))
 }
 
-func runSkillsAndState(updater *selfupdate.Updater, io *cmdutil.IOStreams, stateVersion string, force bool) *skillscheck.SyncResult {
+func runSkillsAndState(updater *selfupdate.Updater, io *cmdutil.IOStreams, stateVersion string, force bool, requestedLayout string) *skillscheck.SyncResult {
+	layout, _ := skillscheck.ParseLayout(requestedLayout)
 	if !force {
-		if existing, ok := skillscheck.ReadSyncedVersion(); ok && normalizeVersion(existing) == normalizeVersion(stateVersion) {
-			return nil
+		if state, ok, err := skillscheck.ReadState(); err == nil && ok && normalizeVersion(state.Version) == normalizeVersion(stateVersion) {
+			if !state.OfficialSkillsUnknown && (layout == "" || skillscheck.EffectiveLayout(state) == layout) {
+				return nil
+			}
 		}
 	}
 	result := syncSkills(skillscheck.SyncOptions{
 		Version: stateVersion,
+		Layout:  layout,
 		Force:   force,
 		Runner:  updater,
 	})
@@ -383,6 +431,20 @@ func runSkillsAndState(updater *selfupdate.Updater, io *cmdutil.IOStreams, state
 	return result
 }
 
+func reportSkillsFailure(opts *UpdateOptions, io *cmdutil.IOStreams, result *skillscheck.SyncResult) error {
+	return reportSkillsFailureWithFields(opts, io, result, nil)
+}
+
+func reportSkillsFailureWithFields(opts *UpdateOptions, io *cmdutil.IOStreams, result *skillscheck.SyncResult, fields map[string]interface{}) error {
+	if result == nil || result.Err == nil {
+		return nil
+	}
+	typedErr := errs.NewInternalError(errs.SubtypeUnknown, "skills update failed: %s", result.Err).
+		WithHint("retry with `lark-cli update --force`").
+		WithCause(result.Err)
+	return reportErrorWithFields(opts, io, "skills_update_error", typedErr, fields)
+}
+
 // reportAlreadyUpToDate emits the JSON / pretty output for the
 // already-up-to-date branch, including any skills_action / skills_warning
 // fields derived from skillsResult. When check is true, this is the pure
@@ -418,9 +480,11 @@ func applySkillsStatus(env map[string]interface{}, target string) {
 	status := map[string]interface{}{
 		"current": state.Version,
 		"target":  target,
-		"in_sync": normalizeVersion(state.Version) == normalizeVersion(target),
+		"in_sync": normalizeVersion(state.Version) == normalizeVersion(target) && !state.OfficialSkillsUnknown,
 	}
-	if len(state.OfficialSkills) > 0 {
+	if state.OfficialSkillsUnknown {
+		status["official_unknown"] = true
+	} else if len(state.OfficialSkills) > 0 {
 		status["official"] = len(state.OfficialSkills)
 	}
 	if len(state.UpdatedSkills) > 0 {
@@ -429,6 +493,7 @@ func applySkillsStatus(env map[string]interface{}, target string) {
 	if len(state.SkippedDeletedSkills) > 0 {
 		status["skipped_deleted"] = state.SkippedDeletedSkills
 	}
+	status["layout"] = skillscheck.EffectiveLayout(state)
 	env["skills_status"] = status
 }
 
@@ -443,15 +508,23 @@ func applySkillsResult(env map[string]interface{}, r *skillscheck.SyncResult) {
 	default:
 		env["skills_action"] = "synced"
 		env["skills_summary"] = skillsSummary(r)
+		if r.Warning != "" {
+			env["skills_warning"] = r.Warning
+		}
 	}
 }
 
 func skillsSummary(r *skillscheck.SyncResult) map[string]interface{} {
 	summary := map[string]interface{}{
-		"official":        len(r.Official),
 		"updated":         len(r.Updated),
 		"added":           len(r.Added),
 		"skipped_deleted": len(r.SkippedDeleted),
+		"layout":          r.Layout,
+	}
+	if r.OfficialUnknown {
+		summary["official_unknown"] = true
+	} else {
+		summary["official"] = len(r.Official)
 	}
 	if len(r.Failed) > 0 {
 		summary["failed"] = r.Failed
@@ -468,10 +541,13 @@ func emitSkillsTextHints(io *cmdutil.IOStreams, r *skillscheck.SyncResult) {
 			fmt.Fprintf(io.ErrOut, "  Failed skills: %s\n", strings.Join(r.Failed, ", "))
 		}
 		fmt.Fprintf(io.ErrOut, "  To retry all official skills: lark-cli update --force\n")
+	case r.Warning != "":
+		fmt.Fprintf(io.ErrOut, "%s Skills updated using %s layout\n", symOK(), r.Layout)
+		fmt.Fprintf(io.ErrOut, "%s %s\n", symWarn(), r.Warning)
 	case r.Force:
-		fmt.Fprintf(io.ErrOut, "%s Skills updated: restored all %d official skills\n", symOK(), len(r.Official))
+		fmt.Fprintf(io.ErrOut, "%s Skills updated using %s layout: restored all %d official skills\n", symOK(), r.Layout, len(r.Official))
 	default:
-		fmt.Fprintf(io.ErrOut, "%s Skills updated: %d official, %d updated, %d added, %d skipped because deleted locally\n", symOK(), len(r.Official), len(r.Updated), len(r.Added), len(r.SkippedDeleted))
+		fmt.Fprintf(io.ErrOut, "%s Skills updated using %s layout: %d official, %d updated, %d added, %d skipped because deleted locally\n", symOK(), r.Layout, len(r.Official), len(r.Updated), len(r.Added), len(r.SkippedDeleted))
 		if len(r.SkippedDeleted) > 0 {
 			fmt.Fprintf(io.ErrOut, "  To restore all official skills: lark-cli update --force\n")
 		}