Skip to content

version pinning piece 1: misleading 'earlier pin stands' warning, and [PINNED] lost when the gateway's env sets npm config #322

Closed

Description

@ViperJuice

Non-blocking findings from the review panel on #319 (version pinning, piece 1; see #294). Both fail safe (the server runs unpinned or reports [FAILED], never a wrong [PINNED]), but the operator is told the wrong thing.

N1: the invalid-pin warning says an earlier pin stands when it doesn't

The "invalid pin" warning, and the README, say "a pin from an earlier source, if any, stands". That is false when a later overlay replaces the whole server entry with a bad version:.
Repro: ~/.pmcp/manifest.yaml pins firecrawl to 3.25.5, then a PMCP_MANIFEST_PATH overlay replaces the entry with version: "latest". The server ends up unpinned, and that warning is the only pin-related log line.
Fix: different fixed text for an entry's version field (the entry, and any earlier pin, was replaced) than for an overlay's server_version (the earlier pin stands).

N2: [PINNED] disappears when the gateway process itself sets npm config

Main's resolver turns off npm package detection for the whole process when the gateway's own environment sets npm config, so pmcp update prints [FAILED] … Could not determine a registry package for a pinned server. Our team hosts export npm_config_cache, so this hits our own fleet. The README mentions only the case where the entry itself sets an npm key.
Fix: document the process-environment case in the README sentence, and make the 7 [PINNED] tests in tests/test_version_pin.py remove the host's npm_config_* / pnpm_config_* variables themselves (they fail on dev0 unless those are unset).

Nit

src/pmcp/manifest/loader.py imports from collections.abc twice.

See #319, #294.

Activity

  1. ViperJuice commented on Sep 30, 2026

    @ViperJuice
    CollaboratorAuthor

    Resolved by #323 (merged). N1 and its mirror case on server_version: warnings are true in every merge-order case (432-case matrix tests). N2: README lists all three [FAILED] causes, with a test locking two of them; [PINNED] tests clear the host's npm config. Nit: imports merged.

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions