fix: allow the ADL conformance forks under blockExoticSubdeps - #128
Merged
Conversation
pnpm's blockExoticSubdeps defaults to true and rejects any transitive dependency resolved from git, which is exactly what the two ADL conformance suite overrides are: [ERR_PNPM_EXOTIC_SUBDEP] Exotic dependency "super-request" (resolved via git-repository) is not allowed in subdependencies The check is skipped for packages that come straight from the lockfile (resolveDependencies.ts gates on `resolvedVia != null`), so `--frozen-lockfile` in CI kept passing and only a FRESH resolution trips it — `pnpm install` after a dependency change, Renovate regenerating the lockfile for one of the override-bump PRs, or lockfile maintenance. It is also version-dependent: 11.15.1 does not enforce it, 11.20.0 does. pnpm exposes no per-package allowlist for this — it is a single global boolean — and making the packages direct devDependencies does not help either, since the check inspects the subdependency edge itself (both verified). So the guard goes off for the whole tree, which the comment in pnpm-workspace.yaml states plainly along with why it is acceptable here: both packages are dev-only test tooling pinned to immutable commit SHAs and tracked by Renovate, so their content cannot change without a reviewed PR. The note says to delete the setting rather than keep it if the forks are ever retired. Bumps the pinned pnpm 11.15.1 -> 11.20.0 in the same change so CI actually exercises the version that enforces the check; pinning below it would leave the fix unverified and break again on the next pnpm bump. The lockfile is unchanged by both the setting and the version bump. Verified under 11.20.0: fresh full resolution from an empty node_modules, a simulated Renovate override-SHA bump (the exact command that failed), typecheck/lint/fmt/arch, 293 unit tests, 65 integration, and the 37-test ADL conformance suite — the actual consumer of the forked packages.
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.
Resolves:
Why this appeared now
blockExoticSubdepsdefaults totrueand rejects any transitive dependency resolved from git — which is precisely what the two ADL conformance suite overrides are.Two things hid it until now, both confirmed in pnpm's source:
resolveDependencies.tsgates onresolvedVia != null, with the comment "This is already coming from the lockfile, we skip the check in this case for now." Sopnpm install --frozen-lockfile— what CI runs — never trips it. Only a fresh resolution does:pnpm installafter a dependency change, Renovate regenerating the lockfile for chore(deps): update adl-lrs-conformance-tests>super-request digest to 6f960be #87/chore(deps): update adl-lrs-conformance-tests>supertest digest to f02f699 #90, or lockfile maintenance (chore(deps): lock file maintenance #45).So this was already queued to break Renovate's override-bump PRs and every future pnpm upgrade, regardless of the immediate trigger.
The fix, and what it costs
blockExoticSubdeps: false.I looked for something narrower first and there isn't one:
ctx.blockExoticSubdeps && currentDepth > 0 && isExoticDep(...)), with no exception list.So the guard goes off for the whole tree, not just these two packages. That's a real if modest reduction in supply-chain protection, and the comment in
pnpm-workspace.yamlsays so plainly rather than implying the change is free. It's acceptable here because the exposure is narrow and already deliberate: both are dev-only test tooling pinned to immutable commit SHAs and tracked by Renovate, so neither can change content without a reviewed PR. The comment also says to delete the setting rather than keep it if the forks are ever retired (upstream releasing the fixes, or vendoring them) — that's the real long-term fix.Why the pnpm bump is in the same PR
Pinned pnpm goes 11.15.1 → 11.20.0. Without it, CI runs the one version that doesn't enforce the check, so the fix would be unverified and would break again on the next pnpm bump. The lockfile is unchanged by both the setting and the version bump.
Verification (all under 11.20.0)
node_modules