Repository navigation
refactor(no-outdated-packages)!: split registry facts from rule verdict - #191
Merged
Merged
Conversation
This was the only rule whose verdict was computed in the registry layer and stored on the display object. Every other rule goes facts -> rule -> violations -> display; this one had `enrichFromRegistry` stamp `severity`, `scope` and `worstLevel` onto `PackageDistribution.releaseAge`, and the Packages table read the verdict straight back off the package rather than joining to violations the way the Flags column already does. That mixing is why the field names resisted fixing: `ReleaseAgeEntry` was registry facts, rule verdict and display state at once, so no name could be honest about what it held. `releaseAge` is facts only now. The verdict is `overdueTier` on the violation, derived from `upgrades` (which holds breached tiers only) rather than stored a second time. `AvailableUpgrade.level` is deleted outright — it was a total function of `semverBump` on the same object. `installedVersion` becomes `measuredVersion`, which is what it always meant under tree scope. The four `minCompliant*` fields and their negating boolean collapse into one `recommendedTarget`, so a "compliant version" field can no longer hold a version that isn't compliant. Severity is no longer an input to the timeline math at all. `thresholds` and `scope` stay, because the arithmetic genuinely needs them — you cannot say which copies count, or which tier is past its line, without them. Display: package rules now declare columns through a registry (`src/utils/package-columns.ts`), the counterpart to the badge registry in `package-flags.ts`. `no-outdated-packages` contributes Installed / Minimum target through it, so `printPackagesTable` no longer tests for that rule by name. The cell takes its icon by joining the violation, which is what keeps it and the rules table from disagreeing. A breached package with no violation is one governed by an 'off' entry, and renders with no icon — derived now, not read from a stored severity. Rendered output is unchanged; verified against the fixture registry. Closes #189 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
🦋 Changeset detectedLatest commit: f6c90f7 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Contributor
Output Review2 of 30 case(s) changed · 0 invariant breach(es) · all 30 cases Reference:
Open a case to read its diff, its config and its full output. If this is the change you meant, apply the |
Contributor
Coverage Report
File Coverage
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
It had no direct test — only the e2e path exercised it, which the coverage run skips. Pins that the `releaseAge` config block reaches the registry layer as connection/infra only, which is the invariant #189 rests on. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Converts #191 from "strip severity" to the full separation: the registry layer takes no policy at all, and `packages[]` stops being a function of config. The practical complaint this answers is that a payload describing the repo was varying by how the repo is judged. Two repos with identical lockfiles and different thresholds got different `packages[]`, and the block changed shape depending on which rules were on. Now `releases` holds every resolved copy and what was published after each, root and nested alike, flagged but unranked. It is byte-identical whatever the thresholds, severity or scope say, because none of them are inputs to producing it. Scope moved with the rest. It used to collapse two installed copies into one policy-chosen answer before anyone asked, which is why `advisoryBreaches` existed as a stored field — it was "the copies the scope excluded". Both are derived now, by the layer that owns the decision. `assessPackage` is the single place policy is applied, and both consumers call it rather than reading a cached answer: the rule to emit violations, the Packages table to render rows no violation covers. An 'off' entry still shows a recommendation, recomputed rather than stored. Field-level: `worstLevel` -> `overdueTier` on the violation; `installedVersion` -> `measuredVersion`; `breachReleasedDaysAgo` + `thresholdDays` -> `daysOverdue`; `AvailableUpgrade.level` deleted as a total function of `semverBump`; the four `minCompliant*` fields and their negating boolean deleted outright, so a "compliant version" field can no longer hold a version that is not compliant. Adds `comply-release-age-json`, which pins this rule's JSON surface end to end. Until now the only JSON case reaching it covered one package on one path — thin for what is arguably the most consequential rule here. Rendered output is unchanged; verified against the fixture registry. Closes #189 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The gate asked "is there release-age data?" as a proxy for "did `no-outdated-packages` run?". That held only while release facts were themselves gated on the policy — which is exactly the coupling this branch removes. With facts recorded whenever the registry is consulted, a `no-deprecated-packages`-only run grew a Packages table it never had. Asks the resolved rules directly instead, which comply now holds, so a per-repo `overrides[]` entry still counts. Found by output review on comply-deprecated-packages; pinned by an e2e test, since nothing in the unit suite covered the gate. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Gallevy
added
the
output:approved
Second instance of the same proxy the previous commit fixed in comply, and the one that actually reached output: `applies` asked only whether release facts existed. Facts are recorded whenever the registry is consulted now, so a `no-deprecated-packages`-only `scan` grew an Installed/Minimum target pair full of em dashes — columns belonging to a rule that was never configured. `comply` hid it because its own gate short-circuits the whole section; `scan` gates on `output.packages` instead, so that path rendered it. `applies` now takes the column context and needs both halves: the rule is configured AND there is data to show. Covered at both levels — a unit test for the registry decision and an e2e test on the `scan` path that exposed it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
github-actions
Bot
removed
the
output:approved
…d-packages Answers why output review missed the em-dash column bug: six fixture configs enable a registry-backed rule and not one of them was used by a `scan` case. Every scan case ran a config with no registry rules; every registry case ran `comply`, whose own gate short-circuits the Packages section before the column registry is consulted. `scan` × registry-rule was an empty cell in the matrix, and that is the cell the bug lived in. Verified the case actually catches it: reverting `applies` to the data-only check makes scan-deprecated-packages diff against main, and restoring it makes it match again. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Gallevy
added
the
output:approved
Gallevy
enabled auto-merge (squash)
September 13, 2026 20:31
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.
Closes #189.
no-outdated-packageswas the only rule whose verdict was computed in the registry layer and stored on the display object. Every other rule goes facts → rule → violations → display; this one had the enricher stampseverity,scopeandworstLevelontoPackageDistribution.releaseAge, and the Packages table read the verdict back off the package instead of joining to violations the way the Flags column already does.The practical consequence, and the thing this PR actually fixes: a payload describing the repo was varying by how the repo is judged. Two repos with identical lockfiles and different thresholds got different
packages[], and the block changed shape depending on which rules were switched on.Facts
packages[].releaseAge→packages[].releases:Every resolved copy, root and nested alike, flagged but unranked. Byte-identical whatever your thresholds, severity or scope say. The only way config affects it is presence: absent when no registry-backed rule ran, because nothing fetched it.
enrichFromRegistrytakes no policy parameter at all any more — that's the structural guarantee behind the sentence above, not a convention.Verdict
{ "ruleId": "no-outdated-packages", "severity": "error", "packageName": "moment", "measuredVersion": "2.29.4", "overdueTier": "minor", "daysOverdue": 946, "scope": "root" }Scope moved with the rest
Per review: scope belongs to the rule, and the facts should present both copies neutrally. It used to collapse two installed copies into one policy-chosen answer before anyone asked — which is exactly why
advisoryBreachesexisted as a stored field, since it was "the copies the scope excluded". Both are derived now, by the layer that owns the decision. The samereleasespayload produces the root and tree answers; there's a test asserting precisely that.One place policy is applied
assessPackageis it, and both consumers call it rather than reading a cached answer — the rule to emit violations, the Packages table to render rows no violation covers. An'off'entry still shows a recommendation, recomputed rather than stored.printPackagesgained the resolved rule entries to make that possible; both call sites already held the config.Field-level
worstLeveloverdueTieron the violationinstalledVersionmeasuredVersionbreachReleasedDaysAgo+thresholdDaysdaysOverdueAvailableUpgrade.levelsemverBumpminCompliantVersion+…ReleasedDaysAgo+…InWindow+…BumpadvisoryBreaches,evaluatedVersionsinWindowis gone as you asked. The old shape letminCompliantVersionhold a version that was not compliant, rescued only by a boolean beside it saying the recommendation wouldn't clear the breach. Nothing stores that contradiction now.recommendedTargetis gone as a stored field, so its bad name went with it. Where the concept survives internally it isminimumUpgrade— the least disruptive upgrade that actually satisfies the policy.The JSON case
Added
comply-release-age-json, against the same recorded registry ascomply-release-age. You were right that the surface was thin: the only JSON case that reached this rule covered one package on one path. The new case shows the whole thing at once — no-in-window-target (#26), a cross-tier target (#57), coming-due, an'off'entry carrying full facts and no violation, and a version conflict whose nested copy is overdue but out of scope.Verification
pnpm run typecheck/pnpm run lint/pnpm run format— cleanpnpm run test:ci— 1198 passing, 58 filesenricher.test.tskeeps its regression cases whole rather than splitting them, since each pins a specific reported bug (#24, #26, #29, #57) that only means something with both layers in play; its helper now runs the rule so those assertions read the assessment.tests/rules/no-outdated-packages.test.tsunit-tests the assessment on its own.Expect
comply-release-age-jsonto show as a new case andcomply-all-rule-types-jsonto change — both are the intended--jsonreshape, and no human-output case should move.🤖 Generated with Claude Code