feat(setup): ask which package manager to use when the project is ambiguous

cmd/cmdtest.go

@@ -22,6 +22,40 @@ var StubbedSuccessResponse = `{
 // CallCmd runs the root command for integration-style tests. It passes isTerminal always true so
 // the default --output matches an interactive terminal (plaintext); non-TTY JSON defaults are
 // covered in root_test.go.
+// CallCmdCapturingStderr runs a command and returns stdout and stderr separately, so
+// a test can assert on output written deliberately to stderr — a transitional note,
+// say — without it being mistaken for parseable output.
+func CallCmdCapturingStderr(
+	t *testing.T,
+	clients APIClients,
+	trackerFn analytics.TrackerFn,
+	args []string,
+) (stdout []byte, stderr []byte, err error) {
+	rootCmd, err := NewRootCommand(
+		config.NewService(&resources.MockClient{}),
+		trackerFn,
+		clients,
+		"test",
+		false,
+		func() bool { return true },
+		nil,
+	)
+	require.NoError(t, err)
+	cmd := rootCmd.Cmd()
+	out, errOut := bytes.NewBufferString(""), bytes.NewBufferString("")
+	cmd.SetOut(out)
+	cmd.SetErr(errOut)
+	cmd.SetArgs(args)
+
+	tracker := trackerFn("", "", false)
+	if err := cmd.Execute(); err != nil {
+		tracker.SendCommandCompletedEvent(analytics.ERROR)
+		return out.Bytes(), errOut.Bytes(), err
+	}
+	tracker.SendCommandCompletedEvent(analytics.SUCCESS)
+	return out.Bytes(), errOut.Bytes(), nil
+}
+
 func CallCmd(
 	t *testing.T,
 	clients APIClients,

cmd/setup/detect.go

@@ -44,7 +44,17 @@ func runDetect(svc setup.Service) func(*cobra.Command, []string) error {
 
 		outputKind := cliflags.GetOutputKind(cmd)
 		if outputKind == "json" {
-			data, _ := json.Marshal(result)
+			// Candidates are added here rather than in the detection result: they
+			// report which tools are on this machine, which is not a fact about the
+			// project. Callers reading this need both.
+			payload := struct {
+				*setup.DetectResult
+				PackageManagerCandidates []setup.PMCandidate `json:"package_manager_candidates,omitempty"`
+			}{DetectResult: result}
+			if result.PackageManagerConfidence == setup.PMAmbiguous {
+				payload.PackageManagerCandidates = setup.PackageManagerChoiceFor(dir, result.SDKID).Candidates
+			}
+			data, _ := json.Marshal(payload)
 			fmt.Fprintln(cmd.OutOrStdout(), string(data))
 			return nil
 		}
@@ -53,7 +63,12 @@ func runDetect(svc setup.Service) func(*cobra.Command, []string) error {
 		if result.Framework != "" {
 			fmt.Fprintf(cmd.OutOrStdout(), "Framework: %s\n", result.Framework)
 		}
-		fmt.Fprintf(cmd.OutOrStdout(), "Package Manager: %s\n", result.PackageManager)
+		if result.PackageManagerConfidence == setup.PMAmbiguous {
+			fmt.Fprintf(cmd.OutOrStdout(), "Package Manager: %s (uncertain — %s)\n",
+				result.PackageManager, result.PackageManagerReason)
+		} else {
+			fmt.Fprintf(cmd.OutOrStdout(), "Package Manager: %s\n", result.PackageManager)
+		}
 		fmt.Fprintf(cmd.OutOrStdout(), "Recommended SDK: %s\n", result.SDKID)
 		if result.EntryPointExists {
 			fmt.Fprintf(cmd.OutOrStdout(), "Entry Point: %s\n", result.EntryPoint)

cmd/setup/install.go

@@ -34,6 +34,20 @@ func newInstallCmd(svc setup.Service) *cobra.Command {
 	return cmd
 }
 
+// candidateList renders the choices for an error message, marking the ones that
+// are not installed so the user isn't sent to a tool they'd have to install first.
+func candidateList(candidates []setup.PMCandidate) string {
+	names := make([]string, 0, len(candidates))
+	for _, c := range candidates {
+		if c.Installed {
+			names = append(names, c.Name)
+			continue
+		}
+		names = append(names, c.Name+" (not installed)")
+	}
+	return strings.Join(names, ", ")
+}
+
 func runInstall(svc setup.Service) func(*cobra.Command, []string) error {
 	return func(cmd *cobra.Command, args []string) error {
 		dir, _ := cmd.Flags().GetString(pathFlag)
@@ -48,6 +62,29 @@ func runInstall(svc setup.Service) func(*cobra.Command, []string) error {
 		sdkID, _ := cmd.Flags().GetString(sdkIDFlag)
 		pkgMgr, _ := cmd.Flags().GetString("package-manager")
 		dryRun, _ := cmd.Flags().GetBool(dryRunFlag)
+
+		// Without an explicit choice, read the project rather than falling back to
+		// pip or npm regardless of what the project uses. An ambiguous project is an
+		// error: guessing here would install with the wrong manager, and this command
+		// cannot ask.
+		if pkgMgr == "" {
+			choice := setup.PackageManagerChoiceFor(dir, sdkID)
+			if choice.Confidence == setup.PMAmbiguous {
+				return fmt.Errorf(
+					"cannot tell which package manager to use: %s\npass --package-manager with one of: %s",
+					choice.Reason, candidateList(choice.Candidates),
+				)
+			}
+			pkgMgr = choice.Name
+			// This used to fall through to npm or pip whatever the project used, so a
+			// caller that relied on that default now gets a different manager. Say so
+			// once, on stderr, where it cannot disturb output being parsed.
+			fmt.Fprintf(cmd.ErrOrStderr(),
+				"note: --package-manager was not given, so setup read the project and chose %q. "+
+					"This previously defaulted to npm or pip. Pass --package-manager to pin it.\n",
+				pkgMgr)
+		}
+
 		detection := &setup.DetectResult{
 			SDKID:          sdkID,
 			PackageManager: pkgMgr,

cmd/setup/model.go

@@ -33,6 +33,9 @@ const (
 	stepSelectEnvironment
 	stepDetect
 	stepSelectSDK
+	// stepSelectPackageManager is only reached when the project does not identify
+	// its package manager. A project that does skips straight to the plan.
+	stepSelectPackageManager
 	stepPlan
 	stepInstall
 	stepCreateFlag
@@ -66,6 +69,12 @@ type wizardModel struct {
 	// zero-value list.Model.
 	sdkListBuilt bool
 	sdkList      list.Model
+	// pmChoice is the package-manager verdict for the chosen SDK. It is non-nil only
+	// when the project was ambiguous and the user was asked, which also records that
+	// going back from the plan should return to the picker rather than the SDK list.
+	pmChoice    *setup.PMChoice
+	pmList      list.Model
+	pmListBuilt bool
 
 	selectedProject string
 	selectedEnv     string
@@ -118,6 +127,24 @@ func (s sdkItem) Title() string {
 func (s sdkItem) Description() string { return s.language }
 func (s sdkItem) FilterValue() string { return s.name }
 
+// pmItem is a package manager the user can pick. Installed state is shown but does
+// not disable the row: the user may be about to install the tool, and setup never
+// installs tooling on their behalf.
+type pmItem struct {
+	name      string
+	command   string
+	installed bool
+}
+
+func (p pmItem) Title() string {
+	if p.installed {
+		return p.name
+	}
+	return p.name + " (not installed)"
+}
+func (p pmItem) Description() string { return p.command }
+func (p pmItem) FilterValue() string { return p.name }
+
 type projectItem struct {
 	key  string
 	name string

cmd/setup/setup_test.go

@@ -220,6 +220,7 @@ func TestInstall_Plaintext(t *testing.T) {
 		"setup", "install",
 		"--access-token", "test-token",
 		"--sdk-id", "node-server",
+		"--package-manager", "npm",
 	}
 	output, err := cmd.CallCmd(
 		t,
@@ -241,6 +242,7 @@ func TestInstall_Plaintext_WithVersion(t *testing.T) {
 		"setup", "install",
 		"--access-token", "test-token",
 		"--sdk-id", "node-server",
+		"--package-manager", "npm",
 	}
 	output, err := cmd.CallCmd(
 		t,
@@ -319,6 +321,7 @@ func TestInstall_DryRun(t *testing.T) {
 		"setup", "install",
 		"--access-token", "test-token",
 		"--sdk-id", "node-server",
+		"--package-manager", "npm",
 		"--dry-run",
 	}
 	// No Installer provided: dry-run must not invoke it or shell out.
@@ -341,6 +344,7 @@ func TestInstall_JSON(t *testing.T) {
 		"setup", "install",
 		"--access-token", "test-token",
 		"--sdk-id", "node-server",
+		"--package-manager", "npm",
 		"--output", "json",
 	}
 	output, err := cmd.CallCmd(
@@ -362,6 +366,7 @@ func TestInstallStubReturnsError(t *testing.T) {
 		"setup", "install",
 		"--access-token", "test-token",
 		"--sdk-id", "node-server",
+		"--package-manager", "npm",
 	}
 	_, err := cmd.CallCmd(
 		t,
@@ -412,3 +417,34 @@ func TestInitMissingRequiredFlags(t *testing.T) {
 	require.Error(t, err)
 	assert.Contains(t, err.Error(), "required flag")
 }
+
+// Omitting --package-manager used to fall through to npm or pip. The default now
+// reads the project, so a caller relying on the old behaviour gets a note — on
+// stderr, so output being parsed is untouched.
+func TestInstall_AutoSelectedManager_WarnsOnStderr(t *testing.T) {
+	dir := t.TempDir()
+	require.NoError(t, os.WriteFile(filepath.Join(dir, "package.json"),
+		[]byte(`{"packageManager":"pnpm@9.1.0"}`), 0600))
+
+	args := []string{
+		"setup", "install",
+		"--access-token", "test-token",
+		"--sdk-id", "node-server",
+		"--path", dir,
+		"--dry-run",
+	}
+	stdout, stderr, err := cmd.CallCmdCapturingStderr(
+		t,
+		cmd.APIClients{ResourcesClient: &resources.MockClient{}},
+		analytics.NoopClientFn{}.Tracker(),
+		args,
+	)
+
+	require.NoError(t, err)
+	assert.Contains(t, string(stderr), "--package-manager was not given")
+	assert.Contains(t, string(stderr), "previously defaulted to npm or pip")
+	assert.Contains(t, string(stderr), "pnpm")
+	// The note must not reach output a caller parses.
+	assert.NotContains(t, string(stdout), "--package-manager was not given")
+	assert.Contains(t, string(stdout), "pnpm add @launchdarkly/node-server-sdk")
+}