From 1de07cf8c4ccbf6f14960258e29a8041eb5a5833 Mon Sep 17 00:00:00 2001 From: Goon Date: Mon, 18 May 2026 15:53:42 +0700 Subject: [PATCH] fix(packages): support npm workspace dependency installs Retry npm global installs that fail on workspace protocol dependencies by packing the registry tarball, rewriting workspace ranges to published versions, and installing the sanitized package folder. --- docs/project-changelog.md | 15 ++ internal/skills/dep_installer.go | 12 +- internal/skills/npm_update_checker_test.go | 4 + internal/skills/npm_update_executor.go | 18 +- internal/skills/npm_workspace_fallback.go | 242 ++++++++++++++++++ .../skills/npm_workspace_fallback_test.go | 108 ++++++++ 6 files changed, 373 insertions(+), 26 deletions(-) create mode 100644 internal/skills/npm_workspace_fallback.go create mode 100644 internal/skills/npm_workspace_fallback_test.go diff --git a/docs/project-changelog.md b/docs/project-changelog.md index d094481f..ca81d86d 100644 --- a/docs/project-changelog.md +++ b/docs/project-changelog.md @@ -4,6 +4,21 @@ Significant changes, features, and fixes in reverse chronological order. --- +## 2026-05-18 + +### Packages: npm workspace protocol fallback + +**Fixes** + +- Fixed Node package installs for registry packages published with `workspace:` dependency ranges, such as `@agenttasks/cli`. +- GoClaw now retries npm `EUNSUPPORTEDPROTOCOL workspace:` failures by packing the registry tarball, rewriting workspace dependency ranges to published package versions, and installing the sanitized package folder. + +**Tests** + +- Added focused coverage for workspace protocol detection and package.json dependency rewrite behavior. + +--- + ## 2026-05-17 ### Agents: provider switch save fix diff --git a/internal/skills/dep_installer.go b/internal/skills/dep_installer.go index 1b31041e..0628b0ba 100644 --- a/internal/skills/dep_installer.go +++ b/internal/skills/dep_installer.go @@ -119,13 +119,7 @@ func InstallSingleDep(ctx context.Context, dep string) (bool, string) { } defer release() } - if err := os.MkdirAll(npmGlobalPrefix(), 0o750); err != nil { - return false, fmt.Sprintf("npm prefix setup: %v", err) - } - ensureNpmGlobalEnv() - cmd := exec.CommandContext(ctx, npmBinary, "install", "-g", pkg) - cmd.Env = npmCommandEnv() - out, err := cmd.CombinedOutput() + out, err := installNpmPackage(ctx, pkg) if err != nil { msg := fmt.Sprintf("%s: %v", strings.TrimSpace(string(out)), err) slog.Error("skills: dep install failed", "dep", dep, "error", msg) @@ -207,9 +201,7 @@ func InstallDeps(ctx context.Context, manifest *SkillManifest, missing []string) ensureNpmGlobalEnv() var successful []string for _, pkg := range npmPkgs { - cmd := exec.CommandContext(ctx, npmBinary, "install", "-g", pkg) - cmd.Env = npmCommandEnv() - if out, err := cmd.CombinedOutput(); err != nil { + if out, err := installNpmPackage(ctx, pkg); err != nil { result.Errors = append(result.Errors, fmt.Sprintf("npm %s: %s (%v)", pkg, strings.TrimSpace(string(out)), err)) } else { successful = append(successful, pkg) diff --git a/internal/skills/npm_update_checker_test.go b/internal/skills/npm_update_checker_test.go index 079f66a3..0e2aa65c 100644 --- a/internal/skills/npm_update_checker_test.go +++ b/internal/skills/npm_update_checker_test.go @@ -2,6 +2,7 @@ package skills import ( "context" + "os" "os/exec" "path/filepath" "runtime" @@ -31,6 +32,9 @@ func useFixtureNpm(t *testing.T) { t.Helper() restoreNpmBinary(t) restoreNpmLookPath(t) + if os.Getenv("RUNTIME_DIR") == "" && os.Getenv("NPM_CONFIG_PREFIX") == "" { + t.Setenv("RUNTIME_DIR", t.TempDir()) + } npmBinary = filepath.Join("testdata", "npm", "bin", "npm") if runtime.GOOS == "windows" { npmBinary += ".cmd" diff --git a/internal/skills/npm_update_executor.go b/internal/skills/npm_update_executor.go index 4e51d097..bc2c92db 100644 --- a/internal/skills/npm_update_executor.go +++ b/internal/skills/npm_update_executor.go @@ -1,12 +1,9 @@ package skills import ( - "bytes" "context" "fmt" "log/slog" - "os" - "os/exec" "time" ) @@ -47,23 +44,12 @@ func (e *NpmUpdateExecutor) Update(ctx context.Context, name, toVersion string, // suffixes, so the only "@" in the token is our version separator. target := name + "@" + toVersion - if err := os.MkdirAll(npmGlobalPrefix(), 0o750); err != nil { - return fmt.Errorf("npm prefix setup: %w", err) - } - ensureNpmGlobalEnv() - cmd := exec.CommandContext(cctx, npmBinary, "install", "--global", target) - cmd.Env = npmCommandEnv() - cmd.WaitDelay = 2 * time.Second - var stdout, stderr bytes.Buffer - cmd.Stdout = &stdout - cmd.Stderr = &stderr - start := time.Now() - runErr := cmd.Run() + out, runErr := installNpmPackage(cctx, target) durationMs := time.Since(start).Milliseconds() if runErr != nil { - sentinel, reason := ClassifyNpmStderr(stderr.String()) + sentinel, reason := ClassifyNpmStderr(string(out)) if sentinel == nil { sentinel = fmt.Errorf("npm install failed: %w", runErr) } diff --git a/internal/skills/npm_workspace_fallback.go b/internal/skills/npm_workspace_fallback.go new file mode 100644 index 00000000..f6ae060e --- /dev/null +++ b/internal/skills/npm_workspace_fallback.go @@ -0,0 +1,242 @@ +package skills + +import ( + "bytes" + "context" + "encoding/json" + "errors" + "fmt" + "log/slog" + "os" + "os/exec" + "path/filepath" + "strings" + "time" +) + +var npmPackageVersionResolver = npmViewPackageVersion + +func installNpmPackage(ctx context.Context, target string) ([]byte, error) { + out, err := runNpmInstall(ctx, target) + if err == nil || !npmOutputHasWorkspaceProtocolError(string(out)) { + return out, err + } + + slog.Warn("skills: npm package contains workspace protocol deps; retrying with sanitized tarball", "target", target) + fallbackOut, fallbackErr := installNpmPackageWithWorkspaceRewrite(ctx, target) + if fallbackErr == nil { + return fallbackOut, nil + } + return appendNpmFallbackOutput(out, fallbackOut), fallbackErr +} + +func runNpmInstall(ctx context.Context, target string) ([]byte, error) { + if err := os.MkdirAll(npmGlobalPrefix(), 0o750); err != nil { + return nil, fmt.Errorf("npm prefix setup: %w", err) + } + ensureNpmGlobalEnv() + cmd := exec.CommandContext(ctx, npmBinary, "install", "-g", target) + cmd.Env = npmCommandEnv() + cmd.WaitDelay = 2 * time.Second + return cmd.CombinedOutput() +} + +func npmOutputHasWorkspaceProtocolError(out string) bool { + return strings.Contains(out, "EUNSUPPORTEDPROTOCOL") && + (strings.Contains(out, `Unsupported URL Type "workspace:"`) || strings.Contains(out, "workspace:")) +} + +func installNpmPackageWithWorkspaceRewrite(ctx context.Context, target string) ([]byte, error) { + tmpDir, err := os.MkdirTemp("", "goclaw-npm-workspace-*") + if err != nil { + return nil, fmt.Errorf("npm workspace fallback temp dir: %w", err) + } + defer os.RemoveAll(tmpDir) + + tarball, packOut, err := npmPackTarball(ctx, target, tmpDir) + if err != nil { + return packOut, err + } + + extractDir := filepath.Join(tmpDir, "extract") + if err := os.MkdirAll(extractDir, 0o750); err != nil { + return packOut, fmt.Errorf("npm workspace fallback extract dir: %w", err) + } + if err := extractNpmTarballToDir(tarball, extractDir); err != nil { + return packOut, err + } + + packageDir := filepath.Join(extractDir, "package") + rewrites, err := rewriteWorkspacePackageJSON(ctx, filepath.Join(packageDir, "package.json")) + if err != nil { + return packOut, err + } + if rewrites == 0 { + return packOut, errors.New("npm workspace fallback found no workspace dependencies to rewrite") + } + + installOut, err := runNpmInstall(ctx, packageDir) + return appendNpmFallbackOutput(packOut, installOut), err +} + +func npmPackTarball(ctx context.Context, target, destination string) (string, []byte, error) { + cmd := exec.CommandContext(ctx, npmBinary, "pack", "--json", "--pack-destination", destination, target) + cmd.Env = npmCommandEnv() + cmd.WaitDelay = 2 * time.Second + + var stdout, stderr bytes.Buffer + cmd.Stdout = &stdout + cmd.Stderr = &stderr + if err := cmd.Run(); err != nil { + out := appendNpmFallbackOutput(stdout.Bytes(), stderr.Bytes()) + return "", out, fmt.Errorf("npm pack fallback failed: %w", err) + } + + var entries []struct { + Filename string `json:"filename"` + } + if err := json.Unmarshal(stdout.Bytes(), &entries); err != nil { + return "", stdout.Bytes(), fmt.Errorf("npm pack fallback parse: %w", err) + } + if len(entries) == 0 || strings.TrimSpace(entries[0].Filename) == "" { + return "", stdout.Bytes(), errors.New("npm pack fallback returned no tarball") + } + + tarball := filepath.Join(destination, filepath.Base(entries[0].Filename)) + if _, err := os.Stat(tarball); err != nil { + return "", stdout.Bytes(), fmt.Errorf("npm pack fallback tarball missing: %w", err) + } + return tarball, stdout.Bytes(), nil +} + +func rewriteWorkspacePackageJSON(ctx context.Context, path string) (int, error) { + raw, err := os.ReadFile(path) + if err != nil { + return 0, fmt.Errorf("read package.json: %w", err) + } + + var pkg map[string]any + if err := json.Unmarshal(raw, &pkg); err != nil { + return 0, fmt.Errorf("parse package.json: %w", err) + } + + rewrites := 0 + for _, section := range []string{"dependencies", "optionalDependencies", "peerDependencies"} { + deps, ok := pkg[section].(map[string]any) + if !ok { + continue + } + for name, value := range deps { + spec, ok := value.(string) + if !ok || !strings.HasPrefix(spec, "workspace:") { + continue + } + resolved, err := resolveWorkspaceDependencySpec(ctx, name, spec) + if err != nil { + return rewrites, err + } + deps[name] = resolved + rewrites++ + } + } + + if rewrites == 0 { + return 0, nil + } + updated, err := json.MarshalIndent(pkg, "", " ") + if err != nil { + return 0, fmt.Errorf("encode package.json: %w", err) + } + updated = append(updated, '\n') + if err := os.WriteFile(path, updated, 0o600); err != nil { + return 0, fmt.Errorf("write package.json: %w", err) + } + return rewrites, nil +} + +func resolveWorkspaceDependencySpec(ctx context.Context, name, spec string) (string, error) { + suffix := strings.TrimSpace(strings.TrimPrefix(spec, "workspace:")) + switch suffix { + case "", "*": + return npmPackageVersionResolver(ctx, name) + case "^", "~": + version, err := npmPackageVersionResolver(ctx, name) + if err != nil { + return "", err + } + return suffix + version, nil + default: + if strings.HasPrefix(suffix, ".") || strings.HasPrefix(suffix, "/") { + return "", fmt.Errorf("unsupported workspace dependency path for %s: %s", name, spec) + } + return suffix, nil + } +} + +func npmViewPackageVersion(ctx context.Context, name string) (string, error) { + cmd := exec.CommandContext(ctx, npmBinary, "view", name, "version", "--json") + cmd.Env = npmCommandEnv() + cmd.WaitDelay = 2 * time.Second + out, err := cmd.Output() + if err != nil { + return "", fmt.Errorf("npm view %s version: %w", name, err) + } + var version string + if err := json.Unmarshal(out, &version); err != nil { + version = strings.Trim(strings.TrimSpace(string(out)), `"`) + } + if version == "" { + return "", fmt.Errorf("npm view %s version returned empty version", name) + } + return version, nil +} + +func extractNpmTarballToDir(tarball, destination string) error { + files, err := ExtractArchiveAs(tarball, filepath.Base(tarball), 50*1024*1024) + if err != nil { + return fmt.Errorf("extract npm tarball: %w", err) + } + + cleanDest := filepath.Clean(destination) + for _, file := range files { + target := filepath.Join(cleanDest, file.Name) + if !isPathWithin(target, cleanDest) { + return fmt.Errorf("npm tarball contains unsafe path: %s", file.Name) + } + if err := os.MkdirAll(filepath.Dir(target), 0o750); err != nil { + return fmt.Errorf("extract npm tarball parent: %w", err) + } + mode := file.Mode.Perm() + if mode == 0 { + mode = 0o600 + } + if err := os.WriteFile(target, file.Content, mode); err != nil { + return fmt.Errorf("extract npm tarball file: %w", err) + } + } + return nil +} + +func isPathWithin(path, parent string) bool { + cleanPath := filepath.Clean(path) + cleanParent := filepath.Clean(parent) + if cleanPath == cleanParent { + return true + } + return strings.HasPrefix(cleanPath, cleanParent+string(os.PathSeparator)) +} + +func appendNpmFallbackOutput(parts ...[]byte) []byte { + var out []byte + for _, part := range parts { + trimmed := bytes.TrimSpace(part) + if len(trimmed) == 0 { + continue + } + if len(out) > 0 { + out = append(out, '\n') + } + out = append(out, trimmed...) + } + return out +} diff --git a/internal/skills/npm_workspace_fallback_test.go b/internal/skills/npm_workspace_fallback_test.go new file mode 100644 index 00000000..a820431c --- /dev/null +++ b/internal/skills/npm_workspace_fallback_test.go @@ -0,0 +1,108 @@ +package skills + +import ( + "context" + "encoding/json" + "os" + "path/filepath" + "testing" +) + +func TestNpmOutputHasWorkspaceProtocolError(t *testing.T) { + out := `npm error code EUNSUPPORTEDPROTOCOL +npm error Unsupported URL Type "workspace:": workspace:*` + if !npmOutputHasWorkspaceProtocolError(out) { + t.Fatal("expected workspace protocol error to be detected") + } + if npmOutputHasWorkspaceProtocolError("npm error code ERESOLVE") { + t.Fatal("non-workspace npm error should not trigger fallback") + } +} + +func TestResolveWorkspaceDependencySpec(t *testing.T) { + orig := npmPackageVersionResolver + t.Cleanup(func() { npmPackageVersionResolver = orig }) + npmPackageVersionResolver = func(context.Context, string) (string, error) { + return "1.2.3", nil + } + + cases := []struct { + spec string + want string + }{ + {"workspace:*", "1.2.3"}, + {"workspace:^", "^1.2.3"}, + {"workspace:~", "~1.2.3"}, + {"workspace:^1.0.0", "^1.0.0"}, + } + for _, tc := range cases { + got, err := resolveWorkspaceDependencySpec(context.Background(), "@scope/core", tc.spec) + if err != nil { + t.Fatalf("resolveWorkspaceDependencySpec(%q) error: %v", tc.spec, err) + } + if got != tc.want { + t.Fatalf("resolveWorkspaceDependencySpec(%q) = %q, want %q", tc.spec, got, tc.want) + } + } +} + +func TestRewriteWorkspacePackageJSON(t *testing.T) { + orig := npmPackageVersionResolver + t.Cleanup(func() { npmPackageVersionResolver = orig }) + npmPackageVersionResolver = func(_ context.Context, name string) (string, error) { + switch name { + case "@agenttasks/core": + return "0.1.0", nil + case "react": + return "0.1.0", nil + default: + t.Fatalf("unexpected version lookup for %s", name) + return "", nil + } + } + + dir := t.TempDir() + path := filepath.Join(dir, "package.json") + raw := []byte(`{ + "name": "@agenttasks/cli", + "dependencies": { + "@agenttasks/core": "workspace:*", + "ws": "^8.18.3" + }, + "peerDependencies": { + "react": "workspace:^" + } +}`) + if err := os.WriteFile(path, raw, 0o600); err != nil { + t.Fatal(err) + } + + rewrites, err := rewriteWorkspacePackageJSON(context.Background(), path) + if err != nil { + t.Fatalf("rewriteWorkspacePackageJSON error: %v", err) + } + if rewrites != 2 { + t.Fatalf("rewrites = %d, want 2", rewrites) + } + + var pkg struct { + Dependencies map[string]string `json:"dependencies"` + PeerDependencies map[string]string `json:"peerDependencies"` + } + updated, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + if err := json.Unmarshal(updated, &pkg); err != nil { + t.Fatal(err) + } + if got := pkg.Dependencies["@agenttasks/core"]; got != "0.1.0" { + t.Fatalf("@agenttasks/core = %q, want 0.1.0", got) + } + if got := pkg.PeerDependencies["react"]; got != "^0.1.0" { + t.Fatalf("react = %q, want ^0.1.0", got) + } + if got := pkg.Dependencies["ws"]; got != "^8.18.3" { + t.Fatalf("ws = %q, want unchanged", got) + } +}