From aba59bfd1a6dc52e8bad6c7a073fe6a8580f0bbe Mon Sep 17 00:00:00 2001 From: Francisco Fantl Date: Mon, 17 Aug 2026 09:05:32 -0400 Subject: [PATCH 1/3] fix(setup): re-derive entry point and package manager on SDK override Choosing an SDK by hand replaced the detected entry point with that SDK's bare default in the working directory, so a project whose entry file was src/index.js got a second index.js created beside it. The package manager was left describing the language detection guessed first, so a Ruby install never ran bundle add. Both are now re-derived for the chosen SDK from one shared table of entry-point candidates, which detection also reads. Snippet-only SDKs have no entry point, so the final screen no longer names an empty path. Co-Authored-By: Claude Opus 5 (1M context) --- cmd/setup/update.go | 25 ++++----- cmd/setup/view.go | 13 ++++- cmd/setup/wizard_test.go | 44 ++++++++++++++- internal/setup/detector.go | 98 +++++++++++++++++++++++++-------- internal/setup/detector_test.go | 58 +++++++++++++++++++ 5 files changed, 198 insertions(+), 40 deletions(-) diff --git a/cmd/setup/update.go b/cmd/setup/update.go index 669e8096..a14b8e4e 100644 --- a/cmd/setup/update.go +++ b/cmd/setup/update.go @@ -2,7 +2,6 @@ package setup import ( "os" - "path/filepath" "strings" "github.com/charmbracelet/bubbles/key" @@ -351,20 +350,18 @@ func (m wizardModel) handleEnter() (tea.Model, tea.Cmd) { result.SDKID = chosen.id result.Language = chosen.language if chosen.id != m.detectedSDKID { - // The detected entry point belongs to the language we detected, not the - // one the user picked. An append-safe SDK would otherwise write Ruby into - // a Node project's index.js, so start over from that SDK's own default. + // The detected entry point and package manager describe the language we + // detected, not the one the user picked, so re-derive both for the chosen + // SDK rather than carrying the wrong ones forward. Searching again also + // finds an existing file the SDK's bare default would have missed, so we + // append to the user's entry point instead of adding a second one. result.Framework = "" - result.EntryPoint = setup.DefaultEntryPoint(chosen.id) - result.EntryPointExists = false - if dir, err := os.Getwd(); err == nil && result.EntryPoint != "" { - result.EntryPoint = filepath.Join(dir, result.EntryPoint) - // Injection appends to a file that is already there, so report the - // default as found when it exists. Claiming otherwise would promise - // to create a file and then quietly append to the user's. - if info, err := os.Stat(result.EntryPoint); err == nil && !info.IsDir() { - result.EntryPointExists = true - } + if dir, err := os.Getwd(); err == nil { + result.EntryPoint, result.EntryPointExists = setup.EntryPointFor(dir, chosen.id) + result.PackageManager = setup.PackageManagerFor(dir, chosen.id) + } else { + result.EntryPoint = setup.DefaultEntryPoint(chosen.id) + result.EntryPointExists = false } } m.detectResult = &result diff --git a/cmd/setup/view.go b/cmd/setup/view.go index 0a359b67..a6ce9ec6 100644 --- a/cmd/setup/view.go +++ b/cmd/setup/view.go @@ -110,7 +110,7 @@ func (m wizardModel) View() string { } else if m.initResult != nil && m.initResult.Success { body += m.wrap(fmt.Sprintf("Initialization code was added to %s.", m.initResult.FilePath)) + "\n" } else if m.initResult != nil && m.initResult.Snippet != "" { - body += m.wrap(fmt.Sprintf("Then add this initialization code to %s:", m.initResult.FilePath)) + + body += m.wrap(addCodeTo("Then add this initialization code", m.initResult.FilePath)) + "\n\n" + code(m.initResult.Snippet) + "\n" } body += "\n" + m.wrap(fmt.Sprintf("Flag %q was created in project %q.", m.flagKey, m.selectedProject)) + "\n" @@ -119,7 +119,7 @@ func (m wizardModel) View() string { if m.initResult != nil && !m.initResult.Success { body := titleStyle.Render("Manual SDK setup required") + "\n\n" if m.initResult.Snippet != "" { - body += m.wrap(fmt.Sprintf("Add the following %s initialization code to %s:", m.initResult.SDKID, m.initResult.FilePath)) + + body += m.wrap(addCodeTo(fmt.Sprintf("Add the following %s initialization code", m.initResult.SDKID), m.initResult.FilePath)) + "\n\n" + code(m.initResult.Snippet) + "\n\n" } else { body += fmt.Sprintf("No initialization template is available for %s.\n", m.initResult.SDKID) @@ -188,6 +188,15 @@ func (m wizardModel) sdkBoxWidth() int { return w } +// 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 { + if path == "" { + return instruction + ":" + } + return fmt.Sprintf("%s to %s:", instruction, path) +} + // listHeight is the height available to a full-screen list. It never returns a // value below a usable minimum, because a WindowSizeMsg may not have arrived yet // and m.height-4 would then be negative. diff --git a/cmd/setup/wizard_test.go b/cmd/setup/wizard_test.go index 5fc223fb..f3897627 100644 --- a/cmd/setup/wizard_test.go +++ b/cmd/setup/wizard_test.go @@ -447,8 +447,48 @@ func TestWizard_OverrideSDK_DoesNotReuseDetectedEntryPoint(t *testing.T) { "a file we have not found must not be reported as found") assert.Contains(t, m3.detectResult.EntryPoint, "main.rb") assert.Empty(t, m3.detectResult.Framework, "Next.js does not describe a Ruby project") - // The package manager describes the project, not the SDK, so it survives. - assert.Equal(t, "pnpm", m3.detectResult.PackageManager) + // pnpm cannot install a gem, so the manager is re-derived for the chosen SDK. + assert.Equal(t, "gem", m3.detectResult.PackageManager) +} + +// An override must find the file the project already has, rather than falling back +// to the SDK's bare default and creating a second entry point beside it. +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)) + // 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) + + m := wizardModel{step: stepDetect, width: 80, height: 30} + next, _ := m.Update(detectDoneMsg{result: &setup.DetectResult{ + SDKID: "js-client-sdk", + Language: "JavaScript", + }}) + + m2 := selectOtherSDK(t, next.(wizardModel), "node-server") + next2, _ := m2.Update(tea.KeyMsg{Type: tea.KeyEnter}) + m3 := next2.(wizardModel) + + require.Equal(t, stepPlan, m3.step) + assert.Equal(t, filepath.Join(dir, "src/index.js"), m3.detectResult.EntryPoint, + "setup would create a second index.js beside the real entry point") + assert.True(t, m3.detectResult.EntryPointExists) +} + +// 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. +func chdir(t *testing.T, dir string) string { + t.Helper() + original, err := os.Getwd() + require.NoError(t, err) + require.NoError(t, os.Chdir(dir)) + t.Cleanup(func() { _ = os.Chdir(original) }) + resolved, err := os.Getwd() + require.NoError(t, err) + return resolved } // SDKs that only ever return a snippet have no file to name. diff --git a/internal/setup/detector.go b/internal/setup/detector.go index a450fc7e..5d9db9b8 100644 --- a/internal/setup/detector.go +++ b/internal/setup/detector.go @@ -173,15 +173,7 @@ func detectNode(dir string) *DetectResult { } } - // Entry point: https://docs.npmjs.com/cli/v11/configuring-npm/package-json#main - // NestJS bootstraps from src/main.ts: https://docs.nestjs.com/first-steps - ep, exists := entryPoint(dir, "index.js", - "src/index.ts", "src/index.js", - "src/main.ts", "src/main.js", - "index.ts", "index.js", - "server.ts", "server.js", - "app.ts", "app.js", - ) + ep, exists := EntryPointFor(dir, "node-server") return &DetectResult{ Language: "JavaScript", PackageManager: pm, @@ -225,11 +217,7 @@ func detectGo(dir string) *DetectResult { func detectPython(dir string) *DetectResult { for _, indicator := range []string{"requirements.txt", "pyproject.toml", "setup.py", "Pipfile"} { if _, err := os.Stat(filepath.Join(dir, indicator)); err == nil { - // Django entry: https://docs.djangoproject.com/en/stable/ref/django-admin/ - // Flask entry: https://flask.palletsprojects.com/en/stable/quickstart/ - ep, exists := entryPoint(dir, "main.py", - "src/main.py", "manage.py", "app.py", "main.py", - ) + ep, exists := EntryPointFor(dir, "python-server-sdk") return &DetectResult{ Language: "Python", PackageManager: detectPythonPM(dir), @@ -280,22 +268,26 @@ func detectRuby(dir string) *DetectResult { return nil } } - // Gemfile: https://bundler.io/guides/gemfile.html - pm := "gem" - if _, err := os.Stat(filepath.Join(dir, "Gemfile")); err == nil { - pm = "bundle" - } - // config.ru: https://github.com/rack/rack/blob/main/SPEC.rdoc - ep, exists := entryPoint(dir, "main.rb", "config.ru", "app.rb", "main.rb") + ep, exists := EntryPointFor(dir, "ruby-server-sdk") return &DetectResult{ Language: "Ruby", - PackageManager: pm, + PackageManager: detectRubyPM(dir), SDKID: "ruby-server-sdk", EntryPoint: ep, EntryPointExists: exists, } } +// detectRubyPM reports whether the project is Bundler-managed, since a bare `gem +// install` would succeed without recording the SDK for the app. +// Gemfile: https://bundler.io/guides/gemfile.html +func detectRubyPM(dir string) string { + if _, err := os.Stat(filepath.Join(dir, "Gemfile")); err == nil { + return "bundle" + } + return "gem" +} + func detectJava(dir string) *DetectResult { for _, indicator := range []string{"pom.xml", "build.gradle", "build.gradle.kts"} { if _, err := os.Stat(filepath.Join(dir, indicator)); err == nil { @@ -478,6 +470,68 @@ var KnownSDKs = []SDKOption{ {ID: "ruby-server-sdk", Language: "Ruby", Name: "Ruby"}, } +// sdkEntryPoints maps each SDK that writes to a file to its entry-point fallback +// and the candidates to look for. Detection and SDK-override both read this table +// so the two can't disagree about where code goes. Framework-specific layouts +// (Next.js, React) stay inline in detectNode: picking an SDK by hand clears the +// detected framework, so only the framework-neutral list can apply afterwards. +var sdkEntryPoints = map[string]struct { + fallback string + candidates []string +}{ + // Entry point: https://docs.npmjs.com/cli/v11/configuring-npm/package-json#main + // NestJS bootstraps from src/main.ts: https://docs.nestjs.com/first-steps + "node-server": {"index.js", []string{ + "src/index.ts", "src/index.js", + "src/main.ts", "src/main.js", + "index.ts", "index.js", + "server.ts", "server.js", + "app.ts", "app.js", + }}, + // Django entry: https://docs.djangoproject.com/en/stable/ref/django-admin/ + // Flask entry: https://flask.palletsprojects.com/en/stable/quickstart/ + "python-server-sdk": {"main.py", []string{ + "src/main.py", "manage.py", "app.py", "main.py", + }}, + // config.ru: https://github.com/rack/rack/blob/main/SPEC.rdoc + "ruby-server-sdk": {"main.rb", []string{ + "config.ru", "app.rb", "main.rb", + }}, +} + +// EntryPointFor returns the file sdkID should write to in dir, joined to dir, and +// whether that file already exists. SDKs that only ever show a snippet have no +// entry point and return an empty path. +func EntryPointFor(dir, sdkID string) (string, bool) { + spec, ok := sdkEntryPoints[sdkID] + if !ok { + return "", false + } + return entryPoint(dir, spec.fallback, spec.candidates...) +} + +// PackageManagerFor returns the package manager that should install sdkID in dir. +// It derives the value from the project the way detection does, so choosing an SDK +// by hand doesn't leave the manager describing the language we guessed first. +func PackageManagerFor(dir, sdkID string) string { + switch sdkID { + case "node-server", "js-client-sdk", "react-client-sdk", "react-native": + return detectNodePM(dir) + case "python-server-sdk": + return detectPythonPM(dir) + case "ruby-server-sdk": + return detectRubyPM(dir) + case "go-server-sdk": + return "go" + case "dotnet-server-sdk": + return "dotnet" + default: + // Java, Android and Swift are installed by hand, so there is no command + // whose choice of manager could be wrong. + return "" + } +} + // entryPoint returns the first candidate that exists as a file under dir, joined // to dir, together with true. When no candidate exists it returns fallback joined // to dir and false, so callers can tell a file we found from one we suggest. diff --git a/internal/setup/detector_test.go b/internal/setup/detector_test.go index cb48d976..5ab223e2 100644 --- a/internal/setup/detector_test.go +++ b/internal/setup/detector_test.go @@ -792,3 +792,61 @@ func TestFileDetector_Swift_SingleTarget_PrefersMainSwift(t *testing.T) { assert.Equal(t, filepath.Join(dir, "Sources/MyTool/main.swift"), result.EntryPoint) assert.True(t, result.EntryPointExists) } + +func TestEntryPointFor_FindsExistingFileNotBareDefault(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(""), 0600)) + + ep, exists := EntryPointFor(dir, "node-server") + + assert.Equal(t, filepath.Join(dir, "src/index.js"), ep) + assert.True(t, exists) +} + +func TestEntryPointFor_SuggestsFallbackWhenNothingExists(t *testing.T) { + dir := t.TempDir() + + ep, exists := EntryPointFor(dir, "node-server") + + assert.Equal(t, filepath.Join(dir, "index.js"), ep) + assert.False(t, exists) +} + +func TestEntryPointFor_SnippetOnlySDKsHaveNoEntryPoint(t *testing.T) { + for _, sdkID := range []string{"react-client-sdk", "js-client-sdk", "go-server-sdk", "java-server-sdk"} { + t.Run(sdkID, func(t *testing.T) { + ep, exists := EntryPointFor(t.TempDir(), sdkID) + assert.Empty(t, ep) + assert.False(t, exists) + }) + } +} + +func TestPackageManagerFor_DerivesFromProjectNotDetectedLanguage(t *testing.T) { + t.Run("bundler project uses bundle so the gem is recorded", func(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, "Gemfile"), []byte("source 'https://rubygems.org'\n"), 0600)) + assert.Equal(t, "bundle", PackageManagerFor(dir, "ruby-server-sdk")) + }) + + t.Run("ruby without a Gemfile falls back to gem", func(t *testing.T) { + assert.Equal(t, "gem", PackageManagerFor(t.TempDir(), "ruby-server-sdk")) + }) + + t.Run("node lockfile picks the matching manager", func(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, "pnpm-lock.yaml"), []byte(""), 0600)) + assert.Equal(t, "pnpm", PackageManagerFor(dir, "node-server")) + }) + + t.Run("python uv lockfile", func(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, "uv.lock"), []byte(""), 0600)) + assert.Equal(t, "uv", PackageManagerFor(dir, "python-server-sdk")) + }) + + t.Run("manual-install SDKs have no manager", func(t *testing.T) { + assert.Empty(t, PackageManagerFor(t.TempDir(), "java-server-sdk")) + }) +} From 7fb6ccb3f33f3bea5154a4dd5027934d0c091583 Mon Sep 17 00:00:00 2001 From: Francisco Fantl Date: Thu, 20 Aug 2026 13:31:22 -0400 Subject: [PATCH 2/3] feat(setup): report package-manager confidence and stop guessing silently (#783) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(setup): report package-manager confidence and stop guessing silently Detection always returned a manager, so a project that never said which one it used got pip or npm presented as fact. Signals are now split into what the project declares and what it merely implies, and a verdict is definite only when the project names exactly one manager. Recognise the corepack packageManager field, poetry.lock, Pipfile.lock and [tool.pdm], which were previously read as pip or npm. [tool.hatch] is recorded as a signal we cannot act on, since hatch has no dependency-add command. `setup install` now reads the project when --package-manager is omitted, instead of defaulting to pip or npm whatever the lockfiles say, and fails with the candidate list when the project is ambiguous rather than picking for the user. Co-Authored-By: Claude Opus 5 (1M context) * fix(setup): match nested tool tables and let a committed manager win Real pyproject files rarely carry a bare [tool.x] header, so matching only that left the poetry, uv, pdm and hatch signals near-dead: a hatch project configures [tool.hatch.build], not [tool.hatch]. Nested tables now count, with the trailing delimiter required so [tool.uv] does not match [tool.uvicorn]. A manager the project committed to now settles the verdict 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 whoever builds the wheel; previously those projects were marked ambiguous and install refused to run. Recognise pdm.lock, so a PDM project that commits only its lockfile is still identified as one. Co-Authored-By: Claude Opus 5 (1M context) * fix(setup): require a version in the packageManager field pnpm refuses to run at all when package.json names it without a version — "No version specified for pnpm in packageManager" — so treating a versionless field as the project's declared manager routed the user into a command that cannot work. Such a field is no longer a declaration: the lockfiles decide, or the user is asked. When a manager still refuses for that reason, say so. The manifest is malformed rather than the command wrong, and repairing someone's manifest is not ours to do. Co-Authored-By: Claude Opus 5 (1M context) * fix(setup): require one exact version in the packageManager field Corepack accepts only an exact version, so a range is refused outright — "Invalid package manager specification in package.json (pnpm@^11.13.0); expected a semver version" — as is a missing version. Treating either as the project's declared manager routed the user into a command that cannot run. Only an exact MAJOR.MINOR.PATCH, optionally with prerelease or build metadata, now counts; anything else leaves the lockfiles to decide or the user to be asked. When a manager refuses for that reason, say which part is wrong. The manifest is malformed rather than the command, and repairing someone's manifest is not ours to do. Co-Authored-By: Claude Opus 5 (1M context) * fix(setup): read pyproject.toml as TOML to find configured tools Matching [tool.] as text counted comments and strings, so a uv project whose comment mentioned the tool it migrated away from was marked ambiguous and install refused to run. Reading the declared tables instead means only a declaration counts, and nested tables need no special case: TOML creates the parent implicitly, so [tool.hatch.build] alone still declares hatch. A file we cannot parse declares nothing, which leaves the project ambiguous and the user asked — the honest answer when we cannot read what manages it. Co-Authored-By: Claude Opus 5 (1M context) * refactor(setup): read pyproject.toml once per detection Detection parsed the file twice for a Python project: once for the detector's own package manager and again for the confidence verdict. The verdict is now the single source for the languages it models, and the detector leaves the field to it. A language it does not model, such as Java's maven versus gradle, keeps the answer its detector gives. Co-Authored-By: Claude Opus 5 (1M context) * fix(setup): stop misreading other ecosystems' install errors The packageManager guidance matched a missing or non-semver version anywhere in an install failure, so a gem, a Python package or a Go module reporting either phrase had its real error replaced by advice about a file it does not have. The output has to name package.json, which both corepack refusals do. Say what was actually found when a project is set up for more than one manager: a Pipfile and a [tool.*] table count as commitments too, so naming lockfiles sent the reader looking for files that are not there. Note on stderr when no --package-manager was given, since that used to fall through to npm or pip and now reads the project. Callers parsing output are unaffected. Co-Authored-By: Claude Opus 5 (1M context) --------- Co-authored-by: Claude Opus 5 (1M context) --- cmd/cmdtest.go | 34 +++ cmd/setup/detect.go | 19 +- cmd/setup/install.go | 37 +++ cmd/setup/setup_test.go | 36 +++ go.mod | 2 +- internal/setup/detector.go | 320 ++++++++++++++++++++++--- internal/setup/detector_shapes_test.go | 8 +- internal/setup/detector_test.go | 290 ++++++++++++++++++++++ internal/setup/installer.go | 37 +++ internal/setup/installer_test.go | 58 +++++ 10 files changed, 808 insertions(+), 33 deletions(-) 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/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/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") + }) + } +} From 1bd7402ce5622412512b238722b37919ebd616e2 Mon Sep 17 00:00:00 2001 From: Francisco Fantl Date: Thu, 20 Aug 2026 13:32:26 -0400 Subject: [PATCH 3/3] feat(setup): ask which package manager to use when the project is ambiguous (#784) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(setup): report package-manager confidence and stop guessing silently Detection always returned a manager, so a project that never said which one it used got pip or npm presented as fact. Signals are now split into what the project declares and what it merely implies, and a verdict is definite only when the project names exactly one manager. Recognise the corepack packageManager field, poetry.lock, Pipfile.lock and [tool.pdm], which were previously read as pip or npm. [tool.hatch] is recorded as a signal we cannot act on, since hatch has no dependency-add command. `setup install` now reads the project when --package-manager is omitted, instead of defaulting to pip or npm whatever the lockfiles say, and fails with the candidate list when the project is ambiguous rather than picking for the user. Co-Authored-By: Claude Opus 5 (1M context) * fix(setup): match nested tool tables and let a committed manager win Real pyproject files rarely carry a bare [tool.x] header, so matching only that left the poetry, uv, pdm and hatch signals near-dead: a hatch project configures [tool.hatch.build], not [tool.hatch]. Nested tables now count, with the trailing delimiter required so [tool.uv] does not match [tool.uvicorn]. A manager the project committed to now settles the verdict 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 whoever builds the wheel; previously those projects were marked ambiguous and install refused to run. Recognise pdm.lock, so a PDM project that commits only its lockfile is still identified as one. Co-Authored-By: Claude Opus 5 (1M context) * fix(setup): require a version in the packageManager field pnpm refuses to run at all when package.json names it without a version — "No version specified for pnpm in packageManager" — so treating a versionless field as the project's declared manager routed the user into a command that cannot work. Such a field is no longer a declaration: the lockfiles decide, or the user is asked. When a manager still refuses for that reason, say so. The manifest is malformed rather than the command wrong, and repairing someone's manifest is not ours to do. Co-Authored-By: Claude Opus 5 (1M context) * fix(setup): require one exact version in the packageManager field Corepack accepts only an exact version, so a range is refused outright — "Invalid package manager specification in package.json (pnpm@^11.13.0); expected a semver version" — as is a missing version. Treating either as the project's declared manager routed the user into a command that cannot run. Only an exact MAJOR.MINOR.PATCH, optionally with prerelease or build metadata, now counts; anything else leaves the lockfiles to decide or the user to be asked. When a manager refuses for that reason, say which part is wrong. The manifest is malformed rather than the command, and repairing someone's manifest is not ours to do. Co-Authored-By: Claude Opus 5 (1M context) * fix(setup): read pyproject.toml as TOML to find configured tools Matching [tool.] as text counted comments and strings, so a uv project whose comment mentioned the tool it migrated away from was marked ambiguous and install refused to run. Reading the declared tables instead means only a declaration counts, and nested tables need no special case: TOML creates the parent implicitly, so [tool.hatch.build] alone still declares hatch. A file we cannot parse declares nothing, which leaves the project ambiguous and the user asked — the honest answer when we cannot read what manages it. Co-Authored-By: Claude Opus 5 (1M context) * refactor(setup): read pyproject.toml once per detection Detection parsed the file twice for a Python project: once for the detector's own package manager and again for the confidence verdict. The verdict is now the single source for the languages it models, and the detector leaves the field to it. A language it does not model, such as Java's maven versus gradle, keeps the answer its detector gives. Co-Authored-By: Claude Opus 5 (1M context) * fix(setup): stop misreading other ecosystems' install errors The packageManager guidance matched a missing or non-semver version anywhere in an install failure, so a gem, a Python package or a Go module reporting either phrase had its real error replaced by advice about a file it does not have. The output has to name package.json, which both corepack refusals do. Say what was actually found when a project is set up for more than one manager: a Pipfile and a [tool.*] table count as commitments too, so naming lockfiles sent the reader looking for files that are not there. Note on stderr when no --package-manager was given, since that used to fall through to npm or pip and now reads the project. Callers parsing output are unaffected. Co-Authored-By: Claude Opus 5 (1M context) * feat(setup): ask which package manager to use when the project is ambiguous A project with lockfiles for two managers, or none at all, has no answer we can read off disk, and picking one is how a yarn project gets installed with npm. The wizard now asks, and says why it is asking. Installed managers are listed first and the cursor starts on one, but an uninstalled manager stays selectable: the choice is the user's and the install step already warns rather than installing tooling. Projects that state their manager go straight to the plan. Co-Authored-By: Claude Opus 5 (1M context) * fix(setup): keep the package-manager picker inside the terminal The screen draws a title, the reason for asking and a key hint around the list, but the list was sized to the whole window, so the hint — including how to go back — was pushed off the bottom at every terminal size. The list now leaves room for that chrome, and the list's own help line goes away since the screen prints its own. Below fourteen rows the reason is dropped: at that size the question and the choices matter more than the explanation. Co-Authored-By: Claude Opus 5 (1M context) * fix(setup): show each screen's key hints once, inside the list The project and environment screens printed a footer of key hints while the list below already rendered its own help, so every instruction appeared twice. The wizard's own bindings now go into the list's help line, which is the single place a screen states them, and the footers are gone. The package-manager picker follows the same shape, and its height reserve is retuned for the help line the list now draws, including the extra row that line takes when the terminal is narrower than it is. Co-Authored-By: Claude Opus 5 (1M context) * fix(setup): wrap the plan steps to the terminal width A step naming an absolute entry-point path alongside the warning that no entry file was found runs well past a narrow terminal, and overflowing there hides the warning the step exists to give. Steps now wrap, with what wraps indented under the number so a step still reads as one item. The screen that names the injected file wraps for the same reason. Co-Authored-By: Claude Opus 5 (1M context) * fix(setup): stop padding pushing the injected file path off screen Wrapping pads every line to the full width, so the newline left inside the wrapped lead put a whole row of spaces in front of the file path and carried it past the edge of the terminal. The newline now sits outside the wrap, and the closing instruction wraps too. Co-Authored-By: Claude Opus 5 (1M context) * fix(setup): stop the package-manager list quitting on esc The list inherits the same quit binding as the others, so esc arriving on its own ended the session from the picker. Co-Authored-By: Claude Opus 5 (1M context) * feat(setup): say the verify step waits for the app as well as the SDK Verification cannot succeed until the user's application is running, so naming only the SDK left it unclear whether anything was expected of them. Co-Authored-By: Claude Opus 5 (1M context) --------- Co-authored-by: Claude Opus 5 (1M context) --- cmd/setup/model.go | 27 ++++ cmd/setup/update.go | 108 ++++++++++++-- cmd/setup/view.go | 71 +++++++-- cmd/setup/wizard_test.go | 300 +++++++++++++++++++++++++++++++++++++-- 4 files changed, 479 insertions(+), 27 deletions(-) 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/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") + } + }) + } + } +}