diff --git a/cmd/root_integration_test.go b/cmd/root_integration_test.go index a8919d1c..794cb07c 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 c9035cd5..ea5dc225 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) } } -// emitSkillsTextHints prints human-readable feedback about the skills -// sync result for non-JSON output. -func emitSkillsTextHints(io *cmdutil.IOStreams, r *selfupdate.NpmResult) { +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 +} + +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 250aa83d..94c38723 100644 --- a/cmd/update/update_test.go +++ b/cmd/update/update_test.go @@ -8,8 +8,6 @@ import ( "encoding/json" "errors" "fmt" - "os" - "path/filepath" "strings" "testing" @@ -28,7 +26,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 +38,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("lark-calendar\nlark-mail\n") + case "-y skills ls -g": + r.Stdout.WriteString("lark-calendar\ncustom-skill\n") + default: + } + return r + } +} + func TestUpdateAlreadyUpToDate_JSON(t *testing.T) { f, stdout, _ := newTestFactory(t) @@ -168,9 +177,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 +193,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 +222,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 +235,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 +251,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 +327,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 +343,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 +454,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 +652,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 +671,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 +752,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 +786,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 +812,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 +838,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 +861,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) } - 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 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) } - stamp, _ := skillscheck.ReadStamp() - if stamp != "1.0.20" { - t.Errorf("stamp = %q, want \"1.0.20\" (failure must not overwrite)", 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 = %q, want \"1.0.20\" (failure must not overwrite)", state.Version) } } @@ -973,8 +969,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 +982,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 +995,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 +1026,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 +1039,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 +1074,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 +1087,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 +1123,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 +1136,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 +1150,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 +1172,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 +1185,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,38 +1215,26 @@ 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 := 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) } - 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) - } - 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()) } diff --git a/internal/selfupdate/updater.go b/internal/selfupdate/updater.go index d9cb5ab9..365f84ab 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,46 @@ 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(name string) *NpmResult { + r := u.runSkillsInstall("https://open.feishu.cn", name) + if r.Err != nil { + r = u.runSkillsInstall("larksuite/cli", name) + } + 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, name string) *NpmResult { + return u.runSkillsCommand("-y", "skills", "add", source, "-s", name, "-g", "-y") +} + +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 +215,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 f13c80b6..b2da83f5 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", "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 429117a1..029a4d01 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. + version, ok := ReadSyncedVersion() + if !ok { 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. - 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 64525bc5..2674d542 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 b1f97221..c1425fbb 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 052e331c..00000000 --- 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 8e60dfbb..00000000 --- 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 00000000..abb2e2a6 --- /dev/null +++ b/internal/skillscheck/state.go @@ -0,0 +1,90 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package skillscheck + +import ( + "encoding/json" + "errors" + "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" + stateSchemaVersion = 1 +) + +type SkillsState struct { + SchemaVersion int `json:"schema_version"` + Version string `json:"version"` + OfficialSkills []string `json:"official_skills"` + UpdatedSkills []string `json:"updated_skills"` + AddedSkills []string `json:"added_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 state SkillsState + if json.Unmarshal(data, &state) != nil { + state = SkillsState{} + } + if state.SchemaVersion != stateSchemaVersion { + return nil, false, nil + } + return &state, true, nil +} + +func WriteState(state SkillsState) error { + state.SchemaVersion = stateSchemaVersion + 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.AddedSkills == nil { + s.AddedSkills = []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 00000000..77eab85d --- /dev/null +++ b/internal/skillscheck/state_test.go @@ -0,0 +1,153 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package skillscheck + +import ( + "encoding/json" + "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{ + SchemaVersion: 1, + Version: "1.2.3", + OfficialSkills: []string{"lark-doc", "lark-im"}, + UpdatedSkills: []string{"lark-doc"}, + AddedSkills: []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_CorruptOrUnknownSchemaUnreadable(t *testing.T) { + tests := []struct { + name string + data []byte + }{ + {name: "corrupt json", data: []byte(`{"schema_version":`)}, + {name: "unknown schema", data: []byte(`{"schema_version":2,"version":"1.2.3"}`)}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + dir := t.TempDir() + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", dir) + if err := os.WriteFile(filepath.Join(dir, stateFile), tt.data, 0o644); err != nil { + t.Fatal(err) + } + + state, ok, err := ReadState() + if err != nil { + t.Fatalf("ReadState() err = %v, want nil", 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.SchemaVersion != 1 { + t.Fatalf("schema_version = %d, want 1", got.SchemaVersion) + } + 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.AddedSkills == 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 00000000..068707bd --- /dev/null +++ b/internal/skillscheck/sync.go @@ -0,0 +1,265 @@ +// 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]+)?$`) + +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 ParseSkillsList(text string) []string { + seen := map[string]bool{} + for _, line := range strings.Split(text, "\n") { + token := strings.TrimSpace(line) + token = strings.TrimPrefix(token, "-") + token = strings.TrimSpace(token) + if token == "" || strings.Contains(token, " ") || strings.HasSuffix(token, ":") { + continue + } + if !skillNamePattern.MatchString(token) { + continue + } + if at := strings.Index(token, "@"); at > 0 { + token = token[:at] + } + seen[token] = 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) + localOfficial := intersection(input.LocalSkills, officialSet) + + previousOfficial := []string{} + if input.StateReadable && input.PreviousState != nil { + previousOfficial = input.PreviousState.OfficialSkills + } + previousSet := toSet(previousOfficial) + + newOfficial := []string{} + for _, skill := range official { + if !previousSet[skill] { + newOfficial = append(newOfficial, skill) + } + } + + updateSet := toSet(localOfficial) + for _, skill := range newOfficial { + 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(newOfficial), + SkippedDeleted: skipped, + } +} + +type SkillsRunner interface { + ListOfficialSkills() *selfupdate.NpmResult + ListGlobalSkills() *selfupdate.NpmResult + InstallSkill(name string) *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")} + } + + officialResult := opts.Runner.ListOfficialSkills() + if officialResult == nil { + return &SyncResult{Action: "failed", Err: fmt.Errorf("failed to list official skills: empty result")} + } + if officialResult.Err != nil { + return &SyncResult{Action: "failed", Err: fmt.Errorf("failed to list official skills: %w", officialResult.Err), Detail: resultDetail(officialResult)} + } + official := ParseSkillsList(officialResult.Stdout.String()) + + localResult := opts.Runner.ListGlobalSkills() + if localResult == nil { + return &SyncResult{Action: "failed", Official: official, Err: fmt.Errorf("failed to list installed skills: empty result")} + } + if localResult.Err != nil { + return &SyncResult{Action: "failed", Official: official, Err: fmt.Errorf("failed to list installed skills: %w", localResult.Err), Detail: resultDetail(localResult)} + } + local := ParseSkillsList(localResult.Stdout.String()) + + previous, readable, err := ReadState() + if err != nil { + return &SyncResult{Action: "failed", Official: official, Err: fmt.Errorf("failed to read skills state: %w", err)} + } + + 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, + } + + failed := []string{} + var details []string + for _, skill := range plan.ToUpdate { + installResult := opts.Runner.InstallSkill(skill) + if installResult == nil { + failed = append(failed, skill) + details = append(details, skill+": empty result") + continue + } + if installResult.Err != nil { + failed = append(failed, skill) + details = append(details, skill+": "+resultDetail(installResult)) + } + } + if len(failed) > 0 { + result.Action = "failed" + result.Failed = failed + result.Err = fmt.Errorf("%d skill(s) failed", len(failed)) + result.Detail = strings.Join(details, "\n") + return result + } + + state := SkillsState{ + Version: opts.Version, + OfficialSkills: plan.OfficialSkills, + UpdatedSkills: plan.ToUpdate, + AddedSkills: 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 +} + +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 +} + +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 00000000..4b7de39c --- /dev/null +++ b/internal/skillscheck/sync_test.go @@ -0,0 +1,222 @@ +// 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 TestParseSkillsList(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) + want := []string{"custom-skill", "lark-base", "lark-calendar", "lark-cli-harness:dev", "lark-im", "lark-mail"} + if !reflect.DeepEqual(got, want) { + t.Fatalf("ParseSkillsList() = %#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 map[string]error + installed []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(name string) *selfupdate.NpmResult { + f.installed = append(f.installed, name) + r := &selfupdate.NpmResult{} + if f.installErr != nil { + r.Err = f.installErr[name] + } + 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: "lark-calendar\nlark-mail\nlark-new\n", + globalOut: "lark-calendar\nlark-custom\n", + } + 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, []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.AddedSkills, []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_ListFailureDoesNotInstallOrWriteState(t *testing.T) { + dir := t.TempDir() + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", dir) + runner := &fakeSkillsRunner{officialErr: fmt.Errorf("list failed")} + + result := SyncSkills(SyncOptions{Version: "1.0.33", Runner: runner, Now: time.Now}) + if result.Err == nil || !strings.Contains(result.Err.Error(), "failed to list official skills") { + t.Fatalf("SyncSkills() err = %v, want official list failure", result.Err) + } + if len(runner.installed) != 0 { + t.Fatalf("installed = %#v, want none", runner.installed) + } + if _, readable, err := ReadState(); err != nil || readable { + t.Fatalf("ReadState() = (_, %v, %v), want unreadable missing state", readable, err) + } +} + +func TestSyncSkills_GlobalListFailureDoesNotInstallOrWriteState(t *testing.T) { + dir := t.TempDir() + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", dir) + runner := &fakeSkillsRunner{ + officialOut: "lark-calendar\nlark-mail\n", + globalErr: fmt.Errorf("global list failed"), + } + + result := SyncSkills(SyncOptions{Version: "1.0.33", Runner: runner, Now: time.Now}) + if result.Err == nil || !strings.Contains(result.Err.Error(), "failed to list installed skills") { + t.Fatalf("SyncSkills() err = %v, want installed list failure", result.Err) + } + if len(runner.installed) != 0 { + t.Fatalf("installed = %#v, want none", runner.installed) + } + if _, readable, err := ReadState(); err != nil || readable { + t.Fatalf("ReadState() = (_, %v, %v), want unreadable missing state", readable, err) + } +} + +func TestSyncSkills_InstallFailureContinuesAndDoesNotWriteState(t *testing.T) { + dir := t.TempDir() + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", dir) + runner := &fakeSkillsRunner{ + officialOut: "lark-calendar\nlark-mail\n", + globalOut: "lark-calendar\nlark-mail\n", + installErr: map[string]error{"lark-calendar": fmt.Errorf("boom")}, + } + + result := SyncSkills(SyncOptions{Version: "1.0.33", Runner: runner, Now: time.Now}) + if result.Err == nil || !strings.Contains(result.Err.Error(), "1 skill(s) failed") { + t.Fatalf("SyncSkills() err = %v, want install failure", result.Err) + } + assertStrings(t, runner.installed, []string{"lark-calendar", "lark-mail"}) + assertStrings(t, result.Failed, []string{"lark-calendar"}) + if !strings.Contains(result.Detail, "boom") { + t.Fatalf("SyncSkills() detail = %q, want install error text", result.Detail) + } + if _, readable, err := ReadState(); err != nil || readable { + t.Fatalf("ReadState() = (_, %v, %v), want no success state", readable, 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 assertStrings(t *testing.T, got, want []string) { + t.Helper() + if !reflect.DeepEqual(got, want) { + t.Fatalf("got %#v, want %#v", got, want) + } +} diff --git a/scripts/install-wizard.js b/scripts/install-wizard.js index 4bc76f5d..91fa2271 100644 --- a/scripts/install-wizard.js +++ b/scripts/install-wizard.js @@ -10,6 +10,8 @@ const p = require("@clack/prompts"); const PKG = "@larksuite/cli"; const SKILLS_REPO = "https://open.feishu.cn"; const SKILLS_REPO_FALLBACK = "larksuite/cli"; +const CONFIG_DIR = process.env.LARKSUITE_CLI_CONFIG_DIR || path.join(process.env.HOME || process.env.USERPROFILE || "", ".lark-cli"); +const SKILLS_STATE_FILE = path.join(CONFIG_DIR, "skills-state.json"); const isWindows = process.platform === "win32"; // --------------------------------------------------------------------------- @@ -236,7 +238,7 @@ async function stepInstallGlobally(msg) { if (installedVer && !needsUpgrade) { p.log.info(fmt(msg.step1Skip, installedVer)); - return false; + return installedVer; } const s = p.spinner(); @@ -248,41 +250,111 @@ async function stepInstallGlobally(msg) { try { await runSilentAsync("npm", ["install", "-g", PKG], { timeout: 120000 }); s.stop(needsUpgrade ? fmt(msg.step1Upgraded, latestVer) : msg.step1Done); - return needsUpgrade; + return latestVer || getGloballyInstalledVersion() || installedVer || null; } catch (_) { s.stop(fmt(msg.step1Fail, PKG)); process.exit(1); } } -async function skillsAlreadyInstalled() { +function parseSkillsList(text) { + const seen = new Set(); + for (const rawLine of text.split("\n")) { + let token = rawLine.trim(); + if (token.startsWith("-")) token = token.slice(1).trim(); + if (!token || token.includes(" ") || token.endsWith(":")) continue; + if (!/^[A-Za-z0-9][A-Za-z0-9_:-]*(?:@\S+)?$/.test(token)) continue; + const at = token.indexOf("@"); + if (at > 0) token = token.slice(0, at); + seen.add(token); + } + return [...seen].sort(); +} + +function readSkillsState() { try { - const out = await runSilentAsync("npx", ["-y", "skills", "ls", "-g"], { - timeout: 120000, - }); - return /^lark-/m.test(out.toString()); + const state = JSON.parse(fs.readFileSync(SKILLS_STATE_FILE, "utf8")); + if (state.schema_version !== 1 || !Array.isArray(state.official_skills)) return null; + return state; } catch (_) { - return false; + return null; } } -async function stepInstallSkills(msg) { +function writeSkillsState(version, official, updated, added, skipped) { + if (!CONFIG_DIR) return; + fs.mkdirSync(CONFIG_DIR, { recursive: true, mode: 0o700 }); + fs.writeFileSync(SKILLS_STATE_FILE, JSON.stringify({ + schema_version: 1, + version, + official_skills: official, + updated_skills: updated, + added_skills: added, + skipped_deleted_skills: skipped, + updated_at: new Date().toISOString(), + }, null, 2) + "\n"); +} + +async function listOfficialSkills() { + try { + return parseSkillsList(await runSilentAsync("npx", ["-y", "skills", "add", SKILLS_REPO, "--list"], { timeout: 120000 })); + } catch (_) { + return parseSkillsList(await runSilentAsync("npx", ["-y", "skills", "add", SKILLS_REPO_FALLBACK, "--list"], { timeout: 120000 })); + } +} + +async function listGlobalSkills() { + return parseSkillsList(await runSilentAsync("npx", ["-y", "skills", "ls", "-g"], { timeout: 120000 })); +} + +function planSkillsSync(version, official, local, previousState) { + const officialSet = new Set(official); + const previousSet = new Set(previousState ? previousState.official_skills : []); + const localOfficial = local.filter((skill) => officialSet.has(skill)); + const added = official.filter((skill) => !previousSet.has(skill)); + const updateSet = new Set([...localOfficial, ...added]); + const updated = official.filter((skill) => updateSet.has(skill)); + return { + version, + official, + updated, + added, + skipped: official.filter((skill) => !updateSet.has(skill)), + }; +} + +async function installSkill(name) { + try { + await runSilentAsync("npx", ["-y", "skills", "add", SKILLS_REPO, "-s", name, "-g", "-y"], { timeout: 120000 }); + } catch (_) { + await runSilentAsync("npx", ["-y", "skills", "add", SKILLS_REPO_FALLBACK, "-s", name, "-g", "-y"], { timeout: 120000 }); + } +} + +async function stepInstallSkills(msg, cliVersion) { const s = p.spinner(); s.start(msg.step2Spinner); try { - if (await skillsAlreadyInstalled()) { + const official = await listOfficialSkills(); + const local = await listGlobalSkills(); + const plan = planSkillsSync(cliVersion || "unknown", official, local, readSkillsState()); + if (plan.updated.length === 0) { + writeSkillsState(plan.version, plan.official, plan.updated, plan.added, plan.skipped); s.stop(msg.step2Skip); return; } - try { - await runSilentAsync("npx", ["-y", "skills", "add", SKILLS_REPO, "-y", "-g"], { - timeout: 120000, - }); - } catch (_) { - await runSilentAsync("npx", ["-y", "skills", "add", SKILLS_REPO_FALLBACK, "-y", "-g"], { - timeout: 120000, - }); + const failed = []; + for (const skill of plan.updated) { + try { + await installSkill(skill); + } catch (_) { + failed.push(skill); + } } + if (failed.length > 0) { + throw new Error(`${failed.length} skill(s) failed: ${failed.join(", ")}`); + } + writeSkillsState(plan.version, plan.official, plan.updated, plan.added, plan.skipped); s.stop(msg.step2Done); } catch (_) { s.stop(fmt(msg.step2Fail, SKILLS_REPO_FALLBACK)); @@ -361,15 +433,15 @@ async function main() { if (isInteractive) { p.intro(msg.setup); - await stepInstallGlobally(msg); - await stepInstallSkills(msg); + const cliVersion = await stepInstallGlobally(msg); + await stepInstallSkills(msg, cliVersion); await stepConfigInit(msg, lang); await stepAuthLogin(msg); p.outro(msg.done); } else { console.log(msg.setup); - await stepInstallGlobally(msg); - await stepInstallSkills(msg); + const cliVersion = await stepInstallGlobally(msg); + await stepInstallSkills(msg, cliVersion); console.log(msg.nonTtyHint); } }