From 5ec0f33b694ff3987c0fb54b849d812b6429ac33 Mon Sep 17 00:00:00 2001 From: leave330 Date: Wed, 1 Jul 2026 16:38:23 +0800 Subject: [PATCH 1/7] fix: detect pnpm global install in self-update --- internal/selfupdate/updater.go | 67 ++++++++++++++++++++--------- internal/selfupdate/updater_test.go | 60 ++++++++++++++++++++++++++ 2 files changed, 107 insertions(+), 20 deletions(-) diff --git a/internal/selfupdate/updater.go b/internal/selfupdate/updater.go index 9304f7c3e1..2a9e0e87c8 100644 --- a/internal/selfupdate/updater.go +++ b/internal/selfupdate/updater.go @@ -32,6 +32,7 @@ type InstallMethod int const ( InstallNpm InstallMethod = iota + InstallPnpm InstallManual ) @@ -53,22 +54,32 @@ var ( // DetectResult holds installation detection results. type DetectResult struct { - Method InstallMethod - ResolvedPath string - NpmAvailable bool + Method InstallMethod + ResolvedPath string + NpmAvailable bool + PnpmAvailable bool } // CanAutoUpdate returns true if the CLI can update itself automatically. func (d DetectResult) CanAutoUpdate() bool { - return d.Method == InstallNpm && d.NpmAvailable + switch d.Method { + case InstallNpm: + return d.NpmAvailable + case InstallPnpm: + return d.PnpmAvailable + } + return false } // ManualReason returns a human-readable explanation of why auto-update is unavailable. func (d DetectResult) ManualReason() string { - if d.Method == InstallNpm && !d.NpmAvailable { + switch { + case d.Method == InstallNpm && !d.NpmAvailable: return "installed via npm, but npm is not available in PATH" + case d.Method == InstallPnpm && !d.PnpmAvailable: + return "installed via pnpm, but pnpm is not available in PATH" } - return "not installed via npm" + return "not installed via npm or pnpm" } // NpmResult holds the result of an npm install or skills update execution. @@ -106,8 +117,8 @@ type Updater struct { // New creates an Updater with default (real) behavior. func New() *Updater { return &Updater{} } -// DetectInstallMethod determines how the CLI was installed and whether -// npm is available for auto-update. +// DetectInstallMethod determines how the CLI was installed and whether the +// owning package manager is available for auto-update. func (u *Updater) DetectInstallMethod() DetectResult { if u.DetectOverride != nil { return u.DetectOverride() @@ -120,24 +131,40 @@ func (u *Updater) DetectInstallMethod() DetectResult { if err != nil { return DetectResult{Method: InstallManual, ResolvedPath: exe} } + _, npmErr := exec.LookPath("npm") + _, pnpmErr := exec.LookPath("pnpm") + return detectFromResolved(resolved, npmErr == nil, pnpmErr == nil) +} +// detectFromResolved classifies the resolved binary path into an install +// method and records package-manager availability. Split out from +// DetectInstallMethod so the classification is unit-testable without touching +// the filesystem or PATH. +func detectFromResolved(resolved string, npmOnPath, pnpmOnPath bool) DetectResult { method := InstallManual if strings.Contains(resolved, "node_modules") { - method = InstallNpm - } - - npmAvailable := false - if method == InstallNpm { - if _, err := exec.LookPath("npm"); err == nil { - npmAvailable = true + if containsPnpmMarker(resolved) { + method = InstallPnpm + } else { + method = InstallNpm } } - - return DetectResult{ - Method: method, - ResolvedPath: resolved, - NpmAvailable: npmAvailable, + d := DetectResult{Method: method, ResolvedPath: resolved} + switch method { + case InstallNpm: + d.NpmAvailable = npmOnPath + case InstallPnpm: + d.PnpmAvailable = pnpmOnPath } + return d +} + +// containsPnpmMarker reports whether the path contains pnpm's virtual-store +// directory segment (".pnpm"), which distinguishes a pnpm global install from +// an npm one. Both POSIX ("/.pnpm/") and Windows ("\.pnpm\") separators are +// checked so the classification is OS-independent and unit-testable anywhere. +func containsPnpmMarker(p string) bool { + return strings.Contains(p, "/.pnpm/") || strings.Contains(p, `\.pnpm\`) } // RunNpmInstall executes npm install -g @larksuite/cli@. diff --git a/internal/selfupdate/updater_test.go b/internal/selfupdate/updater_test.go index 65426eeb12..b5f154df67 100644 --- a/internal/selfupdate/updater_test.go +++ b/internal/selfupdate/updater_test.go @@ -371,3 +371,63 @@ func TestListOfficialSkillsFallsBack(t *testing.T) { t.Fatalf("fallback call = %q, want larksuite/cli --list", called[1]) } } + +func TestContainsPnpmMarker(t *testing.T) { + cases := []struct { + path string + want bool + }{ + {"/Users/x/Library/pnpm/global/5/node_modules/.pnpm/@larksuite+cli@1.0.44/node_modules/@larksuite/cli/bin/lark-cli", true}, + {"/usr/local/lib/node_modules/@larksuite/cli/bin/lark-cli", false}, + {"/opt/homebrew/.pnpmfoo/node_modules/@larksuite/cli/bin/lark-cli", false}, + {`C:\Users\x\AppData\Local\pnpm\global\5\node_modules\.pnpm\@larksuite+cli@1.0.44\node_modules\@larksuite\cli\bin\lark-cli.exe`, true}, + {"/usr/local/bin/lark-cli", false}, + } + for _, c := range cases { + if got := containsPnpmMarker(c.path); got != c.want { + t.Errorf("containsPnpmMarker(%q) = %v, want %v", c.path, got, c.want) + } + } +} + +func TestDetectInstallMethod_Pnpm(t *testing.T) { + u := &Updater{DetectOverride: nil} + u.DetectOverride = func() DetectResult { + // Exercise the real classification by feeding a resolved path via a small shim. + return detectFromResolved("/x/node_modules/.pnpm/@larksuite+cli@1.0.44/node_modules/@larksuite/cli/bin/lark-cli", true, true) + } + got := u.DetectInstallMethod() + if got.Method != InstallPnpm { + t.Errorf("Method = %v, want InstallPnpm", got.Method) + } + if !got.PnpmAvailable { + t.Errorf("PnpmAvailable = false, want true") + } +} + +func TestDetectInstallMethod_NpmVsManual(t *testing.T) { + if m := detectFromResolved("/usr/local/lib/node_modules/@larksuite/cli/bin/lark-cli", true, false).Method; m != InstallNpm { + t.Errorf("npm path Method = %v, want InstallNpm", m) + } + if m := detectFromResolved("/usr/local/bin/lark-cli", false, false).Method; m != InstallManual { + t.Errorf("manual path Method = %v, want InstallManual", m) + } +} + +func TestCanAutoUpdate_Pnpm(t *testing.T) { + if !(DetectResult{Method: InstallPnpm, PnpmAvailable: true}).CanAutoUpdate() { + t.Error("pnpm available should CanAutoUpdate") + } + if (DetectResult{Method: InstallPnpm, PnpmAvailable: false}).CanAutoUpdate() { + t.Error("pnpm unavailable should not CanAutoUpdate") + } +} + +func TestManualReason_Pnpm(t *testing.T) { + if got := (DetectResult{Method: InstallPnpm, NpmAvailable: false, PnpmAvailable: false}).ManualReason(); got != "installed via pnpm, but pnpm is not available in PATH" { + t.Errorf("pnpm reason = %q", got) + } + if got := (DetectResult{Method: InstallManual}).ManualReason(); got != "not installed via npm or pnpm" { + t.Errorf("manual reason = %q", got) + } +} From f204f2ea5419c78c0dbd21081197dc492ea847be Mon Sep 17 00:00:00 2001 From: leave330 Date: Wed, 1 Jul 2026 16:44:47 +0800 Subject: [PATCH 2/7] fix: add pnpm install runner for self-update --- internal/selfupdate/updater.go | 24 ++++++++++++++++++++++++ internal/selfupdate/updater_test.go | 23 +++++++++++++++++++++++ 2 files changed, 47 insertions(+) diff --git a/internal/selfupdate/updater.go b/internal/selfupdate/updater.go index 2a9e0e87c8..4c5367332b 100644 --- a/internal/selfupdate/updater.go +++ b/internal/selfupdate/updater.go @@ -103,6 +103,7 @@ func (r *NpmResult) CombinedOutput() string { type Updater struct { DetectOverride func() DetectResult NpmInstallOverride func(version string) *NpmResult + PnpmInstallOverride func(version string) *NpmResult SkillsIndexFetchOverride func() *NpmResult SkillsCommandOverride func(args ...string) *NpmResult VerifyOverride func(expectedVersion string) error @@ -190,6 +191,29 @@ func (u *Updater) RunNpmInstall(version string) *NpmResult { return r } +// RunPnpmInstall executes pnpm add -g @larksuite/cli@. +func (u *Updater) RunPnpmInstall(version string) *NpmResult { + if u.PnpmInstallOverride != nil { + return u.PnpmInstallOverride(version) + } + r := &NpmResult{} + pnpmPath, err := exec.LookPath("pnpm") + if err != nil { + r.Err = fmt.Errorf("pnpm not found in PATH: %w", err) + return r + } + ctx, cancel := context.WithTimeout(context.Background(), npmInstallTimeout) + defer cancel() + cmd := exec.CommandContext(ctx, pnpmPath, "add", "-g", NpmPackage+"@"+version) + cmd.Stdout = &r.Stdout + cmd.Stderr = &r.Stderr + r.Err = cmd.Run() + if ctx.Err() == context.DeadlineExceeded { + r.Err = fmt.Errorf("pnpm install timed out after %s", npmInstallTimeout) + } + return r +} + func (u *Updater) ListOfficialSkillsIndex() *NpmResult { if u.SkillsIndexFetchOverride != nil { return u.SkillsIndexFetchOverride() diff --git a/internal/selfupdate/updater_test.go b/internal/selfupdate/updater_test.go index b5f154df67..e26abf64b2 100644 --- a/internal/selfupdate/updater_test.go +++ b/internal/selfupdate/updater_test.go @@ -431,3 +431,26 @@ func TestManualReason_Pnpm(t *testing.T) { t.Errorf("manual reason = %q", got) } } + +func TestRunPnpmInstall_Override(t *testing.T) { + u := &Updater{PnpmInstallOverride: func(version string) *NpmResult { + r := &NpmResult{} + r.Stdout.WriteString("added @larksuite/cli@" + version) + return r + }} + got := u.RunPnpmInstall("2.0.0") + if got.Err != nil { + t.Fatalf("unexpected err: %v", got.Err) + } + if !strings.Contains(got.CombinedOutput(), "2.0.0") { + t.Errorf("output = %q, want version echoed", got.CombinedOutput()) + } +} + +func TestRunPnpmInstall_Error(t *testing.T) { + wantErr := errors.New("boom") + u := &Updater{PnpmInstallOverride: func(string) *NpmResult { return &NpmResult{Err: wantErr} }} + if got := u.RunPnpmInstall("2.0.0"); !errors.Is(got.Err, wantErr) { + t.Errorf("err = %v, want %v", got.Err, wantErr) + } +} From 3a8157a9a5025871b8738e0326469b03e2be75f7 Mon Sep 17 00:00:00 2001 From: leave330 Date: Wed, 1 Jul 2026 16:52:15 +0800 Subject: [PATCH 3/7] fix: route pnpm installs through pnpm add -g on update --- cmd/update/update.go | 24 ++++++-- cmd/update/update_test.go | 119 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 137 insertions(+), 6 deletions(-) diff --git a/cmd/update/update.go b/cmd/update/update.go index 43a047b2ed..392abd5afb 100644 --- a/cmd/update/update.go +++ b/cmd/update/update.go @@ -102,7 +102,8 @@ func NewCmdUpdate(f *cmdutil.Factory) *cobra.Command { Long: `Update lark-cli to the latest version. Detects the installation method automatically: - - npm install: runs npm install -g @larksuite/cli@ + - npm install: runs npm install -g @larksuite/cli@ + - pnpm install: runs pnpm add -g @larksuite/cli@ - manual/other: shows GitHub Releases download URL Use --json for structured output (for AI agents and scripts). @@ -164,7 +165,7 @@ func updateRun(opts *UpdateOptions) error { if !detect.CanAutoUpdate() { return doManualUpdate(opts, io, cur, latest, detect, updater) } - return doNpmUpdate(opts, io, cur, latest, updater) + return doAutoUpdate(opts, io, cur, latest, detect, updater) } // --- Output helpers --- @@ -226,12 +227,23 @@ func doManualUpdate(opts *UpdateOptions, io *cmdutil.IOStreams, cur, latest stri fmt.Fprintf(io.ErrOut, "To update manually, download the latest release:\n") fmt.Fprintf(io.ErrOut, " Release: %s\n", releaseURL(latest)) fmt.Fprintf(io.ErrOut, " Changelog: %s\n", changelogURL()) - fmt.Fprintf(io.ErrOut, "\nOr install via npm (note: skills will not be synced):\n npm install -g %s@%s\n npx skills add larksuite/cli -y -g # sync skills separately\n", selfupdate.NpmPackage, latest) + if detect.Method == selfupdate.InstallPnpm { + fmt.Fprintf(io.ErrOut, "\nOr install via pnpm (note: skills will not be synced):\n pnpm add -g %s@%s\n npx skills add larksuite/cli -y -g # sync skills separately\n", selfupdate.NpmPackage, latest) + } else { + fmt.Fprintf(io.ErrOut, "\nOr install via npm (note: skills will not be synced):\n npm install -g %s@%s\n npx skills add larksuite/cli -y -g # sync skills separately\n", selfupdate.NpmPackage, latest) + } emitSkillsTextHints(io, skillsResult) return nil } -func doNpmUpdate(opts *UpdateOptions, io *cmdutil.IOStreams, cur, latest string, updater *selfupdate.Updater) error { +func doAutoUpdate(opts *UpdateOptions, io *cmdutil.IOStreams, cur, latest string, detect selfupdate.DetectResult, updater *selfupdate.Updater) error { + pm := "npm" + install := updater.RunNpmInstall + if detect.Method == selfupdate.InstallPnpm { + pm = "pnpm" + install = updater.RunPnpmInstall + } + restore, err := updater.PrepareSelfReplace() if err != nil { return reportError(opts, io, "update_error", @@ -239,10 +251,10 @@ func doNpmUpdate(opts *UpdateOptions, io *cmdutil.IOStreams, cur, latest string, } if !opts.JSON { - fmt.Fprintf(io.ErrOut, "Updating lark-cli %s %s %s via npm ...\n", cur, symArrow(), latest) + fmt.Fprintf(io.ErrOut, "Updating lark-cli %s %s %s via %s ...\n", cur, symArrow(), latest, pm) } - npmResult := updater.RunNpmInstall(latest) + npmResult := install(latest) if npmResult.Err != nil { restore() combined := npmResult.CombinedOutput() diff --git a/cmd/update/update_test.go b/cmd/update/update_test.go index faf2f7629f..49e604839f 100644 --- a/cmd/update/update_test.go +++ b/cmd/update/update_test.go @@ -57,6 +57,27 @@ func mockDetectAndNpm(t *testing.T, result selfupdate.DetectResult, npmFn func(s t.Cleanup(func() { newUpdater = origNew }) } +// mockDetectAndPnpm mirrors mockDetectAndNpm but wires the pnpm install path +// and fails the test if the npm install path is invoked. +func mockDetectAndPnpm(t *testing.T, result selfupdate.DetectResult, pnpmFn func(string) *selfupdate.NpmResult) { + t.Helper() + origNew := newUpdater + newUpdater = func() *selfupdate.Updater { + u := selfupdate.New() + u.DetectOverride = func() selfupdate.DetectResult { return result } + u.PnpmInstallOverride = pnpmFn + u.NpmInstallOverride = func(string) *selfupdate.NpmResult { + t.Errorf("npm install must not be called for a pnpm install") + return &selfupdate.NpmResult{} + } + u.VerifyOverride = func(string) error { return nil } + u.SkillsIndexFetchOverride = successfulSkillsIndexFetch() + u.SkillsCommandOverride = successfulSkillsCommand() + return u + } + t.Cleanup(func() { newUpdater = origNew }) +} + func successfulSkillsIndexFetch() func() *selfupdate.NpmResult { return func() *selfupdate.NpmResult { r := &selfupdate.NpmResult{} @@ -81,6 +102,104 @@ func successfulSkillsCommand() func(args ...string) *selfupdate.NpmResult { } } +func TestUpdatePnpm_JSON(t *testing.T) { + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) + f, stdout, _ := newTestFactory(t) + cmd := NewCmdUpdate(f) + cmd.SetArgs([]string{"--json"}) + origFetch := fetchLatest + fetchLatest = func() (string, error) { return "2.0.0", nil } + defer func() { fetchLatest = origFetch }() + origVersion := currentVersion + currentVersion = func() string { return "1.0.0" } + defer func() { currentVersion = origVersion }() + mockDetectAndPnpm(t, + selfupdate.DetectResult{Method: selfupdate.InstallPnpm, ResolvedPath: "/x/node_modules/.pnpm/@larksuite+cli@1.0.0/node_modules/@larksuite/cli/bin/lark-cli", PnpmAvailable: true}, + func(string) *selfupdate.NpmResult { return &selfupdate.NpmResult{} }, + ) + if err := cmd.Execute(); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if out := stdout.String(); !strings.Contains(out, `"action": "updated"`) { + t.Errorf("expected updated in output, got: %s", out) + } +} + +func TestUpdatePnpm_Human(t *testing.T) { + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) + f, _, stderr := newTestFactory(t) + cmd := NewCmdUpdate(f) + cmd.SetArgs([]string{}) + origFetch := fetchLatest + fetchLatest = func() (string, error) { return "2.0.0", nil } + defer func() { fetchLatest = origFetch }() + origVersion := currentVersion + currentVersion = func() string { return "1.0.0" } + defer func() { currentVersion = origVersion }() + mockDetectAndPnpm(t, + selfupdate.DetectResult{Method: selfupdate.InstallPnpm, ResolvedPath: "/x/node_modules/.pnpm/@larksuite+cli@1.0.0/node_modules/@larksuite/cli/bin/lark-cli", PnpmAvailable: true}, + func(string) *selfupdate.NpmResult { return &selfupdate.NpmResult{} }, + ) + if err := cmd.Execute(); err != nil { + t.Fatalf("unexpected error: %v", err) + } + out := stderr.String() + if !strings.Contains(out, "via pnpm") { + t.Errorf("expected 'via pnpm' in stderr, got: %s", out) + } + if !strings.Contains(out, "Successfully updated") { + t.Errorf("expected success message, got: %s", out) + } +} + +func TestUpdatePnpm_InstallError_JSON(t *testing.T) { + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) + f, stdout, _ := newTestFactory(t) + cmd := NewCmdUpdate(f) + cmd.SetArgs([]string{"--json"}) + origFetch := fetchLatest + fetchLatest = func() (string, error) { return "2.0.0", nil } + defer func() { fetchLatest = origFetch }() + origVersion := currentVersion + currentVersion = func() string { return "1.0.0" } + defer func() { currentVersion = origVersion }() + mockDetectAndPnpm(t, + selfupdate.DetectResult{Method: selfupdate.InstallPnpm, ResolvedPath: "/x/node_modules/.pnpm/@larksuite+cli@1.0.0/node_modules/@larksuite/cli/bin/lark-cli", PnpmAvailable: true}, + func(string) *selfupdate.NpmResult { return &selfupdate.NpmResult{Err: errors.New("pnpm boom")} }, + ) + err := cmd.Execute() + if err == nil { + t.Fatal("expected error exit") + } + if out := stdout.String(); !strings.Contains(out, `"ok": false`) || !strings.Contains(out, "update_error") { + t.Errorf("expected failure envelope, got: %s", out) + } +} + +func TestUpdatePnpm_Unavailable_ManualFallback(t *testing.T) { + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) + f, _, stderr := newTestFactory(t) + cmd := NewCmdUpdate(f) + cmd.SetArgs([]string{}) + origFetch := fetchLatest + fetchLatest = func() (string, error) { return "2.0.0", nil } + defer func() { fetchLatest = origFetch }() + origVersion := currentVersion + currentVersion = func() string { return "1.0.0" } + defer func() { currentVersion = origVersion }() + mockDetect(t, selfupdate.DetectResult{Method: selfupdate.InstallPnpm, ResolvedPath: "/x/node_modules/.pnpm/@larksuite+cli@1.0.0/node_modules/@larksuite/cli/bin/lark-cli", PnpmAvailable: false}) + if err := cmd.Execute(); err != nil { + t.Fatalf("unexpected error: %v", err) + } + out := stderr.String() + if !strings.Contains(out, "installed via pnpm, but pnpm is not available in PATH") { + t.Errorf("expected pnpm manual reason, got: %s", out) + } + if !strings.Contains(out, "pnpm add -g") { + t.Errorf("expected pnpm add -g hint, got: %s", out) + } +} + func TestNormalizeVersion(t *testing.T) { tests := []struct { input string From e3afad54fe771f01b0e9871c6e6720a56867d9ee Mon Sep 17 00:00:00 2001 From: leave330 Date: Wed, 1 Jul 2026 17:08:12 +0800 Subject: [PATCH 4/7] fix: report actual package manager in update failure message --- cmd/update/update.go | 2 +- cmd/update/update_test.go | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/cmd/update/update.go b/cmd/update/update.go index 392abd5afb..a6accb1a27 100644 --- a/cmd/update/update.go +++ b/cmd/update/update.go @@ -261,7 +261,7 @@ func doAutoUpdate(opts *UpdateOptions, io *cmdutil.IOStreams, cur, latest string if opts.JSON { output.PrintJson(io.Out, map[string]interface{}{ "ok": false, "error": map[string]interface{}{ - "type": "update_error", "message": fmt.Sprintf("npm install failed: %s", npmResult.Err), + "type": "update_error", "message": fmt.Sprintf("%s install failed: %s", pm, npmResult.Err), "detail": selfupdate.Truncate(combined, maxNpmOutput), "hint": permissionHint(combined), }, diff --git a/cmd/update/update_test.go b/cmd/update/update_test.go index 49e604839f..fd2f1bbf98 100644 --- a/cmd/update/update_test.go +++ b/cmd/update/update_test.go @@ -174,6 +174,9 @@ func TestUpdatePnpm_InstallError_JSON(t *testing.T) { if out := stdout.String(); !strings.Contains(out, `"ok": false`) || !strings.Contains(out, "update_error") { t.Errorf("expected failure envelope, got: %s", out) } + if out := stdout.String(); !strings.Contains(out, "pnpm install failed") { + t.Errorf("expected message to report pnpm as the package manager, got: %s", out) + } } func TestUpdatePnpm_Unavailable_ManualFallback(t *testing.T) { From 1099e4fc3536d0731bcd6d325da3cdefd99fdf34 Mon Sep 17 00:00:00 2001 From: leave330 Date: Wed, 1 Jul 2026 20:53:46 +0800 Subject: [PATCH 5/7] fix: cover pnpm global store layout and skills sync via pnpm dlx --- cmd/update/update.go | 2 +- internal/selfupdate/updater.go | 42 +++++++++++++++++++++----- internal/selfupdate/updater_test.go | 47 +++++++++++++++++++++++++++-- 3 files changed, 80 insertions(+), 11 deletions(-) diff --git a/cmd/update/update.go b/cmd/update/update.go index a6accb1a27..7621c88dd5 100644 --- a/cmd/update/update.go +++ b/cmd/update/update.go @@ -228,7 +228,7 @@ func doManualUpdate(opts *UpdateOptions, io *cmdutil.IOStreams, cur, latest stri fmt.Fprintf(io.ErrOut, " Release: %s\n", releaseURL(latest)) fmt.Fprintf(io.ErrOut, " Changelog: %s\n", changelogURL()) if detect.Method == selfupdate.InstallPnpm { - fmt.Fprintf(io.ErrOut, "\nOr install via pnpm (note: skills will not be synced):\n pnpm add -g %s@%s\n npx skills add larksuite/cli -y -g # sync skills separately\n", selfupdate.NpmPackage, latest) + fmt.Fprintf(io.ErrOut, "\nOr install via pnpm (note: skills will not be synced):\n pnpm add -g %s@%s\n pnpm dlx skills add larksuite/cli -y -g # sync skills separately\n", selfupdate.NpmPackage, latest) } else { fmt.Fprintf(io.ErrOut, "\nOr install via npm (note: skills will not be synced):\n npm install -g %s@%s\n npx skills add larksuite/cli -y -g # sync skills separately\n", selfupdate.NpmPackage, latest) } diff --git a/internal/selfupdate/updater.go b/internal/selfupdate/updater.go index 4c5367332b..41f6be6cbf 100644 --- a/internal/selfupdate/updater.go +++ b/internal/selfupdate/updater.go @@ -160,12 +160,17 @@ func detectFromResolved(resolved string, npmOnPath, pnpmOnPath bool) DetectResul return d } -// containsPnpmMarker reports whether the path contains pnpm's virtual-store -// directory segment (".pnpm"), which distinguishes a pnpm global install from -// an npm one. Both POSIX ("/.pnpm/") and Windows ("\.pnpm\") separators are -// checked so the classification is OS-independent and unit-testable anywhere. +// containsPnpmMarker reports whether the resolved binary path belongs to a +// pnpm-managed install. pnpm exposes two layouts: the classic virtual store +// (a ".pnpm" directory segment) and the global content-addressable store, +// whose resolved path runs through pnpm's home directory (e.g. +// "~/Library/pnpm/store/v11/links/...") — a "pnpm" directory segment with no +// ".pnpm". Matching either "pnpm" or ".pnpm" as a path segment covers both. +// Both POSIX ("/") and Windows ("\") separators are checked so the +// classification is OS-independent and unit-testable anywhere. func containsPnpmMarker(p string) bool { - return strings.Contains(p, "/.pnpm/") || strings.Contains(p, `\.pnpm\`) + return strings.Contains(p, "/.pnpm/") || strings.Contains(p, `\.pnpm\`) || + strings.Contains(p, "/pnpm/") || strings.Contains(p, `\pnpm\`) } // RunNpmInstall executes npm install -g @larksuite/cli@. @@ -312,19 +317,40 @@ func (u *Updater) runSkillsInstall(source string, nameList []string) *NpmResult return u.runSkillsCommand(args...) } +// skillsInvocation decides how to launch the `skills` CLI. When the lark-cli +// itself was installed via pnpm and pnpm is available, it uses `pnpm dlx` so +// pnpm-only environments (pnpm's standalone installer bundles Node without +// putting npm/npx on PATH) can still sync skills after a self-update. +// Otherwise it uses `npx`. The npx auto-confirm flag "-y", when present as the +// leading arg, maps to `pnpm dlx`'s default non-interactive behavior and is +// dropped for the pnpm launcher. Kept pure (no exec/PATH access) so the +// launcher selection is unit-testable on any platform. +func skillsInvocation(method InstallMethod, pnpmAvailable bool, args []string) (launcher string, rest []string) { + if method == InstallPnpm && pnpmAvailable { + r := args + if len(r) > 0 && r[0] == "-y" { + r = r[1:] + } + return "pnpm", append([]string{"dlx"}, r...) + } + return "npx", args +} + func (u *Updater) runSkillsCommand(args ...string) *NpmResult { if u.SkillsCommandOverride != nil { return u.SkillsCommandOverride(args...) } r := &NpmResult{} - npxPath, err := exec.LookPath("npx") + det := u.DetectInstallMethod() + launcher, cmdArgs := skillsInvocation(det.Method, det.PnpmAvailable, args) + binPath, err := exec.LookPath(launcher) if err != nil { - r.Err = fmt.Errorf("npx not found in PATH: %w", err) + r.Err = fmt.Errorf("%s not found in PATH: %w", launcher, err) return r } ctx, cancel := context.WithTimeout(context.Background(), skillsUpdateTimeout) defer cancel() - cmd := exec.CommandContext(ctx, npxPath, args...) + cmd := exec.CommandContext(ctx, binPath, cmdArgs...) 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 e26abf64b2..5fe4676501 100644 --- a/internal/selfupdate/updater_test.go +++ b/internal/selfupdate/updater_test.go @@ -377,11 +377,20 @@ func TestContainsPnpmMarker(t *testing.T) { path string want bool }{ + // Classic virtual-store layout (.pnpm segment). {"/Users/x/Library/pnpm/global/5/node_modules/.pnpm/@larksuite+cli@1.0.44/node_modules/@larksuite/cli/bin/lark-cli", true}, - {"/usr/local/lib/node_modules/@larksuite/cli/bin/lark-cli", false}, - {"/opt/homebrew/.pnpmfoo/node_modules/@larksuite/cli/bin/lark-cli", false}, {`C:\Users\x\AppData\Local\pnpm\global\5\node_modules\.pnpm\@larksuite+cli@1.0.44\node_modules\@larksuite\cli\bin\lark-cli.exe`, true}, + // Global content-addressable store layout (pnpm 11): resolved path runs + // through the pnpm home store, a "pnpm" segment with no ".pnpm". + {"/Users/x/Library/pnpm/store/v11/links/@larksuite/cli/1.0.59/abc123/node_modules/@larksuite/cli/bin/lark-cli", true}, + {"/home/x/.local/share/pnpm/store/v10/@larksuite/cli/node_modules/@larksuite/cli/bin/lark-cli", true}, + {`C:\Users\x\AppData\Local\pnpm\store\v11\links\@larksuite\cli\node_modules\@larksuite\cli\bin\lark-cli.exe`, true}, + // npm and non-package installs — no pnpm/.pnpm segment. + {"/usr/local/lib/node_modules/@larksuite/cli/bin/lark-cli", false}, {"/usr/local/bin/lark-cli", false}, + // Substrings that must NOT match: segment must be exactly pnpm/.pnpm. + {"/opt/homebrew/.pnpmfoo/node_modules/@larksuite/cli/bin/lark-cli", false}, + {"/opt/pnpmfoo/node_modules/@larksuite/cli/bin/lark-cli", false}, } for _, c := range cases { if got := containsPnpmMarker(c.path); got != c.want { @@ -454,3 +463,37 @@ func TestRunPnpmInstall_Error(t *testing.T) { t.Errorf("err = %v, want %v", got.Err, wantErr) } } + +func TestSkillsInvocation(t *testing.T) { + addArgs := []string{"-y", "skills", "add", "https://open.feishu.cn", "-g", "-y"} + cases := []struct { + name string + method InstallMethod + pnpmAvailable bool + args []string + wantLauncher string + wantRest []string + }{ + {"pnpm install + pnpm available → pnpm dlx, drop leading -y", InstallPnpm, true, addArgs, + "pnpm", []string{"dlx", "skills", "add", "https://open.feishu.cn", "-g", "-y"}}, + {"pnpm install but pnpm unavailable → npx unchanged", InstallPnpm, false, addArgs, + "npx", addArgs}, + {"npm install → npx unchanged", InstallNpm, false, addArgs, + "npx", addArgs}, + {"manual install → npx unchanged", InstallManual, false, []string{"-y", "skills", "ls", "-g"}, + "npx", []string{"-y", "skills", "ls", "-g"}}, + {"pnpm without a leading -y → prepend dlx only", InstallPnpm, true, []string{"skills", "ls", "-g"}, + "pnpm", []string{"dlx", "skills", "ls", "-g"}}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + gotLauncher, gotRest := skillsInvocation(c.method, c.pnpmAvailable, c.args) + if gotLauncher != c.wantLauncher { + t.Errorf("launcher = %q, want %q", gotLauncher, c.wantLauncher) + } + if strings.Join(gotRest, " ") != strings.Join(c.wantRest, " ") { + t.Errorf("rest = %v, want %v", gotRest, c.wantRest) + } + }) + } +} From 7ce605b619137c706ee06aca9e52951d58e74b04 Mon Sep 17 00:00:00 2001 From: leave330 Date: Wed, 1 Jul 2026 21:15:27 +0800 Subject: [PATCH 6/7] fix: tighten pnpm marker to pnpm/store and show skills launcher --- cmd/update/update.go | 6 +++++- cmd/update/update_test.go | 6 ++++++ internal/selfupdate/updater.go | 19 ++++++++++++++----- internal/selfupdate/updater_test.go | 6 +++++- 4 files changed, 30 insertions(+), 7 deletions(-) diff --git a/cmd/update/update.go b/cmd/update/update.go index 7621c88dd5..2d01a4d66a 100644 --- a/cmd/update/update.go +++ b/cmd/update/update.go @@ -316,7 +316,11 @@ func doAutoUpdate(opts *UpdateOptions, io *cmdutil.IOStreams, cur, latest string fmt.Fprintf(io.ErrOut, "\n%s Successfully updated lark-cli from %s to %s\n", symOK(), cur, latest) fmt.Fprintf(io.ErrOut, " Changelog: %s\n", changelogURL()) if skillsResult != nil { - fmt.Fprintf(io.ErrOut, "\nUpdating skills ...\n") + skillsPM := "npx" + if detect.Method == selfupdate.InstallPnpm && detect.PnpmAvailable { + skillsPM = "pnpm dlx" + } + fmt.Fprintf(io.ErrOut, "\nUpdating skills via %s ...\n", skillsPM) } emitSkillsTextHints(io, skillsResult) return nil diff --git a/cmd/update/update_test.go b/cmd/update/update_test.go index fd2f1bbf98..61e546671a 100644 --- a/cmd/update/update_test.go +++ b/cmd/update/update_test.go @@ -147,6 +147,9 @@ func TestUpdatePnpm_Human(t *testing.T) { if !strings.Contains(out, "via pnpm") { t.Errorf("expected 'via pnpm' in stderr, got: %s", out) } + if !strings.Contains(out, "Updating skills via pnpm dlx ...") { + t.Errorf("expected skills sync to report pnpm dlx launcher, got: %s", out) + } if !strings.Contains(out, "Successfully updated") { t.Errorf("expected success message, got: %s", out) } @@ -388,6 +391,9 @@ func TestUpdateNpm_Human(t *testing.T) { if !strings.Contains(out, "Successfully updated") { t.Errorf("expected success message in stderr, got: %s", out) } + if !strings.Contains(out, "Updating skills via npx ...") { + t.Errorf("expected skills sync to report npx launcher for npm install, got: %s", out) + } } func TestUpdateForce_JSON(t *testing.T) { diff --git a/internal/selfupdate/updater.go b/internal/selfupdate/updater.go index 41f6be6cbf..0bfed53ddf 100644 --- a/internal/selfupdate/updater.go +++ b/internal/selfupdate/updater.go @@ -164,13 +164,22 @@ func detectFromResolved(resolved string, npmOnPath, pnpmOnPath bool) DetectResul // pnpm-managed install. pnpm exposes two layouts: the classic virtual store // (a ".pnpm" directory segment) and the global content-addressable store, // whose resolved path runs through pnpm's home directory (e.g. -// "~/Library/pnpm/store/v11/links/...") — a "pnpm" directory segment with no -// ".pnpm". Matching either "pnpm" or ".pnpm" as a path segment covers both. -// Both POSIX ("/") and Windows ("\") separators are checked so the +// "~/Library/pnpm/store/v11/links/...") — a "pnpm" segment immediately +// followed by "store". Matching only these two shapes (rather than any bare +// "pnpm" segment) avoids misclassifying an npm install that merely lives under +// a directory named "pnpm". Windows separators are normalized to "/" so the // classification is OS-independent and unit-testable anywhere. func containsPnpmMarker(p string) bool { - return strings.Contains(p, "/.pnpm/") || strings.Contains(p, `\.pnpm\`) || - strings.Contains(p, "/pnpm/") || strings.Contains(p, `\pnpm\`) + parts := strings.Split(strings.ReplaceAll(p, `\`, "/"), "/") + for i, part := range parts { + if part == ".pnpm" { + return true + } + if part == "pnpm" && i+1 < len(parts) && parts[i+1] == "store" { + return true + } + } + return false } // RunNpmInstall executes npm install -g @larksuite/cli@. diff --git a/internal/selfupdate/updater_test.go b/internal/selfupdate/updater_test.go index 5fe4676501..1ed51b7bc9 100644 --- a/internal/selfupdate/updater_test.go +++ b/internal/selfupdate/updater_test.go @@ -388,9 +388,13 @@ func TestContainsPnpmMarker(t *testing.T) { // npm and non-package installs — no pnpm/.pnpm segment. {"/usr/local/lib/node_modules/@larksuite/cli/bin/lark-cli", false}, {"/usr/local/bin/lark-cli", false}, - // Substrings that must NOT match: segment must be exactly pnpm/.pnpm. + // Substrings that must NOT match: segment must be exactly .pnpm, or + // "pnpm" immediately followed by "store". {"/opt/homebrew/.pnpmfoo/node_modules/@larksuite/cli/bin/lark-cli", false}, {"/opt/pnpmfoo/node_modules/@larksuite/cli/bin/lark-cli", false}, + // A bare "pnpm" directory NOT followed by "store" (e.g. an npm install + // living under a dir named pnpm) must not be misclassified as pnpm. + {"/opt/pnpm/lib/node_modules/@larksuite/cli/bin/lark-cli", false}, } for _, c := range cases { if got := containsPnpmMarker(c.path); got != c.want { From 568883ee18f2e22ea80fd28ac03e54d142d8611f Mon Sep 17 00:00:00 2001 From: leave330 Date: Fri, 3 Jul 2026 13:02:11 +0800 Subject: [PATCH 7/7] fix: cache install detection and make update failure hints pnpm-aware --- cmd/update/update.go | 22 ++++++++++++++-------- cmd/update/update_test.go | 17 +++++++++++++---- internal/selfupdate/updater.go | 21 +++++++++++++++++++++ internal/selfupdate/updater_test.go | 14 ++++++++++++++ 4 files changed, 62 insertions(+), 12 deletions(-) diff --git a/cmd/update/update.go b/cmd/update/update.go index 2d01a4d66a..d42f87e9bb 100644 --- a/cmd/update/update.go +++ b/cmd/update/update.go @@ -263,7 +263,7 @@ func doAutoUpdate(opts *UpdateOptions, io *cmdutil.IOStreams, cur, latest string "ok": false, "error": map[string]interface{}{ "type": "update_error", "message": fmt.Sprintf("%s install failed: %s", pm, npmResult.Err), "detail": selfupdate.Truncate(combined, maxNpmOutput), - "hint": permissionHint(combined), + "hint": permissionHint(combined, pm), }, }) return output.ErrBare(output.ExitAPI) @@ -275,7 +275,7 @@ func doAutoUpdate(opts *UpdateOptions, io *cmdutil.IOStreams, cur, latest string fmt.Fprint(io.ErrOut, npmResult.Stderr.String()) } fmt.Fprintf(io.ErrOut, "\n%s Update failed: %s\n", symFail(), npmResult.Err) - if hint := permissionHint(combined); hint != "" { + if hint := permissionHint(combined, pm); hint != "" { fmt.Fprintf(io.ErrOut, " %s\n", hint) } return output.ErrBare(output.ExitAPI) @@ -286,7 +286,7 @@ func doAutoUpdate(opts *UpdateOptions, io *cmdutil.IOStreams, cur, latest string if err := updater.VerifyBinary(latest); err != nil { restore() msg := fmt.Sprintf("new binary verification failed: %s", err) - hint := verificationFailureHint(updater, latest) + hint := verificationFailureHint(updater, latest, pm) if opts.JSON { output.PrintJson(io.Out, map[string]interface{}{ "ok": false, @@ -326,17 +326,23 @@ func doAutoUpdate(opts *UpdateOptions, io *cmdutil.IOStreams, cur, latest string return nil } -func permissionHint(npmOutput string) string { - if strings.Contains(npmOutput, "EACCES") && !isWindows() { - return "Permission denied. Try: sudo lark-cli update, or adjust your npm global prefix: https://docs.npmjs.com/resolving-eacces-permissions-errors" +func permissionHint(pmOutput, pm string) string { + if !strings.Contains(pmOutput, "EACCES") || isWindows() { + return "" } - return "" + if pm == "pnpm" { + return "Permission denied. Ensure your pnpm global directory is writable — re-run `pnpm setup`, or see https://pnpm.io/pnpm-cli" + } + return "Permission denied. Try: sudo lark-cli update, or adjust your npm global prefix: https://docs.npmjs.com/resolving-eacces-permissions-errors" } -func verificationFailureHint(updater *selfupdate.Updater, latest string) string { +func verificationFailureHint(updater *selfupdate.Updater, latest, pm string) string { if updater.CanRestorePreviousVersion() { return "the previous version has been restored" } + if pm == "pnpm" { + return fmt.Sprintf("automatic rollback is unavailable on this platform; reinstall manually (skills will not be synced): pnpm add -g %s@%s && pnpm dlx skills add larksuite/cli -y -g, or download %s", selfupdate.NpmPackage, latest, releaseURL(latest)) + } 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)) } diff --git a/cmd/update/update_test.go b/cmd/update/update_test.go index 61e546671a..177d4340f3 100644 --- a/cmd/update/update_test.go +++ b/cmd/update/update_test.go @@ -867,9 +867,9 @@ func TestPermissionHint(t *testing.T) { origOS := currentOS defer func() { currentOS = origOS }() - // Linux: EACCES should produce a hint with npm prefix guidance. + // Linux + npm: EACCES should produce a hint with npm prefix guidance. currentOS = "linux" - hint := permissionHint("EACCES: permission denied, access '/usr/local/lib'") + hint := permissionHint("EACCES: permission denied, access '/usr/local/lib'", "npm") if !strings.Contains(hint, "npm global prefix") { t.Errorf("expected npm prefix hint on linux, got: %s", hint) } @@ -877,16 +877,25 @@ func TestPermissionHint(t *testing.T) { t.Errorf("should not suggest raw sudo npm install, got: %s", hint) } + // Linux + pnpm: EACCES should point at pnpm setup, not npm prefix/sudo. + pnpmHint := permissionHint("EACCES: permission denied, access '/Users/x/Library/pnpm'", "pnpm") + if !strings.Contains(pnpmHint, "pnpm setup") { + t.Errorf("expected pnpm setup hint, got: %s", pnpmHint) + } + if strings.Contains(pnpmHint, "npm global prefix") || strings.Contains(pnpmHint, "sudo") { + t.Errorf("pnpm hint must not reference npm prefix or sudo, got: %s", pnpmHint) + } + // Windows: EACCES hint is suppressed (no EACCES on Windows). currentOS = "windows" - hint = permissionHint("EACCES: permission denied") + hint = permissionHint("EACCES: permission denied", "npm") if hint != "" { t.Errorf("expected empty hint on Windows, got: %s", hint) } // Non-EACCES error: always empty. currentOS = "linux" - if got := permissionHint("some other error"); got != "" { + if got := permissionHint("some other error", "npm"); got != "" { t.Errorf("expected empty hint for non-EACCES, got: %s", got) } } diff --git a/internal/selfupdate/updater.go b/internal/selfupdate/updater.go index 0bfed53ddf..4b8f93a181 100644 --- a/internal/selfupdate/updater.go +++ b/internal/selfupdate/updater.go @@ -113,6 +113,17 @@ type Updater struct { // running binary is successfully renamed to .old. Used by // CanRestorePreviousVersion to report whether rollback is possible. backupCreated bool + + // detectCache memoizes the first real DetectInstallMethod result. How this + // binary was installed cannot change during a single process, so caching is + // the correct semantics — and it is required for correctness: the update + // flow mutates the install (pnpm add -g / npm install -g) before syncing + // skills, so a re-detection at skills time could resolve a now-stale + // os.Executable path and misclassify. Seeded pre-update by the first call + // (updateRun), it keeps the post-update skills launcher consistent with the + // launcher reported to the user. Not goroutine-safe; the update flow is + // sequential. + detectCache *DetectResult } // New creates an Updater with default (real) behavior. @@ -124,6 +135,16 @@ func (u *Updater) DetectInstallMethod() DetectResult { if u.DetectOverride != nil { return u.DetectOverride() } + if u.detectCache != nil { + return *u.detectCache + } + result := u.detectInstallMethod() + u.detectCache = &result + return result +} + +// detectInstallMethod performs the real (uncached) detection. +func (u *Updater) detectInstallMethod() DetectResult { exe, err := vfs.Executable() if err != nil { return DetectResult{Method: InstallManual} diff --git a/internal/selfupdate/updater_test.go b/internal/selfupdate/updater_test.go index 1ed51b7bc9..93176265eb 100644 --- a/internal/selfupdate/updater_test.go +++ b/internal/selfupdate/updater_test.go @@ -501,3 +501,17 @@ func TestSkillsInvocation(t *testing.T) { }) } } + +// TestDetectInstallMethod_Caches locks the fix for the post-update re-detection +// hazard: DetectInstallMethod must return the first (pre-update) detection on +// subsequent calls, so the skills launcher chosen after the binary is replaced +// stays consistent with what was detected — and reported — before the update. +func TestDetectInstallMethod_Caches(t *testing.T) { + u := New() + cached := DetectResult{Method: InstallPnpm, PnpmAvailable: true, ResolvedPath: "/x/pnpm/store/v11/links/@larksuite/cli/1.0.0/node_modules/@larksuite/cli/bin/lark-cli"} + u.detectCache = &cached + got := u.DetectInstallMethod() + if got.Method != InstallPnpm || !got.PnpmAvailable { + t.Errorf("expected cached pnpm result to be returned, got %+v", got) + } +}