diff --git a/cmd/update/update.go b/cmd/update/update.go index a6accb1a2..7621c88dd 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 4c5367332..41f6be6cb 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 e26abf64b..5fe467650 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) + } + }) + } +}