feat(presets): add enable/disable toggle and update semantics#1891
feat(presets): add enable/disable toggle and update semantics#1891mbachorik wants to merge 7 commits intogithub:mainfrom
Conversation
Add preset enable/disable CLI commands and update semantics to match the extension system capabilities. Changes: - Add `preset enable` and `preset disable` CLI commands - Add `restore()` method to PresetRegistry for rollback scenarios - Update `get()` and `list()` to return deep copies (prevents mutation) - Update `list_by_priority()` to filter disabled presets by default - Add input validation to `restore()` for defensive programming - Add 16 new tests covering all functionality and edge cases Closes github#1851 Closes github#1852 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR expands the preset system to support safer registry mutation patterns (rollback/restore and copy isolation) and adds CLI enable/disable toggles so presets can be temporarily turned off without uninstalling, while ensuring disabled presets are skipped during template resolution.
Changes:
- Added
PresetRegistry.restore()plus deep-copy isolation forPresetRegistry.get()andPresetRegistry.list(). - Updated
PresetRegistry.list_by_priority()to exclude disabled presets by default (with aninclude_disabledoverride). - Introduced
specify preset enable|disablecommands and added tests covering restore, deep-copy behavior, disabled filtering, CLI flows, and corrupted-state handling.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/specify_cli/presets.py |
Adds restore(), deep-copy semantics for registry reads, and default filtering of disabled presets in priority listing (affects resolution). |
src/specify_cli/__init__.py |
Adds preset enable / preset disable Typer commands with project checks and corrupted-state handling. |
tests/test_presets.py |
Adds coverage for restore validation, deep-copy protection, list-by-priority filtering, CLI enable/disable flows, and resolution behavior when disabled. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
- Fix error message in restore() to match actual validation ("dict" not "non-empty dict")
- Use copy.deepcopy() in restore() to prevent caller mutation
- Apply same fixes to ExtensionRegistry for parity
- Add /defensive-check command for pre-PR validation
- Add tests for restore() validation and deep copy behavior
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Addressed both review comments:
Also applied the same fixes to Added |
There was a problem hiding this comment.
Pull request overview
This PR extends the preset system to support safer registry semantics and operational toggling, bringing presets closer to feature parity with extensions (update/restore patterns and enable/disable behavior).
Changes:
- Add
PresetRegistry.restore()plus defensive validation, and alignExtensionRegistry.restore()behavior (validation + deep copy). - Return deep copies from preset registry
get()/list()and filter disabled presets by default inlist_by_priority(). - Add
specify preset enable/disablecommands and corresponding tests, including behavior around disabled preset resolution.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/specify_cli/presets.py |
Adds restore(), deep-copy protections for registry reads, and default filtering of disabled presets in priority ordering (affects resolution). |
src/specify_cli/extensions.py |
Hardens restore() with input validation and deep-copy to avoid caller mutation and align with presets. |
src/specify_cli/__init__.py |
Introduces preset enable/disable CLI commands for toggling preset enabled state. |
tests/test_presets.py |
Adds coverage for preset restore validation, deep-copy behavior, disabled filtering, CLI enable/disable flows, and resolution skipping. |
tests/test_extensions.py |
Adds coverage for extension restore validation and deep-copy behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
- Add note to enable/disable output clarifying commands/skills remain active - Add include_disabled parameter to ExtensionRegistry.list_by_priority for parity - Add tests for extension disabled filtering Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR extends the preset (and related extension) registry semantics to support safer state management and toggling without uninstalling, aligning presets more closely with extensions and improving robustness during rollback/registry-corruption scenarios.
Changes:
- Add
preset enable/preset disableCLI commands to toggle installed presets without removal. - Add
PresetRegistry.restore()and updateget()/list()to return deep copies (defensive against accidental mutation). - Update
list_by_priority()for presets/extensions to exclude disabled entries by default (with an override to include them).
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/specify_cli/presets.py |
Adds restore() and deep-copy isolation for registry reads; filters disabled presets in priority listing. |
src/specify_cli/extensions.py |
Adds validation/deep-copy to restore() and changes list_by_priority() to exclude disabled by default. |
src/specify_cli/__init__.py |
Introduces specify preset enable/disable commands with corrupted-state handling and user messaging. |
tests/test_presets.py |
Adds unit/integration tests for restore semantics, deep-copy behavior, enable/disable, and disabled preset resolution behavior. |
tests/test_extensions.py |
Adds tests for restore() input validation/deep-copy and for list_by_priority() disabled filtering semantics. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
…entries - Fix _get_all_extensions_by_priority to use include_disabled=True for tracking registered IDs, preventing disabled extensions from being picked up as unregistered directories - Add corrupted entry handling to get() - returns None for non-dict entries - Add integration tests for disabled extension template resolution - Add tests for get() corrupted entry handling in both registries Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds preset enable/disable toggling and safer registry mutation semantics to better support rollback/update workflows and prevent accidental state mutation during resolution.
Changes:
- Introduces
PresetRegistry.restore()and strengthens registry isolation by returning deep copies fromget()/list(). - Adds
specify preset enable|disablecommands and filters disabled presets/extensions out of priority-based resolution by default. - Extends test coverage for restore validation, deep-copy behavior, corrupted registry entries, and disabled template resolution.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
src/specify_cli/presets.py |
Adds restore(), deep-copy returns, and default filtering of disabled presets during priority iteration; updates resolver to avoid reintroducing disabled extensions via fallback scan. |
src/specify_cli/extensions.py |
Makes restore() validate input + deep copy; ensures get() handles corrupted entries; adds include_disabled option to list_by_priority() with disabled excluded by default. |
src/specify_cli/__init__.py |
Adds new preset enable / preset disable CLI commands with corrupted-state handling and messaging about what disabling affects. |
tests/test_presets.py |
Adds tests for preset restore/deep-copy/corruption handling, disabled preset resolution behavior, and disabled extension template resolution regressions. |
tests/test_extensions.py |
Adds tests for extension restore validation/deep-copy, corrupted get behavior, and include/exclude disabled behavior in list_by_priority(). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
- Add defensive handling to list() when presets/extensions is not a dict - Return empty dict instead of crashing on corrupted registry - Apply same fix to both PresetRegistry and ExtensionRegistry for parity - Add tests for corrupted registry handling Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR extends the preset/extension registry semantics to support enable/disable toggles and safer registry mutation patterns (restore/deep-copy), and updates template resolution to skip disabled items.
Changes:
- Add
preset enable/preset disableCLI commands that toggle a preset’senabledflag without removing it. - Add
PresetRegistry.restore()and update registry accessors (get(),list(),list_by_priority()) to use deep copies and filter disabled entries by default. - Update template resolution to skip disabled presets/extensions (including preventing disabled extensions from being re-introduced via the “unregistered dir scan” fallback).
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/specify_cli/presets.py |
Adds restore(), deep-copy protections, and disabled filtering; updates extension resolution logic to respect disabled state. |
src/specify_cli/extensions.py |
Aligns restore() validation/deep-copy and adds disabled filtering + corrupted-registry handling in list(). |
src/specify_cli/__init__.py |
Introduces preset enable/disable Typer commands with user messaging and error handling. |
tests/test_presets.py |
Adds coverage for restore/deep-copy/corruption handling, preset enable/disable CLI, and disabled resolution behavior. |
tests/test_extensions.py |
Adds coverage for restore input validation, deep-copy/corruption handling, and disabled filtering semantics. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
- get() now validates self.data["presets/extensions"] is a dict before accessing - restore() ensures presets/extensions dict exists before writing - Prevents crashes when registry JSON is parseable but has corrupted structure - Applied same fixes to both PresetRegistry and ExtensionRegistry for parity Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR extends the preset/extension registry semantics to support enable/disable toggles, safe rollback via restore(), and defensive deep-copying to prevent accidental in-memory mutation—while ensuring disabled presets/extensions are skipped during template resolution.
Changes:
- Add
PresetRegistry.restore()plus deep-copy isolation inPresetRegistry.get()/list()and default filtering of disabled presets inlist_by_priority(). - Add
preset enable/preset disableCLI commands and update template resolution to exclude disabled presets/extensions (including preventing disabled extensions from being picked up via “unregistered directory” fallback). - Add/extend test coverage for restore validation, deep-copy behavior, corrupted registry handling, disabled filtering, and disabled template resolution.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
src/specify_cli/presets.py |
Adds restore(), deep-copy return semantics, and default exclusion of disabled presets during priority listing/resolution. |
src/specify_cli/extensions.py |
Aligns extension registry behavior with presets (restore validation, deep-copy returns, corrupted-entry handling, disabled filtering). |
src/specify_cli/__init__.py |
Introduces specify preset enable/disable commands to flip the registry enabled flag. |
tests/test_presets.py |
Adds tests for restore/deep-copy/corruption, disabled preset filtering, and disabled extension template resolution regressions. |
tests/test_extensions.py |
Adds tests for extension restore validation/deep-copy/corruption and disabled filtering in list_by_priority(). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| packs = self.data.get("presets") | ||
| if not isinstance(packs, dict): | ||
| return None | ||
| entry = packs.get(pack_id) |
| extensions = self.data.get("extensions") | ||
| if not isinstance(extensions, dict): | ||
| return None | ||
| entry = extensions.get(extension_id) |
| # Check if preset is installed | ||
| if not manager.registry.is_installed(pack_id): | ||
| console.print(f"[red]Error:[/red] Preset '{pack_id}' is not installed") | ||
| raise typer.Exit(1) | ||
|
|
||
| # Get current metadata | ||
| metadata = manager.registry.get(pack_id) | ||
| if metadata is None or not isinstance(metadata, dict): |
| manager = PresetManager(project_root) | ||
|
|
||
| # Check if preset is installed | ||
| if not manager.registry.is_installed(pack_id): |
Summary
preset enableandpreset disableCLI commands for toggling presets without removalrestore()method to PresetRegistry for rollback scenariosget()andlist()to return deep copies (prevents accidental mutation)list_by_priority()to filter disabled presets by defaultAddresses
Defensive Programming
Following feedback from previous PR, this implementation includes:
restore()for None/non-dict metadataTest plan
restore(), deep copy, enable/disable CLI, corrupted state handling🤖 Generated with Claude Code