Skip to content

Commit dce88c7

Browse files
git-hulkclaude
andcommitted
Normalize bare owner/repo npm sources and detect installed bin
Fixes a confusing failure when users pass --npm owner/repo: npm treats it as a GitHub shorthand instead of a scoped package, so no binary is created and the install fails with "binary not found". NewNpmInstaller now rewrites bare owner/repo strings to @owner/repo, and Install snapshots the npm global bin dir to discover binaries with unexpected names and report a helpful error when none are produced. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 3837cfc commit dce88c7

2 files changed

Lines changed: 208 additions & 11 deletions

File tree

internal/installer/npm.go

Lines changed: 81 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"os"
77
osexec "os/exec"
88
"path/filepath"
9+
"sort"
910
"strings"
1011

1112
"github.com/git-hulk/clime/internal/plugin"
@@ -23,9 +24,12 @@ type NpmInstaller struct {
2324
}
2425

2526
// NewNpmInstaller returns an NpmInstaller for the given npm package.
27+
// A bare "owner/repo" string is treated as a scoped package and rewritten to
28+
// "@owner/repo"; npm would otherwise resolve it as a GitHub shorthand, which
29+
// is rarely what callers want when they specify --npm.
2630
func NewNpmInstaller(pkg string) *NpmInstaller {
2731
return &NpmInstaller{
28-
Package: pkg,
32+
Package: normalizeNpmPackageName(pkg),
2933
runNpmInstall: runNpmGlobalInstall,
3034
runNpmUpdate: runNpmGlobalUpdate,
3135
runNpmUninstall: runNpmGlobalUninstall,
@@ -35,27 +39,43 @@ func NewNpmInstaller(pkg string) *NpmInstaller {
3539
}
3640
}
3741

42+
// normalizeNpmPackageName prepends "@" to bare "owner/repo" strings so npm
43+
// treats them as scoped registry packages rather than GitHub shorthand.
44+
// Inputs that are already scoped, unscoped, URLs, protocol-prefixed
45+
// (git+https://, github:, file:, etc.), or local paths are returned as-is.
46+
func normalizeNpmPackageName(pkg string) string {
47+
pkg = strings.TrimSpace(pkg)
48+
if pkg == "" || strings.HasPrefix(pkg, "@") {
49+
return pkg
50+
}
51+
if strings.ContainsAny(pkg, ":\\") || strings.HasPrefix(pkg, ".") || strings.HasPrefix(pkg, "/") {
52+
return pkg
53+
}
54+
if strings.Count(pkg, "/") == 1 {
55+
return "@" + pkg
56+
}
57+
return pkg
58+
}
59+
3860
func (n *NpmInstaller) Install(name string) (string, error) {
3961
if _, err := osexec.LookPath("npm"); err != nil {
4062
return "", fmt.Errorf("npm is not installed or not on PATH: %w", err)
4163
}
4264

65+
npmBinDir, err := n.npmGlobalBinDir()
66+
if err != nil {
67+
return "", fmt.Errorf("failed to determine npm global bin directory: %w", err)
68+
}
69+
before := snapshotDirEntries(npmBinDir)
70+
4371
if err := n.runNpmInstall(n.Package); err != nil {
4472
return "", fmt.Errorf("npm install failed: %w", err)
4573
}
4674

4775
binName := plugin.BinPrefix + name
48-
npmBinDir, err := n.npmGlobalBinDir()
76+
binaryPath, err := locateNpmInstalledBinary(npmBinDir, n.Package, name, binName, before)
4977
if err != nil {
50-
return "", fmt.Errorf("failed to determine npm global bin directory: %w", err)
51-
}
52-
binaryPath := filepath.Join(npmBinDir, binName)
53-
54-
if _, err := os.Stat(binaryPath); err != nil {
55-
binaryPath = filepath.Join(npmBinDir, name)
56-
if _, err := os.Stat(binaryPath); err != nil {
57-
return "", fmt.Errorf("binary %q or %q not found in npm global bin dir %q after install: %w", binName, name, npmBinDir, err)
58-
}
78+
return "", err
5979
}
6080

6181
installDir, err := n.pluginBinDir()
@@ -164,6 +184,56 @@ func npmGlobalBinDir() (string, error) {
164184
return filepath.Join(strings.TrimSpace(string(out)), "bin"), nil
165185
}
166186

187+
// snapshotDirEntries returns the set of entry names directly under dir, or an
188+
// empty set if the directory cannot be read.
189+
func snapshotDirEntries(dir string) map[string]struct{} {
190+
set := make(map[string]struct{})
191+
entries, err := os.ReadDir(dir)
192+
if err != nil {
193+
return set
194+
}
195+
for _, e := range entries {
196+
set[e.Name()] = struct{}{}
197+
}
198+
return set
199+
}
200+
201+
// locateNpmInstalledBinary picks the binary that an npm install created.
202+
// It prefers clime-<name>, then <name>, then falls back to a single new entry
203+
// added to npmBinDir during the install. The error explains why no candidate
204+
// was found, including any unexpected new entries.
205+
func locateNpmInstalledBinary(npmBinDir, pkg, name, binName string, before map[string]struct{}) (string, error) {
206+
if path := filepath.Join(npmBinDir, binName); fileExists(path) {
207+
return path, nil
208+
}
209+
if path := filepath.Join(npmBinDir, name); fileExists(path) {
210+
return path, nil
211+
}
212+
213+
after := snapshotDirEntries(npmBinDir)
214+
var added []string
215+
for entry := range after {
216+
if _, existed := before[entry]; !existed {
217+
added = append(added, entry)
218+
}
219+
}
220+
sort.Strings(added)
221+
222+
switch len(added) {
223+
case 0:
224+
return "", fmt.Errorf("npm install of %q did not create a binary in %q; the package may not provide a CLI (check that the package name is correct, e.g. a scoped package starting with \"@\")", pkg, npmBinDir)
225+
case 1:
226+
return filepath.Join(npmBinDir, added[0]), nil
227+
default:
228+
return "", fmt.Errorf("npm install of %q created multiple binaries in %q (%s); none matched %q or %q — rerun with a plugin name matching one of them", pkg, npmBinDir, strings.Join(added, ", "), binName, name)
229+
}
230+
}
231+
232+
func fileExists(path string) bool {
233+
_, err := os.Stat(path)
234+
return err == nil
235+
}
236+
167237
// getNpmInstalledVersion returns the actual installed version of an npm package.
168238
func getNpmInstalledVersion(pkg string) (string, error) {
169239
out, err := osexec.Command("npm", "list", "-g", pkg, "--json").Output()

internal/installer/npm_test.go

Lines changed: 127 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
package installer
22

33
import (
4+
"os"
45
"path/filepath"
6+
"strings"
57
"testing"
68

79
"github.com/git-hulk/clime/internal/plugin"
@@ -84,6 +86,131 @@ func TestNpmInstallerUpdateUpToDate(t *testing.T) {
8486
}
8587
}
8688

89+
func TestLocateNpmInstalledBinaryPrefersClimePrefix(t *testing.T) {
90+
t.Parallel()
91+
dir := t.TempDir()
92+
mustTouch(t, filepath.Join(dir, "clime-deploy"))
93+
mustTouch(t, filepath.Join(dir, "deploy"))
94+
95+
path, err := locateNpmInstalledBinary(dir, "@myorg/clime-deploy", "deploy", "clime-deploy", map[string]struct{}{})
96+
if err != nil {
97+
t.Fatalf("locateNpmInstalledBinary() error = %v", err)
98+
}
99+
if want := filepath.Join(dir, "clime-deploy"); path != want {
100+
t.Fatalf("path = %q, want %q", path, want)
101+
}
102+
}
103+
104+
func TestLocateNpmInstalledBinaryFallsBackToName(t *testing.T) {
105+
t.Parallel()
106+
dir := t.TempDir()
107+
mustTouch(t, filepath.Join(dir, "codex"))
108+
109+
path, err := locateNpmInstalledBinary(dir, "@openai/codex", "codex", "clime-codex", map[string]struct{}{})
110+
if err != nil {
111+
t.Fatalf("locateNpmInstalledBinary() error = %v", err)
112+
}
113+
if want := filepath.Join(dir, "codex"); path != want {
114+
t.Fatalf("path = %q, want %q", path, want)
115+
}
116+
}
117+
118+
func TestLocateNpmInstalledBinaryDiscoversNewBinary(t *testing.T) {
119+
t.Parallel()
120+
dir := t.TempDir()
121+
mustTouch(t, filepath.Join(dir, "existing"))
122+
before := snapshotDirEntries(dir)
123+
mustTouch(t, filepath.Join(dir, "weirdname"))
124+
125+
path, err := locateNpmInstalledBinary(dir, "some-package", "tool", "clime-tool", before)
126+
if err != nil {
127+
t.Fatalf("locateNpmInstalledBinary() error = %v", err)
128+
}
129+
if want := filepath.Join(dir, "weirdname"); path != want {
130+
t.Fatalf("path = %q, want %q", path, want)
131+
}
132+
}
133+
134+
func TestLocateNpmInstalledBinaryErrorsWhenNoBinaryCreated(t *testing.T) {
135+
t.Parallel()
136+
dir := t.TempDir()
137+
mustTouch(t, filepath.Join(dir, "preexisting"))
138+
before := snapshotDirEntries(dir)
139+
140+
_, err := locateNpmInstalledBinary(dir, "openai/codex", "codex", "clime-codex", before)
141+
if err == nil {
142+
t.Fatal("expected error when npm install produced no new binary")
143+
}
144+
if !strings.Contains(err.Error(), "did not create a binary") {
145+
t.Fatalf("error = %q, want it to mention missing binary", err)
146+
}
147+
if !strings.Contains(err.Error(), "openai/codex") {
148+
t.Fatalf("error = %q, want it to reference the package", err)
149+
}
150+
}
151+
152+
func TestLocateNpmInstalledBinaryErrorsOnAmbiguousNewBinaries(t *testing.T) {
153+
t.Parallel()
154+
dir := t.TempDir()
155+
before := snapshotDirEntries(dir)
156+
mustTouch(t, filepath.Join(dir, "tsc"))
157+
mustTouch(t, filepath.Join(dir, "tsserver"))
158+
159+
_, err := locateNpmInstalledBinary(dir, "typescript", "ts", "clime-ts", before)
160+
if err == nil {
161+
t.Fatal("expected error when multiple new binaries match nothing")
162+
}
163+
if !strings.Contains(err.Error(), "tsc") || !strings.Contains(err.Error(), "tsserver") {
164+
t.Fatalf("error = %q, want it to list both binaries", err)
165+
}
166+
}
167+
168+
func TestNormalizeNpmPackageName(t *testing.T) {
169+
t.Parallel()
170+
cases := []struct {
171+
in string
172+
want string
173+
}{
174+
{"openai/codex", "@openai/codex"},
175+
{" openai/codex ", "@openai/codex"},
176+
{"@openai/codex", "@openai/codex"},
177+
{"@openai/codex@1.0.0", "@openai/codex@1.0.0"},
178+
{"lodash", "lodash"},
179+
{"lodash@4.17.0", "lodash@4.17.0"},
180+
{"git+https://github.com/openai/codex.git", "git+https://github.com/openai/codex.git"},
181+
{"github:openai/codex", "github:openai/codex"},
182+
{"file:./local", "file:./local"},
183+
{"./local-package", "./local-package"},
184+
{"/abs/path/pkg", "/abs/path/pkg"},
185+
{"", ""},
186+
}
187+
for _, c := range cases {
188+
got := normalizeNpmPackageName(c.in)
189+
if got != c.want {
190+
t.Errorf("normalizeNpmPackageName(%q) = %q, want %q", c.in, got, c.want)
191+
}
192+
}
193+
}
194+
195+
func TestNewNpmInstallerNormalizesPackage(t *testing.T) {
196+
t.Parallel()
197+
n := NewNpmInstaller("openai/codex")
198+
if n.Package != "@openai/codex" {
199+
t.Fatalf("Package = %q, want %q", n.Package, "@openai/codex")
200+
}
201+
}
202+
203+
func mustTouch(t *testing.T, path string) {
204+
t.Helper()
205+
f, err := os.Create(path)
206+
if err != nil {
207+
t.Fatalf("create %s: %v", path, err)
208+
}
209+
if err := f.Close(); err != nil {
210+
t.Fatalf("close %s: %v", path, err)
211+
}
212+
}
213+
87214
func TestNpmInstallerPluginType(t *testing.T) {
88215
t.Parallel()
89216
n := NewNpmInstaller("@myorg/clime-deploy")

0 commit comments

Comments
 (0)