From b411e4c6650a864a11c69031494681043b6cf7ae Mon Sep 17 00:00:00 2001 From: "zhangheng.023" Date: Fri, 22 May 2026 19:25:07 +0800 Subject: [PATCH 1/2] fix: sync skills incrementally during update --- cmd/root_integration_test.go | 17 +- cmd/update/update.go | 134 ++++---- cmd/update/update_test.go | 491 ++++++++++++++++++++-------- internal/selfupdate/updater.go | 53 ++- internal/selfupdate/updater_test.go | 85 +++++ internal/skillscheck/check.go | 37 +-- internal/skillscheck/check_test.go | 33 +- internal/skillscheck/notice.go | 8 +- internal/skillscheck/stamp.go | 49 --- internal/skillscheck/stamp_test.go | 113 ------- internal/skillscheck/state.go | 92 ++++++ internal/skillscheck/state_test.go | 139 ++++++++ internal/skillscheck/sync.go | 383 ++++++++++++++++++++++ internal/skillscheck/sync_test.go | 368 +++++++++++++++++++++ 14 files changed, 1586 insertions(+), 416 deletions(-) delete mode 100644 internal/skillscheck/stamp.go delete mode 100644 internal/skillscheck/stamp_test.go create mode 100644 internal/skillscheck/state.go create mode 100644 internal/skillscheck/state_test.go create mode 100644 internal/skillscheck/sync.go create mode 100644 internal/skillscheck/sync_test.go diff --git a/cmd/root_integration_test.go b/cmd/root_integration_test.go index a8919d1ce..794cb07c5 100644 --- a/cmd/root_integration_test.go +++ b/cmd/root_integration_test.go @@ -536,11 +536,8 @@ func TestIntegration_Shortcut_BusinessError_OutputsEnvelope(t *testing.T) { }) } -// TestSetupNotices_ColdStart_NoNotice verifies that a missing stamp -// produces no skills key in the composed notice. Users who installed -// skills via `npx skills add` (no stamp) must not see the misleading -// "not installed" notice — only `lark-cli update` users opt into the -// drift tracker. +// TestSetupNotices_ColdStart_NoNotice verifies that missing state +// produces no skills key in the composed notice. func TestSetupNotices_ColdStart_NoNotice(t *testing.T) { clearNoticeEnv(t) dir := t.TempDir() @@ -571,13 +568,13 @@ func TestSetupNotices_ColdStart_NoNotice(t *testing.T) { } } -// TestSetupNotices_InSync verifies that a matching stamp produces no +// TestSetupNotices_InSync verifies that matching state produces no // skills key in the composed notice. func TestSetupNotices_InSync(t *testing.T) { clearNoticeEnv(t) dir := t.TempDir() t.Setenv("LARKSUITE_CLI_CONFIG_DIR", dir) - if err := skillscheck.WriteStamp("1.0.21"); err != nil { + if err := skillscheck.WriteState(skillscheck.SkillsState{Version: "1.0.21"}); err != nil { t.Fatal(err) } @@ -604,13 +601,13 @@ func TestSetupNotices_InSync(t *testing.T) { } } -// TestSetupNotices_Drift verifies a mismatching stamp produces the +// TestSetupNotices_Drift verifies mismatching state produces the // drift message with both current and target populated. func TestSetupNotices_Drift(t *testing.T) { clearNoticeEnv(t) dir := t.TempDir() t.Setenv("LARKSUITE_CLI_CONFIG_DIR", dir) - if err := skillscheck.WriteStamp("1.0.20"); err != nil { + if err := skillscheck.WriteState(skillscheck.SkillsState{Version: "1.0.20"}); err != nil { t.Fatal(err) } @@ -659,7 +656,7 @@ func TestSetupNotices_BothUpdateAndSkills(t *testing.T) { clearNoticeEnv(t) dir := t.TempDir() t.Setenv("LARKSUITE_CLI_CONFIG_DIR", dir) - if err := skillscheck.WriteStamp("1.0.20"); err != nil { + if err := skillscheck.WriteState(skillscheck.SkillsState{Version: "1.0.20"}); err != nil { t.Fatal(err) } diff --git a/cmd/update/update.go b/cmd/update/update.go index c9035cd5c..ea5dc2253 100644 --- a/cmd/update/update.go +++ b/cmd/update/update.go @@ -31,11 +31,12 @@ var ( currentVersion = func() string { return build.Version } currentOS = runtime.GOOS newUpdater = func() *selfupdate.Updater { return selfupdate.New() } + syncSkills = func(opts skillscheck.SyncOptions) *skillscheck.SyncResult { return skillscheck.SyncSkills(opts) } ) func isWindows() bool { return currentOS == osWindows } -// normalizeVersion canonicalizes a version string for stamp comparison. +// normalizeVersion canonicalizes a version string for state comparison. // Strips a leading "v" so versions written from Makefile (git describe → // "v1.0.0") and npm (no prefix → "1.0.0") compare equal. func normalizeVersion(s string) string { @@ -121,7 +122,9 @@ func updateRun(opts *UpdateOptions) error { cur := currentVersion() updater := newUpdater() - updater.CleanupStaleFiles() + if !opts.Check { + updater.CleanupStaleFiles() + } output.PendingNotice = nil // 1. Fetch latest version @@ -137,13 +140,9 @@ func updateRun(opts *UpdateOptions) error { // 3. Compare versions if !opts.Force && !update.IsNewer(latest, cur) { - // Run skills sync before returning — covers the case where the - // binary is already current but skills were never synced. - // Stamp dedup makes this a no-op if skills are already in sync. - // Skip side-effects under --check (pure report path per spec §3.6). - var skillsResult *selfupdate.NpmResult + var skillsResult *skillscheck.SyncResult if !opts.Check { - skillsResult = runSkillsAndStamp(updater, io, cur, opts.Force) + skillsResult = runSkillsAndState(updater, io, cur, opts.Force) } return reportAlreadyUpToDate(opts, io, cur, latest, skillsResult, opts.Check) } @@ -185,16 +184,7 @@ func reportCheckResult(opts *UpdateOptions, io *cmdutil.IOStreams, cur, latest s "message": fmt.Sprintf("lark-cli %s %s %s available", cur, symArrow(), latest), "url": releaseURL(latest), "changelog": changelogURL(), } - // skills_status: pure report, no side effect, no stamp write. - // ReadStamp errors are silently swallowed — if we can't read the - // stamp we just omit the block rather than fail the --check. - if stamp, err := skillscheck.ReadStamp(); err == nil { - out["skills_status"] = map[string]interface{}{ - "current": stamp, - "target": cur, - "in_sync": stamp == cur, - } - } + applySkillsStatus(out, cur) output.PrintJson(io.Out, out) return nil } @@ -210,7 +200,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 := runSkillsAndStamp(updater, io, cur, opts.Force) + skillsResult := runSkillsAndState(updater, io, cur, opts.Force) reason := detect.ManualReason() if opts.JSON { @@ -288,10 +278,7 @@ func doNpmUpdate(opts *UpdateOptions, io *cmdutil.IOStreams, cur, latest string, return output.ErrBare(output.ExitAPI) } - // Skills update (best-effort) — uses runSkillsAndStamp so the - // stamp gets persisted on success and dedup applies if a previous - // run already stamped this version. - skillsResult := runSkillsAndStamp(updater, io, latest, opts.Force) + skillsResult := runSkillsAndState(updater, io, latest, opts.Force) if opts.JSON { result := map[string]interface{}{ @@ -328,27 +315,21 @@ 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)) } -// runSkillsAndStamp triggers updater.RunSkillsUpdate and persists the -// stamp on success. Skips the npx invocation when the stamp already -// matches stampVersion (unless force is true). The stamp write failure -// emits a warning to io.ErrOut but does NOT fail the update command — -// best-effort. ReadStamp errors are swallowed (fail-closed: treated as -// out-of-sync, so npx re-runs). Returns nil iff skipped due to stamp -// dedup; otherwise returns the underlying *NpmResult with Err semantics -// from RunSkillsUpdate. -func runSkillsAndStamp(updater *selfupdate.Updater, io *cmdutil.IOStreams, stampVersion string, force bool) *selfupdate.NpmResult { +func runSkillsAndState(updater *selfupdate.Updater, io *cmdutil.IOStreams, stateVersion string, force bool) *skillscheck.SyncResult { if !force { - if existing, _ := skillscheck.ReadStamp(); normalizeVersion(existing) == normalizeVersion(stampVersion) { + if existing, ok := skillscheck.ReadSyncedVersion(); ok && normalizeVersion(existing) == normalizeVersion(stateVersion) { return nil } } - r := updater.RunSkillsUpdate() - if r.Err == nil { - if err := skillscheck.WriteStamp(stampVersion); err != nil { - fmt.Fprintf(io.ErrOut, "warning: skills synced but stamp not written: %v\n", err) - } + result := syncSkills(skillscheck.SyncOptions{ + Version: stateVersion, + Force: force, + Runner: updater, + }) + if result.Err != nil && strings.Contains(result.Err.Error(), "state not written") { + fmt.Fprintf(io.ErrOut, "warning: %v\n", result.Err) } - return r + return result } // reportAlreadyUpToDate emits the JSON / pretty output for the @@ -356,7 +337,7 @@ func runSkillsAndStamp(updater *selfupdate.Updater, io *cmdutil.IOStreams, stamp // fields derived from skillsResult. When check is true, this is the pure // report path (spec §3.6): no side-effects, JSON envelope uses // skills_status (spec §4.2) instead of skills_action. -func reportAlreadyUpToDate(opts *UpdateOptions, io *cmdutil.IOStreams, cur, latest string, skillsResult *selfupdate.NpmResult, check bool) error { +func reportAlreadyUpToDate(opts *UpdateOptions, io *cmdutil.IOStreams, cur, latest string, skillsResult *skillscheck.SyncResult, check bool) error { if opts.JSON { out := map[string]interface{}{ "ok": true, "previous_version": cur, "current_version": cur, @@ -364,16 +345,7 @@ func reportAlreadyUpToDate(opts *UpdateOptions, io *cmdutil.IOStreams, cur, late "message": fmt.Sprintf("lark-cli %s is already up to date", cur), } if check { - // Pure report — read stamp directly, emit skills_status block. - // ReadStamp errors are silently swallowed — if we can't read - // the stamp we just omit the block rather than fail the --check. - if stamp, err := skillscheck.ReadStamp(); err == nil { - out["skills_status"] = map[string]interface{}{ - "current": stamp, - "target": cur, - "in_sync": stamp == cur, - } - } + applySkillsStatus(out, cur) } else { applySkillsResult(out, skillsResult) } @@ -387,36 +359,70 @@ func reportAlreadyUpToDate(opts *UpdateOptions, io *cmdutil.IOStreams, cur, late return nil } -// applySkillsResult mutates the JSON envelope to include skills_action -// (and skills_warning when failed). nil result = "in_sync" (dedup hit). -func applySkillsResult(env map[string]interface{}, r *selfupdate.NpmResult) { +func applySkillsStatus(env map[string]interface{}, target string) { + state, readable, err := skillscheck.ReadState() + if err != nil || !readable || state.Version == "" { + return + } + status := map[string]interface{}{ + "current": state.Version, + "target": target, + "in_sync": normalizeVersion(state.Version) == normalizeVersion(target), + } + if len(state.OfficialSkills) > 0 { + status["official"] = len(state.OfficialSkills) + } + if len(state.UpdatedSkills) > 0 { + status["updated"] = len(state.UpdatedSkills) + } + if len(state.SkippedDeletedSkills) > 0 { + status["skipped_deleted"] = state.SkippedDeletedSkills + } + env["skills_status"] = status +} + +func applySkillsResult(env map[string]interface{}, r *skillscheck.SyncResult) { switch { case r == nil: env["skills_action"] = "in_sync" case r.Err != nil: env["skills_action"] = "failed" env["skills_warning"] = fmt.Sprintf("skills update failed: %s", r.Err) - if detail := strings.TrimSpace(r.Stderr.String()); detail != "" { - env["skills_detail"] = selfupdate.Truncate(detail, maxNpmOutput) - } + env["skills_summary"] = skillsSummary(r) default: env["skills_action"] = "synced" + env["skills_summary"] = skillsSummary(r) + } +} + +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), } + if len(r.Failed) > 0 { + summary["failed"] = r.Failed + } + return summary } -// emitSkillsTextHints prints human-readable feedback about the skills -// sync result for non-JSON output. -func emitSkillsTextHints(io *cmdutil.IOStreams, r *selfupdate.NpmResult) { +func emitSkillsTextHints(io *cmdutil.IOStreams, r *skillscheck.SyncResult) { switch { case r == nil: - // dedup hit — silent (already up to date) case r.Err != nil: fmt.Fprintf(io.ErrOut, "%s Skills update failed: %v\n", symWarn(), r.Err) - if detail := strings.TrimSpace(r.Stderr.String()); detail != "" { - fmt.Fprintf(io.ErrOut, " %s\n", selfupdate.Truncate(detail, maxStderrDetail)) + if len(r.Failed) > 0 { + fmt.Fprintf(io.ErrOut, " Failed skills: %s\n", strings.Join(r.Failed, ", ")) } - fmt.Fprintf(io.ErrOut, " Run manually: npx -y skills add larksuite/cli -g -y\n") + fmt.Fprintf(io.ErrOut, " To retry all official skills: lark-cli update --force\n") + case r.Force: + fmt.Fprintf(io.ErrOut, "%s Skills updated: restored all %d official skills\n", symOK(), len(r.Official)) default: - fmt.Fprintf(io.ErrOut, "%s Skills updated\n", symOK()) + 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)) + if len(r.SkippedDeleted) > 0 { + fmt.Fprintf(io.ErrOut, " To restore all official skills: lark-cli update --force\n") + } } } diff --git a/cmd/update/update_test.go b/cmd/update/update_test.go index 250aa83db..67ae5891e 100644 --- a/cmd/update/update_test.go +++ b/cmd/update/update_test.go @@ -5,13 +5,14 @@ package cmdupdate import ( "bytes" + "context" "encoding/json" "errors" "fmt" - "os" - "path/filepath" + "os/exec" "strings" "testing" + "time" "github.com/larksuite/cli/internal/cmdutil" "github.com/larksuite/cli/internal/core" @@ -28,7 +29,6 @@ func newTestFactory(t *testing.T) (*cmdutil.Factory, *bytes.Buffer, *bytes.Buffe } // mockDetect sets up newUpdater to return an Updater with the given DetectResult. -// It preserves any existing NpmInstallOverride/SkillsUpdateOverride that may be set later. func mockDetect(t *testing.T, result selfupdate.DetectResult) { t.Helper() origNew := newUpdater @@ -41,22 +41,34 @@ func mockDetect(t *testing.T, result selfupdate.DetectResult) { } // mockDetectAndNpm sets up newUpdater with detect, npm install, and skills overrides all at once. -func mockDetectAndNpm(t *testing.T, result selfupdate.DetectResult, - npmFn func(string) *selfupdate.NpmResult, - skillsFn func() *selfupdate.NpmResult) { +func mockDetectAndNpm(t *testing.T, result selfupdate.DetectResult, npmFn func(string) *selfupdate.NpmResult) { t.Helper() origNew := newUpdater newUpdater = func() *selfupdate.Updater { u := selfupdate.New() u.DetectOverride = func() selfupdate.DetectResult { return result } u.NpmInstallOverride = npmFn - u.SkillsUpdateOverride = skillsFn u.VerifyOverride = func(string) error { return nil } + u.SkillsCommandOverride = successfulSkillsCommand() return u } t.Cleanup(func() { newUpdater = origNew }) } +func successfulSkillsCommand() func(args ...string) *selfupdate.NpmResult { + return func(args ...string) *selfupdate.NpmResult { + r := &selfupdate.NpmResult{} + switch strings.Join(args, " ") { + case "-y skills add https://open.feishu.cn --list": + r.Stdout.WriteString("Available Skills\n │ lark-calendar\n │ lark-mail\n") + case "-y skills ls -g": + r.Stdout.WriteString("Global Skills\nlark-calendar /tmp/lark-calendar\ncustom-skill /tmp/custom-skill\n") + default: + } + return r + } +} + func TestUpdateAlreadyUpToDate_JSON(t *testing.T) { f, stdout, _ := newTestFactory(t) @@ -168,9 +180,7 @@ func TestUpdateManual_Human(t *testing.T) { } func TestUpdateNpm_JSON(t *testing.T) { - // Isolate config dir: this test mocks fetchLatest="2.0.0" and lets - // runSkillsAndStamp → WriteStamp succeed, which without isolation would - // clobber the real ~/.lark-cli/skills.stamp with "2.0.0". + // Isolate config dir because skills sync writes skills-state.json. t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) f, stdout, _ := newTestFactory(t) @@ -186,7 +196,6 @@ func TestUpdateNpm_JSON(t *testing.T) { 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{} }, - func() *selfupdate.NpmResult { return &selfupdate.NpmResult{} }, ) err := cmd.Execute() @@ -216,7 +225,6 @@ func TestUpdateNpm_Human(t *testing.T) { 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{} }, - func() *selfupdate.NpmResult { return &selfupdate.NpmResult{} }, ) err := cmd.Execute() @@ -230,7 +238,7 @@ func TestUpdateNpm_Human(t *testing.T) { } func TestUpdateForce_JSON(t *testing.T) { - // Same stamp-isolation rationale as TestUpdateNpm_JSON. + // Same state-isolation rationale as TestUpdateNpm_JSON. t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) f, stdout, _ := newTestFactory(t) @@ -246,7 +254,6 @@ func TestUpdateForce_JSON(t *testing.T) { 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{} }, - func() *selfupdate.NpmResult { return &selfupdate.NpmResult{} }, ) err := cmd.Execute() @@ -323,7 +330,7 @@ func TestUpdateInvalidVersion_JSON(t *testing.T) { } func TestUpdateDevVersion_JSON(t *testing.T) { - // Same stamp-isolation rationale as TestUpdateNpm_JSON. + // Same state-isolation rationale as TestUpdateNpm_JSON. t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) f, stdout, _ := newTestFactory(t) @@ -339,7 +346,6 @@ func TestUpdateDevVersion_JSON(t *testing.T) { 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{} }, - func() *selfupdate.NpmResult { return &selfupdate.NpmResult{} }, ) err := cmd.Execute() @@ -451,8 +457,8 @@ func TestUpdateNpmVerifyFail_JSON_NoRestoreHintWhenBackupUnavailable(t *testing. u.NpmInstallOverride = func(version string) *selfupdate.NpmResult { return &selfupdate.NpmResult{} } u.VerifyOverride = func(string) error { return errors.New("bad binary") } u.RestoreAvailableOverride = func() bool { return false } - u.SkillsUpdateOverride = func() *selfupdate.NpmResult { - t.Fatal("skills update should not run when binary verification fails") + u.SkillsCommandOverride = func(args ...string) *selfupdate.NpmResult { + t.Fatal("skills sync should not run when binary verification fails") return nil } return u @@ -649,7 +655,7 @@ func TestPermissionHint(t *testing.T) { func TestUpdateWindows_NpmSuccess_JSON(t *testing.T) { // With the rename trick, Windows npm installs can now auto-update. - // Same stamp-isolation rationale as TestUpdateNpm_JSON. + // Same state-isolation rationale as TestUpdateNpm_JSON. t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) f, stdout, _ := newTestFactory(t) @@ -668,7 +674,6 @@ func TestUpdateWindows_NpmSuccess_JSON(t *testing.T) { mockDetectAndNpm(t, selfupdate.DetectResult{Method: selfupdate.InstallNpm, ResolvedPath: `C:\npm\node_modules\@larksuite\cli\bin\lark-cli.exe`, NpmAvailable: true}, func(version string) *selfupdate.NpmResult { return &selfupdate.NpmResult{} }, - func() *selfupdate.NpmResult { return &selfupdate.NpmResult{} }, ) err := cmd.Execute() @@ -750,7 +755,6 @@ func TestUpdateNpm_SkillsSuccess_JSON(t *testing.T) { 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{} }, - func() *selfupdate.NpmResult { return &selfupdate.NpmResult{} }, ) err := cmd.Execute() @@ -785,8 +789,7 @@ func TestUpdateNpm_SkillsFail_JSON(t *testing.T) { } u.NpmInstallOverride = func(version string) *selfupdate.NpmResult { return &selfupdate.NpmResult{} } u.VerifyOverride = func(string) error { return nil } - // Skills update fails - u.SkillsUpdateOverride = func() *selfupdate.NpmResult { + u.SkillsCommandOverride = func(args ...string) *selfupdate.NpmResult { r := &selfupdate.NpmResult{} r.Stderr.WriteString("npx: command not found") r.Err = fmt.Errorf("exit status 127") @@ -812,8 +815,8 @@ func TestUpdateNpm_SkillsFail_JSON(t *testing.T) { if !strings.Contains(out, "skills_warning") { t.Errorf("expected skills_warning in output, got: %s", out) } - if !strings.Contains(out, "skills_detail") { - t.Errorf("expected skills_detail in output, got: %s", out) + if !strings.Contains(out, "skills_summary") { + t.Errorf("expected skills_summary in output, got: %s", out) } } @@ -838,7 +841,7 @@ func TestUpdateNpm_SkillsFail_Human(t *testing.T) { } u.NpmInstallOverride = func(version string) *selfupdate.NpmResult { return &selfupdate.NpmResult{} } u.VerifyOverride = func(string) error { return nil } - u.SkillsUpdateOverride = func() *selfupdate.NpmResult { + u.SkillsCommandOverride = func(args ...string) *selfupdate.NpmResult { r := &selfupdate.NpmResult{} r.Stderr.WriteString("npx: command not found") r.Err = fmt.Errorf("exit status 127") @@ -861,100 +864,96 @@ func TestUpdateNpm_SkillsFail_Human(t *testing.T) { if !strings.Contains(out, "Skills update failed") { t.Errorf("expected skills failure warning, got: %s", out) } - if !strings.Contains(out, "npx -y skills add") { - t.Errorf("expected manual skills command hint, got: %s", out) + if !strings.Contains(out, "lark-cli update --force") { + t.Errorf("expected force retry hint, got: %s", out) } } -// newTestIO returns a cmdutil.IOStreams backed by bytes.Buffers, suitable -// for direct calls to internals like runSkillsAndStamp that write to -// io.ErrOut. +// newTestIO returns a cmdutil.IOStreams backed by bytes.Buffers. func newTestIO() *cmdutil.IOStreams { return cmdutil.NewIOStreams(&bytes.Buffer{}, &bytes.Buffer{}, &bytes.Buffer{}) } -func TestRunSkillsAndStamp_DedupHit(t *testing.T) { - dir := t.TempDir() - t.Setenv("LARKSUITE_CLI_CONFIG_DIR", dir) - if err := skillscheck.WriteStamp("1.0.21"); err != nil { +func TestRunSkillsAndState_DedupHit(t *testing.T) { + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) + if err := skillscheck.WriteState(skillscheck.SkillsState{Version: "1.0.21"}); err != nil { t.Fatal(err) } called := false updater := &selfupdate.Updater{ - SkillsUpdateOverride: func() *selfupdate.NpmResult { + SkillsCommandOverride: func(args ...string) *selfupdate.NpmResult { called = true return &selfupdate.NpmResult{} }, } - got := runSkillsAndStamp(updater, newTestIO(), "1.0.21", false) + got := runSkillsAndState(updater, newTestIO(), "1.0.21", false) if got != nil { - t.Errorf("runSkillsAndStamp() = %+v, want nil for dedup hit", got) + t.Errorf("runSkillsAndState() = %+v, want nil for dedup hit", got) } if called { - t.Error("SkillsUpdateOverride called, want skipped due to dedup") + t.Error("SkillsCommandOverride called, want skipped due to dedup") } } -func TestRunSkillsAndStamp_DedupForceBypass(t *testing.T) { - dir := t.TempDir() - t.Setenv("LARKSUITE_CLI_CONFIG_DIR", dir) - if err := skillscheck.WriteStamp("1.0.21"); err != nil { +func TestRunSkillsAndState_DedupForceBypass(t *testing.T) { + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) + if err := skillscheck.WriteState(skillscheck.SkillsState{Version: "1.0.21"}); err != nil { t.Fatal(err) } called := false updater := &selfupdate.Updater{ - SkillsUpdateOverride: func() *selfupdate.NpmResult { + SkillsCommandOverride: func(args ...string) *selfupdate.NpmResult { called = true - return &selfupdate.NpmResult{} + return successfulSkillsCommand()(args...) }, } - got := runSkillsAndStamp(updater, newTestIO(), "1.0.21", true) - if got == nil { - t.Fatal("runSkillsAndStamp(force=true) = nil, want non-nil") + got := runSkillsAndState(updater, newTestIO(), "1.0.21", true) + if got == nil || got.Err != nil { + t.Fatalf("runSkillsAndState(force=true) = %+v, want successful result", got) } if !called { - t.Error("SkillsUpdateOverride not called with force=true") + t.Error("SkillsCommandOverride not called with force=true") } } -func TestRunSkillsAndStamp_SuccessWritesStamp(t *testing.T) { - dir := t.TempDir() - t.Setenv("LARKSUITE_CLI_CONFIG_DIR", dir) - updater := &selfupdate.Updater{ - SkillsUpdateOverride: func() *selfupdate.NpmResult { - return &selfupdate.NpmResult{} - }, - } - got := runSkillsAndStamp(updater, newTestIO(), "1.0.21", false) +func TestRunSkillsAndState_SuccessWritesState(t *testing.T) { + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) + updater := &selfupdate.Updater{SkillsCommandOverride: successfulSkillsCommand()} + got := runSkillsAndState(updater, newTestIO(), "1.0.21", false) if got == nil || got.Err != nil { - t.Fatalf("runSkillsAndStamp() = %+v, want non-nil with nil Err", got) + t.Fatalf("runSkillsAndState() = %+v, want non-nil with nil Err", got) + } + state, readable, err := skillscheck.ReadState() + if err != nil || !readable { + t.Fatalf("ReadState() = (_, %v, %v), want readable", readable, err) } - stamp, _ := skillscheck.ReadStamp() - if stamp != "1.0.21" { - t.Errorf("stamp = %q, want \"1.0.21\"", stamp) + if state.Version != "1.0.21" { + t.Errorf("state.Version = %q, want \"1.0.21\"", state.Version) } } -func TestRunSkillsAndStamp_FailureKeepsOldStamp(t *testing.T) { - dir := t.TempDir() - t.Setenv("LARKSUITE_CLI_CONFIG_DIR", dir) - if err := skillscheck.WriteStamp("1.0.20"); err != nil { +func TestRunSkillsAndState_FailureKeepsOldState(t *testing.T) { + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) + if err := skillscheck.WriteState(skillscheck.SkillsState{Version: "1.0.20"}); err != nil { t.Fatal(err) } updater := &selfupdate.Updater{ - SkillsUpdateOverride: func() *selfupdate.NpmResult { + SkillsCommandOverride: func(args ...string) *selfupdate.NpmResult { r := &selfupdate.NpmResult{} r.Err = fmt.Errorf("npx failed") return r }, } - got := runSkillsAndStamp(updater, newTestIO(), "1.0.21", false) + got := runSkillsAndState(updater, newTestIO(), "1.0.21", false) if got == nil || got.Err == nil { - t.Fatalf("runSkillsAndStamp() = %+v, want non-nil with non-nil Err", got) + t.Fatalf("runSkillsAndState() = %+v, want non-nil with non-nil Err", got) + } + state, readable, err := skillscheck.ReadState() + if err != nil || !readable { + t.Fatalf("ReadState() = (_, %v, %v), want readable", readable, err) } - stamp, _ := skillscheck.ReadStamp() - if stamp != "1.0.20" { - t.Errorf("stamp = %q, want \"1.0.20\" (failure must not overwrite)", stamp) + if state.Version != "1.0.20" { + t.Errorf("state.Version = %q, want \"1.0.20\" (failure must not overwrite)", state.Version) } } @@ -973,8 +972,7 @@ func TestTruncate(t *testing.T) { } func TestUpdateRun_AlreadyLatest_RunsSkillsSync(t *testing.T) { - dir := t.TempDir() - t.Setenv("LARKSUITE_CLI_CONFIG_DIR", dir) + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) origFetch := fetchLatest origCur := currentVersion @@ -987,9 +985,9 @@ func TestUpdateRun_AlreadyLatest_RunsSkillsSync(t *testing.T) { t.Cleanup(func() { newUpdater = origNew }) newUpdater = func() *selfupdate.Updater { return &selfupdate.Updater{ - SkillsUpdateOverride: func() *selfupdate.NpmResult { + SkillsCommandOverride: func(args ...string) *selfupdate.NpmResult { skillsCalled = true - return &selfupdate.NpmResult{} + return successfulSkillsCommand()(args...) }, } } @@ -1000,17 +998,19 @@ func TestUpdateRun_AlreadyLatest_RunsSkillsSync(t *testing.T) { t.Fatalf("updateRun() err = %v, want nil", err) } if !skillsCalled { - t.Error("RunSkillsUpdate not called in already-up-to-date branch (cold stamp), want called") + t.Error("skills sync not called in already-up-to-date branch") } - stamp, _ := skillscheck.ReadStamp() - if stamp != "1.0.21" { - t.Errorf("stamp = %q, want \"1.0.21\"", stamp) + state, readable, err := skillscheck.ReadState() + if err != nil || !readable { + t.Fatalf("ReadState() = (_, %v, %v), want readable", readable, err) + } + if state.Version != "1.0.21" { + t.Errorf("state.Version = %q, want \"1.0.21\"", state.Version) } } func TestUpdateRun_Manual_RunsSkillsSync(t *testing.T) { - dir := t.TempDir() - t.Setenv("LARKSUITE_CLI_CONFIG_DIR", dir) + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) origFetch := fetchLatest origCur := currentVersion @@ -1029,9 +1029,9 @@ func TestUpdateRun_Manual_RunsSkillsSync(t *testing.T) { ResolvedPath: "/usr/local/bin/lark-cli", } }, - SkillsUpdateOverride: func() *selfupdate.NpmResult { + SkillsCommandOverride: func(args ...string) *selfupdate.NpmResult { skillsCalled = true - return &selfupdate.NpmResult{} + return successfulSkillsCommand()(args...) }, } } @@ -1042,17 +1042,19 @@ func TestUpdateRun_Manual_RunsSkillsSync(t *testing.T) { t.Fatalf("updateRun() err = %v, want nil", err) } if !skillsCalled { - t.Error("RunSkillsUpdate not called in manual branch, want called") + t.Error("skills sync not called in manual branch") } - stamp, _ := skillscheck.ReadStamp() - if stamp != "1.0.21" { - t.Errorf("stamp = %q, want \"1.0.21\" (manual path stamps cur)", stamp) + state, readable, err := skillscheck.ReadState() + if err != nil || !readable { + t.Fatalf("ReadState() = (_, %v, %v), want readable", readable, err) + } + if state.Version != "1.0.21" { + t.Errorf("state.Version = %q, want \"1.0.21\" (manual path records current binary)", state.Version) } } -func TestUpdateRun_Npm_RunsSkillsSync_StampsLatest(t *testing.T) { - dir := t.TempDir() - t.Setenv("LARKSUITE_CLI_CONFIG_DIR", dir) +func TestUpdateRun_Npm_RunsSkillsSync_WritesLatestState(t *testing.T) { + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) origFetch := fetchLatest origCur := currentVersion @@ -1075,9 +1077,9 @@ func TestUpdateRun_Npm_RunsSkillsSync_StampsLatest(t *testing.T) { return &selfupdate.NpmResult{} }, VerifyOverride: func(expectedVersion string) error { return nil }, - SkillsUpdateOverride: func() *selfupdate.NpmResult { + SkillsCommandOverride: func(args ...string) *selfupdate.NpmResult { skillsCalled = true - return &selfupdate.NpmResult{} + return successfulSkillsCommand()(args...) }, } } @@ -1088,18 +1090,25 @@ func TestUpdateRun_Npm_RunsSkillsSync_StampsLatest(t *testing.T) { t.Fatalf("updateRun() err = %v, want nil", err) } if !skillsCalled { - t.Error("RunSkillsUpdate not called in npm branch") + t.Error("skills sync not called in npm branch") } - stamp, _ := skillscheck.ReadStamp() - if stamp != "1.0.22" { - t.Errorf("stamp = %q, want \"1.0.22\" (npm path stamps latest)", stamp) + state, readable, err := skillscheck.ReadState() + if err != nil || !readable { + t.Fatalf("ReadState() = (_, %v, %v), want readable", readable, err) + } + if state.Version != "1.0.22" { + t.Errorf("state.Version = %q, want \"1.0.22\" (npm path records latest binary)", state.Version) } } func TestUpdateRun_CheckIncludesSkillsStatus(t *testing.T) { - dir := t.TempDir() - t.Setenv("LARKSUITE_CLI_CONFIG_DIR", dir) - if err := skillscheck.WriteStamp("1.0.20"); err != nil { + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) + if err := skillscheck.WriteState(skillscheck.SkillsState{ + Version: "1.0.20", + OfficialSkills: []string{"lark-calendar", "lark-mail"}, + UpdatedSkills: []string{"lark-calendar"}, + SkippedDeletedSkills: []string{"lark-mail"}, + }); err != nil { t.Fatal(err) } @@ -1117,9 +1126,9 @@ func TestUpdateRun_CheckIncludesSkillsStatus(t *testing.T) { DetectOverride: func() selfupdate.DetectResult { return selfupdate.DetectResult{Method: selfupdate.InstallNpm, NpmAvailable: true} }, - SkillsUpdateOverride: func() *selfupdate.NpmResult { + SkillsCommandOverride: func(args ...string) *selfupdate.NpmResult { skillsCalled = true - return &selfupdate.NpmResult{} + return successfulSkillsCommand()(args...) }, } } @@ -1130,7 +1139,7 @@ func TestUpdateRun_CheckIncludesSkillsStatus(t *testing.T) { t.Fatalf("updateRun(--check) err = %v, want nil", err) } if skillsCalled { - t.Error("RunSkillsUpdate called under --check, want skipped (pure report)") + t.Error("skills sync called under --check, want skipped") } var env map[string]interface{} @@ -1144,12 +1153,14 @@ func TestUpdateRun_CheckIncludesSkillsStatus(t *testing.T) { if status["current"] != "1.0.20" || status["target"] != "1.0.21" || status["in_sync"] != false { t.Errorf("skills_status = %+v, want {current:\"1.0.20\", target:\"1.0.21\", in_sync:false}", status) } + if status["official"] != float64(2) || status["updated"] != float64(1) { + t.Errorf("skills_status counts = %+v, want official:2 updated:1", status) + } } func TestUpdateRun_CheckAlreadyLatest_NoSideEffect(t *testing.T) { - dir := t.TempDir() - t.Setenv("LARKSUITE_CLI_CONFIG_DIR", dir) - if err := skillscheck.WriteStamp("1.0.20"); err != nil { + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) + if err := skillscheck.WriteState(skillscheck.SkillsState{Version: "1.0.20"}); err != nil { t.Fatal(err) } @@ -1164,9 +1175,9 @@ func TestUpdateRun_CheckAlreadyLatest_NoSideEffect(t *testing.T) { t.Cleanup(func() { newUpdater = origNew }) newUpdater = func() *selfupdate.Updater { return &selfupdate.Updater{ - SkillsUpdateOverride: func() *selfupdate.NpmResult { + SkillsCommandOverride: func(args ...string) *selfupdate.NpmResult { skillsCalled = true - return &selfupdate.NpmResult{} + return successfulSkillsCommand()(args...) }, } } @@ -1177,12 +1188,15 @@ func TestUpdateRun_CheckAlreadyLatest_NoSideEffect(t *testing.T) { t.Fatalf("updateRun(--check, already-latest) err = %v, want nil", err) } if skillsCalled { - t.Error("RunSkillsUpdate called under --check (already-latest), want skipped (pure report)") + t.Error("skills sync called under --check (already-latest), want skipped") } - stamp, _ := skillscheck.ReadStamp() - if stamp != "1.0.20" { - t.Errorf("stamp mutated to %q under --check, want \"1.0.20\" (pure report must not write stamp)", stamp) + state, readable, err := skillscheck.ReadState() + if err != nil || !readable { + t.Fatalf("ReadState() = (_, %v, %v), want readable", readable, err) + } + if state.Version != "1.0.20" { + t.Errorf("state.Version mutated to %q under --check, want \"1.0.20\"", state.Version) } var env map[string]interface{} @@ -1204,39 +1218,248 @@ func TestUpdateRun_CheckAlreadyLatest_NoSideEffect(t *testing.T) { } } -// TestRunSkillsAndStamp_StampWriteFailureWarns verifies the stderr warning -// emission when RunSkillsUpdate succeeds but WriteStamp fails. -func TestRunSkillsAndStamp_StampWriteFailureWarns(t *testing.T) { - // Force WriteStamp to fail by pointing config dir at a path that exists - // as a regular file (so MkdirAll fails). - tmp := t.TempDir() - badPath := filepath.Join(tmp, "blocker") - if err := os.WriteFile(badPath, []byte("not-a-dir"), 0o644); err != nil { - t.Fatal(err) +func TestRunSkillsAndState_StateWriteFailureWarns(t *testing.T) { + origSync := syncSkills + syncSkills = func(opts skillscheck.SyncOptions) *skillscheck.SyncResult { + return &skillscheck.SyncResult{Err: fmt.Errorf("skills synced but state not written: denied")} } - t.Setenv("LARKSUITE_CLI_CONFIG_DIR", badPath) + t.Cleanup(func() { syncSkills = origSync }) f, _, stderr := newTestFactory(t) - updater := &selfupdate.Updater{ - SkillsUpdateOverride: func() *selfupdate.NpmResult { - return &selfupdate.NpmResult{} // success - }, - } - got := runSkillsAndStamp(updater, f.IOStreams, "1.0.21", false) - if got == nil || got.Err != nil { - t.Fatalf("runSkillsAndStamp() = %+v, want non-nil with nil Err", got) + got := runSkillsAndState(&selfupdate.Updater{}, f.IOStreams, "1.0.21", false) + if got == nil || got.Err == nil { + t.Fatalf("runSkillsAndState() = %+v, want non-nil with write error", got) } - if !strings.Contains(stderr.String(), "warning: skills synced but stamp not written") { + if !strings.Contains(stderr.String(), "warning: skills synced but state not written") { t.Errorf("stderr does not contain warning: %q", stderr.String()) } } -// TestEmitSkillsTextHints_Success verifies the "Skills updated" success -// message is printed to ErrOut on a successful (Err == nil) result. func TestEmitSkillsTextHints_Success(t *testing.T) { f, _, stderr := newTestFactory(t) - emitSkillsTextHints(f.IOStreams, &selfupdate.NpmResult{}) // Err==nil → success + emitSkillsTextHints(f.IOStreams, &skillscheck.SyncResult{Official: []string{"lark-calendar"}, Updated: []string{"lark-calendar"}}) if !strings.Contains(stderr.String(), "Skills updated") { t.Errorf("stderr does not contain 'Skills updated': %q", stderr.String()) } } + +// TestUpdateCommand_RealSkillsSyncRewritesState is a live integration test that +// verifies "lark-cli update" correctly triggers skills sync and rewrites the +// state file. It calls the real npx skills CLI, so the test is skipped when +// npx or the skills registry is unavailable (e.g. no network or fork PRs). +func TestUpdateCommand_RealSkillsSyncRewritesState(t *testing.T) { + // Phase 1: Verify the real npx skills CLI is available; skip otherwise. + if _, err := exec.LookPath("npx"); err != nil { + t.Skipf("npx not found in PATH: %v", err) + } + ctx, cancel := context.WithTimeout(context.Background(), 45*time.Second) + defer cancel() + if err := exec.CommandContext(ctx, "npx", "-y", "skills", "add", "https://open.feishu.cn", "--list").Run(); err != nil { + t.Skipf("real skills CLI unavailable: %v", err) + } + globalOut, err := exec.CommandContext(ctx, "npx", "-y", "skills", "ls", "-g").Output() + if err != nil { + t.Skipf("real global skills CLI unavailable: %v", err) + } + localSkills := skillscheck.ParseSkillsList(string(globalOut)) + if err := ctx.Err(); err != nil { + t.Skipf("real skills CLI availability check timed out: %v", err) + } + + // Phase 2: Seed a previous sync state simulating an upgrade from v1.0.19. + // lark-doc and lark-mail are recorded as skipped/deleted, meaning the user + // intentionally removed them while they were still official skills. + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) + before := skillscheck.SkillsState{ + Version: "1.0.19", + OfficialSkills: []string{"lark-approval", "lark-attendance", "lark-base", "lark-calendar", "lark-contact", "lark-doc", "lark-drive", "lark-event", "lark-im", "lark-mail", "lark-markdown", "lark-minutes", "lark-okr", "lark-openapi-explorer", "lark-shared", "lark-sheets", "lark-skill-maker", "lark-slides", "lark-task", "lark-vc", "lark-vc-agent", "lark-whiteboard", "lark-wiki", "lark-workflow-meeting-summary", "lark-workflow-standup-report"}, + UpdatedSkills: []string{"lark-approval", "lark-apps", "lark-attendance", "lark-base", "lark-calendar", "lark-contact", "lark-doc", "lark-drive", "lark-event", "lark-im", "lark-mail", "lark-markdown", "lark-minutes", "lark-okr", "lark-openapi-explorer", "lark-shared", "lark-sheets", "lark-skill-maker", "lark-slides", "lark-task", "lark-vc", "lark-vc-agent", "lark-whiteboard", "lark-wiki", "lark-workflow-meeting-summary", "lark-workflow-standup-report"}, + AddedOfficialSkills: []string{}, + SkippedDeletedSkills: []string{}, + UpdatedAt: "2026-05-20T00:00:00Z", + } + if err := skillscheck.WriteState(before); err != nil { + t.Fatal(err) + } + state, readable, err := skillscheck.ReadState() + if err != nil || !readable { + t.Fatalf("ReadState() before update = (_, %v, %v), want readable", readable, err) + } + if state.Version != "1.0.19" { + t.Fatalf("state.Version before update = %q, want 1.0.19", state.Version) + } + + // Phase 3: Mock version functions so the update command believes it has + // upgraded from 1.0.19 to 1.0.20, then execute "lark-cli update --json". + // This triggers SyncSkills which calls the real npx skills add command. + origFetch := fetchLatest + origVersion := currentVersion + t.Cleanup(func() { fetchLatest = origFetch; currentVersion = origVersion }) + fetchLatest = func() (string, error) { return "1.0.20", nil } + currentVersion = func() string { return "1.0.20" } + + f, stdout, _ := newTestFactory(t) + cmd := NewCmdUpdate(f) + cmd.SetArgs([]string{"--json"}) + if err := cmd.Execute(); err != nil { + t.Fatalf("lark-cli update --json err = %v, want nil", err) + } + + // Phase 4: Verify the state file was rewritten with the new version, + // non-empty official/updated skill lists, and a refreshed timestamp. + state, readable, err = skillscheck.ReadState() + if err != nil || !readable { + t.Fatalf("ReadState() after update = (_, %v, %v), want readable", readable, err) + } + if state.Version != "1.0.20" { + t.Errorf("state.Version after update = %q, want 1.0.20", state.Version) + } + if len(state.OfficialSkills) == 0 { + t.Fatalf("state.OfficialSkills after real sync is empty: %+v", state) + } + if len(state.UpdatedSkills) == 0 { + t.Fatalf("state.UpdatedSkills after real sync is empty: %+v", state) + } + if state.UpdatedAt == "" || state.UpdatedAt == before.UpdatedAt { + t.Errorf("state.UpdatedAt = %q, want refreshed non-empty timestamp", state.UpdatedAt) + } + // Verify that previously-skipped skills are handled correctly: + // - If locally installed → should appear in UpdatedSkills (updated to latest) + // - If locally absent → should NOT be force-restored in UpdatedSkills, + // and should remain in SkippedDeletedSkills + for _, skill := range []string{"lark-doc", "lark-mail"} { + if containsString(localSkills, skill) { + if !containsString(state.UpdatedSkills, skill) { + t.Errorf("state.UpdatedSkills = %v, want installed skill %q updated", state.UpdatedSkills, skill) + } + continue + } + if containsString(state.UpdatedSkills, skill) { + t.Errorf("state.UpdatedSkills = %v, want deleted skill %q not restored without --force", state.UpdatedSkills, skill) + } + if !containsString(state.SkippedDeletedSkills, skill) { + t.Errorf("state.SkippedDeletedSkills = %v, want deleted skill %q preserved when still official", state.SkippedDeletedSkills, skill) + } + } + + // Phase 5: Verify the JSON output structure is parseable and contains + // the expected action fields for AI agent consumption. + 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"] != "already_up_to_date" { + t.Errorf("action = %v, want already_up_to_date", env["action"]) + } + if env["skills_action"] != "synced" { + t.Errorf("skills_action = %v, want synced", env["skills_action"]) + } +} + +// TestUpdateCommand_SkillsSyncColdStart verifies that when skills-state.json does +// not exist (cold start), the update command installs all official skills and +// writes a fresh state file. No skill should appear in SkippedDeletedSkills +// because there is no previous state to preserve user deletions from. +// This is a live integration test that calls the real npx skills CLI; it is +// skipped when npx or the skills registry is unavailable. +func TestUpdateCommand_SkillsSyncColdStart(t *testing.T) { + // Phase 1: Verify the real npx skills CLI is available; skip otherwise. + if _, err := exec.LookPath("npx"); err != nil { + t.Skipf("npx not found in PATH: %v", err) + } + ctx, cancel := context.WithTimeout(context.Background(), 45*time.Second) + defer cancel() + if err := exec.CommandContext(ctx, "npx", "-y", "skills", "add", "https://open.feishu.cn", "--list").Run(); err != nil { + t.Skipf("real skills CLI unavailable: %v", err) + } + globalOut, err := exec.CommandContext(ctx, "npx", "-y", "skills", "ls", "-g").Output() + if err != nil { + t.Skipf("real global skills CLI unavailable: %v", err) + } + localSkills := skillscheck.ParseSkillsList(string(globalOut)) + if err := ctx.Err(); err != nil { + t.Skipf("real skills CLI availability check timed out: %v", err) + } + + // Phase 2: Use an isolated config dir with no pre-existing skills-state.json. + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) + if _, readable, _ := skillscheck.ReadState(); readable { + t.Fatal("skills-state.json should not exist before update") + } + + // Phase 3: Mock version functions so the update command believes it is at + // v1.0.20, then execute "lark-cli update --json". This triggers SyncSkills + // which calls the real npx skills add command. + origFetch := fetchLatest + origVersion := currentVersion + t.Cleanup(func() { fetchLatest = origFetch; currentVersion = origVersion }) + fetchLatest = func() (string, error) { return "1.0.20", nil } + currentVersion = func() string { return "1.0.20" } + + f, stdout, _ := newTestFactory(t) + cmd := NewCmdUpdate(f) + cmd.SetArgs([]string{"--json"}) + if err := cmd.Execute(); err != nil { + t.Fatalf("lark-cli update --json err = %v, want nil", err) + } + + // Phase 4: Verify the state file was created with all official skills in + // UpdatedSkills and nothing in SkippedDeletedSkills (cold start = no prior + // deletions to honor). Locally installed skills should appear in UpdatedSkills. + state, readable, err := skillscheck.ReadState() + if err != nil || !readable { + t.Fatalf("ReadState() after update = (_, %v, %v), want readable", readable, err) + } + if state.Version != "1.0.20" { + t.Errorf("state.Version = %q, want 1.0.20", state.Version) + } + if len(state.OfficialSkills) == 0 { + t.Fatalf("state.OfficialSkills after real sync is empty: %+v", state) + } + if len(state.UpdatedSkills) == 0 { + t.Fatalf("state.UpdatedSkills after real sync is empty: %+v", state) + } + if state.UpdatedAt == "" { + t.Error("state.UpdatedAt is empty, want non-empty timestamp") + } + // All locally installed official skills must appear in UpdatedSkills. + officialSet := map[string]bool{} + for _, s := range state.OfficialSkills { + officialSet[s] = true + } + for _, skill := range localSkills { + if !officialSet[skill] { + continue + } + if !containsString(state.UpdatedSkills, skill) { + t.Errorf("state.UpdatedSkills = %v, want locally installed official skill %q updated", state.UpdatedSkills, skill) + } + } + // No skill should be in SkippedDeletedSkills on cold start — there is no + // previous state recording a user deletion to preserve. + if len(state.SkippedDeletedSkills) != 0 { + t.Errorf("state.SkippedDeletedSkills = %v, want empty on cold start", state.SkippedDeletedSkills) + } + + // Phase 5: Verify the JSON output structure is parseable and contains + // the expected action fields for AI agent consumption. + 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"] != "already_up_to_date" { + t.Errorf("action = %v, want already_up_to_date", env["action"]) + } + if env["skills_action"] != "synced" { + t.Errorf("skills_action = %v, want synced", env["skills_action"]) + } +} + +func containsString(values []string, target string) bool { + for _, value := range values { + if value == target { + return true + } + } + return false +} diff --git a/internal/selfupdate/updater.go b/internal/selfupdate/updater.go index d9cb5ab97..facb25f1c 100644 --- a/internal/selfupdate/updater.go +++ b/internal/selfupdate/updater.go @@ -84,6 +84,7 @@ type Updater struct { DetectOverride func() DetectResult NpmInstallOverride func(version string) *NpmResult SkillsUpdateOverride func() *NpmResult + SkillsCommandOverride func(args ...string) *NpmResult VerifyOverride func(expectedVersion string) error RestoreAvailableOverride func() bool @@ -166,7 +167,57 @@ func (u *Updater) RunSkillsUpdate() *NpmResult { return r } +func (u *Updater) ListOfficialSkills() *NpmResult { + r := u.runSkillsListOfficial("https://open.feishu.cn") + if r.Err != nil { + r = u.runSkillsListOfficial("larksuite/cli") + } + return r +} + +func (u *Updater) ListGlobalSkills() *NpmResult { + return u.runSkillsListGlobal() +} + +func (u *Updater) InstallSkill(nameList []string) *NpmResult { + r := u.runSkillsInstall("https://open.feishu.cn", nameList) + if r.Err != nil { + r = u.runSkillsInstall("larksuite/cli", nameList) + } + return r +} + +func (u *Updater) InstallAllSkills() *NpmResult { + r := u.runSkillsAdd("https://open.feishu.cn") + if r.Err != nil { + r = u.runSkillsAdd("larksuite/cli") + } + return r +} + func (u *Updater) runSkillsAdd(source string) *NpmResult { + return u.runSkillsCommand("-y", "skills", "add", source, "-g", "-y") +} + +func (u *Updater) runSkillsListOfficial(source string) *NpmResult { + return u.runSkillsCommand("-y", "skills", "add", source, "--list") +} + +func (u *Updater) runSkillsListGlobal() *NpmResult { + return u.runSkillsCommand("-y", "skills", "ls", "-g") +} + +func (u *Updater) runSkillsInstall(source string, nameList []string) *NpmResult { + args := []string{"-y", "skills", "add", source, "-s"} + args = append(args, nameList...) + args = append(args, "-g", "-y") + return u.runSkillsCommand(args...) +} + +func (u *Updater) runSkillsCommand(args ...string) *NpmResult { + if u.SkillsCommandOverride != nil { + return u.SkillsCommandOverride(args...) + } r := &NpmResult{} npxPath, err := exec.LookPath("npx") if err != nil { @@ -175,7 +226,7 @@ func (u *Updater) runSkillsAdd(source string) *NpmResult { } ctx, cancel := context.WithTimeout(context.Background(), skillsUpdateTimeout) defer cancel() - cmd := exec.CommandContext(ctx, npxPath, "-y", "skills", "add", source, "-g", "-y") + cmd := exec.CommandContext(ctx, npxPath, args...) cmd.Stdout = &r.Stdout cmd.Stderr = &r.Stderr r.Err = cmd.Run() diff --git a/internal/selfupdate/updater_test.go b/internal/selfupdate/updater_test.go index f13c80b65..458f3f79b 100644 --- a/internal/selfupdate/updater_test.go +++ b/internal/selfupdate/updater_test.go @@ -8,6 +8,7 @@ import ( "os" "path/filepath" "runtime" + "strings" "testing" "github.com/larksuite/cli/internal/vfs" @@ -166,3 +167,87 @@ func TestVerifyBinaryEmptyOutput(t *testing.T) { t.Fatal("VerifyBinary(empty output) expected error, got nil") } } + +func TestSkillsCommandsUseExpectedArgs(t *testing.T) { + tests := []struct { + name string + run func(*Updater) *NpmResult + want string + }{ + { + name: "list official primary", + run: func(u *Updater) *NpmResult { + return u.runSkillsListOfficial("https://open.feishu.cn") + }, + want: "-y skills add https://open.feishu.cn --list", + }, + { + name: "list global", + run: func(u *Updater) *NpmResult { + return u.runSkillsListGlobal() + }, + want: "-y skills ls -g", + }, + { + name: "install skill primary", + run: func(u *Updater) *NpmResult { + return u.runSkillsInstall("https://open.feishu.cn", []string{"lark-mail"}) + }, + want: "-y skills add https://open.feishu.cn -s lark-mail -g -y", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("uses a POSIX shell script") + } + dir := t.TempDir() + script := filepath.Join(dir, "npx") + logPath := filepath.Join(dir, "npx.log") + if err := os.WriteFile(script, []byte("#!/bin/sh\nprintf '%s\\n' \"$*\" >> \""+logPath+"\"\nexit 0\n"), 0o755); err != nil { + t.Fatal(err) + } + t.Setenv("PATH", dir+string(os.PathListSeparator)+os.Getenv("PATH")) + + result := tt.run(New()) + if result.Err != nil { + t.Fatalf("command err = %v, want nil", result.Err) + } + raw, err := os.ReadFile(logPath) + if err != nil { + t.Fatal(err) + } + if strings.TrimSpace(string(raw)) != tt.want { + t.Fatalf("args = %q, want %q", strings.TrimSpace(string(raw)), tt.want) + } + }) + } +} + +func TestListOfficialSkillsFallsBack(t *testing.T) { + called := []string{} + updater := &Updater{ + SkillsCommandOverride: func(args ...string) *NpmResult { + called = append(called, strings.Join(args, " ")) + r := &NpmResult{} + if strings.Contains(strings.Join(args, " "), "https://open.feishu.cn") { + r.Err = fmt.Errorf("primary failed") + return r + } + r.Stdout.WriteString("lark-calendar\n") + return r + }, + } + + result := updater.ListOfficialSkills() + if result.Err != nil { + t.Fatalf("ListOfficialSkills() err = %v, want nil", result.Err) + } + if len(called) != 2 { + t.Fatalf("called %d commands, want 2: %#v", len(called), called) + } + if !strings.Contains(called[1], "larksuite/cli --list") { + t.Fatalf("fallback call = %q, want larksuite/cli --list", called[1]) + } +} diff --git a/internal/skillscheck/check.go b/internal/skillscheck/check.go index 429117a18..029a4d01f 100644 --- a/internal/skillscheck/check.go +++ b/internal/skillscheck/check.go @@ -3,46 +3,29 @@ package skillscheck -// Init runs the synchronous skills version check. Stores a StaleNotice -// when the local stamp records a version that does not match -// currentVersion. Safe to call from cmd/root.go before rootCmd.Execute(); -// zero network, zero subprocess — only a local stamp file read. +import "strings" + +// Init runs the synchronous skills version check. Stores a StaleNotice when +// the local skills state records a version that does not match currentVersion. +// Safe to call from cmd/root.go before rootCmd.Execute(); zero network, zero +// subprocess — only a local state file read. // // Skip rules: see shouldSkip (CI envs, DEV builds, non-release semver, // LARKSUITE_CLI_NO_SKILLS_NOTIFIER opt-out). -// -// Failure modes (all → no notice, no nag): -// - shouldSkip rule met -// - ReadStamp returns an I/O error other than ENOENT -// - Stamp matches currentVersion (in-sync) -// - Stamp is missing (cold start) — only users who ran `lark-cli update` -// opt into drift tracking; npx-only installs are intentionally silent. func Init(currentVersion string) { - // Clear any stale notice from a prior call so early returns below - // (skip rules / read errors / cold start / in-sync) leave pending == nil - // instead of preserving a stale value from a previous Init invocation. SetPending(nil) if shouldSkip(currentVersion) { return } - stamp, err := ReadStamp() - if err != nil { - // Fail closed — don't nag for a transient FS problem. - return - } - if stamp == "" { - // Cold start: the stamp is written exclusively by `lark-cli update` - // (runSkillsAndStamp). Users who installed skills via - // `npx skills add larksuite/cli -g` have no stamp yet — they must - // not be nagged with "skills not installed", since the on-disk - // skills directory may already be fully populated. + version, ok := ReadSyncedVersion() + if !ok { return } - if stamp == currentVersion { + if strings.TrimPrefix(strings.TrimPrefix(version, "v"), "V") == strings.TrimPrefix(strings.TrimPrefix(currentVersion, "v"), "V") { return } SetPending(&StaleNotice{ - Current: stamp, // guaranteed non-empty under the new contract + Current: version, Target: currentVersion, }) } diff --git a/internal/skillscheck/check_test.go b/internal/skillscheck/check_test.go index 64525bc5a..2674d5424 100644 --- a/internal/skillscheck/check_test.go +++ b/internal/skillscheck/check_test.go @@ -18,9 +18,8 @@ func resetPending(t *testing.T) { func TestInit_InSync_NoNotice(t *testing.T) { clearSkillsSkipEnv(t) resetPending(t) - dir := t.TempDir() - t.Setenv("LARKSUITE_CLI_CONFIG_DIR", dir) - if err := WriteStamp("1.0.21"); err != nil { + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) + if err := WriteState(SkillsState{Version: "1.0.21"}); err != nil { t.Fatal(err) } Init("1.0.21") @@ -39,12 +38,24 @@ func TestInit_ColdStart_NoNotice(t *testing.T) { } } -func TestInit_Drift_NoticeWithStampVersion(t *testing.T) { +func TestInit_NormalizedVersion_NoNotice(t *testing.T) { clearSkillsSkipEnv(t) resetPending(t) - dir := t.TempDir() - t.Setenv("LARKSUITE_CLI_CONFIG_DIR", dir) - if err := WriteStamp("1.0.20"); err != nil { + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) + if err := WriteState(SkillsState{Version: "1.0.21"}); err != nil { + t.Fatal(err) + } + Init("v1.0.21") + if got := GetPending(); got != nil { + t.Errorf("GetPending() = %+v, want nil (normalized versions are in-sync)", got) + } +} + +func TestInit_Drift_NoticeWithStateVersion(t *testing.T) { + clearSkillsSkipEnv(t) + resetPending(t) + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) + if err := WriteState(SkillsState{Version: "1.0.20"}); err != nil { t.Fatal(err) } Init("1.0.21") @@ -61,22 +72,18 @@ func TestInit_Skipped_NoNotice(t *testing.T) { clearSkillsSkipEnv(t) resetPending(t) t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) - // Even with an empty config dir (no stamp), DEV version should skip - // the check entirely and never emit a notice. Init("DEV") if got := GetPending(); got != nil { t.Errorf("GetPending() = %+v, want nil (skip rules met)", got) } } -func TestInit_ReadStampError_FailsClosed(t *testing.T) { +func TestInit_ReadStateError_FailsClosed(t *testing.T) { clearSkillsSkipEnv(t) resetPending(t) dir := t.TempDir() t.Setenv("LARKSUITE_CLI_CONFIG_DIR", dir) - // Make the stamp path a directory so vfs.ReadFile returns a - // non-ENOENT I/O error. - if err := os.MkdirAll(filepath.Join(dir, "skills.stamp"), 0o755); err != nil { + if err := os.MkdirAll(filepath.Join(dir, "skills-state.json"), 0o755); err != nil { t.Fatal(err) } Init("1.0.21") diff --git a/internal/skillscheck/notice.go b/internal/skillscheck/notice.go index b1f972218..c1425fbb7 100644 --- a/internal/skillscheck/notice.go +++ b/internal/skillscheck/notice.go @@ -3,9 +3,8 @@ // Package skillscheck verifies that the locally installed lark-cli // skills are in sync with the running binary version, by comparing -// the current binary version against a stamp file written when skills -// are last synced (by `lark-cli update`). On mismatch it stores a -// notice for injection into JSON envelopes via output.PendingNotice. +// the current binary version against skills-state.json. On mismatch it +// stores a notice for injection into JSON envelopes via output.PendingNotice. package skillscheck import ( @@ -26,8 +25,7 @@ type StaleNotice struct { // Message returns a single-line, AI-agent-parseable description of the // drift plus the canonical fix command. Mirrors internal/update.UpdateInfo.Message // in style ("..., run: lark-cli update" suffix). Current is guaranteed -// non-empty because Init only emits a StaleNotice for the drift case -// (stamp present and != binary version). +// non-empty because Init only emits a StaleNotice for the drift case. func (s *StaleNotice) Message() string { return fmt.Sprintf( "lark-cli skills %s out of sync with binary %s, run: lark-cli update", diff --git a/internal/skillscheck/stamp.go b/internal/skillscheck/stamp.go deleted file mode 100644 index 052e331c9..000000000 --- a/internal/skillscheck/stamp.go +++ /dev/null @@ -1,49 +0,0 @@ -// Copyright (c) 2026 Lark Technologies Pte. Ltd. -// SPDX-License-Identifier: MIT - -package skillscheck - -import ( - "errors" - "io/fs" - "path/filepath" - "strings" - - "github.com/larksuite/cli/internal/core" - "github.com/larksuite/cli/internal/validate" - "github.com/larksuite/cli/internal/vfs" -) - -const stampFile = "skills.stamp" - -// stampPath returns ~/.lark-cli/skills.stamp. -// Uses the BASE config dir (not workspace-aware) because skills install -// globally via `npx -g`; per-workspace tracking would produce false -// drift signals when switching workspaces. -func stampPath() string { - return filepath.Join(core.GetBaseConfigDir(), stampFile) -} - -// ReadStamp returns the version recorded in the stamp file. Returns -// ("", nil) when the file does not exist (interpreted as "never synced"). -// Other I/O errors are returned as-is so callers can fail closed. -func ReadStamp() (string, error) { - data, err := vfs.ReadFile(stampPath()) - if err != nil { - if errors.Is(err, fs.ErrNotExist) { - return "", nil - } - return "", err - } - return strings.TrimSpace(string(data)), nil -} - -// WriteStamp records `version` as the last successfully synced skills -// version. Atomic via tmp + rename (validate.AtomicWrite). Creates -// the base config directory if it does not exist. -func WriteStamp(version string) error { - if err := vfs.MkdirAll(core.GetBaseConfigDir(), 0o700); err != nil { - return err - } - return validate.AtomicWrite(stampPath(), []byte(version), 0o644) -} diff --git a/internal/skillscheck/stamp_test.go b/internal/skillscheck/stamp_test.go deleted file mode 100644 index 8e60dfbb4..000000000 --- a/internal/skillscheck/stamp_test.go +++ /dev/null @@ -1,113 +0,0 @@ -// Copyright (c) 2026 Lark Technologies Pte. Ltd. -// SPDX-License-Identifier: MIT - -package skillscheck - -import ( - "os" - "path/filepath" - "testing" -) - -func TestReadStamp_Missing(t *testing.T) { - t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) - got, err := ReadStamp() - if err != nil { - t.Fatalf("ReadStamp() err = %v, want nil for ENOENT", err) - } - if got != "" { - t.Errorf("ReadStamp() = %q, want \"\" for missing file", got) - } -} - -func TestReadStamp_Normal(t *testing.T) { - dir := t.TempDir() - t.Setenv("LARKSUITE_CLI_CONFIG_DIR", dir) - if err := os.WriteFile(filepath.Join(dir, "skills.stamp"), []byte("1.0.21"), 0o644); err != nil { - t.Fatal(err) - } - got, err := ReadStamp() - if err != nil || got != "1.0.21" { - t.Errorf("ReadStamp() = (%q, %v), want (\"1.0.21\", nil)", got, err) - } -} - -func TestReadStamp_TrailingNewlineTolerated(t *testing.T) { - dir := t.TempDir() - t.Setenv("LARKSUITE_CLI_CONFIG_DIR", dir) - if err := os.WriteFile(filepath.Join(dir, "skills.stamp"), []byte("1.0.21\n"), 0o644); err != nil { - t.Fatal(err) - } - got, _ := ReadStamp() - if got != "1.0.21" { - t.Errorf("ReadStamp() = %q, want \"1.0.21\" (newline trimmed)", got) - } -} - -func TestReadStamp_EmptyFile(t *testing.T) { - dir := t.TempDir() - t.Setenv("LARKSUITE_CLI_CONFIG_DIR", dir) - if err := os.WriteFile(filepath.Join(dir, "skills.stamp"), []byte(""), 0o644); err != nil { - t.Fatal(err) - } - got, err := ReadStamp() - if err != nil || got != "" { - t.Errorf("ReadStamp() = (%q, %v), want (\"\", nil)", got, err) - } -} - -func TestWriteStamp_CreatesDir(t *testing.T) { - dir := filepath.Join(t.TempDir(), "nested") - t.Setenv("LARKSUITE_CLI_CONFIG_DIR", dir) - if err := WriteStamp("1.0.21"); err != nil { - t.Fatalf("WriteStamp() = %v, want nil", err) - } - got, _ := os.ReadFile(filepath.Join(dir, "skills.stamp")) - if string(got) != "1.0.21" { - t.Errorf("file content = %q, want \"1.0.21\"", string(got)) - } -} - -func TestWriteStamp_OverwritesExisting(t *testing.T) { - dir := t.TempDir() - t.Setenv("LARKSUITE_CLI_CONFIG_DIR", dir) - if err := WriteStamp("1.0.20"); err != nil { - t.Fatal(err) - } - if err := WriteStamp("1.0.21"); err != nil { - t.Fatal(err) - } - got, _ := ReadStamp() - if got != "1.0.21" { - t.Errorf("ReadStamp() after overwrite = %q, want \"1.0.21\"", got) - } -} - -func TestWriteStamp_NoTrailingNewline(t *testing.T) { - dir := t.TempDir() - t.Setenv("LARKSUITE_CLI_CONFIG_DIR", dir) - if err := WriteStamp("1.0.21"); err != nil { - t.Fatal(err) - } - raw, _ := os.ReadFile(filepath.Join(dir, "skills.stamp")) - if string(raw) != "1.0.21" { - t.Errorf("raw file = %q, want exactly \"1.0.21\" (no newline)", string(raw)) - } -} - -// TestWriteStamp_MkdirAllFailure verifies WriteStamp returns the mkdir error -// when the base config dir cannot be created (parent path is a regular file). -func TestWriteStamp_MkdirAllFailure(t *testing.T) { - tmp := t.TempDir() - blocker := filepath.Join(tmp, "blocker") - // Create a regular file where MkdirAll wants to create a directory. - if err := os.WriteFile(blocker, []byte("not-a-dir"), 0o644); err != nil { - t.Fatal(err) - } - // Point the config dir at a path UNDER the regular file — MkdirAll must fail. - t.Setenv("LARKSUITE_CLI_CONFIG_DIR", filepath.Join(blocker, "child")) - - if err := WriteStamp("1.0.21"); err == nil { - t.Fatal("WriteStamp() = nil, want non-nil error from MkdirAll failure") - } -} diff --git a/internal/skillscheck/state.go b/internal/skillscheck/state.go new file mode 100644 index 000000000..eddab1cf3 --- /dev/null +++ b/internal/skillscheck/state.go @@ -0,0 +1,92 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package skillscheck + +import ( + "encoding/json" + "errors" + "fmt" + "io/fs" + "path/filepath" + + "github.com/larksuite/cli/internal/core" + "github.com/larksuite/cli/internal/validate" + "github.com/larksuite/cli/internal/vfs" +) + +const ( + stateFile = "skills-state.json" +) + +var ErrUnreadableState = errors.New("skills state is unreadable") + +type SkillsState struct { + Version string `json:"version"` + OfficialSkills []string `json:"official_skills"` + UpdatedSkills []string `json:"updated_skills"` + AddedOfficialSkills []string `json:"added_official_skills"` + SkippedDeletedSkills []string `json:"skipped_deleted_skills"` + UpdatedAt string `json:"updated_at"` +} + +func statePath() string { + return filepath.Join(core.GetBaseConfigDir(), stateFile) +} + +func ReadState() (*SkillsState, bool, error) { + data, err := vfs.ReadFile(statePath()) + if err != nil { + if errors.Is(err, fs.ErrNotExist) { + return nil, false, nil + } + return nil, false, err + } + + var raw map[string]interface{} + if err := json.Unmarshal(data, &raw); err != nil { + return nil, false, fmt.Errorf("%w: %v", ErrUnreadableState, err) + } + + var state SkillsState + if err := json.Unmarshal(data, &state); err != nil { + return nil, false, fmt.Errorf("%w: %v", ErrUnreadableState, err) + } + return &state, true, nil +} + +func WriteState(state SkillsState) error { + state.ensureNonNilSlices() + + if err := vfs.MkdirAll(core.GetBaseConfigDir(), 0o700); err != nil { + return err + } + data, err := json.MarshalIndent(state, "", " ") + if err != nil { + return err + } + return validate.AtomicWrite(statePath(), append(data, '\n'), 0o644) +} + +func ReadSyncedVersion() (string, bool) { + state, ok, err := ReadState() + if err != nil || !ok || state.Version == "" { + return "", false + } + return state.Version, true +} + +func (s *SkillsState) ensureNonNilSlices() { + if s.OfficialSkills == nil { + s.OfficialSkills = []string{} + } + if s.UpdatedSkills == nil { + s.UpdatedSkills = []string{} + } + if s.AddedOfficialSkills == nil { + s.AddedOfficialSkills = []string{} + } + if s.SkippedDeletedSkills == nil { + s.SkippedDeletedSkills = []string{} + } +} diff --git a/internal/skillscheck/state_test.go b/internal/skillscheck/state_test.go new file mode 100644 index 000000000..d69b74635 --- /dev/null +++ b/internal/skillscheck/state_test.go @@ -0,0 +1,139 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package skillscheck + +import ( + "encoding/json" + "errors" + "os" + "path/filepath" + "reflect" + "testing" +) + +func TestReadState_Missing(t *testing.T) { + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) + + state, ok, err := ReadState() + if err != nil { + t.Fatalf("ReadState() err = %v, want nil for missing file", err) + } + if ok { + t.Fatal("ReadState() ok = true, want false for missing file") + } + if state != nil { + t.Fatalf("ReadState() state = %#v, want nil for missing file", state) + } +} + +func TestReadState_Valid(t *testing.T) { + dir := t.TempDir() + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", dir) + want := SkillsState{ + Version: "1.2.3", + OfficialSkills: []string{"lark-doc", "lark-im"}, + UpdatedSkills: []string{"lark-doc"}, + AddedOfficialSkills: []string{"lark-task"}, + SkippedDeletedSkills: []string{"custom-skill"}, + UpdatedAt: "2026-05-18T10:00:00Z", + } + data, err := json.Marshal(want) + if err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, stateFile), data, 0o644); err != nil { + t.Fatal(err) + } + + got, ok, err := ReadState() + if err != nil { + t.Fatalf("ReadState() err = %v, want nil", err) + } + if !ok { + t.Fatal("ReadState() ok = false, want true") + } + if got == nil { + t.Fatal("ReadState() state = nil, want state") + } + if !reflect.DeepEqual(*got, want) { + t.Fatalf("ReadState() state = %#v, want %#v", *got, want) + } +} + +func TestReadState_CorruptStateUnreadable(t *testing.T) { + dir := t.TempDir() + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", dir) + if err := os.WriteFile(filepath.Join(dir, stateFile), []byte(`{"version":`), 0o644); err != nil { + t.Fatal(err) + } + + state, ok, err := ReadState() + if !errors.Is(err, ErrUnreadableState) { + t.Fatalf("ReadState() err = %v, want ErrUnreadableState", err) + } + if ok { + t.Fatal("ReadState() ok = true, want false") + } + if state != nil { + t.Fatalf("ReadState() state = %#v, want nil", state) + } +} + +func TestWriteState_CreatesDirAndWritesState(t *testing.T) { + dir := filepath.Join(t.TempDir(), "nested") + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", dir) + + state := SkillsState{ + Version: "1.2.3", + UpdatedAt: "2026-05-18T10:00:00Z", + } + if err := WriteState(state); err != nil { + t.Fatalf("WriteState() err = %v, want nil", err) + } + + raw, err := os.ReadFile(filepath.Join(dir, stateFile)) + if err != nil { + t.Fatal(err) + } + var got SkillsState + if err := json.Unmarshal(raw, &got); err != nil { + t.Fatalf("written state is invalid JSON: %v", err) + } + if got.Version != state.Version { + t.Fatalf("version = %q, want %q", got.Version, state.Version) + } + if got.OfficialSkills == nil { + t.Fatal("official_skills decoded as nil, want empty slice") + } + if got.UpdatedSkills == nil { + t.Fatal("updated_skills decoded as nil, want empty slice") + } + if got.AddedOfficialSkills == nil { + t.Fatal("added_skills decoded as nil, want empty slice") + } + if got.SkippedDeletedSkills == nil { + t.Fatal("skipped_deleted_skills decoded as nil, want empty slice") + } +} + +func TestReadSyncedVersionFromState(t *testing.T) { + dir := t.TempDir() + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", dir) + + if got, ok := ReadSyncedVersion(); ok || got != "" { + t.Fatalf("ReadSyncedVersion() = (%q, %v), want (\"\", false) for missing state", got, ok) + } + if err := WriteState(SkillsState{Version: "1.2.3"}); err != nil { + t.Fatal(err) + } + if got, ok := ReadSyncedVersion(); !ok || got != "1.2.3" { + t.Fatalf("ReadSyncedVersion() = (%q, %v), want (\"1.2.3\", true)", got, ok) + } + if err := WriteState(SkillsState{}); err != nil { + t.Fatal(err) + } + if got, ok := ReadSyncedVersion(); ok || got != "" { + t.Fatalf("ReadSyncedVersion() = (%q, %v), want (\"\", false) for empty version", got, ok) + } +} diff --git a/internal/skillscheck/sync.go b/internal/skillscheck/sync.go new file mode 100644 index 000000000..29744febe --- /dev/null +++ b/internal/skillscheck/sync.go @@ -0,0 +1,383 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package skillscheck + +import ( + "fmt" + "regexp" + "sort" + "strings" + "time" + + "github.com/larksuite/cli/internal/selfupdate" +) + +var ( + skillNamePattern = regexp.MustCompile(`^[A-Za-z0-9][A-Za-z0-9_:-]*(@[^\s]+)?$`) + ansiPattern = regexp.MustCompile(`\x1b\[[0-?]*[ -/]*[@-~]`) +) + +type SyncInput struct { + Version string + OfficialSkills []string + LocalSkills []string + PreviousState *SkillsState + StateReadable bool + Force bool +} + +type SyncPlan struct { + Version string + OfficialSkills []string + ToUpdate []string + Added []string + SkippedDeleted []string +} + +func stripANSI(s string) string { + return ansiPattern.ReplaceAllString(s, "") +} + +func ParseSkillsList(text string) []string { + text = stripANSI(text) + lines := strings.Split(text, "\n") + + // Detect format type + hasGlobalSkills := strings.Contains(text, "Global Skills") + hasAvailableSkills := strings.Contains(text, "Available Skills") + + if hasGlobalSkills { + // Format 1: locally installed skills list from "npx -y skills ls -g" + return parseGlobalSkillsList(lines) + } else if hasAvailableSkills { + // Format 2: official skills list from "npx -y skills add ... --list" + return parseOfficialSkillsList(lines) + } + return nil +} + +// parseGlobalSkillsList parses the output of "npx -y skills ls -g" +func parseGlobalSkillsList(lines []string) []string { + seen := map[string]bool{} + + for _, line := range lines { + trimmed := strings.TrimSpace(line) + + // Skip header + if strings.HasPrefix(trimmed, "Global Skills") { + continue + } + + // Skip empty lines + if trimmed == "" { + continue + } + if strings.HasPrefix(trimmed, "Tip:") { + continue + } + + // Skip indented lines (Agents: ...) + if strings.HasPrefix(line, " ") || strings.HasPrefix(line, "\t") { + continue + } + + // Extract skill name, format is typically "skill-name /path/to/skill" + parts := strings.Fields(trimmed) + if len(parts) == 0 { + continue + } + + candidate := parts[0] + + // Validate and add + if candidate == "" || strings.Contains(candidate, " ") || strings.HasSuffix(candidate, ":") { + continue + } + if !skillNamePattern.MatchString(candidate) { + continue + } + if at := strings.Index(candidate, "@"); at > 0 { + candidate = candidate[:at] + } + seen[candidate] = true + } + + return sortedKeys(seen) +} + +// parseOfficialSkillsList parses the output of "npx -y skills add ... --list" +func parseOfficialSkillsList(lines []string) []string { + seen := map[string]bool{} + inAvailableSection := false + + for _, line := range lines { + // Check if we've reached the "Available Skills" section + if strings.Contains(line, "Available Skills") { + inAvailableSection = true + continue + } + + // If there's an "Available Skills" section but we haven't reached it yet, keep skipping + if !inAvailableSection { + continue + } + + // Process lines containing "│", e.g. " │ lark-approval " + if strings.Contains(line, "│") { + // Remove all "│" characters and spaces, extract the first valid token in order + parts := strings.FieldsFunc(line, func(r rune) bool { + return r == '│' || r == ' ' + }) + + if len(parts) > 0 { + candidate := parts[0] + // Check if it's a valid skill name + if strings.Contains(candidate, "-") || + strings.HasPrefix(candidate, "lark-") || + strings.HasPrefix(candidate, "skill-") || + strings.HasPrefix(candidate, "cli-") { + if candidate != "" && !strings.Contains(candidate, " ") && !strings.HasSuffix(candidate, ":") { + if skillNamePattern.MatchString(candidate) { + if at := strings.Index(candidate, "@"); at > 0 { + candidate = candidate[:at] + } + seen[candidate] = true + } + } + } + } + } + } + + return sortedKeys(seen) +} + +func PlanSync(input SyncInput) SyncPlan { + official := uniqueSorted(input.OfficialSkills) + if input.Force { + return SyncPlan{ + Version: input.Version, + OfficialSkills: official, + ToUpdate: official, + Added: []string{}, + SkippedDeleted: []string{}, + } + } + + officialSet := toSet(official) + installedOfficial := intersection(input.LocalSkills, officialSet) + + previousOfficial := []string{} + if input.StateReadable && input.PreviousState != nil { + previousOfficial = input.PreviousState.OfficialSkills + } + previousSet := toSet(previousOfficial) + + newAddedOfficial := []string{} + for _, skill := range official { + if !previousSet[skill] { + newAddedOfficial = append(newAddedOfficial, skill) + } + } + + updateSet := toSet(installedOfficial) + for _, skill := range newAddedOfficial { + updateSet[skill] = true + } + toUpdate := sortedKeys(updateSet) + updateSet = toSet(toUpdate) + + skipped := []string{} + for _, skill := range official { + if !updateSet[skill] { + skipped = append(skipped, skill) + } + } + + return SyncPlan{ + Version: input.Version, + OfficialSkills: official, + ToUpdate: toUpdate, + Added: uniqueSorted(newAddedOfficial), + SkippedDeleted: skipped, + } +} + +type SkillsRunner interface { + ListOfficialSkills() *selfupdate.NpmResult + ListGlobalSkills() *selfupdate.NpmResult + InstallSkill(nameList []string) *selfupdate.NpmResult + InstallAllSkills() *selfupdate.NpmResult +} + +type SyncOptions struct { + Version string + Force bool + Runner SkillsRunner + Now func() time.Time +} + +type SyncResult struct { + Action string + Official []string + Updated []string + Added []string + SkippedDeleted []string + Failed []string + Err error + Detail string + Force bool +} + +func SyncSkills(opts SyncOptions) *SyncResult { + if opts.Now == nil { + opts.Now = time.Now + } + if opts.Runner == nil { + return &SyncResult{Action: "failed", Err: fmt.Errorf("skills runner is nil")} + } + + // --- Step 1: List official skills --- + officialResult := opts.Runner.ListOfficialSkills() + if officialResult == nil || officialResult.Err != nil { + return fallbackFullInstall(opts, resultDetail(officialResult)) + } + official := ParseSkillsList(officialResult.Stdout.String()) + + if len(official) == 0 && strings.TrimSpace(officialResult.Stdout.String()) != "" { + return fallbackFullInstall(opts, "official skills list parsed as empty despite non-empty stdout") + } + + // --- Step 2: List local (installed) skills --- + local := []string{} + localResult := opts.Runner.ListGlobalSkills() + if localResult != nil && localResult.Err == nil { + local = ParseSkillsList(localResult.Stdout.String()) + } + + // --- Step 3: Read previous state --- + previous, readable, err := ReadState() + if err != nil { + readable = false + previous = nil + } + + plan := PlanSync(SyncInput{ + Version: opts.Version, + OfficialSkills: official, + LocalSkills: local, + PreviousState: previous, + StateReadable: readable, + Force: opts.Force, + }) + + result := &SyncResult{ + Action: "synced", + Official: plan.OfficialSkills, + Updated: plan.ToUpdate, + Added: plan.Added, + SkippedDeleted: plan.SkippedDeleted, + Force: opts.Force, + } + + if len(plan.ToUpdate) > 0 { + installResult := opts.Runner.InstallSkill(plan.ToUpdate) + if installResult == nil || installResult.Err != nil { + // incremental install failed, retry with full install. + return fallbackFullInstall(opts, resultDetail(installResult)) + } + } + + state := SkillsState{ + Version: opts.Version, + OfficialSkills: plan.OfficialSkills, + UpdatedSkills: plan.ToUpdate, + AddedOfficialSkills: plan.Added, + SkippedDeletedSkills: plan.SkippedDeleted, + UpdatedAt: opts.Now().UTC().Format(time.RFC3339), + } + if err := WriteState(state); err != nil { + result.Action = "failed" + result.Err = fmt.Errorf("skills synced but state not written: %w", err) + return result + } + + return result +} + +// fallbackFullInstall performs a full skills install (npx -y skills add -g -y) +// when incremental sync is not possible. It does not write a state file because +// we cannot determine the official skill list reliably. +func fallbackFullInstall(opts SyncOptions, reason string) *SyncResult { + installResult := opts.Runner.InstallAllSkills() + if installResult == nil { + return &SyncResult{ + Action: "fallback_failed", + Err: fmt.Errorf("full skills install failed: empty result (reason: %s)", reason), + Detail: reason, + Force: opts.Force, + } + } + if installResult.Err != nil { + return &SyncResult{ + Action: "fallback_failed", + Err: fmt.Errorf("full skills install failed: %w (reason: %s)", installResult.Err, reason), + Detail: reason + "\n" + resultDetail(installResult), + Force: opts.Force, + } + } + return &SyncResult{ + Action: "fallback_synced", + Force: opts.Force, + } +} + +func resultDetail(result *selfupdate.NpmResult) string { + if result == nil { + return "" + } + parts := []string{} + if output := strings.TrimSpace(result.CombinedOutput()); output != "" { + parts = append(parts, output) + } + if result.Err != nil { + parts = append(parts, result.Err.Error()) + } + return strings.Join(parts, "\n") +} + +func uniqueSorted(values []string) []string { + return sortedKeys(toSet(values)) +} + +func toSet(values []string) map[string]bool { + out := map[string]bool{} + for _, value := range values { + value = strings.TrimSpace(value) + if value != "" { + out[value] = true + } + } + return out +} + +// result = { x | x ∈ values ∧ x ∈ allowed } +func intersection(values []string, allowed map[string]bool) []string { + out := map[string]bool{} + for _, value := range values { + if allowed[value] { + out[value] = true + } + } + return sortedKeys(out) +} + +func sortedKeys(values map[string]bool) []string { + out := make([]string, 0, len(values)) + for value := range values { + out = append(out, value) + } + sort.Strings(out) + return out +} diff --git a/internal/skillscheck/sync_test.go b/internal/skillscheck/sync_test.go new file mode 100644 index 000000000..9654845d7 --- /dev/null +++ b/internal/skillscheck/sync_test.go @@ -0,0 +1,368 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package skillscheck + +import ( + "fmt" + "os" + "path/filepath" + "reflect" + "strings" + "testing" + "time" + + "github.com/larksuite/cli/internal/selfupdate" +) + +func TestParseSkillsListIgnoresUnsupportedFormat(t *testing.T) { + input := `Installed skills: +- lark-calendar +- lark-mail +lark-im +custom-skill +lark-base@1.0.0 +lark-cli-harness:dev@0.1.0 +` + got := ParseSkillsList(input) + if len(got) != 0 { + t.Fatalf("ParseSkillsList() = %#v, want empty result for unsupported format", got) + } +} + +func TestParseGlobalSkillsList(t *testing.T) { + input := `Global Skills + +lark-approval ~/.agents/skills/lark-approval + Agents: TRAE CN, TRAE, TRAE-SOLO, TRAE CLI, TRAE CLI (Coco) +3 more +lark-attendance ~/.agents/skills/lark-attendance + Agents: TRAE CN, TRAE, TRAE-SOLO, TRAE CLI, TRAE CLI (Coco) +3 more +lark-base ~/.agents/skills/lark-base + Agents: TRAE CN, TRAE, TRAE-SOLO, TRAE CLI, TRAE CLI (Coco) +3 more +lark-calendar ~/.agents/skills/lark-calendar + Agents: TRAE CN, TRAE, TRAE-SOLO, TRAE CLI, TRAE CLI (Coco) +3 more +dogfood ~/.hermes/skills/dogfood + Agents: Hermes Agent +yuanbao ~/.hermes/skills/yuanbao + Agents: Hermes Agent +` + got := ParseSkillsList(input) + want := []string{"dogfood", "lark-approval", "lark-attendance", "lark-base", "lark-calendar", "yuanbao"} + if !reflect.DeepEqual(got, want) { + t.Fatalf("ParseSkillsList() (Global Skills) = %#v, want %#v", got, want) + } +} + +func TestParseGlobalSkillsListWithANSI(t *testing.T) { + input := "\x1b[1mGlobal Skills\x1b[0m\n\n" + + "\x1b[36mlark-calendar\x1b[0m \x1b[38;5;102m~/.agents/skills/lark-calendar\x1b[0m\n" + + " \x1b[38;5;102mAgents:\x1b[0m TRAE CN, TRAE +3 more\n" + + "\x1b[36mdogfood\x1b[0m \x1b[38;5;102m~/.hermes/skills/dogfood\x1b[0m\n" + + " \x1b[38;5;102mAgents:\x1b[0m Hermes Agent\n" + + "\nTip: Use the -y flag to run in non-interactive mode (for CI and AI agents).\n" + got := ParseSkillsList(input) + want := []string{"dogfood", "lark-calendar"} + if !reflect.DeepEqual(got, want) { + t.Fatalf("ParseSkillsList() (ANSI Global Skills) = %#v, want %#v", got, want) + } +} + +func TestPlanNormal_WithReadableStatePreservesDeletedAndAddsNew(t *testing.T) { + previous := &SkillsState{OfficialSkills: []string{"lark-calendar", "lark-mail"}} + got := PlanSync(SyncInput{ + Version: "1.0.33", + OfficialSkills: []string{"lark-calendar", "lark-mail", "lark-new"}, + LocalSkills: []string{"lark-calendar", "lark-custom"}, + PreviousState: previous, + StateReadable: true, + Force: false, + }) + + assertStrings(t, got.ToUpdate, []string{"lark-calendar", "lark-new"}) + assertStrings(t, got.Added, []string{"lark-new"}) + assertStrings(t, got.SkippedDeleted, []string{"lark-mail"}) +} + +func TestPlanNormal_MissingStateInstallsAllOfficial(t *testing.T) { + got := PlanSync(SyncInput{ + Version: "1.0.33", + OfficialSkills: []string{"lark-calendar", "lark-mail", "lark-new"}, + LocalSkills: []string{"lark-calendar"}, + StateReadable: false, + Force: false, + }) + + assertStrings(t, got.ToUpdate, []string{"lark-calendar", "lark-mail", "lark-new"}) + assertStrings(t, got.Added, []string{"lark-calendar", "lark-mail", "lark-new"}) + assertStrings(t, got.SkippedDeleted, []string{}) +} + +func TestPlanForceRestoresAllOfficial(t *testing.T) { + got := PlanSync(SyncInput{ + Version: "1.0.33", + OfficialSkills: []string{"lark-calendar", "lark-mail", "lark-new"}, + LocalSkills: []string{"lark-calendar"}, + PreviousState: &SkillsState{OfficialSkills: []string{"lark-calendar", "lark-mail"}}, + StateReadable: true, + Force: true, + }) + + assertStrings(t, got.ToUpdate, []string{"lark-calendar", "lark-mail", "lark-new"}) + assertStrings(t, got.Added, []string{}) + assertStrings(t, got.SkippedDeleted, []string{}) +} + +type fakeSkillsRunner struct { + officialOut string + globalOut string + officialErr error + globalErr error + installErr error + installAllErr error + installed [][]string + installedAll int +} + +func officialSkillsOutput(names ...string) string { + var b strings.Builder + b.WriteString("Available Skills\n") + for _, name := range names { + b.WriteString("│ ") + b.WriteString(name) + b.WriteString("\n") + } + return b.String() +} + +func globalSkillsOutput(names ...string) string { + var b strings.Builder + b.WriteString("Global Skills\n\n") + for _, name := range names { + b.WriteString(name) + b.WriteString(" ~/.agents/skills/") + b.WriteString(name) + b.WriteString("\n Agents: Claude Code\n") + } + return b.String() +} + +func (f *fakeSkillsRunner) ListOfficialSkills() *selfupdate.NpmResult { + r := &selfupdate.NpmResult{} + r.Stdout.WriteString(f.officialOut) + r.Err = f.officialErr + return r +} + +func (f *fakeSkillsRunner) ListGlobalSkills() *selfupdate.NpmResult { + r := &selfupdate.NpmResult{} + r.Stdout.WriteString(f.globalOut) + r.Err = f.globalErr + return r +} + +func (f *fakeSkillsRunner) InstallSkill(nameList []string) *selfupdate.NpmResult { + f.installed = append(f.installed, nameList) + r := &selfupdate.NpmResult{} + r.Err = f.installErr + return r +} + +func (f *fakeSkillsRunner) InstallAllSkills() *selfupdate.NpmResult { + f.installedAll++ + r := &selfupdate.NpmResult{} + r.Err = f.installAllErr + return r +} + +func TestSyncSkills_WritesStateAndDoesNotWriteStamp(t *testing.T) { + dir := t.TempDir() + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", dir) + if err := WriteState(SkillsState{ + Version: "1.0.30", + OfficialSkills: []string{"lark-calendar", "lark-mail"}, + UpdatedAt: "2026-05-18T00:00:00Z", + }); err != nil { + t.Fatal(err) + } + + runner := &fakeSkillsRunner{ + officialOut: officialSkillsOutput("lark-calendar", "lark-mail", "lark-new"), + globalOut: globalSkillsOutput("lark-calendar", "lark-custom"), + } + result := SyncSkills(SyncOptions{ + Version: "1.0.33", + Runner: runner, + Now: func() time.Time { return time.Date(2026, 5, 18, 12, 0, 0, 0, time.UTC) }, + }) + + if result.Err != nil { + t.Fatalf("SyncSkills() err = %v, want nil", result.Err) + } + assertStrings(t, runner.installed[0], []string{"lark-calendar", "lark-new"}) + + state, readable, err := ReadState() + if err != nil || !readable { + t.Fatalf("ReadState() = (_, %v, %v), want readable", readable, err) + } + assertStrings(t, state.OfficialSkills, []string{"lark-calendar", "lark-mail", "lark-new"}) + assertStrings(t, state.UpdatedSkills, []string{"lark-calendar", "lark-new"}) + assertStrings(t, state.AddedOfficialSkills, []string{"lark-new"}) + assertStrings(t, state.SkippedDeletedSkills, []string{"lark-mail"}) + if _, err := os.Stat(filepath.Join(dir, "skills.stamp")); !os.IsNotExist(err) { + t.Fatalf("skills.stamp exists or stat failed with unexpected err: %v", err) + } +} + +func TestSyncSkills_ListOfficialFailureFallsBackToFullInstall(t *testing.T) { + dir := t.TempDir() + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", dir) + runner := &fakeSkillsRunner{ + officialErr: fmt.Errorf("list failed"), + installAllErr: nil, + } + + result := SyncSkills(SyncOptions{Version: "1.0.33", Runner: runner, Now: time.Now}) + if result.Action != "fallback_synced" { + t.Fatalf("SyncSkills() action = %q, want fallback_synced", result.Action) + } + if runner.installedAll != 1 { + t.Fatalf("installedAll = %d, want 1", runner.installedAll) + } + if len(runner.installed) != 0 { + t.Fatalf("installed = %#v, want no incremental installs", runner.installed) + } +} + +func TestSyncSkills_ListOfficialFailureAndFullInstallFails(t *testing.T) { + dir := t.TempDir() + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", dir) + runner := &fakeSkillsRunner{ + officialErr: fmt.Errorf("list failed"), + installAllErr: fmt.Errorf("full install failed"), + } + + result := SyncSkills(SyncOptions{Version: "1.0.33", Runner: runner, Now: time.Now}) + if result.Action != "fallback_failed" { + t.Fatalf("SyncSkills() action = %q, want fallback_failed", result.Action) + } + if result.Err == nil { + t.Fatalf("SyncSkills() err = nil, want error") + } + if !strings.Contains(result.Err.Error(), "full skills install failed") { + t.Fatalf("SyncSkills() err = %v, want full install failure", result.Err) + } +} + +func TestSyncSkills_GlobalListFailureDegradesToColdStart(t *testing.T) { + dir := t.TempDir() + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", dir) + runner := &fakeSkillsRunner{ + officialOut: officialSkillsOutput("lark-calendar", "lark-mail"), + globalErr: fmt.Errorf("global list failed"), + } + + result := SyncSkills(SyncOptions{Version: "1.0.33", Runner: runner, Now: time.Now}) + if result.Err != nil { + t.Fatalf("SyncSkills() err = %v, want nil (degraded to cold start)", result.Err) + } + if result.Action != "synced" { + t.Fatalf("SyncSkills() action = %q, want synced", result.Action) + } + assertStrings(t, result.Updated, []string{"lark-calendar", "lark-mail"}) + assertStrings(t, result.SkippedDeleted, []string{}) +} + +func TestSyncSkills_InstallFailureFallsBackToFullInstall(t *testing.T) { + dir := t.TempDir() + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", dir) + runner := &fakeSkillsRunner{ + officialOut: officialSkillsOutput("lark-calendar", "lark-mail"), + globalOut: globalSkillsOutput("lark-calendar", "lark-mail"), + installErr: fmt.Errorf("incremental boom"), + installAllErr: nil, + } + + result := SyncSkills(SyncOptions{Version: "1.0.33", Runner: runner, Now: time.Now}) + if result.Action != "fallback_synced" { + t.Fatalf("SyncSkills() action = %q, want fallback_synced", result.Action) + } + if len(runner.installed) != 1 { + t.Fatalf("installed = %d calls, want 1", len(runner.installed)) + } + if runner.installedAll != 1 { + t.Fatalf("installedAll = %d, want 1 (fallback triggered)", runner.installedAll) + } +} + +func TestSyncSkills_InstallFailureAndFullInstallFails(t *testing.T) { + dir := t.TempDir() + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", dir) + runner := &fakeSkillsRunner{ + officialOut: officialSkillsOutput("lark-calendar", "lark-mail"), + globalOut: globalSkillsOutput("lark-calendar", "lark-mail"), + installErr: fmt.Errorf("incremental boom"), + installAllErr: fmt.Errorf("full install boom"), + } + + result := SyncSkills(SyncOptions{Version: "1.0.33", Runner: runner, Now: time.Now}) + if result.Action != "fallback_failed" { + t.Fatalf("SyncSkills() action = %q, want fallback_failed", result.Action) + } + if result.Err == nil { + t.Fatalf("SyncSkills() err = nil, want error") + } + if !strings.Contains(result.Detail, "incremental boom") { + t.Fatalf("SyncSkills() detail = %q, want incremental error text", result.Detail) + } + if !strings.Contains(result.Err.Error(), "full skills install failed") { + t.Fatalf("SyncSkills() err = %v, want full install failure", result.Err) + } +} + +func TestSyncSkills_NilRunnerFails(t *testing.T) { + result := SyncSkills(SyncOptions{Version: "1.0.33", Now: time.Now}) + if result.Err == nil || !strings.Contains(result.Err.Error(), "skills runner is nil") { + t.Fatalf("SyncSkills() err = %v, want nil runner failure", result.Err) + } +} + +func TestSyncSkills_ParseEmptyWithNonEmptyStdoutFallsBack(t *testing.T) { + dir := t.TempDir() + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", dir) + runner := &fakeSkillsRunner{ + officialOut: "Some unrecognized output format\n", + installAllErr: nil, + } + + result := SyncSkills(SyncOptions{Version: "1.0.33", Runner: runner, Now: time.Now}) + if result.Action != "fallback_synced" { + t.Fatalf("SyncSkills() action = %q, want fallback_synced", result.Action) + } + if runner.installedAll != 1 { + t.Fatalf("installedAll = %d, want 1", runner.installedAll) + } +} + +func TestSyncSkills_ParseEmptyWithNonEmptyStdoutAndFullInstallFails(t *testing.T) { + dir := t.TempDir() + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", dir) + runner := &fakeSkillsRunner{ + officialOut: "Some unrecognized output format\n", + installAllErr: fmt.Errorf("full install failed"), + } + + result := SyncSkills(SyncOptions{Version: "1.0.33", Runner: runner, Now: time.Now}) + if result.Action != "fallback_failed" { + t.Fatalf("SyncSkills() action = %q, want fallback_failed", result.Action) + } + if result.Err == nil { + t.Fatalf("SyncSkills() err = nil, want error") + } +} + +func assertStrings(t *testing.T, got, want []string) { + t.Helper() + if !reflect.DeepEqual(got, want) { + t.Fatalf("got %#v, want %#v", got, want) + } +} From ead4f9769a0827cfb4175a3e3fac40609a427d62 Mon Sep 17 00:00:00 2001 From: "zhangheng.023" Date: Fri, 22 May 2026 19:51:26 +0800 Subject: [PATCH 2/2] fix: handle skills sync parse fallbacks --- cmd/update/update.go | 4 +++- cmd/update/update_test.go | 19 +++++++++++++++++++ internal/skillscheck/sync.go | 3 +++ internal/skillscheck/sync_test.go | 21 +++++++++++++++++++++ 4 files changed, 46 insertions(+), 1 deletion(-) diff --git a/cmd/update/update.go b/cmd/update/update.go index ea5dc2253..6b8ce5091 100644 --- a/cmd/update/update.go +++ b/cmd/update/update.go @@ -40,7 +40,9 @@ func isWindows() bool { return currentOS == osWindows } // Strips a leading "v" so versions written from Makefile (git describe → // "v1.0.0") and npm (no prefix → "1.0.0") compare equal. func normalizeVersion(s string) string { - return strings.TrimPrefix(strings.TrimSpace(s), "v") + s = strings.TrimSpace(s) + s = strings.TrimPrefix(s, "v") + return strings.TrimPrefix(s, "V") } func releaseURL(version string) string { diff --git a/cmd/update/update_test.go b/cmd/update/update_test.go index 67ae5891e..5cfe52477 100644 --- a/cmd/update/update_test.go +++ b/cmd/update/update_test.go @@ -69,6 +69,25 @@ func successfulSkillsCommand() func(args ...string) *selfupdate.NpmResult { } } +func TestNormalizeVersion(t *testing.T) { + tests := []struct { + input string + want string + }{ + {input: "1.2.3", want: "1.2.3"}, + {input: "v1.2.3", want: "1.2.3"}, + {input: "V1.2.3", want: "1.2.3"}, + {input: " v1.2.3 ", want: "1.2.3"}, + } + for _, tt := range tests { + t.Run(tt.input, func(t *testing.T) { + if got := normalizeVersion(tt.input); got != tt.want { + t.Fatalf("normalizeVersion(%q) = %q, want %q", tt.input, got, tt.want) + } + }) + } +} + func TestUpdateAlreadyUpToDate_JSON(t *testing.T) { f, stdout, _ := newTestFactory(t) diff --git a/internal/skillscheck/sync.go b/internal/skillscheck/sync.go index 29744febe..c0090c4c2 100644 --- a/internal/skillscheck/sync.go +++ b/internal/skillscheck/sync.go @@ -254,6 +254,9 @@ func SyncSkills(opts SyncOptions) *SyncResult { localResult := opts.Runner.ListGlobalSkills() if localResult != nil && localResult.Err == nil { local = ParseSkillsList(localResult.Stdout.String()) + if len(local) == 0 && strings.TrimSpace(localResult.Stdout.String()) != "" { + return fallbackFullInstall(opts, "global skills list parsed as empty despite non-empty stdout") + } } // --- Step 3: Read previous state --- diff --git a/internal/skillscheck/sync_test.go b/internal/skillscheck/sync_test.go index 9654845d7..b2a19b93f 100644 --- a/internal/skillscheck/sync_test.go +++ b/internal/skillscheck/sync_test.go @@ -272,6 +272,27 @@ func TestSyncSkills_GlobalListFailureDegradesToColdStart(t *testing.T) { assertStrings(t, result.SkippedDeleted, []string{}) } +func TestSyncSkills_ParseEmptyGlobalListWithNonEmptyStdoutFallsBack(t *testing.T) { + dir := t.TempDir() + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", dir) + runner := &fakeSkillsRunner{ + officialOut: officialSkillsOutput("lark-calendar", "lark-mail"), + globalOut: "Some unrecognized output format\n", + installAllErr: nil, + } + + result := SyncSkills(SyncOptions{Version: "1.0.33", Runner: runner, Now: time.Now}) + if result.Action != "fallback_synced" { + t.Fatalf("SyncSkills() action = %q, want fallback_synced", result.Action) + } + if runner.installedAll != 1 { + t.Fatalf("installedAll = %d, want 1", runner.installedAll) + } + if len(runner.installed) != 0 { + t.Fatalf("installed = %#v, want no incremental installs", runner.installed) + } +} + func TestSyncSkills_InstallFailureFallsBackToFullInstall(t *testing.T) { dir := t.TempDir() t.Setenv("LARKSUITE_CLI_CONFIG_DIR", dir)