{{ message }}
Repository navigation
fix: un-deprecate extensions removed from the extension-control file - #2261
Merged
Merged
Conversation
processDeprecatedExtensions only ever applied entries the daily extension-control job's fetched extensions.json still lists. An entry removed from the file entirely, rather than flipped to false, was never seen by that loop, so extension.deprecated stayed true forever - nothing else ever clears it. Reconcile instead: after applying every entry the file lists, un-deprecate any extension the database still has marked deprecated but that no longer appears in the file. Fixes #2260. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Un-deprecation leaves any existing replacement persisted and exposed.
Review effort: Balanced
Findings: 1
What changed in this PR
Reconciles database deprecation state with the extension-control file so removed entries become active again.
Changes:
- Adds repository lookup for deprecated extensions.
- Reconciles missing entries case-insensitively.
- Adds regression tests for removal and casing behavior.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
RepositoryService already had findDeprecatedExtensions(Extension) - extensions deprecated in favor of a given replacement - which is unrelated to the new no-arg query. Reusing the name meant RepositoryServiceSmokeTest's assertAll only ever exercised the existing overload, so its "did we call every public method" check at the end failed for the new one. Renamed to findAllDeprecatedExtensions (matching findAllActiveExtensions right above it) and added it to the smoke test. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
updateExtension only changed extension.replacement when replacementId was non-null, so un-deprecating (an explicit false entry, or the reconciliation pass just added) left a stale replacement in place - still surfaced by LocalRegistryService#toReplacementJson regardless of the deprecated flag. Clear it unconditionally once deprecated is false. Addresses a Copilot review comment on PR #2261. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
cstamas
approved these changes
Sep 29, 2026
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.
Fixes #2260.
ExtensionControlJobRequestHandler.processDeprecatedExtensionsonly ever applied the entries the daily job's freshly fetchedextensions.jsonstill lists. If an entry flips fromtruetofalse(or the object form dropsdisallowInstall), that key is still present andupdateExtensioncorrectly un-deprecates it. But an entry removed from the file entirely — the more natural way to say "this is no longer deprecated" — was never seen by that loop at all: nothing else ever callsextension.setDeprecated(false), and there is no admin API to toggle it by hand either. So the extension stayed deprecated forever.Fix
After applying every entry the control file lists, reconcile: load every extension currently marked
deprecated = truein the database, and un-deprecate any of them that no longer appear in the file (updateExtension(id, false, null, true), the same call an explicitfalseentry already takes).Matching is done case-insensitively, consistent with every other namespace/extension lookup in the codebase (
ExtensionRepository#findByNameIgnoreCaseAndNamespaceNameIgnoreCase), so a casing difference alone between the file and the stored extension is never mistaken for a removal.Changes
ExtensionRepository.findByDeprecatedTrue()/RepositoryService.findDeprecatedExtensions(): fetch the extensions currently marked deprecated.ExtensionControlJobRequestHandler.processDeprecatedExtensions: tracks the ids seen in the control file, then reconciles against the database afterward.Tests
ExtensionControlJobRequestHandlerTest:unDeprecatesExtensionNoLongerListedInControlFile— a deprecated extension absent from the file gets un-deprecated.doesNotUnDeprecateExtensionStillListedInControlFile— an extension still listed (even astrue) is left alone by the reconciliation pass.matchesControlFileEntriesCaseInsensitively— a casing difference between the file and the stored extension is not treated as a removal.Confirmed these fail (in fact don't compile, since
findDeprecatedExtensionsdoesn't exist yet) without the fix../gradlew spotlessCheckpasses for every touched file. Fullextension_controlpackage suite (24 tests) is green../gradlew testcouldn't be run in full in this environment — no Docker daemon available for the Testcontainers-backed integration tests — so say so rather than claiming a full green run.🤖 Generated with Claude Code