feat(presets): preset stacks — named, reusable preset lineups - #4102
feat(presets): preset stacks — named, reusable preset lineups#4102ira-at-work wants to merge 6 commits into
Conversation
Introduces named, reusable "preset stacks" defined in .specify/preset-stacks.yml: PresetStackEntry/PresetStack/PresetStacksConfig dataclasses, load_stacks_config() validation, and apply_stack() which drives installs through the existing PresetManager.install_from_directory/ install_from_zip/remove primitives only — no new install/uninstall logic. apply_stack() diffs against .stack-state.json on reapply so entries dropped from a stack are uninstalled, unless another applied stack still lists them. download_pack() gains bypass_install_allowed (default False) so a stack entry resolved through the catalog can skip the install_allowed gate: listing a preset in one's own stack is itself the trust decision (FR-2025).
Adds the specify preset stack list/add/remove CLI verbs, mirroring preset_catalog_add/remove's exact YAML-dict-edit pattern: list shows every stack defined in .specify/preset-stacks.yml; add/remove only edit that config file (never install or uninstall anything). (The install verb and its wiring into stacks.py were added in the prior commit.)
There was a problem hiding this comment.
Pull request overview
Adds reusable preset stacks with configuration, CLI management, automatic init application, synchronization, and documentation.
Changes:
- Implements stack parsing, validation, installation, and state synchronization.
- Adds stack CLI commands and
init --preset-stack. - Adds comprehensive tests and preset documentation.
Show a summary per file
| File | Description |
|---|---|
tests/test_preset_stacks.py |
Tests stack configuration, application, synchronization, and CLI behavior. |
src/specify_cli/presets/stacks.py |
Implements preset stack models and application logic. |
src/specify_cli/presets/_commands.py |
Adds stack list, install, add, and remove commands. |
src/specify_cli/presets/__init__.py |
Supports trusted stack-driven catalog downloads. |
src/specify_cli/commands/init.py |
Adds stack selection and implicit default application. |
src/specify_cli/commands/bundle/__init__.py |
Updates the in-process init invocation. |
presets/README.md |
Documents preset stack usage. |
presets/ARCHITECTURE.md |
Documents stack architecture and control flow. |
Review details
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
- Files reviewed: 8/8 changed files
- Comments generated: 6
- Review effort level: Balanced
|
Please address Copilot feedback and limit the changes to presets as indicated by the PR title and description |
Applies a named stack (or the implicit "default" stack, if defined and no flag is given) automatically at init time via apply_stack(). --preset-stack none skips stack resolution entirely; --preset and --preset-stack are mutually exclusive. bundle/_run_init() invokes init's raw Typer callback with a fully-enumerated kwarg list, bypassing Click's option-default resolution, so the new preset_stack parameter has to be passed through there as well; without it the parameter keeps the raw typer.Option sentinel and bundle-driven bootstrap fails on a string comparison.
Adds a Preset Stacks section to README.md (config format, CLI verbs, --preset-stack) and ARCHITECTURE.md (apply_stack() flow diagram, module cross-references), and corrects ARCHITECTURE.md's Module Structure listing to reflect the real presets/ package layout.
Addresses Copilot review feedback on the stack sync logic: - Stack membership now follows stack.entries, not a run's install outcomes. A transient failure previously dropped the entry from current_ids, so the diff treated a still-listed preset as removed and uninstalled a working installation. - Successful entries are recorded under the ID their manifest actually declares. PresetManager keys the registry off the manifest, so a source shipping a different ID left the requested ID in stack state and the real one orphaned on removal. - A run with any failing entry now defers uninstalls (deferred_removals) instead of guessing: a failed entry yields no manifest ID, so a previously tracked ID that differs from the requested one cannot be attributed to it, and removal is destructive. Also moves stack selection (select_stack) and result rendering (render_apply_result) into stacks.py, so `specify init` and `specify preset stack install` share them and the init.py diff shrinks from 83 to 40 lines. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Addresses Copilot review feedback: - Only `none` is reserved; `default` is an ordinary, definable stack name that init picks when no --preset-stack is given. - apply_stack() installs entries in listed order; `priority` is the resolver precedence recorded on the install, not an install order. - The non-zero exit claim only holds for `specify preset stack install`; `specify init` treats stack application as best-effort, like --preset. - Documents membership/ID tracking and deferred removals. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
c33d797 to
e6bd416
Compare
|
Thanks — Copilot's six comments are all addressed, and I've clarified the scope question in the PR description. Copilot feedback Two of them were real defects in the sync logic, now fixed in
The other four were documentation inaccuracies, all corrected:
Scope Everything new is under
Happy to split the Full suite: 6682 passed, with the same 4 pre-existing branch-naming failures present on |
There was a problem hiding this comment.
Review details
Suppressed comments (5)
src/specify_cli/presets/stacks.py:486
PresetManager.remove()returnsFalsewhen a tracked ID is already absent, but this still records and renders it as successfully removed. Only include IDs for which removal actually occurred soStackApplyResult.removedand the CLI output remain accurate.
manager.remove(pid)
removed.append(pid)
src/specify_cli/presets/stacks.py:143
- Priorities below 1 pass this validator even though
PresetManager.install_from_directoryrejects them. A manually authored stack therefore loads as valid and only fails during application; reject non-positive values here so the config and installer enforce the same contract.
This issue also appears on line 485 of the same file.
priority = int(raw_priority)
src/specify_cli/presets/_commands.py:924
stack add --priority 0currently reports success and persists an entry thatPresetManagercan never install. Constrain this CLI option to the installer's minimum so the command cannot create an unusable stack.
priority: int = typer.Option(10, "--priority", help="Install priority (lower = higher priority)"),
src/specify_cli/presets/stacks.py:455
- A local source can pass manifest validation and then raise
OSErrorwhile being copied or registered. Since onlyPresetErroris collected, that filesystem failure aborts the entire apply, prevents later entries from running, and skips state persistence instead of producing the promised per-entry failure result.
except PresetError as e:
any_failed = True
entries.append(
StackEntryResult(
preset=entry.preset,
success=False,
error=f"stack '{stack.name}', preset '{entry.preset}': {e}",
)
)
member_ids.append(entry.preset)
src/specify_cli/presets/init.py:4845
- The stack test replaces
download_packwith a fake, so this new trust-gate branch is never exercised in production code. Add a directPresetCatalog.download_packtest proving a discovery-only entry is still rejected by default and proceeds only whenbypass_install_allowed=True.
if not bypass_install_allowed and not pack_info.get("_install_allowed", True):
- Files reviewed: 8/8 changed files
- Comments generated: 0 new
- Review effort level: Balanced
|
Apologies, @ira-at-work — I should have raised this earlier in the review rather than after you'd built the whole thing out. Digging into this, I think
Could you try modeling your target workflow as a bundle and running Posted by @mnriem via GitHub Copilot (model: Claude Opus 4.8). |
Summary
.specify/preset-stacks.ymlsupport: named, ordered lists of preset entries (id/priority/optional source), equivalent to a saved sequence ofspecify preset addcalls.defaultis applied automatically atspecify init(no flags needed);specify init --preset-stack <name>|noneselects a specific stack or opts out.--presetand--preset-stackare mutually exclusive.specify preset stack list/add/remove(alongside the existinginstall), mirroringpreset catalog add/remove's exact edit pattern —add/removeonly ever touch the config file, never install/uninstall anything.sourcebypasses the discovery-onlyinstall_allowedgate when resolved through the catalog, since listing a preset in your own stack is itself the trust decision.Scope
Everything new lives under
presets/andsrc/specify_cli/presets/. Two files outside that tree are touched, both required by--preset-stackitself and nothing else:src/specify_cli/commands/init.py(+40): the--preset-stackoption and its call intopresets.stacks. Stack selection and output rendering live inpresets/stacks.py(select_stack(),render_apply_result()), shared withspecify preset stack install.src/specify_cli/commands/bundle/__init__.py(+1):_run_init()invokes init's Typer callback with a fully-enumerated kwarg list, bypassing Click's option-default resolution, so every new init parameter has to be listed there too. Without the line,preset_stackkeeps the rawtyper.Optionsentinel and bundle-driven bootstrap fails on a string comparison.Test plan
pytest tests/test_preset_stacks.py tests/test_presets.py— all green (641 passed)pytest) — 6682 passed, 4 pre-existing/unrelated failures (branch-naming short-word retention, confirmed viagit stashbaseline diff, not caused by this change)specs/001-preset-stacks/quickstart.mdagainst a real scratch project with the built CLIpresets/README.mdandpresets/ARCHITECTURE.md🤖 Generated with Claude Code