feat(presets): add enable/disable toggle and update semantics#1891
feat(presets): add enable/disable toggle and update semantics#1891mbachorik wants to merge 10 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.
- _load() now validates json.load() result is a dict before returning - is_installed() validates presets/extensions is a dict before checking membership - Prevents crashes when registry file is valid JSON but wrong type (e.g., array) - Applied same fixes to both registries 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 system to support reversible state changes (enable/disable + restore semantics) and hardens registry access patterns to avoid accidental mutation and improve corrupted-registry resilience, while aligning preset/extension behaviors during template resolution.
Changes:
- Added
specify preset enable|disablecommands to toggle presets without uninstalling. - Introduced
PresetRegistry.restore()and updated registry reads (get(),list()) to return deep copies and handle corrupted entries more safely. - Updated priority ordering to exclude disabled presets/extensions by default and ensured disabled extensions are not reintroduced via unregistered-dir scanning during 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 isolation for reads, defensive handling for corrupted entries, and disables presets in list_by_priority() by default; updates extension resolution to respect disabled state. |
src/specify_cli/extensions.py |
Aligns extension registry behavior with presets: dict validation on load, deep-copy reads, defensive corrupted-entry handling, and list_by_priority(include_disabled=...). |
src/specify_cli/__init__.py |
Adds new Typer CLI commands preset enable / preset disable that flip the registry enabled flag with user messaging. |
tests/test_presets.py |
Adds unit + integration tests for restore/deep-copy behavior, corrupted registry handling, enable/disable commands, and resolution skipping for disabled presets/extensions. |
tests/test_extensions.py |
Adds tests for restore input validation, deep-copy behavior, corrupted registry handling, and list_by_priority() disabled filtering. |
💡 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.
- _load() now normalizes the presets/extensions field to {} if not a dict
- Makes corrupted registries recoverable for add/update/remove operations
- Applied same fix to both registries for parity
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds safer preset/extension registry mutation semantics and introduces preset enable/disable toggles so presets can be temporarily excluded from template resolution without uninstalling.
Changes:
- Added
PresetRegistry.restore()for rollback scenarios and hardened registry loading against corrupted JSON structures. - Updated
get()/list()to return deep copies and updatedlist_by_priority()to exclude disabled entries by default (with an opt-in to include them). - Added
preset enable/preset disableCLI commands and expanded tests for restore/deep-copy behavior, corrupted registry handling, and disabled template 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 |
|---|---|
tests/test_presets.py |
Adds tests for restore(), deep-copy protection, preset enable/disable CLI, and resolution behavior when presets/extensions are disabled. |
tests/test_extensions.py |
Adds tests for ExtensionRegistry.restore() validation, deep-copy behavior, corrupted entries, and disabled filtering in list_by_priority(). |
src/specify_cli/presets.py |
Implements PresetRegistry.restore(), hardens _load(), returns deep copies from get()/list(), adds disabled filtering to list_by_priority(), and updates resolver extension handling. |
src/specify_cli/extensions.py |
Hardens _load(), validates/deep-copies restore(), returns None for corrupted entries in get(), returns {} for corrupted in list(), and adds disabled filtering to list_by_priority(). |
src/specify_cli/__init__.py |
Adds specify preset enable/disable CLI commands with user messaging about scope of the toggle. |
💡 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.
src/specify_cli/presets.py
Outdated
| registered_extension_ids = {ext_id for ext_id, _ in registered_extensions} | ||
| # Get ALL registered extensions (including disabled) to know which dirs are tracked | ||
| all_registered = registry.list_by_priority(include_disabled=True) | ||
| registered_extension_ids = {ext_id for ext_id, _ in all_registered} |
tests/test_presets.py
Outdated
| # Template should NOT be found when disabled | ||
| resolver2 = PresetResolver(project_dir) | ||
| result2 = resolver2.resolve("test-template", "template") | ||
| assert result2 is None or "test-pack" not in str(result2) |
- Use registry.list().keys() instead of list_by_priority() for tracking - Corrupted entries are now treated as tracked, not picked up as unregistered - Tighten test assertion for disabled preset resolution - Update test to match new expected behavior for corrupted entries Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
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