Skip to content

Commit 3868825

Browse files
dmealingclaude
andcommitted
fix(prerelease): unlink names the lockfile residue it cannot repin away (#336)
Reported against 0.24.0-rc.5 by an adopter verifying the #330/#323/#334 fixes. A regression the #330 fix introduced, and the detector was already catching it correctly — what was missing is that nothing told the user the fix is one install command. WHAT BROKE. `link` correctly stopped deleting lockfiles (#330: a deletion is a destructive edit to committed state unlink cannot undo, and it turns a targeted pin into a full re-resolve, so an RC evaluation ends up testing "does the RC work" and "does a fresh resolve work" in one diff). But `unlink` repins MANIFESTS only. A kept lockfile keeps its `-rc` versions and private-registry tarball URLs, and `npm ci` resolves straight from the lockfile — so a committed lockfile in that state sends every other machine and every CI run to a registry it cannot reach. Closing one hole opened another; before #330 this state was unreachable because the lockfile was gone. The detector caught it. `unlink` then reported "left pre-release references behind — fix them before pushing", which is true, unhelpful, and reads like a defect in the tool rather than a remaining step. `unlink` now names the offending lockfiles BEFORE running the detector, so the detector's verdict reads as confirmation rather than a mystery, and prints the reconcile command per ecosystem. The failure message says the lockfile is the residue and what to run. IT STILL DOES NOT RUN INSTALLS, and that is deliberate rather than laziness: an install is network-dependent and can fail, and running the wrong one silently migrates a project's package manager — which is the exact hazard `npm_manager_for` already exists to avoid. So the command is derived per install root from that root's own lockfile and PRINTED. Extended to every ecosystem whose lockfile `link` used to drop, not just npm: uv, poetry, pipenv, NuGet. The residue check reads `PRERELEASE_RE` OUT OF THE DETECTOR rather than restating it. A second copy of that regex would drift, and this function exists precisely to explain the detector's own verdict — the two disagreeing would be worse than not explaining it at all. ALSO FIXED, and it is the part that misleads at the worst moment: the detector's remediation text still said unlink "drops the lockfile". It has not since #330, so the tool was telling the user something untrue exactly when they were trying to work out why it failed. It now states that unlink does NOT touch the lockfile and that repinning therefore cannot clear a reference inside one. Verified against a reproduction of the reported state (manifest repinned, lockfile still on rc.5 with a private tarball URL): unlink names `package-lock.json`, prints `npm install`, repins the manifest to 0.23.2, leaves the lockfile untouched, and exits non-zero with the actionable message. CONTROLLED against an already-reconciled lockfile, which unlinks clean with no residue reported — a false positive here would be worse than the bug, since it would teach adopters to ignore the one check that prevents shipping a private-registry lockfile. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent c4f995f commit 3868825

2 files changed

Lines changed: 75 additions & 3 deletions

File tree

tools/prerelease/detect-prerelease-pins.sh

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -355,8 +355,13 @@ Fix — from the project directory:
355355
356356
tools/prerelease/prerelease-link.sh unlink --to <released-version>
357357
358-
That removes the registry config, repins every vendor dependency, drops the lockfile,
359-
and re-runs this check.
358+
That removes the registry config, repins every vendor dependency, and re-runs this check.
359+
360+
It does NOT delete or rewrite your lockfile — a lockfile is committed state `unlink` could
361+
not restore. So if the references above are in one (`package-lock.json`, `uv.lock`, …),
362+
repinning cannot clear them: reconcile with your project's own install command
363+
(`npm install` / `uv sync` / `dotnet restore --force-evaluate`) and run `check` again.
364+
`unlink` names the offending lockfiles and the exact command for them.
360365
MSG
361366
exit 1
362367
fi

tools/prerelease/prerelease-link.sh

Lines changed: 68 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -291,6 +291,57 @@ npm_install_hint() {
291291
printf '%s' "$(IFS=' / '; echo "${cmds[*]}")"
292292
}
293293

294+
# Lockfiles that STILL resolve from the pre-release after a repin — the residue `unlink`
295+
# leaves behind and cannot fix by editing manifests.
296+
#
297+
# WHY THIS EXISTS (#336). Before the #330 fix, `link` deleted the lockfile, so the
298+
# post-unlink install regenerated it from the public registry and this state was
299+
# unreachable. Keeping the lockfile closed one hole (a destructive edit to committed state,
300+
# and a full re-resolve that confounded "does the RC work" with "does a fresh resolve
301+
# work") and opened another: a repin rewrites the MANIFEST, while the lockfile keeps its
302+
# `-rc` versions and private-registry tarball URLs. `npm ci` then resolves straight from
303+
# the lockfile, sending every other machine and every CI run to a registry it cannot reach.
304+
#
305+
# Matched against the SAME `PRERELEASE_RE` the detector uses, sourced from it rather than
306+
# restated here — a second copy of that regex would drift, and this function exists to
307+
# explain the detector's own verdict.
308+
lockfiles_with_prerelease() {
309+
local detector="$PROJECT/tools/prerelease/detect-prerelease-pins.sh"
310+
[ -x "$detector" ] || detector="$HERE/detect-prerelease-pins.sh"
311+
local re
312+
re="$(grep -m1 "^PRERELEASE_RE=" "$detector" | cut -d"'" -f2)"
313+
[ -n "$re" ] || return 0
314+
315+
local root prefix f
316+
while IFS= read -r root; do
317+
prefix="${root#"$PROJECT"/}"; [ "$prefix" = "$root" ] && prefix="" || prefix="$prefix/"
318+
for f in package-lock.json npm-shrinkwrap.json yarn.lock pnpm-lock.yaml bun.lock; do
319+
[ -e "$root/$f" ] || continue
320+
grep -Eq -- "$re" "$root/$f" 2>/dev/null && printf '%s\n' "${prefix}${f}"
321+
done
322+
done < <(npm_install_roots)
323+
# The same residue exists for every other ecosystem whose lockfile `link` used to drop.
324+
for f in uv.lock poetry.lock Pipfile.lock packages.lock.json; do
325+
[ -e "$PROJECT/$f" ] || continue
326+
grep -Eq -- "$re" "$PROJECT/$f" 2>/dev/null && printf '%s\n' "$f"
327+
done
328+
}
329+
330+
# The reconcile command per ecosystem that still holds pre-release residue. `unlink` PRINTS
331+
# these rather than running them: an install is network-dependent, can fail, and — the
332+
# reason that matters — running the wrong one silently migrates a project's package manager,
333+
# which is exactly what npm_manager_for exists to avoid.
334+
reconcile_hints() {
335+
local residue="$1"
336+
case "$residue" in *package-lock.json*|*npm-shrinkwrap.json*|*yarn.lock*|*pnpm-lock.yaml*|*bun.lock*)
337+
printf ' %s\n' "$(npm_install_hint)" ;;
338+
esac
339+
case "$residue" in *uv.lock*) printf ' %s\n' "uv lock && uv sync" ;; esac
340+
case "$residue" in *poetry.lock*) printf ' %s\n' "poetry lock --no-update && poetry install" ;; esac
341+
case "$residue" in *Pipfile.lock*) printf ' %s\n' "pipenv lock && pipenv sync" ;; esac
342+
case "$residue" in *packages.lock.json*) printf ' %s\n' "dotnet restore --force-evaluate" ;; esac
343+
}
344+
294345
# Lockfiles are NOT dropped by default. Deleting one is a destructive edit to committed
295346
# state that `unlink` cannot undo, and it turns a targeted pin into a full re-resolution of
296347
# every unrelated transitive dependency — so a pre-release evaluation ends up testing "does
@@ -574,8 +625,24 @@ case "$ACTION" in
574625
has_mvn && repin_mvn "$TO"
575626
drop_lockfiles
576627
echo
628+
# #336: a repin rewrites MANIFESTS; a kept lockfile still resolves the pre-release. Say
629+
# so BEFORE the detector runs, so its verdict reads as confirmation rather than a
630+
# mystery the user has to diagnose.
631+
residue="$(lockfiles_with_prerelease)"
632+
if [ -n "$residue" ]; then
633+
warn "these lockfiles still resolve from the pre-release — repinning cannot fix a lockfile:"
634+
printf ' %s\n' $residue
635+
say "reconcile with:"
636+
reconcile_hints "$residue"
637+
echo
638+
fi
577639
if run_detector; then
578-
ok "unlinked and verified clean — reinstall to regenerate the lockfile from public registries"
640+
ok "unlinked and verified clean"
641+
elif [ -n "$residue" ]; then
642+
# The detector is RIGHT and this is not a false positive — the residue is real and
643+
# would send every `npm ci` to a private registry. It just has a one-command fix,
644+
# which the old message left the user to work out.
645+
die "unlink is incomplete: the lockfile(s) above still carry the pre-release. Run the reconcile command, then 'check'."
579646
else
580647
die "unlink left pre-release references behind (listed above) — fix them before pushing"
581648
fi

0 commit comments

Comments
 (0)