diff --git a/cmd/cmdtest.go b/cmd/cmdtest.go index 54ea73bd..90e65c7f 100644 --- a/cmd/cmdtest.go +++ b/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, diff --git a/cmd/setup/detect.go b/cmd/setup/detect.go index b7d0ced2..aa14e873 100644 --- a/cmd/setup/detect.go +++ b/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) diff --git a/cmd/setup/install.go b/cmd/setup/install.go index 42b85465..96628eb4 100644 --- a/cmd/setup/install.go +++ b/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, diff --git a/cmd/setup/model.go b/cmd/setup/model.go index 15563393..fe09b7c9 100644 --- a/cmd/setup/model.go +++ b/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 diff --git a/cmd/setup/setup_test.go b/cmd/setup/setup_test.go index 17130c2f..78a22b78 100644 --- a/cmd/setup/setup_test.go +++ b/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") +} diff --git a/cmd/setup/update.go b/cmd/setup/update.go index a14b8e4e..a484a3c9 100644 --- a/cmd/setup/update.go +++ b/cmd/setup/update.go @@ -29,6 +29,9 @@ func (m wizardModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { if m.sdkListBuilt { m.sdkList.SetSize(m.sdkBoxWidth()-2, m.sdkList.Height()) } + if m.pmListBuilt { + m.pmList.SetSize(m.sdkBoxWidth(), m.pmListHeight()) + } case tea.KeyMsg: switch msg.String() { @@ -79,6 +82,7 @@ func (m wizardModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { m.projectList.Title = "Select a project:" m.projectList.SetShowStatusBar(false) keepEscFromQuitting(&m.projectList) + m.projectList.AdditionalShortHelpKeys = listHints(false) return m, nil case envsFetchedMsg: @@ -96,6 +100,7 @@ func (m wizardModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { m.envList.Title = "Select an environment:" m.envList.SetShowStatusBar(false) keepEscFromQuitting(&m.envList) + m.envList.AdditionalShortHelpKeys = listHints(true) return m, nil case envDetailsFetchedMsg: @@ -178,6 +183,10 @@ func (m wizardModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { if len(m.environments) > 0 { m.envList, cmd = m.envList.Update(msg) } + case stepSelectPackageManager: + if m.pmListBuilt { + m.pmList, cmd = m.pmList.Update(msg) + } case stepSelectSDK: // Two panels when a detected SDK is shown: the detected panel (focus 0) // and the list of other SDKs (focus 1). Arrows move focus between them. @@ -225,6 +234,8 @@ func (m wizardModel) isFiltering() bool { return m.projectList.FilterState() == list.Filtering case stepSelectEnvironment: return m.envList.FilterState() == list.Filtering + case stepSelectPackageManager: + return m.pmList.FilterState() == list.Filtering case stepSelectSDK: return m.sdkList.FilterState() == list.Filtering } @@ -252,6 +263,60 @@ func (m *wizardModel) enterSDKStep() { m.step = stepSelectSDK } +// listHints adds the wizard's own bindings to a list's help line. Screens showed +// the list's help and a footer of ours, so every instruction appeared twice; the +// list's help is the one place they belong. +func listHints(back bool) func() []key.Binding { + return func() []key.Binding { + hints := []key.Binding{ + key.NewBinding(key.WithKeys("enter"), key.WithHelp("enter", "select")), + } + if back { + hints = append(hints, key.NewBinding(key.WithKeys("left"), key.WithHelp("←", "back"))) + } + return hints + } +} + +// enterPackageManagerStep builds the picker from the ambiguous verdict. Installed +// managers are listed first and the cursor starts on one, but an uninstalled +// manager stays selectable: the choice is the user's, and setup warns at install +// time rather than installing the tool itself. +func (m *wizardModel) enterPackageManagerStep() { + installed := make([]list.Item, 0, len(m.pmChoice.Candidates)) + missing := make([]list.Item, 0, len(m.pmChoice.Candidates)) + for _, c := range m.pmChoice.Candidates { + item := pmItem{name: c.Name, command: c.Command, installed: c.Installed} + if c.Installed { + installed = append(installed, item) + continue + } + missing = append(missing, item) + } + items := append(installed, missing...) + + m.pmList = list.New(items, list.NewDefaultDelegate(), m.sdkBoxWidth(), m.pmListHeight()) + m.pmList.Title = "Select a package manager:" + m.pmList.SetShowStatusBar(false) + keepEscFromQuitting(&m.pmList) + m.pmList.AdditionalShortHelpKeys = listHints(true) + m.pmListBuilt = true + m.step = stepSelectPackageManager +} + +// enterPlanStep computes the preview shown before anything is written or run. +func (m *wizardModel) enterPlanStep() { + // Resolved against the project directory so the previewed command is the one + // that runs, virtualenv pip included. + dir, _ := os.Getwd() + args, _ := setup.InstallArgs(dir, m.detectResult.SDKID, m.detectResult.PackageManager) + m.planInstallCmd = strings.Join(args, " ") + if dir != "" { + m.planAlready = setup.IsInstalled(dir, m.detectResult.SDKID) + } + m.step = stepPlan +} + // acceptsEnvs reports whether an environment list still describes the project the // user has selected, and whether the wizard is still choosing one. A list fetched // for a project the user has since left would otherwise be shown under the new @@ -300,7 +365,15 @@ func (m wizardModel) handleBack() (tea.Model, tea.Cmd) { m.resetEnvSelection() case stepSelectSDK: m.step = stepSelectEnvironment + case stepSelectPackageManager: + m.step = stepSelectSDK case stepPlan: + // The picker only exists for an ambiguous project, so going back must return + // to whichever screen the user actually came from. + if m.pmChoice != nil { + m.step = stepSelectPackageManager + break + } m.step = stepSelectSDK } return m, nil @@ -365,15 +438,32 @@ func (m wizardModel) handleEnter() (tea.Model, tea.Cmd) { } } m.detectResult = &result - // Compute the plan preview shown before any action is taken. It is resolved - // against the project directory so the previewed command is the one that runs. - planDir, _ := os.Getwd() - args, _ := setup.InstallArgs(planDir, chosen.id, result.PackageManager) - m.planInstallCmd = strings.Join(args, " ") - if planDir != "" { - m.planAlready = setup.IsInstalled(planDir, chosen.id) - } - m.step = stepPlan + + // Ask which manager to use when the project doesn't say. Picking one for the + // user here is how a yarn project ends up installed with npm. + m.pmChoice = nil + if dir, err := os.Getwd(); err == nil { + if choice := setup.PackageManagerChoiceFor(dir, chosen.id); choice.Confidence == setup.PMAmbiguous { + m.pmChoice = &choice + m.enterPackageManagerStep() + return m, nil + } else if choice.Name != "" { + result.PackageManager = choice.Name + m.detectResult = &result + } + } + m.enterPlanStep() + return m, nil + + case stepSelectPackageManager: + selected, ok := m.pmList.SelectedItem().(pmItem) + if !ok { + return m, nil + } + result := *m.detectResult + result.PackageManager = selected.name + m.detectResult = &result + m.enterPlanStep() return m, nil case stepPlan: diff --git a/cmd/setup/view.go b/cmd/setup/view.go index a6ce9ec6..d47de6b0 100644 --- a/cmd/setup/view.go +++ b/cmd/setup/view.go @@ -47,7 +47,7 @@ func (m wizardModel) View() string { m.wrap("This access token can't see any projects. Create a project in LaunchDarkly, or use a token with access to one, then run this command again.") + "\n" + quitHint } - return m.projectList.View() + "\n" + mutedStyle.Render("q quit") + return m.projectList.View() case stepSelectEnvironment: if !m.envsLoaded { @@ -58,7 +58,7 @@ func (m wizardModel) View() string { m.wrap(fmt.Sprintf("Project %q has no environments this access token can see. Press ← to pick another project.", m.selectedProject)) + "\n" + mutedStyle.Render("← back · q quit") + "\n" } - return m.envList.View() + "\n" + mutedStyle.Render("← back · q quit") + return m.envList.View() case stepDetect: return m.spinner.View() + " Detecting project type..." @@ -66,6 +66,9 @@ func (m wizardModel) View() string { case stepSelectSDK: return m.sdkSelectView() + case stepSelectPackageManager: + return m.packageManagerView() + case stepPlan: return m.planView() @@ -79,17 +82,20 @@ func (m wizardModel) View() string { return m.spinner.View() + " Injecting initialization code..." case stepWaitForApp: - lead := "SDK initialization code has been injected into:\n" + // The newline stays outside the wrap: wrapping pads each line to the full + // width, so a trailing one inside would put a row of spaces in front of the + // path and push it past the edge of the terminal. + lead := "SDK initialization code has been injected into:" if m.initResult.AlreadyInitialized { - lead = "This file already initializes the LaunchDarkly SDK, so it was left as it is:\n" + lead = "This file already initializes the LaunchDarkly SDK, so it was left as it is:" } return titleStyle.Render("Start your application") + "\n\n" + - lead + - " " + m.initResult.FilePath + "\n\n" + - "Please start your application now, then press Enter to verify the connection.\n" + m.wrap(lead) + "\n" + + m.wrap(" "+m.initResult.FilePath) + "\n\n" + + m.wrap("Please start your application now, then press Enter to verify the connection.") + "\n" case stepVerify: - return m.spinner.View() + " Waiting for SDK to connect..." + return m.spinner.View() + " Waiting for your app to start and its SDK to connect..." case stepDone: if m.installResult != nil && m.installResult.Failed { @@ -188,6 +194,46 @@ func (m wizardModel) sdkBoxWidth() int { return w } +// pmListHeight is the height available to the package-manager list. The screen +// draws a title, the reason it is asking and a key hint around the list, so giving +// the list the whole window pushes the hint — including how to go back — off the +// bottom of the terminal. +func (m wizardModel) pmListHeight() int { + chrome := 3 // the question, a blank line, and the list's own trailing row + if m.pmShowReason() { + chrome += 3 // the reason, which wraps to two lines when narrow + } + // The list's help line runs to about seventy columns, so on anything narrower + // it wraps and costs a second row. + if m.width < 72 { + chrome++ + } + h := m.height - chrome + if h < 3 { + h = 3 + } + return h +} + +// pmShowReason reports whether there is room to explain why we are asking. On a +// very short terminal the question and the choices have to win: dropping the +// explanation is better than pushing the key hint off the bottom. +func (m wizardModel) pmShowReason() bool { return m.height >= 14 } + +// packageManagerView asks which package manager to use. It says why it is asking: +// a wizard that stops to ask without explaining itself reads as one that failed to +// look, and the reason is also what tells the user whether our reading of their +// project is wrong. +func (m wizardModel) packageManagerView() string { + reason := "" + if m.pmShowReason() && m.pmChoice != nil && m.pmChoice.Reason != "" { + reason = m.wrap(strings.ToUpper(m.pmChoice.Reason[:1])+m.pmChoice.Reason[1:]+".") + "\n\n" + } + return titleStyle.Render(m.wrap("Which package manager should install the SDK?")) + "\n\n" + + reason + + m.pmList.View() +} + // addCodeTo phrases an "add this code" instruction. SDKs that only show a snippet // have no entry point, so naming a destination would print an empty path. func addCodeTo(instruction, path string) string { @@ -290,7 +336,14 @@ func (m wizardModel) planView() string { var steps []string add := func(s string) { - steps = append(steps, selectedStyle.Render(fmt.Sprintf("%d.", len(steps)+1))+" "+s) + marker := selectedStyle.Render(fmt.Sprintf("%d.", len(steps)+1)) + // Wrap to leave room for the marker and indent what wraps, so a step too long + // for the terminal still reads as one numbered item instead of overflowing. + lines := strings.Split(wrapText(s, m.width-len("1. ")), "\n") + for i := 1; i < len(lines); i++ { + lines[i] = strings.Repeat(" ", len("1. ")) + lines[i] + } + steps = append(steps, marker+" "+strings.Join(lines, "\n")) } switch { diff --git a/cmd/setup/wizard_test.go b/cmd/setup/wizard_test.go index f3897627..9647c02e 100644 --- a/cmd/setup/wizard_test.go +++ b/cmd/setup/wizard_test.go @@ -1,8 +1,10 @@ package setup import ( + "fmt" "os" "path/filepath" + "strings" "testing" "github.com/charmbracelet/bubbles/spinner" @@ -342,8 +344,8 @@ func TestWizard_Plan_ExistingEntryPoint_SaysAdd(t *testing.T) { } view := m.planView() - assert.Contains(t, view, "Add initialization code to src/index.js") - assert.NotContains(t, view, "Create src/index.js") + assert.Contains(t, flat(view), "Add initialization code to src/index.js") + assert.NotContains(t, flat(view), "Create src/index.js") } // A guessed entry point means we would write a file the project does not load, so @@ -363,14 +365,16 @@ func TestWizard_Plan_MissingEntryPoint_SaysCreate(t *testing.T) { } view := m.planView() - assert.Contains(t, view, "Create instrumentation.ts") - assert.Contains(t, view, "no entry file found") - assert.NotContains(t, view, "Add initialization code to") + assert.Contains(t, flat(view), "Create instrumentation.ts") + assert.Contains(t, flat(view), "no entry file found") + assert.NotContains(t, flat(view), "Add initialization code to") } // The SDK screen rebuilds detectResult, and the plan and install steps read it, so // every detected value has to survive that step — not just the SDK. func TestWizard_SelectSDK_CarriesDetectionThrough(t *testing.T) { + // A Gemfile makes Bundler the project's stated manager, so the picker is skipped. + gemfileProject(t) m := wizardModel{step: stepDetect, width: 80, height: 30} next, _ := m.Update(detectDoneMsg{result: &setup.DetectResult{ @@ -395,6 +399,7 @@ func TestWizard_SelectSDK_CarriesDetectionThrough(t *testing.T) { } func TestWizard_SelectSDK_PlanUsesDetectedPackageManager(t *testing.T) { + gemfileProject(t) m := wizardModel{step: stepDetect, width: 80, height: 30} next, _ := m.Update(detectDoneMsg{result: &setup.DetectResult{ @@ -425,6 +430,8 @@ func selectOtherSDK(t *testing.T, m wizardModel, id string) wizardModel { // The detected entry point belongs to the detected language. ruby-server-sdk is // append-safe, so reusing it would append Ruby to a Node project's index.js. func TestWizard_OverrideSDK_DoesNotReuseDetectedEntryPoint(t *testing.T) { + // A Gemfile states the manager, so the override lands on the plan without asking. + gemfileProject(t) m := wizardModel{step: stepDetect, width: 80, height: 30} next, _ := m.Update(detectDoneMsg{result: &setup.DetectResult{ SDKID: "node-server", @@ -448,7 +455,7 @@ func TestWizard_OverrideSDK_DoesNotReuseDetectedEntryPoint(t *testing.T) { assert.Contains(t, m3.detectResult.EntryPoint, "main.rb") assert.Empty(t, m3.detectResult.Framework, "Next.js does not describe a Ruby project") // pnpm cannot install a gem, so the manager is re-derived for the chosen SDK. - assert.Equal(t, "gem", m3.detectResult.PackageManager) + assert.Equal(t, "bundle", m3.detectResult.PackageManager) } // An override must find the file the project already has, rather than falling back @@ -457,6 +464,8 @@ func TestWizard_OverrideSDK_FindsExistingEntryPoint(t *testing.T) { dir := t.TempDir() require.NoError(t, os.MkdirAll(filepath.Join(dir, "src"), 0755)) require.NoError(t, os.WriteFile(filepath.Join(dir, "src/index.js"), []byte("console.log(1)\n"), 0600)) + // A lockfile states the manager, so the override lands on the plan without asking. + require.NoError(t, os.WriteFile(filepath.Join(dir, "package-lock.json"), []byte("{}"), 0600)) // macOS resolves /var to /private/var, and the override path reads os.Getwd, // so compare against the resolved directory rather than the one we created. dir = chdir(t, dir) @@ -477,6 +486,15 @@ func TestWizard_OverrideSDK_FindsExistingEntryPoint(t *testing.T) { assert.True(t, m3.detectResult.EntryPointExists) } +// gemfileProject moves into a project whose package manager is unambiguous, so the +// package-manager picker does not intervene. +func gemfileProject(t *testing.T) string { + t.Helper() + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, "Gemfile"), []byte("source 'https://rubygems.org'\n"), 0600)) + return chdir(t, dir) +} + // chdir moves into dir for the duration of the test and returns the working // directory as the process sees it. The override path reads os.Getwd to re-derive // the entry point. @@ -538,6 +556,12 @@ func overrideToSDK(t *testing.T, detected *setup.DetectResult, id string) wizard m2 := selectOtherSDK(t, next.(wizardModel), id) next2, _ := m2.Update(tea.KeyMsg{Type: tea.KeyEnter}) m3 := next2.(wizardModel) + // An override into a project that doesn't state its package manager asks first. + // These callers are about entry points, so accept the highlighted manager. + if m3.step == stepSelectPackageManager { + next3, _ := m3.Update(tea.KeyMsg{Type: tea.KeyEnter}) + m3 = next3.(wizardModel) + } require.Equal(t, stepPlan, m3.step) return m3 } @@ -558,8 +582,8 @@ func TestWizard_OverrideSDK_DefaultEntryPointAlreadyPresent(t *testing.T) { assert.Equal(t, filepath.Join(dir, "main.rb"), m.detectResult.EntryPoint) assert.True(t, m.detectResult.EntryPointExists) view := m.View() - assert.Contains(t, view, "Add initialization code to") - assert.NotContains(t, view, "no entry file found") + assert.Contains(t, flat(view), "Add initialization code to") + assert.NotContains(t, flat(view), "no entry file found") } func TestWizard_OverrideSDK_DefaultEntryPointMissing(t *testing.T) { @@ -571,7 +595,7 @@ func TestWizard_OverrideSDK_DefaultEntryPointMissing(t *testing.T) { }, "ruby-server-sdk") assert.False(t, m.detectResult.EntryPointExists) - assert.Contains(t, m.View(), "no entry file found") + assert.Contains(t, flat(m.View()), "no entry file found") } func TestWizard_Done_DeclinedInstall_ShowsReasonWithoutCommand(t *testing.T) { @@ -854,3 +878,261 @@ func TestWizard_EnvsFetched_ForSupersededProject_IsIgnored(t *testing.T) { fresh, _ := m.Update(envsFetchedMsg{project: "proj-b", environments: []envItem{{key: "b-prod", name: "B Prod"}}}) assert.Contains(t, fresh.(wizardModel).View(), "B Prod") } + +// A project that states its manager must not be interrupted; the happy path gains +// no keystrokes from the picker existing. +func TestWizard_DefinitePackageManager_SkipsPicker(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, "package.json"), []byte(`{"packageManager":"pnpm@9.1.0"}`), 0600)) + chdir(t, dir) + + m := wizardModel{step: stepDetect, width: 80, height: 30} + next, _ := m.Update(detectDoneMsg{result: &setup.DetectResult{ + SDKID: "node-server", Language: "JavaScript", PackageManager: "pnpm", + }}) + next2, _ := next.(wizardModel).Update(tea.KeyMsg{Type: tea.KeyEnter}) + m3 := next2.(wizardModel) + + require.Equal(t, stepPlan, m3.step) + assert.Nil(t, m3.pmChoice, "nothing was ambiguous, so nothing was asked") + assert.Equal(t, "pnpm add @launchdarkly/node-server-sdk", m3.planInstallCmd) +} + +// Two lockfiles from different managers is the case no guess can get right. +func TestWizard_ConflictingLockfiles_AsksAndUsesTheAnswer(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, "package.json"), []byte(`{}`), 0600)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "yarn.lock"), []byte(""), 0600)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "package-lock.json"), []byte("{}"), 0600)) + chdir(t, dir) + + m := wizardModel{step: stepDetect, width: 80, height: 30} + next, _ := m.Update(detectDoneMsg{result: &setup.DetectResult{ + SDKID: "node-server", Language: "JavaScript", PackageManager: "yarn", + }}) + next2, _ := next.(wizardModel).Update(tea.KeyMsg{Type: tea.KeyEnter}) + picker := next2.(wizardModel) + + require.Equal(t, stepSelectPackageManager, picker.step) + require.NotNil(t, picker.pmChoice) + assert.Contains(t, picker.pmChoice.Reason, "more than one manager") + + // The view has to say why it is asking, or it reads as a tool that failed to look. + view := picker.View() + assert.Contains(t, view, "Which package manager") + assert.Contains(t, view, "more than one manager") + + // Pick whatever is highlighted and confirm the plan follows the answer. + next3, _ := picker.Update(tea.KeyMsg{Type: tea.KeyEnter}) + planned := next3.(wizardModel) + require.Equal(t, stepPlan, planned.step) + selected := planned.detectResult.PackageManager + assert.Contains(t, planned.planInstallCmd, selected, + "the plan must run the manager the user chose") +} + +// Installed managers come first and the cursor starts on one, but an uninstalled +// manager stays selectable — setup never installs tooling for the user. +func TestWizard_Picker_ListsInstalledFirstAndKeepsMissingSelectable(t *testing.T) { + m := wizardModel{step: stepSelectSDK, width: 80, height: 30} + m.detectResult = &setup.DetectResult{SDKID: "node-server"} + m.pmChoice = &setup.PMChoice{ + Name: "npm", + Confidence: setup.PMAmbiguous, + Reason: "this project doesn't say which package manager it uses", + Candidates: []setup.PMCandidate{ + {Name: "npm", Installed: false, Command: "npm install x"}, + {Name: "yarn", Installed: true, Command: "yarn add x"}, + {Name: "pnpm", Installed: true, Command: "pnpm add x"}, + }, + } + m.enterPackageManagerStep() + + items := m.pmList.Items() + require.Len(t, items, 3) + assert.Equal(t, "yarn", items[0].(pmItem).name, "installed managers come first") + assert.Equal(t, "pnpm", items[1].(pmItem).name) + assert.Equal(t, "npm", items[2].(pmItem).name) + assert.Contains(t, items[2].(pmItem).Title(), "not installed") + + // Selecting the uninstalled one is allowed; the install step warns later. + m.pmList.Select(2) + next, _ := m.Update(tea.KeyMsg{Type: tea.KeyEnter}) + chosen := next.(wizardModel) + require.Equal(t, stepPlan, chosen.step) + assert.Equal(t, "npm", chosen.detectResult.PackageManager) +} + +// The picker's list quits on esc for the same reason the others did. +func TestWizard_Picker_EscDoesNotQuit(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, "package.json"), []byte(`{}`), 0600)) + chdir(t, dir) + + m := wizardModel{step: stepDetect, width: 80, height: 24} + next, _ := m.Update(detectDoneMsg{result: &setup.DetectResult{ + SDKID: "node-server", Language: "JavaScript", + }}) + next2, _ := next.(wizardModel).Update(tea.KeyMsg{Type: tea.KeyEnter}) + picker := next2.(wizardModel) + require.Equal(t, stepSelectPackageManager, picker.step) + + after, cmd := picker.Update(tea.KeyMsg{Type: tea.KeyEsc}) + assert.False(t, after.(wizardModel).quitting) + assert.False(t, quitsOn(cmd), "the list quit the wizard on esc") + assert.Equal(t, stepSelectPackageManager, after.(wizardModel).step) +} + +// Back must return to the picker, not skip over it to the SDK list. +func TestWizard_Picker_BackReturnsToPickerFromPlan(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, "package.json"), []byte(`{}`), 0600)) + chdir(t, dir) + + m := wizardModel{step: stepDetect, width: 80, height: 30} + next, _ := m.Update(detectDoneMsg{result: &setup.DetectResult{ + SDKID: "node-server", Language: "JavaScript", + }}) + next2, _ := next.(wizardModel).Update(tea.KeyMsg{Type: tea.KeyEnter}) + picker := next2.(wizardModel) + require.Equal(t, stepSelectPackageManager, picker.step) + + next3, _ := picker.Update(tea.KeyMsg{Type: tea.KeyEnter}) + planned := next3.(wizardModel) + require.Equal(t, stepPlan, planned.step) + + back, _ := planned.Update(tea.KeyMsg{Type: tea.KeyLeft}) + assert.Equal(t, stepSelectPackageManager, back.(wizardModel).step) + + backAgain, _ := back.(wizardModel).Update(tea.KeyMsg{Type: tea.KeyLeft}) + assert.Equal(t, stepSelectSDK, backAgain.(wizardModel).step) +} + +// The picker draws its question and the reason for asking around the list, so the +// list has to be sized for less than the whole window or the instructions are +// pushed off the bottom. Rows are counted the way a terminal shows them, with +// over-wide lines wrapping. +// +// Widths below 72 are left out: the list widget's own help line runs to about +// seventy columns and wraps there. That affects every list screen in the wizard, +// not this one, and no height reserve fixes it. +func TestWizard_Picker_FitsTerminalHeight(t *testing.T) { + for _, dims := range [][2]int{{100, 30}, {80, 30}, {80, 24}, {80, 20}, {80, 16}} { + t.Run(fmt.Sprintf("%dx%d", dims[0], dims[1]), func(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, "package.json"), []byte(`{}`), 0600)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "yarn.lock"), []byte(""), 0600)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "package-lock.json"), []byte("{}"), 0600)) + chdir(t, dir) + + m := wizardModel{step: stepDetect, width: dims[0], height: dims[1]} + next, _ := m.Update(detectDoneMsg{result: &setup.DetectResult{ + SDKID: "node-server", Language: "JavaScript", + }}) + next2, _ := next.(wizardModel).Update(tea.KeyMsg{Type: tea.KeyEnter}) + picker := next2.(wizardModel) + require.Equal(t, stepSelectPackageManager, picker.step) + + view := picker.View() + assert.LessOrEqual(t, terminalRows(view, dims[0]), dims[1], + "the instructions would be pushed off the bottom") + // However short the terminal, the way out must stay on screen. + assert.Contains(t, view, "back") + assert.Contains(t, view, "Which package manager") + }) + } +} + +// flat collapses whitespace in a rendered view, so assertions about a phrase hold +// wherever wrapping happens to fall. +func flat(view string) string { return strings.Join(strings.Fields(view), " ") } + +// terminalRows counts the rows a terminal of the given width would use, so a line +// wider than the window counts as the several rows it actually occupies. +func terminalRows(view string, width int) int { + rows := 0 + for _, line := range strings.Split(strings.TrimRight(view, "\n"), "\n") { + if w := len([]rune(line)); w > width { + rows += (w + width - 1) / width + continue + } + rows++ + } + return rows +} + +// The plan names an absolute entry-point path and explains why it is creating the +// file, which together run well past a narrow terminal. Overflowing there hides +// the very warning the step exists to give. +func TestWizard_Plan_WrapsStepsToTerminalWidth(t *testing.T) { + for _, width := range []int{100, 80, 60, 40} { + t.Run(fmt.Sprintf("width%d", width), func(t *testing.T) { + m := wizardModel{ + step: stepPlan, + selectedProject: "my-scratch-project", + selectedEnv: "production", + detectResult: &setup.DetectResult{ + SDKID: "python-server-sdk", + EntryPoint: "/Users/someone/code/launchdarkly/test-app/main.py", + EntryPointExists: false, + }, + planInstallCmd: "pip3 install launchdarkly-server-sdk", + width: width, + height: 30, + } + + view := m.planView() + + for _, line := range strings.Split(view, "\n") { + assert.LessOrEqual(t, len([]rune(line)), width, + "a plan step overflows a %d-column terminal", width) + } + // The warning must survive wrapping, not be truncated away. + assert.Contains(t, flat(view), "no entry file found") + assert.Contains(t, flat(view), "main.py") + // Wrapped text is indented under its number so the step still reads as one. + assert.Regexp(t, `(?m)^ {3}\S`, view) + }) + } +} + +// Wrapping pads every line to the full width, so a newline left inside a wrapped +// string put a whole row of spaces in front of the injected file path and pushed it +// off the terminal. +func TestWizard_WaitForApp_WrapsWithoutLeadingPadding(t *testing.T) { + for _, width := range []int{80, 60, 40} { + for _, already := range []bool{false, true} { + t.Run(fmt.Sprintf("width%d_already%v", width, already), func(t *testing.T) { + path := "/Users/someone/code/launchdarkly/test-app/main.py" + m := wizardModel{ + step: stepWaitForApp, + width: width, + height: 24, + initResult: &setup.InitResult{ + FilePath: path, + AlreadyInitialized: already, + }, + } + + view := m.View() + + for _, line := range strings.Split(view, "\n") { + assert.LessOrEqual(t, len([]rune(line)), width, + "a line overflows a %d-column terminal", width) + } + // The path must start near the left edge, not after a row of padding. + for _, line := range strings.Split(view, "\n") { + if idx := strings.Index(line, "/Users/someone"); idx >= 0 { + assert.LessOrEqual(t, idx, 2, "the path is pushed right by padding") + } + } + // A path has no spaces to wrap on, so a narrow terminal hard-breaks it. + // Compare with whitespace removed to check nothing was lost. + assert.Contains(t, strings.Join(strings.Fields(view), ""), path) + if already { + assert.Contains(t, flat(view), "already initializes the LaunchDarkly SDK") + } + }) + } + } +} diff --git a/go.mod b/go.mod index 0449eb4a..87c1277d 100644 --- a/go.mod +++ b/go.mod @@ -27,6 +27,7 @@ require ( github.com/muesli/reflow v0.3.0 github.com/oapi-codegen/oapi-codegen/v2 v2.7.1 github.com/oapi-codegen/runtime v1.1.2 + github.com/pelletier/go-toml/v2 v2.2.4 github.com/pkg/browser v0.0.0-20240102092130-5ac0b6a4141c github.com/pkg/errors v0.9.1 github.com/samber/lo v1.51.0 @@ -81,7 +82,6 @@ require ( github.com/oasdiff/yaml3 v0.0.14 // indirect github.com/onsi/gomega v1.27.6 // indirect github.com/patrickmn/go-cache v2.1.0+incompatible // indirect - github.com/pelletier/go-toml/v2 v2.2.4 // indirect github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect github.com/rivo/uniseg v0.4.7 // indirect github.com/sagikazarmark/locafero v0.11.0 // indirect diff --git a/internal/setup/detector.go b/internal/setup/detector.go index 5d9db9b8..5869f091 100644 --- a/internal/setup/detector.go +++ b/internal/setup/detector.go @@ -1,13 +1,16 @@ package setup import ( - "bytes" "encoding/json" "errors" + "fmt" "io/fs" "os" "path/filepath" + "regexp" "strings" + + "github.com/pelletier/go-toml/v2" ) // DetectResult contains information about the user's project detected from the working directory. @@ -21,6 +24,12 @@ type DetectResult struct { // suggest. Callers must not write initialization code into a suggested path // without telling the user, since the project does not load that file. EntryPointExists bool `json:"entry_point_exists"` + // PackageManagerConfidence says whether the project identifies its package + // manager or PackageManager is only a conventional default. Callers must not + // install against an ambiguous verdict without asking first. + PackageManagerConfidence PMConfidence `json:"package_manager_confidence,omitempty"` + // PackageManagerReason explains an ambiguous verdict, phrased for the user. + PackageManagerReason string `json:"package_manager_reason,omitempty"` } // Detector inspects a directory to determine the language, framework, package manager, @@ -60,6 +69,18 @@ func (FileDetector) Detect(dir string) (*DetectResult, error) { detectNode, } { if result := detect(dir); result != nil { + // One read of the project decides the manager and how sure we are, rather + // than each detector working it out again. An empty name means a language + // this does not model — Java's maven versus gradle — so the detector's own + // answer stands. Candidates carry which tools are installed, which + // describes the machine rather than the project, so they are left to + // callers that need to present a choice. + choice := PackageManagerChoiceFor(dir, result.SDKID) + if choice.Name != "" { + result.PackageManager = choice.Name + } + result.PackageManagerConfidence = choice.Confidence + result.PackageManagerReason = choice.Reason return result, nil } } @@ -184,19 +205,65 @@ func detectNode(dir string) *DetectResult { } func detectNodePM(dir string) string { - if _, err := os.Stat(filepath.Join(dir, "pnpm-lock.yaml")); err == nil { - return "pnpm" + return nodePMSignals(dir).best("npm") +} + +// exactSemver matches the exact versions corepack requires: a bare MAJOR.MINOR.PATCH +// with optional prerelease and build metadata, and no range operator. +var exactSemver = regexp.MustCompile(`^\d+\.\d+\.\d+(-[0-9A-Za-z.-]+)?(\+[0-9A-Za-z.-]+)?$`) + +// corepackPM reads the packageManager field, which names the manager and version +// the project expects. It is the most explicit statement a Node project can make, +// so it outranks lockfiles. +// https://nodejs.org/api/corepack.html +func corepackPM(dir string) string { + b, err := os.ReadFile(filepath.Join(dir, "package.json")) + if err != nil { + return "" } - if _, err := os.Stat(filepath.Join(dir, "yarn.lock")); err == nil { - return "yarn" + var pkg struct { + PackageManager string `json:"packageManager"` + } + if json.Unmarshal(b, &pkg) != nil { + return "" } - // Lockfiles: https://bun.com/docs/install/lockfile - for _, lock := range []string{"bun.lock", "bun.lockb"} { - if _, err := os.Stat(filepath.Join(dir, lock)); err == nil { - return "bun" + // The field must be "@". Corepack accepts nothing else, so + // a missing version ("No version specified for pnpm in packageManager") or a + // range ("Invalid package manager specification (pnpm@^11.13.0); expected a + // semver version") both stop the manager from running at all. Treating either as + // the project's declared manager would route the user into a command that cannot + // work, so the lockfiles decide instead, or the user is asked. + name, version, _ := strings.Cut(pkg.PackageManager, "@") + if !exactSemver.MatchString(version) { + return "" + } + switch name { + case "npm", "yarn", "pnpm", "bun": + return name + } + return "" +} + +// nodePMSignals reports what the project says about its Node package manager. +// Lockfiles: https://bun.com/docs/install/lockfile +func nodePMSignals(dir string) pmSignals { + if declared := corepackPM(dir); declared != "" { + return pmSignals{declared: declared} + } + var s pmSignals + for _, lock := range []struct{ file, pm string }{ + {"pnpm-lock.yaml", "pnpm"}, + {"yarn.lock", "yarn"}, + {"bun.lock", "bun"}, + {"bun.lockb", "bun"}, + {"package-lock.json", "npm"}, + {"npm-shrinkwrap.json", "npm"}, + } { + if _, err := os.Stat(filepath.Join(dir, lock.file)); err == nil { + s.addLocked(lock.pm) } } - return "npm" + return s } func detectGo(dir string) *DetectResult { @@ -219,8 +286,8 @@ func detectPython(dir string) *DetectResult { if _, err := os.Stat(filepath.Join(dir, indicator)); err == nil { ep, exists := EntryPointFor(dir, "python-server-sdk") return &DetectResult{ - Language: "Python", - PackageManager: detectPythonPM(dir), + Language: "Python", + // PackageManager is filled in by Detect from the same read. SDKID: "python-server-sdk", EntryPoint: ep, EntryPointExists: exists, @@ -236,23 +303,64 @@ func detectPython(dir string) *DetectResult { // // https://docs.astral.sh/uv/concepts/projects/layout/ // https://pipenv.pypa.io/en/latest/ -// https://python-poetry.org/docs/pyproject/ -func detectPythonPM(dir string) string { - if _, err := os.Stat(filepath.Join(dir, "uv.lock")); err == nil { - return "uv" - } - if _, err := os.Stat(filepath.Join(dir, "Pipfile")); err == nil { - return "pipenv" - } - if b, err := os.ReadFile(filepath.Join(dir, "pyproject.toml")); err == nil { - if bytes.Contains(b, []byte("[tool.poetry]")) { - return "poetry" + +// pythonPMSignals reports what the project says about its Python package manager. +// A lockfile is treated as the project having committed to a tool; a [tool.*] +// section counts the same way, since the tool owns that config. +func pythonPMSignals(dir string) pmSignals { + var s pmSignals + for _, lock := range []struct{ file, pm string }{ + {"uv.lock", "uv"}, + {"poetry.lock", "poetry"}, + {"pdm.lock", "pdm"}, + {"Pipfile.lock", "pipenv"}, + {"Pipfile", "pipenv"}, + } { + if _, err := os.Stat(filepath.Join(dir, lock.file)); err == nil { + s.addLocked(lock.pm) } - if bytes.Contains(b, []byte("[tool.uv]")) { - return "uv" + } + tools := configuredTools(dir) + for _, section := range []struct{ tool, pm string }{ + {"poetry", "poetry"}, + {"uv", "uv"}, + {"pdm", "pdm"}, + // hatch has no dependency-add command, so it is a signal we cannot act on. + // Recording it keeps the project ambiguous instead of silently falling + // through to pip. + {"hatch", ""}, + } { + if tools[section.tool] { + s.addLocked(section.pm) } } - return "pip" + return s +} + +// configuredTools reports which tools pyproject.toml configures, by the [tool.*] +// tables it declares. Reading the tables rather than matching text means a comment +// or a string that mentions another tool is not mistaken for a declaration, and +// nested tables need no special case: TOML creates the parent table implicitly, so +// [tool.hatch.build] on its own still declares hatch. +// +// A file we cannot parse declares nothing. That leaves the project ambiguous and the +// user asked, which is the honest answer when we cannot read what manages it. +func configuredTools(dir string) map[string]bool { + b, err := os.ReadFile(filepath.Join(dir, "pyproject.toml")) + if err != nil { + return nil + } + var doc struct { + Tool map[string]any `toml:"tool"` + } + if err := toml.Unmarshal(b, &doc); err != nil { + return nil + } + tools := make(map[string]bool, len(doc.Tool)) + for name := range doc.Tool { + tools[name] = true + } + return tools } func detectRuby(dir string) *DetectResult { @@ -282,10 +390,18 @@ func detectRuby(dir string) *DetectResult { // install` would succeed without recording the SDK for the app. // Gemfile: https://bundler.io/guides/gemfile.html func detectRubyPM(dir string) string { + return rubyPMSignals(dir).best("gem") +} + +// rubyPMSignals reports what the project says about its Ruby package manager. A +// Gemfile is Bundler's own manifest, so it settles the question; a gemspec alone +// does not, since the gem could be developed either way. +func rubyPMSignals(dir string) pmSignals { + var s pmSignals if _, err := os.Stat(filepath.Join(dir, "Gemfile")); err == nil { - return "bundle" + s.addLocked("bundle") } - return "gem" + return s } func detectJava(dir string) *DetectResult { @@ -518,7 +634,7 @@ func PackageManagerFor(dir, sdkID string) string { case "node-server", "js-client-sdk", "react-client-sdk", "react-native": return detectNodePM(dir) case "python-server-sdk": - return detectPythonPM(dir) + return PackageManagerChoiceFor(dir, sdkID).Name case "ruby-server-sdk": return detectRubyPM(dir) case "go-server-sdk": @@ -585,3 +701,149 @@ func findFileUnder(dir, root string, names ...string) string { } return "" } + +// PMConfidence says whether the project itself identifies its package manager. +type PMConfidence string + +const ( + // PMDefinite means the project names one manager and only one. + PMDefinite PMConfidence = "definite" + // PMAmbiguous means the project does not say, or contradicts itself. Callers + // must ask rather than guess. + PMAmbiguous PMConfidence = "ambiguous" +) + +// PMCandidate is a package manager the user could pick, with the command it would +// run. Installed reports whether the tool is on PATH; it describes the machine, not +// the project, so it never makes a candidate more likely to be the right one. +type PMCandidate struct { + Name string `json:"name"` + Installed bool `json:"installed"` + Command string `json:"command"` +} + +// PMChoice is the package manager for a project plus how sure we are. +type PMChoice struct { + Name string `json:"name"` + Confidence PMConfidence `json:"confidence"` + // Reason explains an ambiguous verdict in the words the picker shows the user. + Reason string `json:"reason,omitempty"` + Candidates []PMCandidate `json:"candidates,omitempty"` +} + +// pmSignals collects what a project says about its package manager. declared holds +// an explicit statement, which settles the question on its own; locked holds +// managers implied by lockfiles or tool config, where more than one means the +// project contradicts itself. An empty entry in locked marks a tool we recognise +// but cannot drive. +type pmSignals struct { + declared string + locked []string + unactionable bool +} + +func (s *pmSignals) addLocked(pm string) { + if pm == "" { + s.unactionable = true + return + } + for _, existing := range s.locked { + if existing == pm { + return + } + } + s.locked = append(s.locked, pm) +} + +// best returns the manager to use, falling back to fallback when the project says +// nothing. It preserves the old detection behaviour for callers that only want a +// name, including the first-match-wins ordering when lockfiles conflict. +func (s pmSignals) best(fallback string) string { + if s.declared != "" { + return s.declared + } + if len(s.locked) > 0 { + return s.locked[0] + } + return fallback +} + +// choose turns signals into a verdict. options lists every manager valid for the +// language, in the order the picker should show them, and fallback is the +// conventional default when the project is silent. +func (s pmSignals) choose(options []string, fallback string, argvFor func(string) []string) PMChoice { + candidates := make([]PMCandidate, 0, len(options)) + for _, name := range options { + argv := argvFor(name) + // Installed tracks the executable the command actually runs, not the label. + // "pip" resolves to pip3 on a stock macOS box, and reporting that as missing + // would steer the user away from the option that works. + installed := len(argv) > 0 && onPath(argv[0]) + candidates = append(candidates, PMCandidate{ + Name: name, + Installed: installed, + Command: strings.Join(argv, " "), + }) + } + + switch { + case s.declared != "": + return PMChoice{Name: s.declared, Confidence: PMDefinite} + // A manager the project committed to settles it even when an unactionable tool + // is also configured: hatchling is a common build backend for uv and poetry + // projects, and uv can add the dependency regardless of who builds the wheel. + case len(s.locked) == 1: + return PMChoice{Name: s.locked[0], Confidence: PMDefinite} + case len(s.locked) > 1: + // Say what was actually found. A lockfile, a Pipfile and a [tool.*] table all + // count as a project committing to a manager, so naming lockfiles would send + // the reader looking for files that are not there. + return PMChoice{ + Name: s.locked[0], + Confidence: PMAmbiguous, + Reason: fmt.Sprintf("this project is set up for more than one manager (%s)", + strings.Join(s.locked, ", ")), + Candidates: candidates, + } + case s.unactionable: + return PMChoice{ + Name: fallback, + Confidence: PMAmbiguous, + Reason: "this project is managed by a tool that cannot add dependencies for us", + Candidates: candidates, + } + default: + return PMChoice{ + Name: fallback, + Confidence: PMAmbiguous, + Reason: "this project doesn't say which package manager it uses", + Candidates: candidates, + } + } +} + +// PackageManagerChoiceFor reports the package manager for sdkID in dir and whether +// the project actually identifies it. Languages with a single toolchain are always +// definite; there is nothing to ask. +func PackageManagerChoiceFor(dir, sdkID string) PMChoice { + cmdFor := func(pm string) []string { + args, _ := InstallArgs(dir, sdkID, pm) + return args + } + + switch sdkID { + case "node-server", "js-client-sdk", "react-client-sdk", "react-native": + return nodePMSignals(dir).choose([]string{"npm", "yarn", "pnpm", "bun"}, "npm", cmdFor) + case "python-server-sdk": + return pythonPMSignals(dir).choose([]string{"pip", "uv", "poetry", "pipenv", "pdm"}, "pip", cmdFor) + case "ruby-server-sdk": + return rubyPMSignals(dir).choose([]string{"bundle", "gem"}, "gem", cmdFor) + case "go-server-sdk": + return PMChoice{Name: "go", Confidence: PMDefinite} + case "dotnet-server-sdk": + return PMChoice{Name: "dotnet", Confidence: PMDefinite} + default: + // Java, Android and Swift are installed by hand. + return PMChoice{Name: "", Confidence: PMDefinite} + } +} diff --git a/internal/setup/detector_shapes_test.go b/internal/setup/detector_shapes_test.go index 56df04eb..058370f3 100644 --- a/internal/setup/detector_shapes_test.go +++ b/internal/setup/detector_shapes_test.go @@ -367,7 +367,13 @@ func TestFileDetector_ProjectShapes(t *testing.T) { require.NoError(t, err) want := shape.want want.EntryPoint = filepath.Join(dir, want.EntryPoint) - assert.Equal(t, want, *result) + // These shapes assert language, SDK and entry point. Package-manager + // confidence has its own tests, so it is cleared rather than restated on + // every shape. + got := *result + got.PackageManagerConfidence = "" + got.PackageManagerReason = "" + assert.Equal(t, want, got) }) } } diff --git a/internal/setup/detector_test.go b/internal/setup/detector_test.go index 5ab223e2..02e6e047 100644 --- a/internal/setup/detector_test.go +++ b/internal/setup/detector_test.go @@ -850,3 +850,293 @@ func TestPackageManagerFor_DerivesFromProjectNotDetectedLanguage(t *testing.T) { assert.Empty(t, PackageManagerFor(t.TempDir(), "java-server-sdk")) }) } + +func TestPackageManagerChoice_Definite(t *testing.T) { + tests := []struct { + name string + files map[string]string + sdkID string + want string + }{ + // The most explicit statement a Node project can make outranks lockfiles. + {"corepack field", map[string]string{ + "package.json": `{"packageManager":"pnpm@9.1.0"}`, + }, "node-server", "pnpm"}, + {"corepack field with prerelease", map[string]string{ + "package.json": `{"packageManager":"pnpm@9.1.0-beta.1"}`, + }, "node-server", "pnpm"}, + // corepack writes this hash form itself when it pins a manager. + {"corepack field with build metadata", map[string]string{ + "package.json": `{"packageManager":"yarn@4.1.0+sha224.abcdef"}`, + }, "node-server", "yarn"}, + {"corepack field beats a conflicting lockfile", map[string]string{ + "package.json": `{"packageManager":"pnpm@9.1.0"}`, + "yarn.lock": "", + }, "node-server", "pnpm"}, + {"single node lockfile", map[string]string{ + "package.json": `{}`, + "pnpm-lock.yaml": "", + }, "node-server", "pnpm"}, + {"package-lock only", map[string]string{ + "package.json": `{}`, + "package-lock.json": "", + }, "node-server", "npm"}, + {"uv lockfile", map[string]string{"uv.lock": ""}, "python-server-sdk", "uv"}, + {"poetry lockfile", map[string]string{"poetry.lock": ""}, "python-server-sdk", "poetry"}, + {"tool.uv section", map[string]string{ + "pyproject.toml": "[project]\nname=\"a\"\n[tool.uv]\n", + }, "python-server-sdk", "uv"}, + {"tool.pdm section", map[string]string{ + "pyproject.toml": "[project]\nname=\"a\"\n[tool.pdm]\n", + }, "python-server-sdk", "pdm"}, + {"Gemfile", map[string]string{"Gemfile": "source 'x'"}, "ruby-server-sdk", "bundle"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + dir := t.TempDir() + for name, body := range tt.files { + require.NoError(t, os.WriteFile(filepath.Join(dir, name), []byte(body), 0600)) + } + + choice := PackageManagerChoiceFor(dir, tt.sdkID) + + assert.Equal(t, PMDefinite, choice.Confidence) + assert.Equal(t, tt.want, choice.Name) + assert.Empty(t, choice.Candidates, "a definite verdict needs no choice") + }) + } +} + +func TestPackageManagerChoice_Ambiguous(t *testing.T) { + tests := []struct { + name string + files map[string]string + sdkID string + wantReason string + wantOptions []string + }{ + // No detection tuning can fix this: the project contradicts itself. + {"conflicting node lockfiles", map[string]string{ + "package.json": `{}`, + "yarn.lock": "", + "package-lock.json": "", + }, "node-server", "more than one manager", []string{"npm", "yarn", "pnpm", "bun"}}, + {"bare package.json", map[string]string{ + "package.json": `{}`, + }, "node-server", "doesn't say", []string{"npm", "yarn", "pnpm", "bun"}}, + // Corepack needs one exact version, so honouring anything else would route the + // user into a manager that refuses to run. + {"packageManager without a version", map[string]string{ + "package.json": `{"packageManager":"pnpm"}`, + }, "node-server", "doesn't say", []string{"npm", "yarn", "pnpm", "bun"}}, + {"packageManager with a caret range", map[string]string{ + "package.json": `{"packageManager":"pnpm@^11.13.0"}`, + }, "node-server", "doesn't say", []string{"npm", "yarn", "pnpm", "bun"}}, + {"packageManager with a comparator range", map[string]string{ + "package.json": `{"packageManager":"pnpm@>=11"}`, + }, "node-server", "doesn't say", []string{"npm", "yarn", "pnpm", "bun"}}, + {"packageManager with a partial version", map[string]string{ + "package.json": `{"packageManager":"pnpm@11"}`, + }, "node-server", "doesn't say", []string{"npm", "yarn", "pnpm", "bun"}}, + // uv does not require committing the lock, and PEP 621 has no uv marker. + {"PEP 621 pyproject only", map[string]string{ + "pyproject.toml": "[project]\nname=\"a\"\n", + }, "python-server-sdk", "doesn't say", []string{"pip", "uv", "poetry", "pipenv", "pdm"}}, + {"requirements.txt only", map[string]string{ + "requirements.txt": "flask\n", + }, "python-server-sdk", "doesn't say", []string{"pip", "uv", "poetry", "pipenv", "pdm"}}, + // hatch cannot add dependencies for us, so we must not pick it or silently pip. + {"hatch project", map[string]string{ + "pyproject.toml": "[project]\nname=\"a\"\n[tool.hatch]\n", + }, "python-server-sdk", "cannot add dependencies", []string{"pip", "uv", "poetry", "pipenv", "pdm"}}, + {"gemspec without Gemfile", map[string]string{ + "a.gemspec": "Gem::Specification.new", + }, "ruby-server-sdk", "doesn't say", []string{"bundle", "gem"}}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + dir := t.TempDir() + for name, body := range tt.files { + require.NoError(t, os.WriteFile(filepath.Join(dir, name), []byte(body), 0600)) + } + + choice := PackageManagerChoiceFor(dir, tt.sdkID) + + assert.Equal(t, PMAmbiguous, choice.Confidence) + assert.Contains(t, choice.Reason, tt.wantReason) + names := make([]string, 0, len(choice.Candidates)) + for _, c := range choice.Candidates { + names = append(names, c.Name) + assert.NotEmpty(t, c.Command, "every candidate needs the command it would run") + } + assert.Equal(t, tt.wantOptions, names) + }) + } +} + +// Installed state describes the machine, so it must never change the verdict. +func TestPackageManagerChoice_InstalledStateDoesNotDecide(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, "package.json"), []byte(`{}`), 0600)) + + stubPath(t, "pnpm") // the only manager on this machine + choice := PackageManagerChoiceFor(dir, "node-server") + + assert.Equal(t, PMAmbiguous, choice.Confidence, "one installed tool is not evidence about the project") + assert.Equal(t, "npm", choice.Name, "the conventional default stands until the user picks") + for _, c := range choice.Candidates { + assert.Equal(t, c.Name == "pnpm", c.Installed) + } +} + +func TestPackageManagerChoice_SingleToolchainsAreAlwaysDefinite(t *testing.T) { + for sdkID, want := range map[string]string{ + "go-server-sdk": "go", + "dotnet-server-sdk": "dotnet", + "java-server-sdk": "", + } { + t.Run(sdkID, func(t *testing.T) { + choice := PackageManagerChoiceFor(t.TempDir(), sdkID) + assert.Equal(t, PMDefinite, choice.Confidence) + assert.Equal(t, want, choice.Name) + }) + } +} + +// Real pyproject files rarely carry a bare [tool.x] header; the tables that matter +// are nested. Matching only the bare header made these signals near-dead. +func TestPackageManagerChoice_NestedToolTables(t *testing.T) { + tests := []struct { + name string + pyproject string + wantName string + wantDefinite bool + }{ + {"hatch build table only", "[project]\nname=\"a\"\n[tool.hatch.build.targets.wheel]\npackages=[\"a\"]\n", "pip", false}, + {"hatch version table only", "[project]\nname=\"a\"\n[tool.hatch.version]\npath=\"a/__init__.py\"\n", "pip", false}, + {"poetry dependencies table only", "[project]\nname=\"a\"\n[tool.poetry.dependencies]\npython=\"^3.12\"\n", "poetry", true}, + {"uv sources table only", "[project]\nname=\"a\"\n[tool.uv.sources]\nx={git=\"...\"}\n", "uv", true}, + {"pdm dev-dependencies table only", "[project]\nname=\"a\"\n[tool.pdm.dev-dependencies]\ntest=[]\n", "pdm", true}, + // The trailing delimiter matters: [tool.uv] must not match [tool.uvicorn]. + {"uvicorn is not uv", "[project]\nname=\"a\"\n[tool.uvicorn]\nport=8000\n", "pip", false}, + {"hatchling ruff etc are not hatch", "[project]\nname=\"a\"\n[tool.ruff]\nline-length=100\n", "pip", false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, "pyproject.toml"), []byte(tt.pyproject), 0600)) + + choice := PackageManagerChoiceFor(dir, "python-server-sdk") + + assert.Equal(t, tt.wantName, choice.Name) + if tt.wantDefinite { + assert.Equal(t, PMDefinite, choice.Confidence) + } else { + assert.Equal(t, PMAmbiguous, choice.Confidence) + } + }) + } +} + +// A PDM project that commits only its lockfile is still a PDM project. +func TestPackageManagerChoice_PdmLockfile(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, "pdm.lock"), []byte(""), 0600)) + + choice := PackageManagerChoiceFor(dir, "python-server-sdk") + + assert.Equal(t, PMDefinite, choice.Confidence) + assert.Equal(t, "pdm", choice.Name) +} + +// hatchling is a common build backend for uv and poetry projects. The manager the +// project committed to can still add the dependency, whoever builds the wheel. +func TestPackageManagerChoice_ActionableSignalBeatsHatch(t *testing.T) { + tests := []struct { + name string + files map[string]string + want string + }{ + {"uv lockfile alongside hatch build backend", map[string]string{ + "pyproject.toml": "[project]\nname=\"a\"\n[tool.hatch.build.targets.wheel]\npackages=[\"a\"]\n", + "uv.lock": "", + }, "uv"}, + {"uv table alongside hatch", map[string]string{ + "pyproject.toml": "[project]\nname=\"a\"\n[tool.uv]\n[tool.hatch.version]\npath=\"x\"\n", + }, "uv"}, + {"poetry alongside hatch", map[string]string{ + "pyproject.toml": "[project]\nname=\"a\"\n[tool.poetry]\n[tool.hatch.build]\n", + }, "poetry"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + dir := t.TempDir() + for name, body := range tt.files { + require.NoError(t, os.WriteFile(filepath.Join(dir, name), []byte(body), 0600)) + } + + choice := PackageManagerChoiceFor(dir, "python-server-sdk") + + assert.Equal(t, PMDefinite, choice.Confidence, + "setup would refuse to install though %s can add the dependency", tt.want) + assert.Equal(t, tt.want, choice.Name) + }) + } +} + +// pyproject.toml is read as TOML rather than searched as text, so only the tables it +// actually declares count as a project committing to a tool. +func TestPackageManagerChoice_ToolTablesAreParsedNotMatched(t *testing.T) { + tests := []struct { + name string + pyproject string + wantName string + wantDefinite bool + }{ + // A note about the tool a project migrated away from is not a declaration. + {"comment mentions another tool", "[project]\nname=\"a\"\n# migrated away from [tool.poetry] in March\n[tool.uv]\n", "uv", true}, + // Nor is a table name inside a string. + {"multi-line string mentions another tool", + "[project]\nname=\"a\"\ndescription=\"\"\"\nsee [tool.poetry] for history\n\"\"\"\n[tool.uv]\n", "uv", true}, + {"single-quoted string mentions another tool", + "[project]\nname=\"a\"\nsummary='see [tool.poetry]'\n[tool.uv]\n", "uv", true}, + // A trailing comment on the header itself is still a declaration. + {"header with a trailing comment", "[project]\nname=\"a\"\n[tool.uv] # the real one\n", "uv", true}, + // Parent tables are implicit, so a nested table declares its tool. + {"nested table only", "[project]\nname=\"a\"\n[tool.poetry.dependencies]\npython=\"^3.12\"\n", "poetry", true}, + // A different tool whose name merely starts the same way is not a match. + {"similarly named tool", "[project]\nname=\"a\"\n[tool.uvicorn]\nport=8000\n", "pip", false}, + // Unreadable TOML declares nothing, so the user is asked rather than guessed at. + {"malformed toml", "[project\nname=\"a\"\n[tool.uv]\n", "pip", false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, "pyproject.toml"), []byte(tt.pyproject), 0600)) + + choice := PackageManagerChoiceFor(dir, "python-server-sdk") + + assert.Equal(t, tt.wantName, choice.Name) + if tt.wantDefinite { + assert.Equal(t, PMDefinite, choice.Confidence) + } else { + assert.Equal(t, PMAmbiguous, choice.Confidence) + } + }) + } +} + +// A [tool.*] table and a Pipfile count as commitments too, so the reason must not +// send the reader looking for lockfiles that are not there. +func TestPackageManagerChoice_ConflictReasonNamesWhatWasFound(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, "pyproject.toml"), + []byte("[project]\nname=\"a\"\n[tool.uv]\n[tool.poetry]\n"), 0600)) + + choice := PackageManagerChoiceFor(dir, "python-server-sdk") + + require.Equal(t, PMAmbiguous, choice.Confidence) + assert.Contains(t, choice.Reason, "set up for more than one manager") + assert.NotContains(t, choice.Reason, "lockfile", + "neither signal here is a lockfile") +} diff --git a/internal/setup/installer.go b/internal/setup/installer.go index cab87917..170c4e14 100644 --- a/internal/setup/installer.go +++ b/internal/setup/installer.go @@ -143,6 +143,15 @@ func (p PackageInstaller) Install(dir string, detection *DetectResult) (*Install out, err := runner(dir, args) command := strings.Join(args, " ") if err != nil { + if reason := packageManagerSpecReason(out); reason != "" { + return &InstallResult{ + SDKID: detection.SDKID, + Package: pkg, + Command: command, + Failed: true, + FailureReason: reason, + }, nil + } if reason := externallyManagedReason(dir, out); reason != "" { // No Command: the reason says not to run this pip, and the done screen // offers a non-empty Command as "install it yourself with". @@ -190,6 +199,31 @@ func dotnetProjectArg(dir string) (args []string, reason string) { } } +// packageManagerSpecReason recognises a Node manager refusing to run because the +// packageManager field in package.json is not a spec corepack accepts: it requires +// an exact version, so both a missing one and a range are rejected. The manifest is +// malformed rather than the command wrong, and repairing someone's manifest is not +// ours to do, so say what is wrong and let them fix it. +func packageManagerSpecReason(out []byte) string { + // package.json has to be named in the output. Both corepack refusals mention it, + // and without that check any failure whose text happens to mention a missing or + // non-semver version — from a gem, a Python package, a Go module — would have its + // real error replaced by advice about a field it does not have. + if !bytes.Contains(out, []byte("package.json")) { + return "" + } + badSpec := bytes.Contains(out, []byte("No version specified")) || + bytes.Contains(out, []byte("expected a semver version")) || + bytes.Contains(out, []byte("Invalid package manager specification")) + if !badSpec { + return "" + } + return "the packageManager field in package.json is not a specification your package " + + "manager accepts: it needs one exact version, so a missing version or a range such as " + + "\"pnpm@^11.13.0\" is refused. Pin it (for example \"pnpm@11.13.0\") or remove the field, " + + "then run setup again." +} + // externallyManagedReason recognises a PEP 668 refusal and says what to do about // it. Homebrew and most current Linux distributions mark their Python as managed // by the OS package manager, so pip declines to write into it. A virtualenv is the @@ -269,6 +303,7 @@ var installHints = map[string]string{ "poetry": "see https://python-poetry.org/docs/#installation", "uv": "see https://docs.astral.sh/uv/getting-started/installation", "pipenv": "see https://pipenv.pypa.io/en/latest/installation.html", + "pdm": "see https://pdm-project.org/en/latest/#installation", "npm": "install Node.js from https://nodejs.org", "yarn": "see https://yarnpkg.com/getting-started/install", "pnpm": "see https://pnpm.io/installation", @@ -369,6 +404,8 @@ func pythonInstallCmd(dir, pm, pkg string) []string { return []string{"uv", "add", pkg} case "pipenv": return []string{"pipenv", "install", pkg} + case "pdm": + return []string{"pdm", "add", pkg} default: return pipInstallCmd(dir, pkg) } diff --git a/internal/setup/installer_test.go b/internal/setup/installer_test.go index 62a194d1..96390154 100644 --- a/internal/setup/installer_test.go +++ b/internal/setup/installer_test.go @@ -881,3 +881,61 @@ func TestInstall_PinnedSdkVersion_NoWarning(t *testing.T) { require.NoError(t, err) assert.Empty(t, result.Warning) } + +// Corepack needs one exact version, so both a missing version and a range stop the +// manager running. Repairing someone's manifest is not ours to do, so the failure +// has to say what is wrong. +func TestInstall_BadPackageManagerSpec_ExplainsIt(t *testing.T) { + for _, out := range []string{ + `No version specified for pnpm in "packageManager" of package.json`, + "Invalid package manager specification in package.json (pnpm@^11.13.0); expected a semver version", + } { + t.Run(out[:24], func(t *testing.T) { + stubVirtualEnv(t, "") + stubPath(t, "pnpm") + installer := PackageInstaller{ + run: func(string, []string) ([]byte, error) { + return []byte(out), errors.New("exit status 1") + }, + } + + result, err := installer.Install(t.TempDir(), &DetectResult{ + SDKID: "node-server", PackageManager: "pnpm", + }) + + require.NoError(t, err, "a malformed manifest must not dead-end the flow") + assert.True(t, result.Failed) + assert.Contains(t, result.FailureReason, "packageManager field") + assert.Contains(t, result.FailureReason, "one exact version") + assert.Contains(t, result.FailureReason, "pnpm@11.13.0") + }) + } +} + +// Only a Node failure about package.json may be answered with advice about that +// file. Another ecosystem's error keeps its own text, whatever phrases it contains. +func TestInstall_OtherEcosystemErrorsKeepTheirText(t *testing.T) { + for _, out := range []string{ + "ERROR: Could not find a valid gem 'x' (>= 0), here is why:\n No version specified", + "ERROR: Could not find a version that satisfies the requirement x; expected a semver version", + "go: module x: invalid version: expected a semver version", + } { + t.Run(out[:20], func(t *testing.T) { + stubVirtualEnv(t, "") + stubPath(t, "npm") + installer := PackageInstaller{ + run: func(string, []string) ([]byte, error) { + return []byte(out), errors.New("exit status 1") + }, + } + + _, err := installer.Install(t.TempDir(), &DetectResult{ + SDKID: "node-server", PackageManager: "npm", + }) + + require.Error(t, err, "the real failure must reach the caller") + assert.Contains(t, err.Error(), out, "the real error text is kept") + assert.NotContains(t, err.Error(), "packageManager field") + }) + } +}