Skip to content

feat(discovery): add project discovery package#262

Open
pjcdawkins wants to merge 2 commits intorefactor/abstract-fs-2-fsysfrom
refactor/abstract-fs-3-discovery
Open

feat(discovery): add project discovery package#262
pjcdawkins wants to merge 2 commits intorefactor/abstract-fs-2-fsysfrom
refactor/abstract-fs-3-discovery

Conversation

@pjcdawkins
Copy link
Contributor

Summary

Stacked on #261.

Extract project detection logic from question handlers into a new discovery/ package.

  • Discoverer struct operates on fs.FS with memoized detection methods: Stack(), Type(), DependencyManagers(), BuildSteps(), ApplicationRoot(), Environment()
  • stack.go rewritten to delegate to Discoverer.Stack() instead of inline detection
  • Comprehensive tests for all discovery methods
  • Adds Symfony, Ibexa, Shopware stack constants to platformifier
  • Adds CountFiles utility for runtime detection heuristic

Author: @akalipetis

🤖 Generated with Claude Code

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR introduces a new discovery/ package to centralize project detection logic (stack/runtime/dependency managers/build steps/etc.) over an fs.FS, and wires stack discovery into the existing interactive questionnaire flow.

Changes:

  • Add discovery.Discoverer with memoized detection methods and accompanying unit tests.
  • Refactor stack detection in internal/question/stack.go to delegate to Discoverer.Stack().
  • Extend platform stack constants and add a CountFiles utility to support runtime/type heuristics.

Reviewed changes

Copilot reviewed 19 out of 19 changed files in this pull request and generated 8 comments.

Show a summary per file
File Description
platformifier/models.go Adds new stack constants (Symfony, Ibexa, Shopware) and Stack.Name() mappings.
internal/utils/utils.go Adds CountFiles(fs.FS) used for runtime/type heuristics.
internal/question/working_directory.go Initializes answers.Discoverer based on the chosen working directory.
internal/question/stack.go Uses Discoverer.Stack() instead of inline filesystem heuristics.
internal/question/stack_test.go Adds tests verifying stack question behavior with fstest.MapFS.
internal/question/models/answer.go Adds Discoverer to Answers.
internal/question/build_steps.go Restores manage.py constant at package scope after stack refactor.
discovery/* New package implementing discovery logic (stack/type/env/build steps/app root/dependency managers) with tests.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +155 to +156
if keywordsVal, ok := keywords.([]string); ok && slices.Contains(keywordsVal, "shopware") {
return platformifier.Shopware, nil
Copy link

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

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

GetJSONValue() unmarshals JSON arrays as []any, so asserting keywords.([]string) will never succeed and this Shopware detection path is effectively dead code. Consider handling []any and checking each element for the "shopware" string (or adjust GetJSONValue to decode arrays of strings when appropriate).

Suggested change
if keywordsVal, ok := keywords.([]string); ok && slices.Contains(keywordsVal, "shopware") {
return platformifier.Shopware, nil
// Handle both []string and []any returned by GetJSONValue for JSON arrays.
if keywordsVal, ok := keywords.([]string); ok {
if slices.Contains(keywordsVal, "shopware") {
return platformifier.Shopware, nil
}
} else if keywordsSlice, ok := keywords.([]any); ok {
for _, kw := range keywordsSlice {
if kwStr, ok := kw.(string); ok && kwStr == "shopware" {
return platformifier.Shopware, nil
}
}

Copilot uses AI. Check for mistakes.
Comment on lines +40 to +42
"yarn/yarn.lock": &fstest.MapFile{},
"poetry/poetry.lock": &fstest.MapFile{},
"composer/composer-lock.json": &fstest.MapFile{},
Copy link

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

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

Test fixture uses composer/composer-lock.json, but the code detects Composer via composer.lock. As written, this test doesn’t exercise Composer detection at all. Rename the fixture to composer/composer.lock (or adjust the detector if the intended file is different).

Suggested change
"yarn/yarn.lock": &fstest.MapFile{},
"poetry/poetry.lock": &fstest.MapFile{},
"composer/composer-lock.json": &fstest.MapFile{},
"yarn/yarn.lock": &fstest.MapFile{},
"poetry/poetry.lock": &fstest.MapFile{},
"composer/composer.lock": &fstest.MapFile{},

Copilot uses AI. Check for mistakes.
Comment on lines +47 to +59
{
name: "Pipenv Django with Yarn build",
fields: fields{
fileSystem: fstest.MapFS{
"project/manage.py": &fstest.MapFile{},
"package.json": &fstest.MapFile{Data: []byte(`{"scripts": {"build": "nuxt build"}}`)},
},
memory: map[string]any{
"stack": platformifier.Django,
"type": "python",
"dependency_managers": []string{"poetry", "yarn"},
"application_root": ".",
},
Copy link

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

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

The test case name says "Pipenv Django with Yarn build", but the setup uses dependency_managers: []string{"poetry", "yarn"} and asserts Poetry commands. Rename the case (or change the fixture) so the test name matches what it actually covers.

Copilot uses AI. Check for mistakes.
@@ -51,166 +40,69 @@ func (q *Stack) Ask(ctx context.Context) error {
}()

answers.Stack = models.GenericStack
Copy link

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

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

answers.Discoverer is dereferenced without a nil check. If Stack.Ask is ever called without WorkingDirectory.Ask having initialized Discoverer, this will panic. Consider lazily initializing it from answers.WorkingDirectory when nil (or returning a clear error) to make the question handler more robust.

Suggested change
answers.Stack = models.GenericStack
answers.Stack = models.GenericStack
if answers.Discoverer == nil {
return fmt.Errorf("stack discoverer is not initialized")
}

Copilot uses AI. Check for mistakes.
HasGit bool `json:"has_git"`
FilesCreated []string `json:"files_created"`
Locations map[string]map[string]interface{} `json:"locations"`
Discoverer *discovery.Discoverer
Copy link

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

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

Discoverer is a runtime helper and likely shouldn’t be serialized with Answers (it has no json tag, unlike most fields). Consider adding json:"-" to prevent accidental inclusion in any JSON output/telemetry and to clarify it’s not user-provided data.

Suggested change
Discoverer *discovery.Discoverer
Discoverer *discovery.Discoverer `json:"-"`

Copilot uses AI. Check for mistakes.
Comment on lines +90 to +102
func (d *Discoverer) nodeExecPrefix() string {
dependencyManagers, err := d.DependencyManagers()
if err != nil {
return ""
}

if slices.Contains(dependencyManagers, "yarn") {
return "yarn exec"
}

if slices.Contains(dependencyManagers, "npm") {
return "npm exec "
}
Copy link

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

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

nodeExecPrefix() returns "yarn exec" without a trailing space, but callers concatenate it with the command (e.g. +"next build"), producing an invalid command like yarn execnext build. Return "yarn exec " (or add spacing at the call site) to ensure valid build steps for Yarn.

Copilot uses AI. Check for mistakes.
"# Install Pipenv as a global tool",
"python -m venv /app/.global",
"pip install pipenv==$PIPENV_TOOL_VERSION",
"pipenv install",
Copy link

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

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

The Pipenv build step uses pipenv install, but Pipenv is only detected via Pipfile.lock (see dependencyManagersMap), and the existing question flow uses pipenv sync to install from the lockfile. Using install can update the lock and produce non-reproducible builds. Switch this step to pipenv sync (or otherwise ensure it installs strictly from the lock).

Suggested change
"pipenv install",
"pipenv sync",

Copilot uses AI. Check for mistakes.
Comment on lines +118 to +127
if managePyPath := utils.FindFile(
d.fileSystem,
appRoot,
managePyFile,
); managePyPath != "" {
buildSteps = append(
buildSteps,
"# Collect static files",
fmt.Sprintf("%spython %s collectstatic --noinput", d.pythonPrefix(), managePyPath),
)
Copy link

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

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

managePyPath is used directly in the collectstatic command, but FindFile() returns a path relative to the repo root (e.g. project/manage.py). If build hooks execute from application_root (common), this becomes the wrong path when application_root != ".". Convert managePyPath to a path relative to appRoot (similar to the existing question implementation) before formatting the command.

Copilot uses AI. Check for mistakes.
Antonis Kalipetis and others added 2 commits March 24, 2026 01:18
Extract project detection logic from question handlers into a new
discovery/ package. The Discoverer operates on an fs.FS and provides
memoized methods for detecting stack, runtime, dependency managers,
build steps, application root, and environment.

- Add discovery package with Stack, Type, DependencyManagers,
  BuildSteps, ApplicationRoot, Environment detection
- Add comprehensive tests for all discovery methods
- Add Discoverer field to Answers, initialized in WorkingDirectory
- Rewrite stack.go to delegate to Discoverer.Stack() instead of
  inline detection
- Add Symfony, Ibexa, Shopware stack constants to platformifier
- Add CountFiles utility for runtime detection heuristic

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Add missing trailing space in nodeExecPrefix() for yarn, which caused
  "yarn execnext build" instead of "yarn exec next build".
- Add missing Rails detection via config.ru, which was dropped during
  the refactor from question/stack.go to discovery/stack.go.
- Add Rails case to discoverType() to return "ruby".
- Change pipenv build step from "pipenv install" to "pipenv sync" to
  match the original behavior (installs from lock file only).
- Remove redundant zero-value map initialization in discoverType().

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@pjcdawkins pjcdawkins force-pushed the refactor/abstract-fs-3-discovery branch from a2655d1 to c8068a9 Compare March 24, 2026 01:20
@pjcdawkins pjcdawkins force-pushed the refactor/abstract-fs-2-fsys branch from 9fd9c6f to ac58a58 Compare March 24, 2026 01:20
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