Refactor/deps checker - #170
Merged
Merged
Conversation
Contributor
Dependency ReviewThe following issues were found:
License Issues.github/workflows/generator-versions.yml
OpenSSF Scorecard
Scanned Files
|
Contributor
|
❌ Commit message validation failed. Please ensure your commits follow the Conventional Commits format: Allowed types: feat, fix, docs, style, refactor, perf, test, chore, ci, revert See CONTRIBUTING.md for details. |
Allows() cannot distinguish "there is nothing newer" from "what's newer breaks the constraint" — it rejects a version below the anchor for the same reason it rejects one above the permitted range. Callers classifying dependency drift need to tell those apart.
Pins every npm package the generators scaffold, in one place, with exactly one pin per package. Generators name the packages they need; the catalog says which version they get. One version per package, repo-wide, is the point. When versions lived inline in 40 generator files the same package could drift to two versions unnoticed, and it did — vitest sat at ^4.1.7 and ^4.1.8, bcryptjs at ^2.4.3 and ^3.0.3. With one key per package that is unrepresentable rather than merely discouraged. Ecosystem is declared by the pin, not inferred from which file a generator happens to write. Only npm is live; Cargo, Maven and Go come back when a generator actually needs them. Refs: ADR-0002
Generators now name the packages they need; internal/deps supplies the version. No generator source carries a version string any more. Three generators built the package *name* in a Go variable — auth_clerk_frontend, sentry_frontend and storybook_setup. That made @clerk/nextjs, @sentry/nextjs and @storybook/nextjs invisible to any scan driven by an empty answer set, and made the packages on the default branch impossible to patch by matching a source literal. deps.NPM() takes a runtime string, so the conditionals survive intact while every package stays visible in the catalog. Five deliberate output changes ride along, because the catalog admits one pin per package and these could not be carried forward as-is: bcryptjs ^2.4.3 / ^3.0.3 -> ^3.0.3 (drift; identical default import) vitest ^4.1.7 / ^4.1.8 -> ^4.1.8 (drift) @vitest/coverage-v8 ^4.1.7 / ^4.1.8 -> ^4.1.8 (drift) @version14/ui 0.8.0 -> ^0.8.0 (an exact pin admits no update) @types/bcryptjs removed (deprecated stub; bcryptjs 3.x ships types) react-router-dom is dropped and the templates now import from react-router, completing the v7 -> v8 major that was previously merged as a version bump while the templates still imported the v7 package. Verified output-neutral for 56 of 61 generators; all 36 test-flow fixtures pass a full run.
The catalog migration was output-neutral for 56 of 61 generators. These five scaffold something genuinely different, so their manifest versions move: auth_jwt_mvc_route minor bcryptjs 2.x -> 3.x react_router_v7 minor react-router-dom dropped, imports on v8 auth_jwt_clean_arch_module patch @types/bcryptjs stub removed version14_ui patch exact pin -> caret vitest_testing_library patch vitest ^4.1.7 -> ^4.1.8
Thirteen doc pages disagreed with their manifest. The old dependency bot wrote these rows and then discarded them every single run: it staged with `git add generators/`, so nothing under docs/ was ever committed. No bot commit in the repo's history touches docs/. express_test_setup and monorepo_ts_workspaces had no doc page at all.
The old scanner ran Generate() on every generator with an empty Answers map and read back whatever package.json fell out. ADR-0001 called this "strictly accurate"; it was not. Generators branch on Answers, so an empty map executes one arbitrary branch, and three generators pick their package name from the framework answer — @clerk/nextjs, @sentry/nextjs and @storybook/nextjs were never checked once in the tool's lifetime. Worse, the scanner read *values* out of generated JSON while the patcher rewrote *source literals* it located by regex. Nothing guaranteed such a literal existed. Where it did not, the patch errored, the bash driver broke out of its loop and ran `git checkout main` without resetting the working tree, and the next iteration's `git add generators/` swept the already-patched files into an unrelated commit. The scan is now a map read over the catalog: no generator runs, no answers, no branch can hide a package. The patcher writes internal/deps/npm.go and nothing else, so the value the scan read is the literal the patch writes. Classification delegates to internal/versioning: an update is low-risk iff the new version satisfies the pin's existing constraint. That gets zero-major right for free — ^0.45.2 does not permit 0.46.0 — where the hand-rolled comparison it replaces called drizzle-orm 0.45.2 -> 0.46.0 a safe "minor" and would have shipped a breaking release inside a batch. Cargo, Maven and Go extractors are removed; pom.xml was even routed to an extractor that did not exist. No generator writes those files. Refs: ADR-0002
Replaying a flow headlessly from a recorded answer map was private to test-flow. Fingerprinting needs the same thing, and two copies of a flow runner would be two things to keep in step.
What a generator contributes cannot be recovered after the fact. Generators merge into shared files — above all package.json — and the finished state records only that a generator touched a file, never what it put there. Bracketing each call is the only way to attribute the edit.
A fingerprint hashes a generator's contribution — the files it adds and the edits it makes to files other generators own — across every fixture that exercises it. It answers the only question that matters when deciding whether a manifest version must move: did what this generator scaffolds actually change? A contribution is a diff, not a file hash, and that distinction is the whole point. Hashing the resulting files would mean bumping the react pin moved the fingerprint of every generator that touches package.json, when only two name react. That over-triggering is not cosmetic: it is exactly the defect behind commit d53c302, which bumped 15 generator manifests for a change to one generator. Create and modify are deliberately not distinguished either. Whether a generator creates package.json or merges into one another generator already created is an accident of topological order, so encoding it would demand a version bump whenever a new generator was inserted ahead of an existing one.
dot gen-fingerprint generator -> fingerprint, computed across the fixtures
dot gen-check --docs every doc version row equals its manifest version
dot gen-check --bumped --base=<fingerprints.json>
every generator whose fingerprint moved has been bumped
Entirely in memory — no pnpm install — so the whole fixture suite runs in seconds.
A comment fix or a gofmt moves no fingerprint and demands nothing; a template edit
does.
Rule 1 a generator whose fingerprint moved has a bumped manifest version Rule 2 every generator's doc version row equals its manifest version Rule 1 deliberately does not fire on a diff that only touches internal/deps/. A catalog pin change alters what many generators scaffold, but forcing those manifest bumps into the dependency PR is what made merge order corrupt them: a bump computed against main at PR-creation time is a relative operation applied at merge time, when main has moved. Those bumps are derived at release instead.
Closing dependency drift is not one kind of work, and the old script treated it as
if it were.
Rollup every update that satisfies its pin's constraint, in one batched PR.
Bot-owned and disposable: rebuilt from scratch each run. Excludes any
package holding an open dep PR of its own.
Migration an update that breaks its pin's constraint. One PR per package,
human-owned from the moment it is raised. The bot never touches that
branch again, so it rebases cleanly for months.
Deprecation an issue, never a PR. A deprecation is not fixed by a version bump —
the replacement is a different package, or none.
The bot patches internal/deps/npm.go and nothing else. The old script ran
`git add generators/`, which committed whatever happened to be dirty in the working
tree; that is how 997e489, titled "bump @clerk/clerk-react", came to contain 13
files of unrelated packages and no clerk changes.
It no longer bumps manifests — those are derived at release — and it no longer
claims to run test-flows, a gate ADR-0001 promised and never implemented. The PR's
own CI runs test-flows; duplicating it would double the most expensive job in CI.
Deprecation issues are keyed on a single package name. The old key was the issue
title, which contained a variable package list, which is why the same
@types/bcryptjs deprecation was filed twice under two different titles.
`make release-prep` bumps every generator whose fingerprint has moved since the last tag, once, from the final state. A version bump stored inside a PR is a relative operation — 0.8.0 -> 0.9.0 — computed against main at PR-creation time and applied at merge time, when main has moved. With several dependency PRs open, the version you end up with is a function of merge order. It is fatal for long-lived migrations: one open for two months collides with every weekly rollup touching the same manifest. Deriving the bump here makes merge order provably irrelevant. Any sequence of merges produces the identical manifest.
ADR-0002 replaces ADR-0001, whose central claim — that instantiating a generator and reading back its output is "strictly accurate" and "can never drift" — is the thing that broke. CONTEXT.md is the glossary. It separates two concepts the codebase had been running together under one word: pin drift (our pinned version has fallen behind the registry) and generator drift (a project's generator version no longer satisfies its recorded constraint). It also names rollup and migration, which are different kinds of work rather than two sizes of the same one. docs/contributor/dep-checker.md described a test-flows gate that never existed.
mathieusouflis
force-pushed
the
refactor/deps-checker
branch
from
July 12, 2026 10:24
4a0c54e to
0f05a06
Compare
|
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.
Summary
Closes #
Type of change
Changes
Testing
Checklist
Screenshots