Skip to content

feat: throw error on duplicate module names in workspace#886

Open
pyramation wants to merge 1 commit intomainfrom
devin/1774306216-error-on-duplicate-modules
Open

feat: throw error on duplicate module names in workspace#886
pyramation wants to merge 1 commit intomainfrom
devin/1774306216-error-on-duplicate-modules

Conversation

@pyramation
Copy link
Copy Markdown
Contributor

Summary

Adds a DUPLICATE_MODULE error that is thrown when multiple modules in a pgpm workspace share the same .control file name, similar to how pnpm/yarn workspaces reject duplicate package names.

Before: getModules() silently returned all duplicates (causing duplicate entries in pgpm deploy's interactive prompt), and listModules() silently picked the shortest path.

After: Both methods throw a DUPLICATE_MODULE error listing the conflicting paths.

Two files changed:

  • pgpm/types/src/error-factory.ts — new DUPLICATE_MODULE error definition
  • pgpm/core/src/core/class/pgpm.ts — duplicate detection in getModules() and listModules()

Review & Testing Checklist for Human

  • listModules() behavior change: The old code intentionally handled naming collisions by picking the shortest path (lines 334-343 before). This PR replaces that with a hard error. Verify this is the desired behavior — any workspace with legitimately duplicated .control file names (e.g. nested node_modules, symlinks) will now fail instead of deduplicating gracefully.
  • Breaking change scope: Any existing workspace that has duplicate module names will start failing on any command that calls getModules() or listModules(). Confirm no existing workspaces besides agentic-db are affected, or that this is acceptable.
  • getModuleName() reliability in getModules(): The new code calls proj.getModuleName() on each PgpmPackage instance. Verify this works correctly for all module types (the original code didn't need the module name, just isInModule()).
  • No unit tests added — consider whether test coverage for the new error path is needed.
  • Test manually: Set up a workspace with two directories containing .control files that share the same base name, run pgpm deploy, and confirm the error message is clear and actionable.

Notes

  • Only the first duplicate group triggers the error (the for loop throws immediately). Users with multiple duplicate groups will need to fix them one at a time.
  • The error uses HTTP code 400 consistent with other validation errors in the factory.

Link to Devin session: https://app.devin.ai/sessions/c26111a26a9a44f891534104a809dcdb
Requested by: @pyramation

Similar to how pnpm/yarn workspaces error when multiple packages share the
same name, pgpm now throws a DUPLICATE_MODULE error when getModules() or
listModules() encounters multiple modules with the same .control file name.

Previously getModules() returned all matches (causing duplicate entries in
pgpm deploy's interactive prompt) and listModules() silently picked the
shortest path. Both now throw a descriptive error listing the conflicting
paths.
@devin-ai-integration
Copy link
Copy Markdown
Contributor

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

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.

1 participant