Skip to content

fix(no-ticket): show pnpm hosts when the pnpm launcher is missing - #554

Open
cloudsmith-iduffy wants to merge 2 commits into
masterfrom
iduffy/fix-pnpm-status-missing-launcher
Open

cloudsmith-iduffy wants to merge 2 commits into
masterfrom
iduffy/fix-pnpm-status-missing-launcher

Conversation

@cloudsmith-iduffy

@cloudsmith-iduffy cloudsmith-iduffy commented Oct 24, 2026 •

Copy link
Copy Markdown
Collaborator

Description

If the pnpm launcher is gone but .npmrc still points pnpm at it, cloudsmith credential-helper list showed pnpm with no hosts. That hid the broken state: pnpm still tries to run the missing launcher for those hosts and fails.

PNPMInstaller.status() set the launcher path to None when the file did not exist. Then it looked for tokenHelper entries with the value "None", and found none. Now status() matches the .npmrc entries against the expected launcher path, and it still reports the launcher as missing. Docker and Cargo already report hosts whatever the launcher state is.

I also corrected the status() docstring. It described Docker's config.json credHelpers block, not .npmrc.

Before (launcher deleted):

pnpm  (pnpm credential helper for Cloudsmith registries)
  launcher : not installed
  hosts    : none configured

After (launcher deleted):

pnpm  (pnpm credential helper for Cloudsmith registries)
  launcher : not installed
  hosts    : npm.cloudsmith.io

To reproduce, use a temporary HOME and set NPM_CONFIG_USERCONFIG=$HOME/.npmrc. Run cloudsmith credential-helper install pnpm --no-discover, then delete $HOME/.cloudsmith/bin/pnpm-credential-cloudsmith, then run cloudsmith credential-helper list.

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation update
  • Refactoring
  • Other (please describe)

Additional Notes

This touches the same status() method as #549, but not the same lines, so the code merges cleanly. The only conflict is the ### Fixed heading in CHANGELOG.md, because both PRs add it.

Hosts installed with --bin-dir didn't show either, because their tokenHelper value is not the default launcher path. NPMRC.helped_hosts() now matches the launcher file name in any directory, the same way Docker and Cargo match on their helper name. uninstall pnpm uses the same lookup, so it now removes those .npmrc entries too, as Docker and Cargo already do. The "launcher" docstring now says it is a str, not a Path.

🤖 Generated with Claude Code

PNPMInstaller.status() set the launcher path to None when the launcher
file did not exist. It then matched .npmrc tokenHelper entries against
the string "None", so `credential-helper list` showed no hosts. Match
the entries against the expected launcher path. Also correct the
docstring, which described the Docker config.json.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings October 24, 2026 17:56

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

The focused fix preserves the status return contract, includes regression coverage, and has no identified blocking issues.

No open issues 🎉

What changed in this PR

Fixes pnpm status reporting so cloudsmith credential-helper list shows configured hosts even when their expected launcher is missing.

Changes:

  • Matches .npmrc entries against the expected launcher path while still reporting the launcher as missing.
  • Adds a regression test for a deleted launcher.
  • Corrects the status docstring and documents the fix in the changelog.
File Description
cloudsmith_cli/​credential_helpers/​pnpm/​installer.py Preserves host detection when the launcher is missing and corrects the docstring.
cloudsmith_cli/​cli/​tests/​commands/​test_credential_helper_install.py Tests host reporting after launcher deletion.
CHANGELOG.md Documents the corrected pnpm status output.

Review effort: Balanced

NPMRC.helped_hosts() matched only the tokenHelper value that is the
launcher path in the default bin directory. `credential-helper list`
did not show hosts that you installed with --bin-dir. Match the
launcher file name in any directory, as Docker and Cargo do. Uninstall
uses the same match, so it now removes these entries too. Also correct
the type of "launcher" in the status() docstring.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@cloudsmith-iduffy
cloudsmith-iduffy marked this pull request as ready for review October 24, 2026 17:59
@cloudsmith-iduffy
cloudsmith-iduffy requested a review from a team as a code owner October 24, 2026 17:59

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants