feat(engines): propose runtime bumps, which were resolved and never proposed - #1428
Merged
Conversation
…the file
`resolveDockerDigest` was written with a docstring explaining exactly why it
matters — "moving the tag without moving the digest would leave the old image
running while the file claims otherwise" — and never called. Three defects
followed, two of them silent.
Parsing split on the *last* colon, and a digest reference contributes two.
`FROM node:20@sha256:abc…` therefore parsed as the image `node:20@sha256` at
version `abc…`. That is not an image, is never found in any registry, and
appears on the dependency dashboard as though it were real. The guard meant
to catch this tested `version.startsWith('sha256:')`, which the bare hex
never does. The digest is now split off before the tag is read, and a
reference pinned by digest alone — with no tag to move — is skipped as it was
always meant to be.
Updating then dropped the pin. `[^\s]+` swallowed `20@sha256:abc…` as the
version and replaced the lot, so `FROM node:20@sha256:…` became
`FROM node:22` — silently unpinning an image somebody pinned on purpose. The
tag and digest are now captured separately and moved together, with the new
digest resolved through the OCI client. Where it cannot be resolved the line
is left alone: writing the new tag beside the old digest would produce
exactly the mismatch the original docstring warns about.
Registry credentials are threaded through, so a private image's digest is
re-resolved with the same credentials the scan used to find its tag.
The third was pre-existing and unrelated to digests. The trailing group was
`(\s.*)?$`, and `\s` matches a newline — so with the `m` flag the match ran
past the end of its line and took every following line with it. A
multi-stage Dockerfile building from the same image twice had both `FROM`
lines collapsed into one, deleting a build stage. Horizontal whitespace only
now, with a regression test that reads the surviving `FROM` lines back.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UWouahBJT3msK7V2VDYY6T
…roposed `extractEngines` put every known runtime under `engines` on the dashboard. `resolveEngineVersion` knew which registry to ask for each — GitHub for Node and Deno, npm for the package managers — and `bumpEngineConstraint` knew how to raise a constraint without changing its shape. Both were tested and exported from the public API. No scan step called either, so an engine was listed and never proposed. Had one been proposed, it would not have been written. `generateAllFileUpdates` rewrites the four dependency sections and nothing else, so an `engines` update entered that loop, logged "Package node not found", and produced no change. Opt-in via `packages.engines: true`, off by default, because the module's own docstring already said why: an engine constraint is a deployment decision rather than a dependency one. Raising `node: '>=20'` to `'>=22'` is a statement about every environment the project runs in. The constraint keeps its shape at the author's precision — `>=20` becomes `>=22`, never `22.3.0`, which would break every contributor on a different patch. `bumpEngineConstraint` compares at that precision rather than by `satisfies`, so `^9.0.0` follows 9.1.0 to `^9.1.0` the way a dependency's caret range would, while `>=9` is not moved by a 9.x release at all. That is the existing function's behaviour, and the tests now say so explicitly. Rewriting is scoped to the `engines` object. `npm`, `pnpm` and `yarn` are often both an engine and a devDependency in the same file, and a rewrite that matched by name alone would move the wrong one. The GitHub lookup reuses `fetchLatestActionVersion`: it takes `owner/repo` and returns the latest release tag, which is exactly the shape a runtime's GitHub releases have. The npm lookup is the instance's registry client, so it shares the scan's cache. Not addressed: Node's latest release is the current line, not the LTS one, so `>=20` is offered `>=23` when 23 is current. Preferring even majors for Node would be an opinion the resolver has not expressed, and the opt-in makes the proposal deliberate either way. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UWouahBJT3msK7V2VDYY6T
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
1 participant
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.
extractEnginesput every known runtime underengineson the dashboard.resolveEngineVersionknew which registry to ask for each.bumpEngineConstraintknew how to raise a constraint without changing its shape. Both were tested and exported from the public API.No scan step called either. So an engine was listed and never proposed.
Had one been proposed, it would not have been written:
generateAllFileUpdatesrewrites the four dependency sections and nothing else, so anenginesupdate entered that loop, loggedPackage node not found, and produced no change.What's wired
checkEnginesForUpdatesbeside the Docker/Zig checks; npm engines via the registry client (shared cache), Node/Deno via GitHub releases>=20→>=22, never22.3.0applyEngineUpdates, scoped to theenginesobjectpackages.enginestyped, validated as a boolean, documentedOff by default because the module's own docstring already said why: "bumping it is a deployment decision". Raising
node: '>=20'to'>=22'is a statement about every environment the project runs in.Two things worth your eye
Scoping the rewrite.
npm,pnpmandyarnare often both an engine and a devDependency in the same file. A rewrite matching by name alone would move the wrong one — there's a test where both are present and only the engine moves.Precision, not
satisfies.bumpEngineConstraintcompares at the author's precision, so^9.0.0follows 9.1.0 to^9.1.0the way a dependency's caret would, while>=9isn't moved by a 9.x release at all. That's the existing function's behaviour — its docstring says "null when it already admits latest", which isn't quite what it does. I kept the behaviour and made the tests state it explicitly rather than silently pick a side.Not addressed
Node's latest release is the current line, not LTS — so
>=20is offered>=23when 23 is current. Preferring even majors would be an opinion the resolver hasn't expressed; the opt-in makes the proposal deliberate either way. Flagging rather than deciding.Also cosmetic: an engines row lands in the npm table with Mend badges keyed on the npm package name, which for
nodeis a different package. Harmless, wrong-looking.Tests
2127 → 2143.
Lint ✓ · typecheck ✓ ·
check:docs✓ (222 config keys now) · 0 fail🤖 Generated with Claude Code
https://claude.ai/code/session_01UWouahBJT3msK7V2VDYY6T