Add support for custom registries to updateNotifier

[jdalton]

No description provided.

[claude]

Claude finished @jdalton's task —— View job

---

Code Review: Add support for custom registries to updateNotifier

Todo List:

• Read repository CLAUDE.md for guidelines (no CLAUDE.md found)
• Analyze changed files and understand implementation
• Review the patch file (likely core functionality)
• Review CLI changes in src/cli.mts
• Provide feedback on code quality and best practices
• Check for security implications
• Assess performance considerations
• Evaluate architecture and design decisions

Analysis

This PR adds support for custom npm registries to the update notifier functionality by:

1. Patching tiny-updater (via patches/tiny-updater#3.5.3.patch) to support:

   • Custom registry URLs (registryUrl parameter)
   • Authentication info (authInfo parameter with type and token)
   • Modified fetch logic to include Authorization headers

2. Updating CLI usage (src/cli.mts:52-60) to pass:

   • Registry URL from registry-url package
   • Auth token from registry-auth-token package
   • Proper configuration to updateNotifier

Code Quality Review

✅ Strengths:

• Type Safety: The patch adds comprehensive TypeScript types for all new parameters (AuthInfo, UtilsGetLatestVersionOptions, etc.)
• Backward Compatibility: New parameters are optional with sensible defaults (registry defaults to npmjs.org)
• Proper Integration: Uses existing ecosystem libraries (registry-url, registry-auth-token) for consistent behavior
• Clean API Design: The destructured options pattern with __proto__: null is well-implemented

⚠️ Areas for Improvement:

1. URL Handling Logic (patches/tiny-updater#3.5.3.patch:143-144):

   const maybeSlash = registryUrl.endsWith('/') ? '' : '/';
   const latestUrl = `${registryUrl}${maybeSlash}${name}/latest`;

   This could be simplified and made more robust using URL constructor for proper URL joining.

2. Error Handling: The patch doesn't add specific error handling for authentication failures or invalid registry URLs. Network errors are caught generically with .catch(Utils.noop) which might suppress important auth-related errors.

3. Security Considerations: While the auth token handling looks correct, there's no validation of the authInfo.type parameter - it's directly interpolated into the Authorization header.

Security Review

✅ Security Positives:

• Auth tokens are properly handled through established registry libraries
• No hardcoded credentials or sensitive data
• Uses proper Authorization header format

⚠️ Potential Concerns:

• authInfo.type is directly concatenated without validation - should validate against known auth types (Bearer, Basic, etc.)
• No input validation on registryUrl - could potentially be exploited if user-controlled

Performance Considerations

Minimal Impact: The changes add minimal overhead:

• Optional parameter destructuring
• Simple string operations for URL construction
• One additional header in HTTP requests when auth is present

Architecture & Design

✅ Well-Architected:

• Leverages existing npm ecosystem patterns and libraries
• Maintains separation of concerns (CLI handles config lookup, updater handles the actual checking)
• Follows the existing codebase patterns and style

Suggestions for Future Enhancement:

1. Consider adding registry URL validation
2. Add specific error types for authentication failures
3. Consider caching registry-specific settings to avoid repeated lookups

Overall Assessment

This is a solid implementation that successfully adds custom registry support while maintaining backward compatibility and following good practices. The approach of patching the upstream library and integrating with standard npm ecosystem tools is pragmatic and appropriate.

Recommendation: ✅ Approved - The implementation is functional and follows good practices, with only minor suggestions for potential future improvements.