Skip to content

Conversation

domdomegg
Copy link
Member

Summary

  • Add URL encoding for package identifiers and versions in NPM registry requests
  • Fixes issue where scoped packages like @hellocoop/admin-mcp failed validation due to unencoded @ and / symbols
  • Add unit tests for scoped NPM packages to prevent regression

Fixes #409

@Avish34
Copy link
Contributor

Avish34 commented Sep 11, 2025

@domdomegg - #424 just added the extra validation to fail fast before we send a request. To clarify, it wasn't the fix for the URL encoding issue that we saw. Do we want to remove this validation?

@domdomegg domdomegg marked this pull request as draft September 11, 2025 06:12
@domdomegg
Copy link
Member Author

Sorry I have no idea what happened here! I didn't mean to remove this validation at all. I thought I had a PR up to URL encode the params, but think I mangled it in a force push... fixing...

@domdomegg domdomegg closed this Sep 11, 2025
@domdomegg domdomegg force-pushed the adamj/fix-npm-scoped-packages branch from 736901d to 17eb56d Compare September 11, 2025 06:14
- Add URL encoding for package identifiers and versions in NPM registry requests
- Fixes issue where scoped packages like @hellocoop/admin-mcp failed validation
- Add unit tests for scoped NPM packages

Fixes #409

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>

:house: Remote-Dev: homespace
@domdomegg domdomegg reopened this Sep 11, 2025
@domdomegg
Copy link
Member Author

Okay, correct implementation restored. @Avish34 would you be able to review this now corrected PR?

@domdomegg domdomegg marked this pull request as ready for review September 11, 2025 06:17
@Avish34
Copy link
Contributor

Avish34 commented Sep 11, 2025

No worries.
Also, maybe I can add checks for version formatting too else user can pass anything and will still be go ahead and make request. What's your thought?

Copy link
Contributor

@Avish34 Avish34 left a comment

Choose a reason for hiding this comment

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

LGTM @domdomegg .
Thanks!

@domdomegg domdomegg merged commit a100367 into main Sep 11, 2025
5 checks passed
@domdomegg domdomegg deleted the adamj/fix-npm-scoped-packages branch September 11, 2025 16:34
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.

NPM package validation failing for certain packages
3 participants