Skip to content

feat: add update skill sync scope flags #1189

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
nguyenngothuong wants to merge 1 commit into
base: main
Choose a base branch
from
+153 −9
Draft

feat: add update skill sync scope flags #1189

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 33 additions & 9 deletions cmd/update/update.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,10 +85,12 @@ 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
CLIOnly bool
WithSkills bool
}

// NewCmdUpdate creates the update command.
Expand All @@ -105,7 +107,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.
Use --cli-only to update only the CLI package and skip skills sync.
Use --with-skills to explicitly update both the CLI package and official skills.`,
RunE: func(cmd *cobra.Command, args []string) error {
return updateRun(opts)
},
Expand All @@ -114,13 +118,19 @@ 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().BoolVar(&opts.CLIOnly, "cli-only", false, "update only lark-cli; do not install or sync skills")
cmd.Flags().BoolVar(&opts.WithSkills, "with-skills", false, "update lark-cli and sync official skills (default)")
cmdutil.SetRisk(cmd, "high-risk-write")

return cmd
}

func updateRun(opts *UpdateOptions) error {
io := opts.Factory.IOStreams
if opts.CLIOnly && opts.WithSkills {
return reportError(opts, io, output.ExitValidation, "validation", "--cli-only and --with-skills are mutually exclusive")
}

cur := currentVersion()
updater := newUpdater()

Expand All @@ -144,7 +154,7 @@ 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 = runSkillsAndStateIfEnabled(opts, updater, io, cur)
}
return reportAlreadyUpToDate(opts, io, cur, latest, skillsResult, opts.Check)
}
Expand Down Expand Up @@ -202,7 +212,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 := runSkillsAndStateIfEnabled(opts, updater, io, cur)

reason := detect.ManualReason()
if opts.JSON {
Expand Down Expand Up @@ -280,7 +290,7 @@ func doNpmUpdate(opts *UpdateOptions, io *cmdutil.IOStreams, cur, latest string,
return output.ErrBare(output.ExitAPI)
}

skillsResult := runSkillsAndState(updater, io, latest, opts.Force)
skillsResult := runSkillsAndStateIfEnabled(opts, updater, io, latest)

if opts.JSON {
result := map[string]interface{}{
Expand All @@ -296,7 +306,7 @@ func doNpmUpdate(opts *UpdateOptions, io *cmdutil.IOStreams, cur, latest string,

fmt.Fprintf(io.ErrOut, "\n%s Successfully updated lark-cli from %s to %s\n", symOK(), cur, latest)
fmt.Fprintf(io.ErrOut, " Changelog: %s\n", changelogURL())
if skillsResult != nil {
if skillsResult != nil && skillsResult.Action != "skipped" {
fmt.Fprintf(io.ErrOut, "\nUpdating skills ...\n")
}
emitSkillsTextHints(io, skillsResult)
Expand All @@ -317,6 +327,16 @@ func verificationFailureHint(updater *selfupdate.Updater, latest string) string
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 runSkillsAndStateIfEnabled(opts *UpdateOptions, updater *selfupdate.Updater, io *cmdutil.IOStreams, stateVersion string) *skillscheck.SyncResult {
if opts.CLIOnly {
return &skillscheck.SyncResult{
Action: "skipped",
Detail: "skills sync skipped by --cli-only",
}
}
return runSkillsAndState(updater, io, stateVersion, opts.Force)
}

func runSkillsAndState(updater *selfupdate.Updater, io *cmdutil.IOStreams, stateVersion string, force bool) *skillscheck.SyncResult {
if !force {
if existing, ok := skillscheck.ReadSyncedVersion(); ok && normalizeVersion(existing) == normalizeVersion(stateVersion) {
Expand Down Expand Up @@ -387,6 +407,8 @@ func applySkillsResult(env map[string]interface{}, r *skillscheck.SyncResult) {
switch {
case r == nil:
env["skills_action"] = "in_sync"
case r.Action == "skipped":
env["skills_action"] = "skipped"
case r.Err != nil:
env["skills_action"] = "failed"
env["skills_warning"] = fmt.Sprintf("skills update failed: %s", r.Err)
Expand All @@ -413,6 +435,8 @@ func skillsSummary(r *skillscheck.SyncResult) map[string]interface{} {
func emitSkillsTextHints(io *cmdutil.IOStreams, r *skillscheck.SyncResult) {
switch {
case r == nil:
case r.Action == "skipped":
fmt.Fprintf(io.ErrOut, "Skills sync skipped (--cli-only)\n")
case r.Err != nil:
fmt.Fprintf(io.ErrOut, "%s Skills update failed: %v\n", symWarn(), r.Err)
if len(r.Failed) > 0 {
Expand Down
120 changes: 120 additions & 0 deletions cmd/update/update_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,126 @@ func TestUpdateNpm_JSON(t *testing.T) {
}
}

func TestUpdateNpm_CLIOnlySkipsSkillsSync_JSON(t *testing.T) {
t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir())

f, stdout, _ := newTestFactory(t)
cmd := NewCmdUpdate(f)
cmd.SetArgs([]string{"--json", "--cli-only"})

origFetch := fetchLatest
fetchLatest = func() (string, error) { return "2.0.0", nil }
t.Cleanup(func() { fetchLatest = origFetch })
origVersion := currentVersion
currentVersion = func() string { return "1.0.0" }
t.Cleanup(func() { currentVersion = origVersion })

mockDetectAndNpm(t,
selfupdate.DetectResult{Method: selfupdate.InstallNpm, ResolvedPath: "/node_modules/@larksuite/cli/bin/lark-cli", NpmAvailable: true},
func(version string) *selfupdate.NpmResult { return &selfupdate.NpmResult{} },
)

origSync := syncSkills
syncSkills = func(opts skillscheck.SyncOptions) *skillscheck.SyncResult {
t.Fatal("skills sync called under --cli-only")
return nil
}
t.Cleanup(func() { syncSkills = origSync })

if err := cmd.Execute(); err != nil {
t.Fatalf("unexpected error: %v", err)
}
var env map[string]interface{}
if err := json.Unmarshal(stdout.Bytes(), &env); err != nil {
t.Fatalf("json.Unmarshal stdout: %v\nstdout: %s", err, stdout.String())
}
if env["action"] != "updated" {
t.Errorf("action = %v, want updated", env["action"])
}
if env["skills_action"] != "skipped" {
t.Errorf("skills_action = %v, want skipped; stdout: %s", env["skills_action"], stdout.String())
}
if _, readable, err := skillscheck.ReadState(); err != nil || readable {
t.Fatalf("ReadState() = (_, %v, %v), want unreadable/nil after --cli-only", readable, err)
}
}

func TestUpdateNpm_WithSkillsRunsSkillsSync_JSON(t *testing.T) {
t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir())

f, stdout, _ := newTestFactory(t)
cmd := NewCmdUpdate(f)
cmd.SetArgs([]string{"--json", "--with-skills"})

origFetch := fetchLatest
fetchLatest = func() (string, error) { return "2.0.0", nil }
t.Cleanup(func() { fetchLatest = origFetch })
origVersion := currentVersion
currentVersion = func() string { return "1.0.0" }
t.Cleanup(func() { currentVersion = origVersion })

mockDetectAndNpm(t,
selfupdate.DetectResult{Method: selfupdate.InstallNpm, ResolvedPath: "/node_modules/@larksuite/cli/bin/lark-cli", NpmAvailable: true},
func(version string) *selfupdate.NpmResult { return &selfupdate.NpmResult{} },
)

called := false
origSync := syncSkills
syncSkills = func(opts skillscheck.SyncOptions) *skillscheck.SyncResult {
called = true
return &skillscheck.SyncResult{
Action: "synced",
Official: []string{"lark-calendar"},
Updated: []string{"lark-calendar"},
}
}
t.Cleanup(func() { syncSkills = origSync })

if err := cmd.Execute(); err != nil {
t.Fatalf("unexpected error: %v", err)
}
if !called {
t.Fatal("skills sync not called under --with-skills")
}
var env map[string]interface{}
if err := json.Unmarshal(stdout.Bytes(), &env); err != nil {
t.Fatalf("json.Unmarshal stdout: %v\nstdout: %s", err, stdout.String())
}
if env["skills_action"] != "synced" {
t.Errorf("skills_action = %v, want synced; stdout: %s", env["skills_action"], stdout.String())
}
}

func TestUpdateSkillSyncFlagsMutuallyExclusive(t *testing.T) {
f, _, _ := newTestFactory(t)
cmd := NewCmdUpdate(f)
cmd.SetArgs([]string{"--cli-only", "--with-skills"})
cmd.SilenceErrors = true
cmd.SilenceUsage = true

origFetch := fetchLatest
fetchLatest = func() (string, error) {
t.Fatal("fetchLatest called before validating mutually exclusive flags")
return "", nil
}
t.Cleanup(func() { fetchLatest = origFetch })

err := cmd.Execute()
if err == nil {
t.Fatal("expected validation error")
}
var exitErr *output.ExitError
if !errors.As(err, &exitErr) {
t.Fatalf("expected *output.ExitError, got %T: %v", err, err)
}
if exitErr.Code != output.ExitValidation {
t.Fatalf("exit code = %d, want %d", exitErr.Code, output.ExitValidation)
}
if !strings.Contains(exitErr.Error(), "--cli-only and --with-skills are mutually exclusive") {
t.Fatalf("error message = %q", exitErr.Error())
}
}

func TestUpdateNpm_Human(t *testing.T) {
// Same isolation as TestUpdateNpm_JSON — see comment there.
t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir())
Expand Down
Loading