Skip to content

chore(setup): add SDK/project detection library - #749

Open
ffantl-ld wants to merge 4 commits into
setup-ldfrom
ffantl/setup-ld/1-detector
Open

chore(setup): add SDK/project detection library#749
ffantl-ld wants to merge 4 commits into
setup-ldfrom
ffantl/setup-ld/1-detector

Conversation

@ffantl-ld

@ffantl-ld ffantl-ld commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Describe the solution you've provided

First layer of the guided setup command: the internal/setup detection library. Inspects a project directory and identifies the language/package manager and the most likely LaunchDarkly SDK (Detector interface, FileDetector, KnownSDKs).

Pure library with no consumer yet — the setup command wires it up later in this stack.

Related issues

Part of the setup-ld feature. Stacked PR — base is setup-ld.

Requirements

  • I have added test coverage for new or changed functionality

Note

Low Risk
New read-only library with no command integration yet; main future risk is mishandling suggested entry points when setup writes init code.

Overview
Adds internal/setup project detection for the upcoming guided setup flow: a Detector interface, FileDetector that inspects the working directory, and KnownSDKs for manual --sdk-id overrides when auto-detection fails.

FileDetector walks common manifest/layout signals and returns language, framework, package manager, a LaunchDarkly sdk_id, and an entry-point path. Node handling distinguishes Next.js (server node-server, prefers instrumentation.ts over app/pages routes), React Native vs React, other browser stacks (Vue, Angular, etc. → js-client-sdk), and lockfile-based PMs (pnpm, yarn, bun). Similar logic covers Go, Python (pip/poetry/uv/pipenv), Ruby, Java vs Android (manifest + nested MainActivity search), Swift (SPM/Xcode), and .NET.

EntryPointExists separates a path that was found on disk from a suggested fallback so later setup steps do not silently inject code into files the app never loads. StubDetector remains as an explicit not-implemented placeholder. Broad unit tests cover frameworks, edge cases (multi-binary Go, ambiguous Swift targets), and helpers.

Reviewed by Cursor Bugbot for commit 3fefe37. Bugbot is set up for automated code reviews on this repo. Configure here.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
if result := detectDotnet(dir); result != nil {
return result, nil
}
return nil, errors.New("could not detect project language from directory; try specifying --sdk-id manually")

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"could not detect project language from directory")

Should probably mean "Hey, here's a link to the docs and a blurb about how to do this install manually"

If we're doing a hybrid approach to setup, then we should definitely

@ffantl-ld
ffantl-ld marked this pull request as ready for review July 27, 2026 17:23
@ffantl-ld
ffantl-ld requested review from Vadman97 and erangeles July 27, 2026 17:23
Comment thread internal/setup/detector.go Outdated
Comment thread internal/setup/detector.go Outdated
Comment thread internal/setup/detector.go Outdated
Comment thread internal/setup/detector.go
Comment thread internal/setup/detector.go
@ffantl-ld
ffantl-ld requested review from a team and removed request for Vadman97 July 28, 2026 16:37
@erangeles

erangeles commented Jul 28, 2026

Copy link
Copy Markdown

@ffantl-ld there is some valid bug bot issues flagged, wanted to make sure you were aware of them (either to fix now or punt on)

Comment thread internal/setup/detector.go Outdated
// firstExistingIn returns the first candidate that exists as a file in dir,
// or the last candidate if none exist (as a suggested path).
// Returns an empty string if candidates is empty.
func firstExistingIn(dir string, candidates []string) string {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: we should return whether the entry point actually exists so downstream steps can prompt the user instead

type DetectResult struct {
      // ...
      EntryPoint       string `json:"entry_point"`
      EntryPointExists bool   `json:"entry_point_exists"`
}

// firstExistingIn returns the first candidate that exists in dir and true,
// or the last candidate and false as a suggestion if none exist.
func firstExistingIn(dir string, candidates []string) (string, bool) {
      if len(candidates) == 0 {
              return "", false
      }
      for _, c := range candidates {
              if _, err := os.Stat(filepath.Join(dir, c)); err == nil {
                      return c, true
              }
      }
      return candidates[len(candidates)-1], false
}

Then the wizard can branch: exists → inject; doesn't → ask the user for the entry file (or show the snippet).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DetectResult now carries EntryPointExists. Good find. :)

@erangeles

Copy link
Copy Markdown

@ffantl-ld bump, just wanted to make sure you saw my earlier comments

DetectResult carries EntryPointExists so callers can tell an entry file the
detector found from one it merely suggests, and never write initialization code
into a path the project does not load.

PackageManager names the tool that manages the project's dependencies: bundle
rather than gem when a Gemfile is present, and poetry, uv or pipenv rather than
always pip.

Locate MainActivity and Main under their real package directory rather than
assuming an unqualified class name, derive the Android source root from
whichever manifest matched, find the Swift entry point where SwiftPM and Xcode
nest it, look for src/main.tsx where Vite mounts a React app, and recognise
bun.lockb.

Rename the Android SDK ID to android for consistency with the other IDs.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Comment thread internal/setup/detector.go
ffantl-ld and others added 2 commits July 31, 2026 13:57
Next.js detection targeted whichever page module happened to exist, and
node-server is append-safe, so setup wrote server SDK init — including the SDK
key — into app/page.tsx or pages/index.tsx. A page module may carry 'use client'
or be imported by something that does, which bundles it for the browser, and
nothing in the detector can tell which. Only instrumentation.ts, Next's
server-startup hook, is guaranteed to stay server-side, so suggest creating it
rather than picking a page.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Each candidate list encodes a claim about where a toolchain puts its entry file.
Link the documentation that claim rests on so it can be rechecked when the
frameworks move.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 3fefe37. Configure here.

return result, nil
}
return nil, errors.New("could not detect project language from directory; try specifying --sdk-id manually")
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Node detection shadows other languages

High Severity

Detect always prefers detectNode whenever a parseable package.json exists, so polyglot projects never reach later detectors. Common layouts like Rails (Gemfile + package.json), Go repos that ship npm packaging (this repo included), or Django with frontend tooling get node-server instead of the backend SDK.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 3fefe37. Configure here.

"App.swift", "ContentView.swift", "AppDelegate.swift",
findFileUnder(dir, appRoot, "*App.swift", "ContentView.swift", "AppDelegate.swift"),
findFileUnder(dir, "Sources", "main.swift", "*App.swift"),
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Swift ignores multi-target ambiguity

Medium Severity

swiftEntryCandidates walks all of Sources for main.swift and *App.swift before applying the soleSubdir guard. In a multi-target package, an arbitrary target file can be returned with EntryPointExists true, contradicting the stated rule that multi-target layouts must only suggest a path.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 3fefe37. Configure here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants