Conversation
k80bowman
requested changes
Mar 2, 2026
Contributor
k80bowman
left a comment
There was a problem hiding this comment.
I'm excited about using hooks for this, I think that's a great idea. And I love the idea of adding the plugin health check as well, that's awesome. I do have a few implementation comments, but this is great. I appreciate you figuring this out.
…updates This change improves the user experience when updating the Heroku CLI by adding better handling for npm authentication and plugin installation issues. Key changes: - Add preupdate hook (check-npm-auth) that detects private plugins and prompts users to authenticate with npm before attempting the update - Add post-update hook (check-plugin-health) that verifies all plugins installed correctly and provides recovery instructions for missing plugins - Remove deprecated v6 plugin migration code from plugin-migrate hook - Add comprehensive unit tests for both new hooks The check-npm-auth hook: - Reads installed plugins from package.json - Checks which plugins are private (require authentication) - Verifies npm authentication status - Prompts user to login if needed before proceeding with update - Handles user cancellation gracefully with clear messaging The check-plugin-health hook: - Runs after plugin installation during update - Checks if all configured plugins exist in node_modules - Warns users about missing plugins with recovery instructions - Suggests both reinstall and uninstall options This resolves issues where users would get cryptic npm errors during CLI updates when logged out of npm with private plugins installed.
Refactor check-npm-auth hook to check multiple plugins concurrently instead of sequentially. This significantly reduces the time to check plugin privacy when users have many plugins installed. - Process plugins in batches of 5 using Promise.all - Maintains the same behavior and debug logging - Reduces check time from O(n) sequential to O(n/5) batches
- Extract NpmAuth class with testable wrapper methods (exec, spawn) - Add isNpmAvailable() check to skip auth flow when npm is not installed - Treat 404 errors as potentially private packages (npm returns 404 for private packages when not authenticated) - Consolidate duplicate warning messages - Simplify authentication flow to never block updates - always warn and continue - Reduce check-npm-auth hook from 230 lines to 118 lines Tests: - Add 14 unit tests for NpmAuth class methods - Add 9 comprehensive tests for hook orchestration logic - Remove weak "doesn't throw" assertions in favor of specific behavioral checks - All 23 tests passing
- Fix FileHandle deprecation warning by explicitly closing historyStream on REPL exit - Fix terms-of-service hook to properly await async fs.createFile - Simplify check-plugin-health error message by removing redundant text
The hook was registered as 'preupdate' in package.json but was located in the hooks/update directory. Move it to hooks/preupdate to match its actual hook type and improve code organization.
1e78521 to
3f367e5
Compare
k80bowman
approved these changes
Mar 3, 2026
Contributor
k80bowman
left a comment
There was a problem hiding this comment.
This looks awesome, thank you for all of the fixes!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR improves the user experience when updating the Heroku CLI by adding better handling for npm authentication and plugin installation issues. Users previously encountered cryptic npm errors when updating while logged out of npm with private plugins installed. This change proactively detects and handles these scenarios with clear messaging and interactive prompts.
Key Changes
1. Pre-update npm authentication check (
hooks/preupdate/check-npm-auth.ts)preupdatehook that runs before the CLI update begins2. Post-update plugin health check (
hooks/update/check-plugin-health.ts)updatehook that runs after plugin installation3. Reusable npm authentication library (
lib/npm-auth.ts)isNpmAvailable(),isAuthenticated(),isPrivatePackage(),login()4. Cleanup
hooks/update/plugin-migrate.ts)hooks/init/terms-of-service.tslib/repl.ts(missinghistoryStream.close())Type of Change
Feature Additions (minor semver update)
Testing
Notes:
The hooks integrate into the existing oclif update lifecycle. The check-npm-auth hook runs before the update starts (preupdate), while check-plugin-health runs after plugin installation (update). Both hooks are non-blocking - errors won't prevent updates from completing. Since this is part of the lifecycle, we'll test this once this gets into the beta release.
Steps:
heroku plugins:install @private/some-pluginnpm logoutheroku updateTo test plugin health check:
heroku updateRelated Issues
https://gus.lightning.force.com/lightning/r/ADM_Work__c/a07EE00002VgjNJYAZ/view