mirror of
https://github.com/github/spec-kit.git
synced 2026-08-03 06:26:30 +08:00
fix: register extensions for the active integration only (#3459)
* fix: register extensions for the active integration only extension add registered commands for every detected agent, and integration upgrade back-filled enabled extensions for non-active integrations. Maintainer direction on #2948: treat the project as single-active. Only the active integration gets extension artifacts; use/switch rescaffold the target when the user selects it. - extension add now routes through the all-agents pass restricted to the active integration (only_agent), keeping detection and missing-skills-dir recovery safeguards. Projects without recorded init-options fall back to detection-based registration. - integration upgrade re-registers extensions only when upgrading the active integration, reversing the #2886 back-fill for non-active targets at maintainer request. Fixes #2948 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: address review feedback on active-only extension registration - Restrict the extension-add active-integration fallback to projects with no recorded active key at all. A recorded but unsupported key (e.g. "generic", deliberately excluded from AGENT_CONFIGS) no longer falls back to registering every detected agent. - Apply the same single-active rule to preset command overrides: PresetManager._register_commands now scopes registration to the active integration via only_agent. - Add PresetManager.register_enabled_presets_for_agent, mirroring ExtensionManager.register_enabled_extensions_for_agent, and call it from integration use/switch/upgrade (active only) alongside the existing extension re-registration so presets are rescaffolded on activation instead of being written for inactive integrations. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: address second round of review feedback (priority order, fail-closed, docs) - register_enabled_presets_for_agent now processes presets in reverse priority order (lowest-precedence first) so the highest-precedence preset is written last and actually wins after `integration use` rescaffolds two overlapping preset command overrides. Verified this reproduces the previously reported reversed-priority bug and that the fix resolves it. - _register_commands_for_active_agent now checks for the "ai" key's presence separately from its value: a missing key still falls back to detection-based registration for all agents, but a recorded, malformed value (non-string or empty, e.g. [] or null) now fails closed (registers nothing) instead of being treated as "no active integration" or reaching AGENT_CONFIGS.get() with an unhashable key and raising TypeError. - Updated docs/reference/presets.md and docs/reference/integrations.md to describe active-only preset/extension registration and clarify that `integration use`/`switch` is the activation point for installed extensions and presets, and that `upgrade` only re-registers them for the active integration. Adds regression tests: two enabled presets overriding the same command with different priorities (priority winner must survive `use` rescaffolding), and a malformed recorded `ai` value ([]) for `extension add`. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: address third round of review feedback (multi-integration semantics) Fixes five deeper active-only registration bugs surfaced by Copilot review after2486c08, all in the presets/extensions single-active integration rule (#2948): 1. presets: _reconcile_composed_commands (run after install/remove) bypassed the active-only filter entirely, writing composition-winner command files for every detected non-skill agent via register_commands_for_non_skill_agents. Added an only_agent param to that registrar method (mirroring register_commands_for_all_agents) and threaded it through all 5 reconciliation call sites. 2. presets: `integration use copilot` with --skills (ai_skills: true) wrote both the static .agent.md command file AND the SKILL.md mirror for the same override. Mirrored the extension path's ai_skills guard in both _register_commands and the reconciliation pass: a command-backed active agent running in skills mode is excluded from non-skill command registration. 3. presets: registered_skills was a flat list, so switching between two skill-mode agents (e.g. Claude -> Codex) and then removing the preset only restored the currently active agent's directory, permanently orphaning the other. _unregister_skills now restores every existing skill-mode agent directory instead of only the active one. 4. extensions: load_init_options() collapses "no file" and "corrupted file" into the same {}, so the round-2 fail-closed fix didn't actually distinguish them. Added a shared resolve_active_agent_for_registration() helper in _init_options.py that checks file existence separately from parse success, returning a distinct sentinel for "file absent" vs None for "corrupted or invalid". extensions/__init__.py now uses this helper. 5. presets: same corruption-collapsing bug in _register_commands's active_agent resolution. Now uses the same shared helper as (4). Adds regression tests for all five: reconciliation active-only filtering, copilot --skills dual-write prevention, multi-skill-agent switch+remove, and corrupted init-options fail-closed behavior for both extension add and preset add. Each test was verified to fail against the pre-fix code and pass with the fix. Targeted (883) and full (3923 passed, 109 skipped) suites pass; ruff check clean. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: address fourth round of review feedback (skill registration provenance) Replace the "enumerate every skill-mode directory and restore all of them" approach from the previous round with precise per-agent provenance tracking, per reviewer feedback that the enumerate-and-restore-everything design was unsound: - registered_skills changes from a flat List[str] to Dict[str, List[str]] (agent name -> skill names actually written), mirroring the shape registered_commands already uses. _register_skills now returns this per-agent mapping instead of a bare list, and every call site (register_enabled_presets_for_agent, install_from_directory, the _reconcile_skills "was this skill previously managed" check) is updated to read/merge the new shape. Legacy flat-list registry entries from before this change are still readable: writes self-migrate the format, and _normalize_registered_skills() handles the transitional read paths. - _unregister_skills now restores exactly the agent directories recorded for a preset instead of guessing at every skill-mode integration that happens to exist on disk. This fixes two problems with the old enumerate-everything design: (1) it could silently overwrite or delete another preset's (or a user's) override in an agent directory the current preset never actually touched, and (2) it depended on transient per-process integration state (_skills_mode), which is unset in a fresh CLI invocation for mode-selectable integrations like Copilot --skills, permanently orphaning their overrides after a process restart. Registries written before this change (flat list, no agent provenance) fall back to best-effort restoration under only the currently active agent, matching the pre-existing guarantee level. - Every directory resolved from persisted provenance is now validated through the project's shared symlink/containment guard (_ensure_safe_shared_directory) before any file in it is read, written, or removed, since restoration may target an agent that isn't currently active and its directory can't be assumed safe just because a name was recorded for it. - _tracked_skill_agent_dirs() (the enumeration helper introduced last round) is removed; it's superseded by the provenance-based design. Adds regression tests: a symlinked skills directory is rejected during removal; removing one preset does not disturb a different preset's override in another agent's directory; and a Copilot --skills registration installed, then removed after switching agents in a fresh PresetManager instance (simulating a new process), is still correctly restored. Updates existing skill-registration assertions across test_presets.py and test_integration_claude.py for the new per-agent registry shape. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: address fifth round of review feedback (symlink presence, rescaffold reconciliation, shared skills dir) - _init_options.py: resolve_active_agent_for_registration() now treats a dangling init-options.json symlink as present (path.is_symlink() check alongside path.exists()), since Path.exists() follows symlinks and returns False for a broken one. Previously a broken symlink fell back to the legacy "no file" path and registered every detected agent instead of failing closed. - presets/__init__.py (register_enabled_presets_for_agent): the integration use/switch rescaffold path now collects affected command names across all presets processed and runs _reconcile_composed_commands/_reconcile_skills once after the loop, matching install/remove. Previously rescaffolding wrote each preset's raw content directly with no follow-up reconciliation, so a project-level override (the highest-priority layer) could be clobbered by a lower-precedence preset after switching agents. - presets/__init__.py (_unregister_skills): multiple integrations can share one physical skills directory (agy/codex/zed all resolve to .agents/skills). Provenance restoration now groups recorded agent entries by resolved directory and restores each physical directory exactly once, preferring the currently active agent's renderer when it owns that directory (otherwise any recorded owner, chosen deterministically). Previously each recorded agent key triggered its own restore pass against the same directory, with whichever agent was iterated last silently winning regardless of which agent was active. Adds regression tests for each: a dangling init-options.json symlink failing closed for both preset resolution and extension add; integration use rescaffold preserving a project override over a lower-priority preset; and a codex/agy shared-directory removal restoring the directory exactly once in the active agent's format. Targeted (tests/integrations/test_integration_subcommand.py, tests/test_presets.py, tests/test_extensions.py, tests/test_extension_skills.py, tests/integrations/test_integration_opencode.py, tests/integrations/test_integration_claude.py): 930 passed. Full suite: 3930 passed, 109 skipped. ruff check: clean on files touched by this change. Refs #2948 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: guard skill subdirectories and active-agent scoping in preset reconciliation Fix 4 issues from round-6 review of the active-only integration registration work (#2948): - remove(): removed_cmd_names only collected primary command names from registered_commands + manifest aliases, missing commands that were only ever registered via skills mode (ai_skills guard returns no command names for command-backed integrations in skills mode). This skipped reconciliation entirely when removing a higher-priority skills-mode preset, causing _unregister_skills() to fall back to core/extension content instead of the surviving lower-priority preset's override. Now every command template's primary name is added to removed_cmd_names unconditionally. - _reconcile_composed_commands(): the "composed is None" branch (fires when no replace-strategy layer remains for a command, e.g. after removing a wrap/append preset's base) called unregister_commands() across every configured non-skill agent, ignoring only_agent. This deleted historical artifacts from integrations that were never active for the preset. Now filtered by only_agent like the rest of the file. - Added _validate_skill_subdir() helper (reusing _ensure_safe_shared_directory/_validate_safe_shared_directory from shared_infra.py) and applied it at every site that reads or writes an individual skill subdirectory (_register_skills, _unregister_skills_in_dir, _reconcile_skills' override_skills restoration loop). _safe_skills_dir_for_agent only validated the parent skills directory; a symlinked leaf subdirectory (e.g. .claude/skills/speckit-specify) would slip past that check since is_dir()/exists() follow symlinks, letting write_text/rmtree operate through it to an arbitrary location outside the project. Added regression tests: removing a higher-priority skills-only preset restores the surviving lower-priority preset's content; composed-is-None unregistration only touches the active agent; symlinked skill subdirectory rejected on restore; symlinked skill subdirectory rejected on write. Targeted (934) and full (3934 passed, 109 skipped) test suites and ruff check pass clean. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: persist command registration before fallible skills phase on rescaffold Fix remaining round-6 review findings on the active-only integration registration work (#2948): - register_enabled_presets_for_agent(): registered_commands and registered_skills were merged and persisted together in a single registry.update() call after both the commands and skills phases ran. If _register_skills() raised, the per-preset try/except swallowed it before that update() call was reached, even though _register_commands() had already written a real command file to disk. That file became untracked, so preset removal could no longer clean it up. install_from_directory() already persists registered_commands immediately after the commands phase, before starting the independently fallible skills phase; rescaffold now does the same. - test_presets.py: renamed a misleading claude_dir variable (pointing at Gemini's command directory) in test_composed_none_unregister_respects_active_agent to reuse the existing gemini_commands_dir variable already defined earlier in the same test. Added regression test test_rescaffold_persists_commands_before_fallible_skills_phase: simulates a skills-phase failure during rescaffold and asserts the command file already written to disk is still tracked in registered_commands. Verified all other round-6 findings (preset active-integration scoping, preset reconciliation/remove paths, skills-mode switching, override precedence during rescaffold, skill-subdirectory symlink safety) are already addressed by prior commits in this branch; re-checked each against current code before concluding no further change was needed. Targeted (tests/test_presets.py, tests/test_extensions.py: 689 passed) and full (3935 passed, 109 skipped) suites and ruff check on changed files pass clean. Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: unregister stale opposite-mode preset artifact on same-agent skills toggle Fix an Important gap in register_enabled_presets_for_agent() surfaced by quality review (#2948): toggling ai_skills for the *same already-active* command-backed agent (e.g. `integration upgrade copilot` after flipping ai_skills, with copilot staying active throughout) left a stale artifact from the previous mode behind, violating the command/skill mutual- exclusion invariant this PR otherwise enforces. - command -> skills: _register_commands()'s ai_skills guard makes the commands phase a no-op, but the previously-written command file (e.g. .agent.md) and its registered_commands[agent] entry were never cleaned up, so it lingered alongside the newly written SKILL.md. - skills -> command: _get_skills_dir() stops resolving a skills directory once ai_skills is off, making the skills phase a no-op, but the previously-written SKILL.md and its registered_skills[agent] entry were never cleaned up, so it lingered alongside the newly (re)written command file. register_enabled_presets_for_agent() now resolves once per call whether agent_name is a command-backed integration (extension != "/SKILL.md") and the current ai_skills state, then narrowly unregisters the stale opposite- mode entry for that agent via the existing _unregister_commands / _unregister_skills helpers before persisting updated tracking — mirroring the same per-agent, per-preset isolation already used elsewhere in this method. Native skill-only agents (claude, codex, ...) are unaffected: they have no command/skill toggle, so registered_commands and registered_skills legitimately co-exist for them by design. The trailing reconciliation pass, project-override precedence, and per-preset partial-failure isolation are all unchanged. Added red-first regression tests exercising the real install + register_enabled_presets_for_agent rescaffold path in both toggle directions: - test_rescaffold_toggle_command_to_skills_removes_stale_command_file - test_rescaffold_toggle_skills_to_command_removes_stale_skill_file Both failed against the prior code (stale artifact persisted / registry still tracked it) and pass after the fix. Targeted (tests/test_presets.py, tests/test_extensions.py: 691 passed) and full (3937 passed, 109 skipped) suites and ruff check on changed files pass clean. Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: migrate legacy flat-list registered_skills on rescaffold even when unchanged Fix a valid finding from GitHub Copilot's review of HEADb9d9053(#2948): register_enabled_presets_for_agent() normalizes a legacy flat-list registered_skills value (predating per-agent provenance) to the {agent_name: [...]} dict shape in memory via _normalize_registered_skills, but the persistence check only compared the two *normalized* forms. When the freshly rescaffolded skill names are identical to what the legacy list already held — the common case, since nothing about the preset or skill actually changed — that comparison is a no-op and registry.update() is skipped, leaving the *raw* on-disk value as the un-migrated flat list. A later switch to a different skill-mode agent and removal then follows _unregister_skills's legacy best-effort path (restore only the currently active agent's directory) instead of the per-agent provenance path, permanently orphaning the first agent's override. Fix: track the raw (pre-normalization) existing value and force persistence whenever it's a non-empty list, independent of whether the normalized content changed. Traced registered_commands for the same class of bug: its registry value has always been Dict[str, List[str]] (no legacy flat-list format ever existed for it — the existing `if not isinstance(existing_commands, dict): existing_commands = {}` guard is not a lossy migration path), so this fix stays scoped to registered_skills only. Added red-first regression test test_rescaffold_migrates_legacy_flat_list_registered_skills: installs a preset, overwrites its registry entry with a raw legacy flat list, rescaffolds the *same* active agent with unchanged skill names, and asserts the raw registry is migrated to per-agent dict form. Extends the scenario with a switch to a second skill-mode agent and preset removal to prove both agents' directories restore cleanly instead of orphaning the first. Failed against the prior code (raw value stayed a list) and passes after the fix. Targeted (tests/test_presets.py, tests/test_extensions.py: 692 passed) and full (3938 passed, 109 skipped) suites and ruff check on changed files pass clean. Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: reconcile before fallible skills phase, infer legacy skill provenance, and unregister stale extension artifacts on toggle Three findings from the Copilot review on HEAD b9d9053/3a1e749: 1. `register_enabled_presets_for_agent()` only recorded a preset's command names into `affected_cmd_names` (the set later passed to `_reconcile_composed_commands`/`_reconcile_skills`) in the loop that ran *after* `_register_skills()`, inside the same per-preset `try` block. If `_register_skills` raised, the `except` caught it and `continue`d before that loop ever ran — so a preset whose commands phase already wrote real content to disk never got reconciled against the full priority stack, leaving its raw content in place instead of a project override or higher-precedence preset's content. Fix: record the manifest's command names immediately after the commands phase succeeds and persists, before calling the independently fallible `_register_skills()`. 2. The legacy flat-list `registered_skills` migration (added for the previous review round) attributed every name in the list to whichever agent was currently being (re)activated. If the first operation after upgrading from a pre-#2948 registry was a direct switch to a *different* skill-mode agent (e.g. a legacy Claude override, then `integration use codex` with no intervening Claude rescaffold), the migrated dict only recorded `{"codex": [...]}`, permanently losing Claude's actual provenance and orphaning its override on later removal. Fix: added `_infer_legacy_skill_provenance()`, which probes every configured skill-mode agent's directory (via the same safe, symlink-validated helpers already used for restore/removal) for a `SKILL.md` whose frontmatter records this exact preset as the owner (`metadata.source == "preset:<pack_id>"`). A name found under more than one directory is attributed to every matching agent (the preset may have been active while the user switched between several skill-mode agents before provenance tracking existed); names that can't be matched to any directory still fall back to the previously-active best-effort behaviour. Directory grouping for shared-path aliases (e.g. agy/codex/zed all resolving to `.agents/skills`) intentionally does not call `.resolve()` on the path, since doing so diverges from `project_root`'s own resolution state on platforms where a path component is itself a symlink (e.g. macOS's `/var` -> `/private/var`) and made every subsequent containment check spuriously fail. 3. `register_enabled_extensions_for_agent()` has the same command/skill mutual-exclusion gap the preset path had (fixed in a previous round): toggling `ai_skills` for the *same active* agent left the opposite mode's artifact behind. Command -> skills left the extension's `.agent.md` file and its `registered_commands[agent]` entry in place once `skills_mode_active` made the commands phase a no-op. Skills -> command left the extension's `SKILL.md` file in place, since an empty `_register_extension_skills()` result (because this agent's skills directory no longer resolves once `ai_skills` is off) was treated as "nothing to register" rather than "this was rendered here before and is now stale". This diverges from the preset path in one respect: `registered_skills` for extensions has always been a flat list with no per-agent provenance (extension skills are only ever rendered for the active agent, never per-preset-per-agent tracked), so the fix resolves ownership by checking which of the extension's tracked skill names still exist as directories under this specific agent's directory before removing them — mirroring the same technique `unregister_agent_artifacts` already uses for full agent deactivation, but scoped narrowly to firing only when a toggle is actually detected (`skills_mode_active` / `command_mode_active`), so a same-mode re-run never disturbs already-correct artifacts or a user's manual customizations. Regression tests (all confirmed red before their respective fix, green after): - tests/test_presets.py::TestPresetSkills::test_rescaffold_reconciles_override_even_when_skills_phase_fails - tests/test_presets.py::TestPresetSkills::test_rescaffold_legacy_flat_list_direct_switch_preserves_original_agent - tests/test_extension_skills.py::TestExtensionSkillRegistration::test_rescaffold_toggle_command_to_skills_removes_stale_extension_command_file - tests/test_extension_skills.py::TestExtensionSkillRegistration::test_rescaffold_toggle_skills_to_command_removes_stale_extension_skill_file Verification: tests/test_presets.py + tests/test_extensions.py + tests/test_extension_skills.py (753 passed), tests/integrations/ (1768 passed, 1 skipped), full suite `pytest tests -q` (3942 passed, 109 skipped), `ruff check` on changed files clean. Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: broaden legacy skill provenance inference to command-backed agents _infer_legacy_skill_provenance() only probed agents whose registrar config statically declares extension == "/SKILL.md", excluding command-backed agents (e.g. Copilot) that can also render preset overrides as SKILL.md files when ai_skills is enabled. A real preset-owned .github/skills/.../SKILL.md written while Copilot was the active skills-mode agent was therefore never probed and got misattributed entirely to whichever agent activated first after the upgrade, permanently orphaning Copilot's override on later removal. Broaden the candidate set to every configured integration (CommandRegistrar.AGENT_CONFIGS), reusing the existing safe-path helper (_safe_skills_dir_for_agent, itself built on the shared _get_skills_dir resolver) rather than inventing new path-construction logic. The existing preset-marker match (metadata.source == "preset:<pack_id>") continues to gate every attribution, so command-mode agents that never rendered this preset's skill are not falsely attributed. Add red-first regression tests: a legacy flat-list entry owned by Copilot in skills mode, switched directly to Claude with no intervening Copilot rescaffold, now migrates to a per-agent dict covering both agents, and removal restores both agents' files instead of orphaning Copilot's override; plus a negative-case test confirming a command-mode Copilot with no preset-owned skill marker is not falsely attributed during the same migration. Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: preserve extension skill tracking for mirrors in other agent dirs The skills -> command toggle cleanup in register_enabled_extensions_for_agent() recomputed the remaining tracked registered_skills names by checking only the toggling agent's own skills directory. Since registered_skills is a single flat list shared across every agent an extension has ever been activated under (skills are only ever rendered for the active agent, so there is no per-agent registry key), a name whose mirror still existed under a *different*, previously-active agent's directory was incorrectly dropped from tracking as soon as the current agent's own copy was removed. A later full removal only iterates registered_skills, so the orphaned mirror under the other agent's directory was never found or cleaned up. Add _extension_owned_skill_names(), which re-verifies ownership across every configured agent's skills directory (deduped by shared path) the same way the existing _unregister_extension_skills() fallback scan already does, keeping a name only when a SKILL.md with a matching metadata.source == "extension:<id>" marker is found somewhere - read-only, no directory creation, no symlink escape. Use it instead of re-checking only the toggling agent's own directory when recomputing what remains tracked after narrow stale-mirror cleanup. Add a red-first regression test: Auggie is activated in skills mode first (writing a mirror), then Copilot is activated in skills mode (writing its own mirror for the same names), then Copilot toggles to command mode. Before the fix, registered_skills lost both names entirely even though Auggie's mirrors were untouched on disk; after the fix tracking is preserved and a subsequent full removal correctly cleans up Auggie's remaining mirrors too. Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: reject symlinked skills-directory escape in extension skill scans _extension_owned_skill_names() and the fast/fallback paths of its sibling _unregister_extension_skills() called skills_candidate.resolve() and then checked children relative to that already-resolved candidate. If the candidate directory itself (e.g. .gemini/skills) was a symlink pointing outside the project root, both the resolve() call and the subsequent containment check silently passed through the symlink instead of rejecting it: - _extension_owned_skill_names() would falsely attribute ownership to a marker-matching SKILL.md living outside the project. - _unregister_extension_skills()'s fast path (an explicit skills_dir, as passed by the toggle-cleanup call site) and its fallback scan (used during full extension removal) would both shutil.rmtree() the external directory, deleting unrelated content outside the project. Fix by validating the candidate directory itself with the existing _validate_safe_shared_directory() shared-infra helper before any probe or delete: it rejects a symlink at any path component (walking down from the project root, including the final component) without ever resolving through it, and is already used elsewhere in the codebase for the same class of shared-directory containment check. Unsafe candidates are skipped/refused rather than followed. Add red-first security regression tests reproducing each of the three call sites with a `.gemini/skills` symlink pointing at an external directory containing a marker-matching SKILL.md and an unrelated precious_file.txt: provenance inference must not attribute the name, and both the explicit-skills_dir fast path and the None-skills_dir fallback scan must leave the external directory and file untouched. Existing valid shared/deduped directory tests (e.g. agy/amp/codex/zed sharing .agents/skills) continue to pass, confirming legitimate shared directories still clean up correctly. Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix unscoped extension-skill removal and legacy preset provenance on direct remove - _unregister_extension_skills(): omitting skills_dir now always triggers the full multi-directory fallback scan instead of narrowing to the currently active agent's directory. Previously, remove() (the only caller that omits skills_dir) would resolve the active agent's dir and take the scoped fast path, orphaning a previously-active second agent's extension skill mirror during full removal. - PresetManager.remove(): infer legacy flat-list registered_skills provenance (reusing _infer_legacy_skill_provenance from the prior rescaffold fix) before invoking _unregister_skills, so a direct `preset remove` with no intervening rescaffold/switch also restores every previously-active agent's directory instead of only the currently active one. Added regression tests: - test_remove_while_second_agent_still_in_skills_mode_cleans_up_first_agent_mirror - test_remove_infers_legacy_flat_list_provenance_without_prior_rescaffold Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Keep unregister_agent_artifacts scoped to its agent when directory is absent ExtensionManager.unregister_agent_artifacts() converted its resolved agent_skills_dir to None whenever that directory didn't exist, before calling _unregister_extension_skills(). After1d8f9e3, omitting skills_dir means "genuinely unscoped removal": scan every configured agent's directory, reserved for ExtensionManager.remove()'s full project cleanup. Since unregister_agent_artifacts is agent-scoped (used by switch to clean up the previous integration's artifacts), this caused it to delete every other agent's live extension skill mirrors whenever the target agent's own directory happened to be absent, e.g. unregistering an agent that was never activated. Fix: always pass the explicit, agent-scoped skills_dir, even when it doesn't exist on disk, so the fast path is a safe no-op for an absent directory instead of falling back to the all-agents scan. Registry reconciliation (dropping removed names from the flat registered_skills list) now only runs when the agent's directory actually exists, so an absent directory can't be misread as "these names were removed everywhere" and wipe tracking for mirrors that still legitimately live under other agents' directories. Added regression test: - test_unregister_agent_artifacts_stays_scoped_when_agent_dir_absent Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Preserve global skill tracking across agents in unregister_agent_artifacts The present-directory branch of ExtensionManager.unregister_agent_artifacts() recomputed "remaining" registered_skills only by checking whether each name still existed under the just-cleaned agent's own directory. registered_skills is a single flat list shared across every agent an extension was ever activated under (skills are only ever rendered for the currently active agent, so there's no per-agent registry key). Repro: auggie and copilot both have mirrors for the same extension; unregister_agent_artifacts("auggie") correctly removes auggie's own mirror, sees the names absent from auggie's (now empty) directory, and stores an empty registered_skills list - even though copilot's mirror is still live on disk and now untracked. A later full remove() then reads an empty registry and leaves copilot's mirror orphaned. Fix: after the agent-scoped cleanup, recompute remaining names with _extension_owned_skill_names(), which scans every safe, configured agent skills directory (not just the one just cleaned) and keeps a name only if a marker-verified SKILL.md for this extension still exists somewhere. This is the same helper already used for the analogous same-agent toggle-cleanup case, so no new abstraction was introduced. Explicit per-agent cleanup, marker ownership verification, and symlink/containment safety are unchanged. Added regression test: - test_unregister_agent_artifacts_preserves_tracking_for_other_agent_mirror Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Reconcile every historical agent on preset removal; validate child skill dirs Fixes 3 findings from the Copilot review on HEAD31c9b97(#2948): 1. presets/__init__.py: remove()'s command reconciliation only recreated the surviving preset's content for the currently active agent, even though the removed preset's registered_commands could span multiple historical (now-inactive) agents recorded via prior rescaffolds. Now remove() captures every historical agent registered_commands actually targeted (before mutation) and passes it as extra_agents through _reconcile_composed_commands -> _register_for_non_skill_agents / _register_command_from_path -> registrar.register_commands_for_non_ skill_agents, so the active-only restriction for install/use is preserved while post-removal reconciliation restores every touched directory. 2. presets/__init__.py: the analogous gap existed for skills. _unregister_ skills() now returns {skills_dir: renderer_agent} for every directory it actually restored, and _reconcile_skills() accepts extra_skills_dirs to reconcile each of those directories (via a new apply_to_dir() helper), not only the currently active skills directory. _register_skills() gained optional target_dir/target_agent overrides (forcing create_missing_skills off for non-active directories) so a historical directory is only ever restored, never seeded with brand-new skills. 3. extensions/__init__.py: _extension_owned_skill_names() and both the fast and fallback paths of _unregister_extension_skills() validated only the parent skills_dir for symlink escape, then resolved skills_dir / skill_name and checked containment relative to that already-resolved parent. A per-skill child that is itself a symlink to a different, legitimate skill directory within the same (safe) root passed that containment check, so deleting/attributing through the symlink name could destroy or misattribute an unrelated skill reached only via the alias. All three call sites now run the shared _validate_safe_shared_directory() component-wise check against the full skills_dir / skill_name path (not just the parent) before any read or delete, rejecting a symlinked child outright rather than following it, even when its resolved target remains in-bounds. Regression tests added (all confirmed red against pre-fix code, green after): - test_remove_reconciles_command_for_every_historical_agent - test_remove_reconciles_skill_for_every_historical_agent - test_extension_owned_skill_names_rejects_symlinked_child_skill_dir - test_unregister_extension_skills_explicit_dir_rejects_symlinked_child - test_unregister_extension_skills_fallback_rejects_symlinked_child Tests: tests/test_presets.py (361), tests/test_extension_skills.py (69), tests/test_extensions.py (338) all pass; tests/integrations (1768 passed, 1 skipped) pass; full suite 3902 passed / 74 skipped (90 pre-existing, environment-only git-signing tests deselected — confirmed failing identically on the pre-change baseline due to local 1Password SSH-agent signing, unrelated to this change). ruff check clean on all changed files. Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Persist historical reconciliation ownership; defer destructive toggle cleanup; validate registry-provided skill names Round 11 review findings (5 comments on HEADab6c28c), three root causes: A) Historical-agent reconciliation wrote surviving content to disk but discarded the returned per-agent write map, so the preset's own registered_commands/registered_skills never learned about directories reconciliation restored on its behalf. A later removal of that same preset then orphaned those directories. Added _merge_pack_registered_commands/_merge_pack_registered_skills and wired them into _reconcile_composed_commands and _reconcile_skills's apply_to_dir so every actual write is merged back into the winning preset's registry metadata. B) Command<->skills toggle on an already-active agent deleted the old artifact before the replacement registration ran, in both presets/__init__.py's register_enabled_presets_for_agent and extensions/__init__.py's register_enabled_extensions_for_agent. If the replacement step raised, both artifacts were lost. Deferred the destructive cleanup until after the replacement phase completes without raising (register-new-then-remove-old ordering); the mirror skills->command direction was already safe since the new command file is always registered unconditionally before any cleanup runs. C) _unregister_skills_in_dir and _infer_legacy_skill_provenance joined a registry-provided (untrusted) skill name directly onto a directory before any name-shape validation. An absolute in-project name discards the intended parent directory entirely (Path's "/" operator drops the left side for an absolute right side), letting a corrupted registry entry escape the intended skills subtree while still resolving inside the project root - passing the existing containment/symlink check. Added a centralized _is_safe_registry_skill_name guard (rejecting non-strings, empty strings, absolute paths, multi-component paths, and "."/".." ) and applied it before every path join derived from registry-provided skill names in both functions. Also fixed _infer_legacy_skill_provenance's unmatched-name fallback, which previously still attributed rejected names to fallback_agent even after the loop skipped them. Added red-first regressions for all three root causes, covering: a two-preset historical-command-agent survivor scenario, an analogous skill-agent survivor scenario, injected skills-phase failure during a preset command->skills toggle and the extension equivalent, a direct unit test of the new name-safety guard, an absolute-path escape attempt against _unregister_skills_in_dir, and a false-attribution attempt against _infer_legacy_skill_provenance. Tests: tests/test_presets.py (367 passed), tests/test_extension_skills.py + tests/test_extensions.py (408 passed), tests/integrations (1768 passed, 1 skipped), full suite tests -q deselecting the pre-existing 1Password-signing-affected tests/extensions/git/test_git_extension.py (3909 passed, 74 skipped, 90 deselected). ruff check clean on all changed files. Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Verify replacement actually landed before retiring stale toggle artifacts The command<->skills toggle cleanup added for #2948 deferred destructive removal of the old-mode artifact until after the replacement registration call completed without raising. That was necessary but not sufficient: none of _register_skills(), _register_commands(), register_commands_for_agent(), or _register_extension_skills() raise on a missing source template, a safety-validation skip, or a corrupted manifest entry — they simply return an empty or partial result. Treating "did not raise" as "fully replaced" meant a stale artifact could still be deleted (or its tracking dropped) even though its specific replacement never actually landed, leaving neither artifact in place for that logical command/skill. Fix all four affected toggle directions by checking the replacement call's actual return value before allowing any destructive step: - presets command->skills (register_enabled_presets_for_agent): only unregister a stale command name once its corresponding skill name (via the existing _skill_names_for_command() helper) is confirmed present in the skills call's returned names for that agent; the remainder stays tracked and on disk. - presets skills->command (register_enabled_presets_for_agent): only unregister a stale skill name once its corresponding command name is confirmed present in the commands call's returned names for that agent, using the same helper. - extensions skills->command (register_enabled_extensions_for_agent): only remove a skill mirror once the matching command (mapped via the existing HookExecutor._skill_name_from_command() helper) is confirmed present in register_commands_for_agent's returned names. - extensions command->skills (register_enabled_extensions_for_agent): only remove a deferred stale command once its matching skill name is confirmed present in _register_extension_skills()'s returned names. All four reuse the existing command<->skill name-derivation helpers rather than inventing new mapping logic. Registry tracking is updated to retain exactly the unreplaced subset rather than being popped wholesale, so partially-successful toggles leave correct, minimal tracking behind. Added 8 new regression tests (4 presets, 4 extensions) covering both the fully-empty and genuinely-partial result cases for each of the four toggle directions, using real missing-source-file scenarios (not mocked return values) to exercise the actual code paths. Confirmed red before the fix and green after for all 8. Focused (test_presets.py, test_extension_skills.py, test_extensions.py, tests/integrations): 2551 passed, 1 skipped. Full suite (tests, excluding the pre-existing environment-local 1Password-signing git-extension failures): 3917 passed, 74 skipped, 90 deselected. ruff check: clean. Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Retire alias command groups on toggle; scope preset cleanup to switched-away agent (#2948) Fixes three current Copilot review findings on HEADd0d152e: 1. Command->skills toggle cleanup only matched a stale command's own name against the returned replacement skill name. Aliases (CommandRegistrar tracks and returns primary + alias names flattened into one list) never have their own skill rendered -- only the primary command's skill is rendered -- so an alias's name could never match, leaving its command artifact and tracking behind forever even after the primary's replacement landed. Fixed identically in both presets (register_enabled_presets_for_agent) and extensions (register_enabled_extensions_for_agent): build a primary->alias mapping from the manifest, group stale names by primary, and retire/keep the whole group together based solely on whether the primary's skill replacement actually landed. 2. `integration switch` to a not-yet-installed target unregistered the old agent's extension artifacts but had no preset equivalent, so a preset's command overrides (including custom preset commands) and skill mirrors for the deactivated agent lingered as orphans. Added `PresetManager.unregister_agent_artifacts()`, mirroring `ExtensionManager.unregister_agent_artifacts()`: scoped strictly to the given agent, migrates a legacy flat-list `registered_skills` entry via existing on-disk provenance inference before removing anything (so other agents' real ownership is preserved rather than guessed or dropped), and guards against double-processing an artifact through both the commands and skills paths for native SKILL.md agents. Wired via a new `_unregister_presets_for_agent()` helper into the integration switch command's existing old-agent cleanup phase. Added red-first regression tests: - tests/test_presets.py: alias-group retire/keep/partial-multi-group tests for the command->skills toggle; unregister_agent_artifacts scoping tests for commands and legacy-list skill provenance. - tests/test_extension_skills.py: alias-group retire/keep tests for the extension command->skills toggle. - tests/integrations/test_integration_subcommand.py: end-to-end switch test proving a preset's custom command override is cleaned up when switching to a not-yet-installed integration, with tracking updated correctly and the new agent's registration unaffected. All new tests confirmed red (AttributeError / orphaned file assertions) before the fix and green after. Full suite: 3980 passed, 109 skipped. ruff check clean on all changed files. Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: track reconciled extension artifacts Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix native skill preset reconciliation Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix shared native skill cleanup Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix partial preset rescaffold tracking Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix preset agent skill lifecycle Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Clarify preset removal reconciliation Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(integrations): address upgrade review feedback Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(presets): reconcile partial command writes Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: address active artifact cleanup review Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: defer preset skill cleanup to winning command Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: track reconciled and partial preset skills Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: reconcile project overrides to legacy skills Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: harden preset skill writes and rollback Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(presets): harden legacy skill restoration Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(presets): preserve non-owned legacy skills Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: validate reconciled skill paths Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(presets): preserve reconciled skill ownership Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(presets): clean reconciled agent skills Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: keep legacy cleanup project-local Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(presets): keep active agent's artifacts in its current mode on remove A partially failed command<->skills toggle leaves stale tracking (registered_commands or registered_skills) for the active agent, and remove() replayed that history regardless of the agent's current mode: - extra_agents re-admitted the active skills-mode agent into command reconciliation, recreating its command file from a surviving lower preset even though only_agent excluded it. - _unregister_skills restored (and _reconcile_skills reapplied) a skill artifact for the active command-mode agent instead of deleting the preset-owned leftover. The active agent's participation is now decided exclusively by its current mode: reconciliation strips it from extra_agents, and removal routes its stale skills through _delete_agent_preset_skills. Historical replay still applies to inactive agents only (#2948). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: filter uninstalled-extension commands in reconciliation; allow active-agent layout change with presets Two follow-ups to the upstream-main merge: - Preset reconciliation (_reconcile_composed_commands) now skips extension-scoped commands (speckit.<ext>.<cmd>) whose extension is not installed, at the single chokepoint every install/remove/rescaffold pass funnels through. Registration already refused them, so reconciliation could materialize files no registry entry tracks. The duplicated per-call-site filters collapse into one _extension_installed_for_command helper. - The #3415 layout-change guard predates this PR's agent-scoped preset rescaffold: for the active integration, _register_presets_for_agent now re-registers enabled presets in the new layout and retires the old layout's stale files, so an active-agent command<->skills toggle proceeds and reconciles instead of being rejected. The guard still rejects non-active agents (no rescaffold runs for them) and still fails closed on an unreadable registry. _installed_presets_affecting_agent also understands the per-agent dict shape of registered_skills this PR writes, instead of raising 'malformed'. Regression tests: rescaffold with an uninstalled extension's command, CLI-level legacy<->skills toggle with an installed preset (both directions), secondary-agent rejection, and dict-shaped registered_skills in the guard helper. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: reject active layout change while a disabled preset owns artifacts The post-upgrade preset rescaffold iterates enabled presets only, and a disabled preset's artifacts are deliberately frozen until removal, so an active-agent command<->skills layout change cannot reconcile them. _installed_presets_affecting_agent now reports each preset's enabled state and the guard rejects the migration while any affected preset is disabled, with re-enable/remove guidance. Enabled presets and non-active rejection behave as before. Regression test: disabled preset blocks the toggle untouched; re-enabling unblocks it and reconciles. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs: replace placeholder prefix in two safety comments Comment-only: spell out why skill deletion is restricted to project-local directories (flat/legacy provenance cannot prove home-directory ownership) instead of an undefined placeholder word. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs: correct guard-helper docstring to active-only registration model Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: fail closed on non-list values in per-agent preset provenance A dict-shaped registered_skills/registered_commands entry with a non-list value (e.g. null) left ownership undecidable but read as "no artifacts", letting a layout-changing upgrade proceed on a malformed registry. Validate values are lists and raise _PresetRegistryUnreadableError otherwise, matching the guard's fail-closed contract. Unit test covers both fields. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: drop eager extension unregister on layout-changing upgrade Unregistering the agent's extension artifacts before re-registration deleted files and registry tracking up front, so a failed or partial re-registration left the extension with no artifacts at all. Retirement of each opposite-mode artifact already belongs to register_enabled_extensions_for_agent's deferred toggle cleanup, which removes an old artifact only after its replacement is confirmed. Also keeps disabled extensions consistent with disabled presets: artifacts stay frozen in place with intact tracking. Regression test corrupts the installed extension manifest so re-registration fails, then asserts the old-layout artifacts and their registry tracking survive the upgrade. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: rescaffold fallback integration after failed switch rollback When Phase 2 of a switch fails, rollback restores another installed integration as the default via _set_default_integration but never re-registered extensions or presets for it. Under active-only registration the fallback may never have received any artifacts (it was installed while another integration was active), and Phase 1 already unregistered the outgoing agent's artifacts — leaving the restored default unusable. Rescaffold both extensions and presets (best-effort) after the fallback default is successfully restored. Regression test: secondary codex install with the git extension, a failing switch to generic, then asserts codex ends up with registered extension artifacts after rollback. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs: explain load-bearing pre-create loop in _reconcile_skills The per-skill _validate_skill_subdir(create=True) loop looks like dead code (its result is unused), but it re-creates the tracked skill subdirectories that _unregister_skills just deleted so _register_skills's only-overwrite-existing gate passes during a historical-directory restore. Removing it fails test_skill_reconciliation_preserves_per_directory_names. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: preserve dashed-description skill tracking Use the shared frontmatter parser when verifying surviving extension skill mirrors so delimiter substrings cannot hide provenance metadata. Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: skip absent extension skills during reconciliation Filter extension-scoped commands before skill reconciliation so historical preset tracking and project overrides cannot recreate artifacts for uninstalled extensions. Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: preserve partial native skill cleanup Coordinate native-skill command cleanup with registered skill coverage per agent and command so partial rescaffolds cannot orphan preset artifacts. Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
@@ -94,6 +94,8 @@ Installs the specified integration into the current project. If another integrat
|
||||
|
||||
Installing an additional integration does not change the default integration. Use `specify integration use <key>` to change the default.
|
||||
|
||||
Installed extensions and presets are not registered for a non-default integration at install time — they follow the currently active (default) integration only. `specify integration use <key>` (or `switch <key>`) is what rescaffolds them for the newly active integration.
|
||||
|
||||
> **Note:** All integration management commands require a project already initialized with `specify init`. To start a new project with a specific agent, use `specify init <project> --integration <key>` instead.
|
||||
|
||||
**Version note:** Controlled multi-install support was introduced in Spec Kit 0.8.5. If `specify integration install <key>` says another integration is already installed and only suggests `switch` or `uninstall`, check your local CLI with `specify version` and upgrade it. Running a one-shot command such as `uvx --from git+https://github.com/github/spec-kit.git specify ...` uses a temporary copy for that command only; it does not update the persistent `specify` executable on your `PATH`.
|
||||
@@ -127,7 +129,7 @@ specify integration switch <key>
|
||||
| `--refresh-shared-infra` | Also overwrite shared infrastructure files even if you customized them (otherwise customizations are preserved) |
|
||||
| `--integration-options` | Options for the target integration when it is not already installed |
|
||||
|
||||
If the target integration is not already installed, equivalent to running `uninstall` followed by `install` in a single step. In this mode, `--force` controls whether modified files from the removed integration are deleted. If the target integration is already installed, `switch` only changes the default integration, like `use`; in this mode, `--force` controls whether managed shared templates are overwritten while the default changes. `--integration-options` is rejected for already-installed targets because changing integration options requires reinstalling managed files; run `upgrade <key> --integration-options ...` first, then `use <key>`.
|
||||
If the target integration is not already installed, equivalent to running `uninstall` followed by `install` in a single step. In this mode, `--force` controls whether modified files from the removed integration are deleted. If the target integration is already installed, `switch` only changes the default integration, like `use`; in this mode, `--force` controls whether managed shared templates are overwritten while the default changes. `--integration-options` is rejected for already-installed targets because changing integration options requires reinstalling managed files; run `upgrade <key> --integration-options ...` first, then `use <key>`. Like `use`, `switch` rescaffolds installed extensions and presets for the target integration once it becomes the default.
|
||||
|
||||
## Use an Installed Integration
|
||||
|
||||
@@ -141,6 +143,8 @@ specify integration use <key>
|
||||
|
||||
Sets the default integration without uninstalling any other installed integrations. This also refreshes managed shared templates so command references match the new default integration's invocation style. Modified or untracked shared templates are preserved unless `--force` is used.
|
||||
|
||||
`use` is also the activation point for installed extensions and presets: it re-registers every enabled extension's and preset's command overrides (and skills, for skills-mode agents) for the newly active integration, so artifacts installed while a different integration was active are rescaffolded here rather than at install time.
|
||||
|
||||
## Upgrade an Integration
|
||||
|
||||
```bash
|
||||
@@ -155,6 +159,10 @@ specify integration upgrade [<key>]
|
||||
|
||||
Reinstalls an installed integration with updated templates and commands (e.g., after upgrading Spec Kit). Defaults to the default integration; if a key is provided, it must be one of the installed integrations. Detects locally modified files and blocks the upgrade unless `--force` is used. Stale files from the previous install that are no longer needed are removed automatically. Shared templates stay aligned with the default integration even when upgrading a non-default integration.
|
||||
|
||||
Enabled extensions and presets are re-registered only when upgrading the currently active (default) integration. A non-default upgrade still refreshes that integration's core commands, but does not re-register its extension or preset layers — `use`/`switch` that integration afterward to rescaffold them.
|
||||
|
||||
If an upgrade would change an integration between command and skills layouts while preset artifacts are registered for it, the upgrade is rejected before changing files. Remove the affected presets, run the layout-changing upgrade, then reinstall them.
|
||||
|
||||
## Report Integration Status
|
||||
|
||||
```bash
|
||||
@@ -303,3 +311,7 @@ CLI-based integrations (like Claude Code, Gemini CLI) require the tool to be ins
|
||||
### When should I use `upgrade` vs `switch`?
|
||||
|
||||
Use `upgrade` when you've upgraded Spec Kit and want to refresh an installed integration's managed files. Use `switch` when you want to replace the current default with another integration; if the target is already installed, `switch` behaves like `use`.
|
||||
|
||||
### Do extensions and presets I install apply to every installed integration?
|
||||
|
||||
No. Extensions (`specify extension add`) and presets (`specify preset add`) register their command overrides for the currently active (default) integration only, even if other integrations are installed. A non-default integration does not receive those artifacts until it becomes the default: `specify integration use <key>` (or `switch <key>`) rescaffolds every enabled extension and preset for the newly active integration. `specify integration upgrade` follows the same rule — it only re-registers extensions and presets when upgrading the active integration.
|
||||
|
||||
@@ -139,7 +139,7 @@ catalogs:
|
||||
|
||||
Presets can provide command files, template files (like `plan-template.md`), and script files. Each file name is evaluated independently against the priority stack, so different files can come from different layers.
|
||||
|
||||
Templates and scripts are looked up from the stack when Spec Kit needs them. Commands use the same stack for replacement and composition, but are materialized into detected agent directories instead of being re-resolved by agents. During preset install, Spec Kit registers command files for the preset being installed; post-install and post-removal reconciliation then recomputes and writes the effective command content for affected command names based on the active stack. Agents do not re-resolve the stack each time they run a command.
|
||||
Templates and scripts are looked up from the stack when Spec Kit needs them. Commands use the same stack for replacement and composition, but are materialized into the active integration's directory only, instead of being re-resolved by agents or written to every detected agent directory (#2948). During preset install, Spec Kit registers command files for the preset being installed against the currently active integration; post-install and post-removal reconciliation then recomputes and writes the effective command content for affected command names based on the active stack. Install and rescaffold remain active-only, but removal may also update previously targeted inactive directories recorded by the removed preset to restore the surviving command or skill layer. A non-active installed integration does not otherwise receive these command files until it becomes the default — `specify integration use <key>` (or `switch <key>`) rescaffolds enabled presets for the newly active integration. Agents do not re-resolve the stack each time they run a command.
|
||||
|
||||
By default, files use a **replace** strategy: the first match in the priority stack wins and is used entirely. Templates and commands can also use composition strategies: **prepend** places preset content before lower-priority content, **append** places it after lower-priority content, and **wrap** replaces `{CORE_TEMPLATE}` with lower-priority content. Scripts support **replace** and **wrap**; script wrappers use `$CORE_SCRIPT` as the placeholder.
|
||||
|
||||
|
||||
@@ -3,12 +3,22 @@
|
||||
import json
|
||||
from collections.abc import Mapping
|
||||
from pathlib import Path
|
||||
from typing import Any
|
||||
from typing import Any, Union
|
||||
|
||||
|
||||
INIT_OPTIONS_FILE = ".specify/init-options.json"
|
||||
|
||||
|
||||
class _MissingInitOptionsFile:
|
||||
"""Sentinel: init-options.json does not exist at all (legacy layout)."""
|
||||
|
||||
def __repr__(self) -> str: # pragma: no cover - debug aid only
|
||||
return "MISSING_INIT_OPTIONS_FILE"
|
||||
|
||||
|
||||
MISSING_INIT_OPTIONS_FILE = _MissingInitOptionsFile()
|
||||
|
||||
|
||||
def save_init_options(project_path: Path, options: dict[str, Any]) -> None:
|
||||
"""Persist the CLI options used during ``specify init``."""
|
||||
dest = project_path / INIT_OPTIONS_FILE
|
||||
@@ -34,3 +44,40 @@ def load_init_options(project_path: Path) -> dict[str, Any]:
|
||||
def is_ai_skills_enabled(opts: Mapping[str, Any] | None) -> bool:
|
||||
"""Return True only when init options explicitly enable AI skills."""
|
||||
return isinstance(opts, Mapping) and opts.get("ai_skills") is True
|
||||
|
||||
|
||||
def resolve_active_agent_for_registration(
|
||||
project_path: Path,
|
||||
) -> Union[str, None, _MissingInitOptionsFile]:
|
||||
"""Resolve the active integration key for active-only registration (#2948).
|
||||
|
||||
``load_init_options`` collapses "no file", "unreadable/malformed file",
|
||||
and "valid file with no recorded active agent" into the same ``{}``
|
||||
result, which previously made corrupted-but-present init-options behave
|
||||
like a legacy pre-init-options project and fall back to registering
|
||||
every detected agent. This helper distinguishes those cases explicitly:
|
||||
|
||||
- Returns :data:`MISSING_INIT_OPTIONS_FILE` when init-options.json does
|
||||
not exist at all (pre-init-options layout or direct library use).
|
||||
Callers should fall back to detection-based registration for all
|
||||
agents, matching the original pre-#2948 behavior for such projects.
|
||||
- Returns ``None`` when init-options.json exists but could not provide a
|
||||
valid non-empty string active agent (malformed/unreadable JSON,
|
||||
non-object payload, or a non-string/empty ``ai`` value). Callers must
|
||||
fail closed (register nothing) rather than treat this like "no file"
|
||||
or pass a non-string key into agent-config lookups.
|
||||
- Returns the active agent key (a non-empty string) otherwise.
|
||||
"""
|
||||
path = project_path / INIT_OPTIONS_FILE
|
||||
# A dangling symlink's target doesn't exist, so Path.exists() (which
|
||||
# follows symlinks) returns False even though the path itself is
|
||||
# present as a broken/corrupted entry. Treat any symlink as "present"
|
||||
# so a dangling one fails closed via the invalid-file branch below
|
||||
# instead of being mistaken for "no file at all" (legacy fallback).
|
||||
if not path.is_symlink() and not path.exists():
|
||||
return MISSING_INIT_OPTIONS_FILE
|
||||
|
||||
active_agent = load_init_options(project_path).get("ai")
|
||||
if isinstance(active_agent, str) and active_agent:
|
||||
return active_agent
|
||||
return None
|
||||
|
||||
@@ -10,7 +10,7 @@ import os
|
||||
import re
|
||||
from copy import deepcopy
|
||||
from pathlib import Path
|
||||
from typing import Any, Dict, List, Optional
|
||||
from typing import Any, Dict, Iterable, List, Optional
|
||||
|
||||
import yaml
|
||||
|
||||
@@ -1020,6 +1020,7 @@ class CommandRegistrar:
|
||||
link_outputs: bool = False,
|
||||
create_missing_active_skills_dir: bool = False,
|
||||
extension_id: Optional[str] = None,
|
||||
only_agent: Optional[str] = None,
|
||||
) -> Dict[str, List[str]]:
|
||||
"""Register commands for all detected agents in the project.
|
||||
|
||||
@@ -1037,6 +1038,8 @@ class CommandRegistrar:
|
||||
skills directory) and is skipped when safe resolution or
|
||||
creation fails.
|
||||
extension_id: Extension id when rendering extension-owned commands.
|
||||
only_agent: If set, restrict registration to this single agent
|
||||
while keeping all detection and recovery safeguards (#2948).
|
||||
|
||||
Returns:
|
||||
Dictionary mapping agent names to list of registered commands
|
||||
@@ -1060,6 +1063,8 @@ class CommandRegistrar:
|
||||
)
|
||||
active_created_skills_dir: Optional[Path] = None
|
||||
for agent_name, agent_config in self.AGENT_CONFIGS.items():
|
||||
if only_agent is not None and agent_name != only_agent:
|
||||
continue
|
||||
active_skills_output = (
|
||||
agent_name == active_skills_agent
|
||||
and agent_config.get("extension") == "/SKILL.md"
|
||||
@@ -1165,6 +1170,8 @@ class CommandRegistrar:
|
||||
context_note: Optional[str] = None,
|
||||
link_outputs: bool = False,
|
||||
extension_id: Optional[str] = None,
|
||||
only_agent: Optional[str] = None,
|
||||
extra_agents: Optional[Iterable[str]] = None,
|
||||
) -> Dict[str, List[str]]:
|
||||
"""Register commands for all non-skill agents in the project.
|
||||
|
||||
@@ -1181,13 +1188,29 @@ class CommandRegistrar:
|
||||
link_outputs: If True, create dev-mode symlinks for rendered
|
||||
command files when supported by the OS.
|
||||
extension_id: Extension id when rendering extension-owned commands.
|
||||
only_agent: If set, restrict registration to this single agent
|
||||
(#2948). An agent name that matches no configured agent
|
||||
(e.g. an empty string) yields no registrations at all.
|
||||
extra_agents: Additional agent names to register for besides
|
||||
``only_agent``. Used by post-removal reconciliation to also
|
||||
restore surviving content into historical agent directories
|
||||
a just-removed preset actually wrote to, not only the
|
||||
currently active agent (#2948). Ignored when ``only_agent``
|
||||
is ``None`` (already unrestricted).
|
||||
|
||||
Returns:
|
||||
Dictionary mapping agent names to list of registered commands
|
||||
"""
|
||||
results = {}
|
||||
self._ensure_configs()
|
||||
extra_agents_set = frozenset(extra_agents) if extra_agents else frozenset()
|
||||
for agent_name, agent_config in self.AGENT_CONFIGS.items():
|
||||
if (
|
||||
only_agent is not None
|
||||
and agent_name != only_agent
|
||||
and agent_name not in extra_agents_set
|
||||
):
|
||||
continue
|
||||
if agent_config.get("extension") == "/SKILL.md":
|
||||
continue
|
||||
detect_dir_str = agent_config.get("detect_dir")
|
||||
|
||||
@@ -1071,6 +1071,92 @@ class ExtensionManager:
|
||||
return _ensure_usable(agent_skills_dir)
|
||||
return _ensure_usable(skills_dir)
|
||||
|
||||
def _register_commands_for_active_agent(
|
||||
self,
|
||||
manifest: ExtensionManifest,
|
||||
extension_dir: Path,
|
||||
link_outputs: bool = False,
|
||||
) -> Dict[str, List[str]]:
|
||||
"""Register extension commands for the active integration only.
|
||||
|
||||
Maintainer-requested behavior for #2948: ``extension add`` treats the
|
||||
project as single-active — only the integration recorded in
|
||||
init-options gets command files. Non-active integrations receive them
|
||||
when selected via ``integration use`` / ``switch`` (rescaffold).
|
||||
|
||||
Projects without a recorded active integration at all (pre-init-options
|
||||
layouts or direct library use, i.e. init-options.json does not
|
||||
exist) fall back to detection-based registration for all agents. A
|
||||
*recorded* active key that has no registrar config (e.g. ``generic``,
|
||||
which is deliberately excluded from ``AGENT_CONFIGS``) is not treated
|
||||
as "no active integration" — it must not cause registration to
|
||||
target other detected agents.
|
||||
|
||||
An init-options.json that exists but is corrupted, unreadable, or
|
||||
has a malformed/empty ``ai`` value (e.g. ``[]`` or ``null``) is also
|
||||
not "no active integration" — fail closed (register nothing) rather
|
||||
than fall back to registering every detected agent, which would
|
||||
otherwise happen because a corrupted file loads the same as an
|
||||
absent one.
|
||||
|
||||
Returns:
|
||||
Mapping of agent name to registered command names, matching the
|
||||
``registered_commands`` registry shape.
|
||||
"""
|
||||
from .. import load_init_options
|
||||
from .._init_options import (
|
||||
MISSING_INIT_OPTIONS_FILE,
|
||||
resolve_active_agent_for_registration,
|
||||
)
|
||||
|
||||
registrar = CommandRegistrar()
|
||||
active_agent = resolve_active_agent_for_registration(self.project_root)
|
||||
|
||||
if active_agent is MISSING_INIT_OPTIONS_FILE:
|
||||
return registrar.register_commands_for_all_agents(
|
||||
manifest,
|
||||
extension_dir,
|
||||
self.project_root,
|
||||
link_outputs=link_outputs,
|
||||
create_missing_active_skills_dir=True,
|
||||
)
|
||||
|
||||
if active_agent is None:
|
||||
# init-options.json exists but could not provide a valid active
|
||||
# agent (corrupted/unreadable/non-object JSON, or a malformed
|
||||
# "ai" value). Fail closed instead of falling back to all agents
|
||||
# or passing a non-string key into AGENT_CONFIGS.get() below,
|
||||
# which would raise TypeError for unhashable values like a list.
|
||||
return {}
|
||||
|
||||
init_options = load_init_options(self.project_root)
|
||||
|
||||
# A recorded active key with no registrar config (e.g. "generic",
|
||||
# deliberately excluded from AGENT_CONFIGS) has nothing to register
|
||||
# through this path, but it is still an active integration. Passing
|
||||
# it as only_agent below naturally yields no matches instead of
|
||||
# falling back to registering every detected agent.
|
||||
agent_config = registrar.AGENT_CONFIGS.get(active_agent)
|
||||
if (
|
||||
agent_config
|
||||
and is_ai_skills_enabled(init_options)
|
||||
and agent_config.get("extension") != "/SKILL.md"
|
||||
):
|
||||
# Active agent runs skills mode: extension artifacts render as
|
||||
# skills via _register_extension_skills, not as command files.
|
||||
return {}
|
||||
|
||||
# Route through the all-agents pass restricted to the active agent so
|
||||
# detection and missing-skills-dir recovery safeguards still apply.
|
||||
return registrar.register_commands_for_all_agents(
|
||||
manifest,
|
||||
extension_dir,
|
||||
self.project_root,
|
||||
link_outputs=link_outputs,
|
||||
create_missing_active_skills_dir=True,
|
||||
only_agent=active_agent,
|
||||
)
|
||||
|
||||
def _register_extension_skills(
|
||||
self,
|
||||
manifest: ExtensionManifest,
|
||||
@@ -1271,6 +1357,50 @@ class ExtensionManager:
|
||||
except OSError:
|
||||
return False
|
||||
|
||||
def _extension_skill_trusted_root(self, candidate: Path) -> Optional[Path]:
|
||||
"""Return the project or home root allowed to contain *candidate*."""
|
||||
candidate = Path(os.path.abspath(candidate))
|
||||
for root in (
|
||||
Path(os.path.abspath(self.project_root)),
|
||||
Path(os.path.abspath(Path.home())),
|
||||
):
|
||||
if candidate.is_relative_to(root):
|
||||
return root
|
||||
return None
|
||||
|
||||
def _extension_skill_candidate_dirs(self) -> Dict[Path, Path]:
|
||||
"""Return every configured skill output and its trusted root."""
|
||||
from .. import AGENT_CONFIG, DEFAULT_SKILLS_DIR
|
||||
from ..agents import CommandRegistrar
|
||||
|
||||
candidates: Dict[Path, Path] = {}
|
||||
|
||||
def add_candidate(candidate: Path) -> None:
|
||||
candidate = Path(os.path.abspath(candidate))
|
||||
trusted_root = self._extension_skill_trusted_root(candidate)
|
||||
if trusted_root is not None:
|
||||
candidates[candidate] = trusted_root
|
||||
|
||||
for cfg in AGENT_CONFIG.values():
|
||||
folder = cfg.get("folder", "")
|
||||
if folder:
|
||||
add_candidate(
|
||||
self.project_root / folder.rstrip("/") / "skills"
|
||||
)
|
||||
add_candidate(self.project_root / DEFAULT_SKILLS_DIR)
|
||||
|
||||
registrar = CommandRegistrar()
|
||||
for agent_name, agent_config in registrar.AGENT_CONFIGS.items():
|
||||
if agent_config.get("extension") != "/SKILL.md":
|
||||
continue
|
||||
add_candidate(
|
||||
registrar._resolve_agent_dir(
|
||||
agent_name, agent_config, self.project_root
|
||||
)
|
||||
)
|
||||
|
||||
return candidates
|
||||
|
||||
def _unregister_extension_skills(
|
||||
self,
|
||||
skill_names: List[str],
|
||||
@@ -1282,31 +1412,46 @@ class ExtensionManager:
|
||||
Called during extension removal to clean up skill files that
|
||||
were created by ``_register_extension_skills()``.
|
||||
|
||||
If *skills_dir* is not provided and ``_get_skills_dir()`` returns
|
||||
``None`` (e.g. the user removed init-options.json or toggled
|
||||
ai_skills after installation), we fall back to scanning all known
|
||||
agent skills directories so that orphaned skill directories are
|
||||
still cleaned up. In that case each candidate directory is
|
||||
verified against the SKILL.md ``metadata.source`` field before
|
||||
removal to avoid accidentally deleting user-created skills with
|
||||
the same name.
|
||||
When *skills_dir* is omitted, project-local agent skill directories
|
||||
are scanned. Home-scoped outputs require explicit agent provenance:
|
||||
the legacy flat registry and ``metadata.source`` marker do not
|
||||
identify which project created a global skill.
|
||||
|
||||
Args:
|
||||
skill_names: List of skill names to remove.
|
||||
extension_id: Extension ID used to verify ownership during
|
||||
fallback candidate scanning.
|
||||
skills_dir: Optional explicit skills directory to use instead
|
||||
of resolving via ``_get_skills_dir()``. Useful when the
|
||||
caller needs to target a specific agent's skills directory
|
||||
regardless of the currently-active agent in init-options.
|
||||
skills_dir: Optional explicit skills directory to scope
|
||||
cleanup to. Useful when the caller needs to target a
|
||||
specific agent's skills directory regardless of the
|
||||
currently-active agent in init-options. When omitted,
|
||||
every configured agent's skills directory is scanned
|
||||
instead of resolving just the currently active one.
|
||||
"""
|
||||
if not skill_names:
|
||||
return
|
||||
|
||||
if skills_dir is None:
|
||||
skills_dir = self._get_skills_dir()
|
||||
from ..shared_infra import _validate_safe_shared_directory
|
||||
|
||||
if skills_dir:
|
||||
# Reject the candidate directory itself (any path component,
|
||||
# including the final one) if it's a symlink escaping the
|
||||
# trusted project/home root, before probing or deleting anything
|
||||
# inside it.
|
||||
# A caller-supplied skills_dir (e.g. a specific agent's
|
||||
# directory resolved without side effects) could have been
|
||||
# replaced with a symlink between registration and removal;
|
||||
# resolving it and only checking children relative to the
|
||||
# already-resolved candidate (the previous approach) would
|
||||
# silently follow the symlink instead of rejecting it.
|
||||
trusted_root = self._extension_skill_trusted_root(skills_dir)
|
||||
if trusted_root is None:
|
||||
return
|
||||
try:
|
||||
_validate_safe_shared_directory(trusted_root, skills_dir)
|
||||
except (ValueError, OSError):
|
||||
return
|
||||
|
||||
# Fast path: we know the exact skills directory
|
||||
for skill_name in skill_names:
|
||||
# Guard against path traversal from a corrupted registry entry:
|
||||
@@ -1315,10 +1460,19 @@ class ExtensionManager:
|
||||
sn_path = Path(skill_name)
|
||||
if sn_path.is_absolute() or len(sn_path.parts) != 1:
|
||||
continue
|
||||
skill_subdir = skills_dir / skill_name
|
||||
# Validate every path component down to the skill's own
|
||||
# subdirectory, not just the already-validated parent
|
||||
# skills_dir: a per-skill child can itself be a symlink to
|
||||
# another directory whose *resolved* target still lands
|
||||
# inside this same (safe) skills root, which the previous
|
||||
# resolve()+relative_to() containment check alone would
|
||||
# not catch. Reject the symlink outright rather than
|
||||
# following it, even when the target is otherwise
|
||||
# in-bounds (#2948).
|
||||
try:
|
||||
skill_subdir = (skills_dir / skill_name).resolve()
|
||||
skill_subdir.relative_to(skills_dir.resolve()) # raises if outside
|
||||
except (OSError, ValueError):
|
||||
_validate_safe_shared_directory(trusted_root, skill_subdir)
|
||||
except (ValueError, OSError):
|
||||
continue
|
||||
if not skill_subdir.is_dir():
|
||||
continue
|
||||
@@ -1351,31 +1505,48 @@ class ExtensionManager:
|
||||
shutil.rmtree(skill_subdir)
|
||||
else:
|
||||
# Fallback: scan all possible agent skills directories
|
||||
from .. import AGENT_CONFIG, DEFAULT_SKILLS_DIR
|
||||
|
||||
candidate_dirs: set[Path] = set()
|
||||
for cfg in AGENT_CONFIG.values():
|
||||
folder = cfg.get("folder", "")
|
||||
if folder:
|
||||
candidate_dirs.add(
|
||||
self.project_root / folder.rstrip("/") / "skills"
|
||||
)
|
||||
candidate_dirs.add(self.project_root / DEFAULT_SKILLS_DIR)
|
||||
|
||||
for skills_candidate in candidate_dirs:
|
||||
for (
|
||||
skills_candidate,
|
||||
trusted_root,
|
||||
) in self._extension_skill_candidate_dirs().items():
|
||||
# Only project-local skills directories are eligible: the
|
||||
# flat (non-agent-scoped) registered_skills provenance
|
||||
# cannot prove a home-directory skill belongs to this
|
||||
# project, so deleting there could remove another
|
||||
# project's files. Revisit if registry entries ever record
|
||||
# the owning project/agent.
|
||||
if trusted_root != Path(os.path.abspath(self.project_root)):
|
||||
continue
|
||||
if not skills_candidate.is_dir():
|
||||
continue
|
||||
# Reject the candidate directory itself (any path
|
||||
# component) if it's a symlink escaping the project
|
||||
# root, before probing or deleting anything inside it —
|
||||
# same guard as the fast path above.
|
||||
try:
|
||||
_validate_safe_shared_directory(
|
||||
trusted_root, skills_candidate
|
||||
)
|
||||
except (ValueError, OSError):
|
||||
continue
|
||||
for skill_name in skill_names:
|
||||
# Same path-traversal guard as the fast path above
|
||||
sn_path = Path(skill_name)
|
||||
if sn_path.is_absolute() or len(sn_path.parts) != 1:
|
||||
continue
|
||||
skill_subdir = skills_candidate / skill_name
|
||||
# Validate every path component down to the skill's
|
||||
# own subdirectory, not just the already-validated
|
||||
# candidate parent: a per-skill child can itself be a
|
||||
# symlink to another directory whose resolved target
|
||||
# still lands inside this same candidate, which the
|
||||
# previous resolve()+relative_to() containment check
|
||||
# alone would not catch (#2948).
|
||||
try:
|
||||
skill_subdir = (skills_candidate / skill_name).resolve()
|
||||
skill_subdir.relative_to(
|
||||
skills_candidate.resolve()
|
||||
) # raises if outside
|
||||
except (OSError, ValueError):
|
||||
_validate_safe_shared_directory(
|
||||
trusted_root, skill_subdir
|
||||
)
|
||||
except (ValueError, OSError):
|
||||
continue
|
||||
if not skill_subdir.is_dir():
|
||||
continue
|
||||
@@ -1409,6 +1580,99 @@ class ExtensionManager:
|
||||
continue
|
||||
shutil.rmtree(skill_subdir)
|
||||
|
||||
def _extension_owned_skill_names(
|
||||
self, skill_names: List[str], extension_id: str
|
||||
) -> List[str]:
|
||||
"""Return the subset of *skill_names* still marker-verified anywhere.
|
||||
|
||||
``registered_skills`` is a single flat list shared across every
|
||||
agent this extension has ever been activated under (skills are
|
||||
only ever rendered for the currently active agent, so there is no
|
||||
per-agent registry key to consult). A name can therefore still be
|
||||
globally owned by this extension even after it's removed from one
|
||||
particular agent's directory, if an earlier activation under a
|
||||
*different* agent left its own marker-verified mirror behind.
|
||||
|
||||
This scans the same candidate directories (every configured
|
||||
agent's skills folder, deduped by shared path, plus the default
|
||||
skills directory) as the fallback branch of
|
||||
:meth:`_unregister_extension_skills`, but read-only: no directory
|
||||
is created and a name is only kept if at least one candidate
|
||||
directory contains a ``SKILL.md`` whose ``metadata.source`` field
|
||||
matches this exact extension (the same ownership marker
|
||||
:meth:`_register_extension_skills` writes), so an unrelated
|
||||
directory or user-created skill of the same name can't cause a
|
||||
false positive. Symlink/containment safety mirrors the existing
|
||||
fallback scan: each candidate path is resolved and the resulting
|
||||
skill subdirectory is required to stay within it before any file
|
||||
is read.
|
||||
"""
|
||||
if not skill_names:
|
||||
return []
|
||||
|
||||
from ..shared_infra import _validate_safe_shared_directory
|
||||
|
||||
marker = f"extension:{extension_id}"
|
||||
owned: set = set()
|
||||
for (
|
||||
skills_candidate,
|
||||
trusted_root,
|
||||
) in self._extension_skill_candidate_dirs().items():
|
||||
if len(owned) == len(skill_names):
|
||||
break # every name already confirmed owned somewhere
|
||||
if not skills_candidate.is_dir():
|
||||
continue
|
||||
# Reject the candidate directory itself (any path component)
|
||||
# if it's a symlink escaping the project root, before probing
|
||||
# anything inside it. Resolving it and only checking children
|
||||
# relative to the already-resolved candidate (the previous
|
||||
# approach) would silently follow the symlink instead of
|
||||
# rejecting it, letting a marker-matching SKILL.md outside the
|
||||
# project be falsely attributed.
|
||||
try:
|
||||
_validate_safe_shared_directory(trusted_root, skills_candidate)
|
||||
except (ValueError, OSError):
|
||||
continue
|
||||
for skill_name in skill_names:
|
||||
if skill_name in owned:
|
||||
continue
|
||||
sn_path = Path(skill_name)
|
||||
if sn_path.is_absolute() or len(sn_path.parts) != 1:
|
||||
continue
|
||||
skill_subdir = skills_candidate / skill_name
|
||||
# Validate every path component down to the skill's own
|
||||
# subdirectory, not just the already-validated candidate
|
||||
# parent: a per-skill child can itself be a symlink to
|
||||
# another directory whose resolved target still lands
|
||||
# inside this same candidate, which the previous
|
||||
# resolve()+relative_to() containment check alone would
|
||||
# not catch (#2948).
|
||||
try:
|
||||
_validate_safe_shared_directory(trusted_root, skill_subdir)
|
||||
except (ValueError, OSError):
|
||||
continue
|
||||
if not skill_subdir.is_dir():
|
||||
continue
|
||||
skill_md = skill_subdir / "SKILL.md"
|
||||
if not skill_md.is_file():
|
||||
continue
|
||||
try:
|
||||
from ..agents import CommandRegistrar as _Registrar
|
||||
|
||||
raw = skill_md.read_text(encoding="utf-8")
|
||||
fm, _ = _Registrar.parse_frontmatter(raw)
|
||||
source = (
|
||||
fm.get("metadata", {}).get("source", "")
|
||||
if isinstance(fm, dict)
|
||||
else ""
|
||||
)
|
||||
except (OSError, UnicodeDecodeError, Exception):
|
||||
continue
|
||||
if source == marker:
|
||||
owned.add(skill_name)
|
||||
|
||||
return [name for name in skill_names if name in owned]
|
||||
|
||||
def check_compatibility(
|
||||
self, manifest: ExtensionManifest, speckit_version: str
|
||||
) -> bool:
|
||||
@@ -1942,17 +2206,11 @@ class ExtensionManager:
|
||||
# the restored user config with packaged defaults. Cleanup is deferred
|
||||
# until after registry.add() succeeds (see post-commit cleanup below).
|
||||
|
||||
# Register commands with AI agents
|
||||
# Register commands with AI agents (active integration only, #2948)
|
||||
registered_commands = {}
|
||||
if register_commands:
|
||||
registrar = CommandRegistrar()
|
||||
# Register for all detected agents
|
||||
registered_commands = registrar.register_commands_for_all_agents(
|
||||
manifest,
|
||||
dest_dir,
|
||||
self.project_root,
|
||||
link_outputs=link_commands,
|
||||
create_missing_active_skills_dir=True,
|
||||
registered_commands = self._register_commands_for_active_agent(
|
||||
manifest, dest_dir, link_outputs=link_commands
|
||||
)
|
||||
|
||||
# Auto-register extension commands as agent skills when skills mode
|
||||
@@ -2263,31 +2521,51 @@ class ExtensionManager:
|
||||
metadata.get("registered_skills", [])
|
||||
)
|
||||
if registered_skills and not commands_only:
|
||||
# Only pass the resolved skills_dir when it actually exists.
|
||||
# Otherwise let _unregister_extension_skills fall back to
|
||||
# scanning all known agent skills directories, which is useful
|
||||
# for cleaning up stale entries created by earlier installs.
|
||||
skills_dir = agent_skills_dir if agent_skills_dir.is_dir() else None
|
||||
# Always pass the explicit, agent-scoped skills_dir — even
|
||||
# when it doesn't currently exist on disk. This method must
|
||||
# stay scoped to *this* agent only; omitting skills_dir (a
|
||||
# bare ``None``) tells _unregister_extension_skills "this is
|
||||
# a genuinely unscoped removal", which triggers its
|
||||
# all-configured-agents fallback scan — reserved for
|
||||
# ExtensionManager.remove()'s full project cleanup. If this
|
||||
# agent's directory doesn't exist, there is nothing under it
|
||||
# to clean up; the fast path below is a safe no-op in that
|
||||
# case (every candidate skill_subdir.is_dir() check fails).
|
||||
self._unregister_extension_skills(
|
||||
registered_skills, ext_id, skills_dir=skills_dir
|
||||
registered_skills, ext_id, skills_dir=agent_skills_dir
|
||||
)
|
||||
|
||||
# Only reconcile registry state when cleanup was scoped to a
|
||||
# specific existing directory. When skills_dir is None,
|
||||
# _unregister_extension_skills falls back to scanning multiple
|
||||
# candidate directories, so agent_skills_dir cannot be used to
|
||||
# infer what was removed. When skills_dir is set,
|
||||
# _unregister_extension_skills may intentionally skip deletion
|
||||
# when ownership cannot be verified (e.g., corrupted/missing
|
||||
# SKILL.md or mismatching metadata.source). Only drop registry
|
||||
# entries for skill directories that were actually removed so
|
||||
# future cleanup attempts can still find skipped ones.
|
||||
if skills_dir is not None:
|
||||
remaining_skills = [
|
||||
skill_name
|
||||
for skill_name in registered_skills
|
||||
if (skills_dir / skill_name).is_dir()
|
||||
]
|
||||
# Only reconcile registry state when this agent's directory
|
||||
# actually exists. When it's absent, this agent never had
|
||||
# any of these skills mirrored under its own directory in
|
||||
# the first place, so there is nothing to conclude about
|
||||
# global ``registered_skills`` tracking from that absence —
|
||||
# other agents' directories may still legitimately hold
|
||||
# live mirrors for these same names (the flat list is
|
||||
# agent-agnostic). Recomputing "remaining" against an
|
||||
# absent directory would incorrectly conclude every name
|
||||
# was removed and drop them all from the registry, silently
|
||||
# orphaning any still-live mirrors under other agents'
|
||||
# directories from future cleanup/removal.
|
||||
#
|
||||
# When the directory does exist, _unregister_extension_skills
|
||||
# may intentionally skip deletion when ownership cannot be
|
||||
# verified (e.g., corrupted/missing SKILL.md or mismatching
|
||||
# metadata.source). A name no longer present under *this*
|
||||
# agent's directory isn't necessarily gone everywhere either
|
||||
# — registered_skills is a single flat list shared across
|
||||
# every agent this extension was ever activated under, so
|
||||
# an earlier activation under a different, still-active
|
||||
# agent may have left its own marker-verified mirror behind.
|
||||
# Recompute across every safe, supported skills directory
|
||||
# (the same helper used for the analogous toggle-cleanup
|
||||
# case) rather than just this one, or a still-existing
|
||||
# mirror elsewhere would be silently dropped from tracking
|
||||
# and orphaned on later removal (#2948).
|
||||
if agent_skills_dir.is_dir():
|
||||
remaining_skills = self._extension_owned_skill_names(
|
||||
registered_skills, ext_id
|
||||
)
|
||||
if remaining_skills != registered_skills:
|
||||
updates["registered_skills"] = remaining_skills
|
||||
|
||||
@@ -2298,11 +2576,10 @@ class ExtensionManager:
|
||||
"""Register installed, enabled extensions for ``agent_name``.
|
||||
|
||||
Command-file registration is scoped to the explicit ``agent_name``
|
||||
argument, so this method can be used after install, upgrade, or switch.
|
||||
Extension skill rendering is still scoped to the active ``ai`` /
|
||||
``ai_skills`` settings in init-options, so non-active skills-mode
|
||||
targets receive command files here. Per-agent skills parity is tracked
|
||||
separately in #2948.
|
||||
argument. Since #2948, callers pass the active agent only (``use`` /
|
||||
``switch`` activate the target first; ``upgrade`` calls it only for
|
||||
the active integration), so extension skill rendering — scoped to the
|
||||
active ``ai`` / ``ai_skills`` init-options — matches ``agent_name``.
|
||||
"""
|
||||
if not agent_name:
|
||||
return
|
||||
@@ -2323,6 +2600,22 @@ class ExtensionManager:
|
||||
and bool(agent_config)
|
||||
and agent_config.get("extension") != "/SKILL.md"
|
||||
)
|
||||
# Mirror image of skills_mode_active: this agent is command-backed,
|
||||
# active, and currently in command mode. Used to detect a
|
||||
# skills -> command toggle for this same agent, where the skills
|
||||
# phase below returns empty (its directory no longer resolves) but
|
||||
# a previously-written extension SKILL.md is now stale (#2948).
|
||||
command_mode_active = (
|
||||
active_agent == agent_name
|
||||
and not ai_skills_enabled
|
||||
and bool(agent_config)
|
||||
and agent_config.get("extension") != "/SKILL.md"
|
||||
)
|
||||
agent_skills_dir = None
|
||||
if agent_config and agent_config.get("extension") != "/SKILL.md":
|
||||
from .. import _get_skills_dir as _resolve_agent_skills_dir
|
||||
|
||||
agent_skills_dir = _resolve_agent_skills_dir(self.project_root, agent_name)
|
||||
|
||||
for ext_id, metadata in self.registry.list().items():
|
||||
if not metadata.get("enabled", True):
|
||||
@@ -2339,6 +2632,10 @@ class ExtensionManager:
|
||||
# registration of the remaining enabled extensions for this agent.
|
||||
try:
|
||||
updates: Dict[str, Any] = {}
|
||||
# Set when a command -> skills toggle for this same agent
|
||||
# defers stale command-mode cleanup until the skills
|
||||
# replacement below confirms success (#2948).
|
||||
deferred_stale_commands: Optional[List[str]] = None
|
||||
|
||||
if agent_config and not skills_mode_active:
|
||||
registered = registrar.register_commands_for_agent(
|
||||
@@ -2358,17 +2655,38 @@ class ExtensionManager:
|
||||
new_registered.pop(agent_name, None)
|
||||
if new_registered != registered_commands:
|
||||
updates["registered_commands"] = new_registered
|
||||
elif agent_config and skills_mode_active:
|
||||
# Toggled command -> skills for this same agent: the
|
||||
# commands phase above is skipped. A command file this
|
||||
# extension previously wrote for this agent while
|
||||
# command mode was active is still on disk and still
|
||||
# tracked, but it must NOT be removed yet — the skills
|
||||
# phase below is an independently fallible replacement
|
||||
# step, and deleting the old artifact before it
|
||||
# succeeds would leave neither the old command file nor
|
||||
# a new skill file if skills registration raises. The
|
||||
# actual removal is deferred until after the skills
|
||||
# phase below completes without raising (#2948).
|
||||
registered_commands = metadata.get("registered_commands", {})
|
||||
if isinstance(registered_commands, dict) and registered_commands.get(
|
||||
agent_name
|
||||
):
|
||||
deferred_stale_commands = self._valid_name_list(
|
||||
registered_commands.get(agent_name)
|
||||
)
|
||||
else:
|
||||
deferred_stale_commands = None
|
||||
else:
|
||||
deferred_stale_commands = None
|
||||
|
||||
# Extension *skills* are only ever rendered for the active agent:
|
||||
# `_register_extension_skills` resolves the skills dir and
|
||||
# frontmatter from init-options["ai"], ignoring ``agent_name``.
|
||||
# When this method runs for a non-active agent — as install/upgrade
|
||||
# now do for a secondary integration (#2886) — the skills pass would
|
||||
# re-render the *active* agent's extension skills as a side effect,
|
||||
# Running the skills pass for a non-active agent would re-render
|
||||
# the *active* agent's extension skills as a side effect,
|
||||
# resurrecting skill files the user deliberately deleted. Skip it
|
||||
# unless the target is the active agent; `switch` is unaffected
|
||||
# because it activates the target before registering. (Rendering
|
||||
# skills for a non-active target is tracked separately in #2948.)
|
||||
# unless the target is the active agent (defense in depth: since
|
||||
# #2948 callers only pass the active agent anyway).
|
||||
if agent_name == active_agent:
|
||||
try:
|
||||
registered_skills = self._register_extension_skills(
|
||||
@@ -2399,6 +2717,140 @@ class ExtensionManager:
|
||||
dict.fromkeys(existing_skills + registered_skills)
|
||||
)
|
||||
updates["registered_skills"] = merged_skills
|
||||
elif command_mode_active and agent_skills_dir is not None:
|
||||
# Mirror image: toggled skills -> command for
|
||||
# this same agent. _register_extension_skills
|
||||
# returned empty because this agent's skills
|
||||
# directory no longer resolves once ai_skills is
|
||||
# off, but a SKILL.md this extension wrote while
|
||||
# skills mode was active may still be tracked
|
||||
# and still on disk. Remove it narrowly for this
|
||||
# agent's directory only (#2948).
|
||||
existing_skills = self._valid_name_list(
|
||||
metadata.get("registered_skills", [])
|
||||
)
|
||||
owned_here = [
|
||||
name
|
||||
for name in existing_skills
|
||||
if (agent_skills_dir / name).is_dir()
|
||||
]
|
||||
# Only retire a skill mirror when the
|
||||
# replacement command for the same logical
|
||||
# command was actually written this call —
|
||||
# `registered` (from register_commands_for_agent
|
||||
# above) may be empty or a partial subset
|
||||
# (missing source file, safety rejection,
|
||||
# corrupted manifest), and removing a skill
|
||||
# mirror whose command replacement never
|
||||
# landed would leave neither artifact (#2948).
|
||||
replaced_skill_names = {
|
||||
HookExecutor._skill_name_from_command(cmd_name)
|
||||
for cmd_name in (registered or [])
|
||||
}
|
||||
to_remove = [
|
||||
name for name in owned_here
|
||||
if name in replaced_skill_names
|
||||
]
|
||||
if to_remove:
|
||||
self._unregister_extension_skills(
|
||||
to_remove, ext_id, skills_dir=agent_skills_dir
|
||||
)
|
||||
# registered_skills is a single flat list
|
||||
# shared across every agent this extension
|
||||
# was ever activated under (unlike presets'
|
||||
# per-agent dict), so a name removed from
|
||||
# *this* agent's directory may still have a
|
||||
# marker-verified mirror under a different,
|
||||
# previously-active agent's directory.
|
||||
# Recompute across every safe, supported
|
||||
# skills directory rather than just this
|
||||
# one, or a still-existing mirror elsewhere
|
||||
# would be silently dropped from tracking
|
||||
# and orphaned on later removal (#2948).
|
||||
remaining = self._extension_owned_skill_names(
|
||||
existing_skills, ext_id
|
||||
)
|
||||
if remaining != existing_skills:
|
||||
updates["registered_skills"] = remaining
|
||||
|
||||
# The skills phase above completed without raising
|
||||
# (this ``else:`` is only reached on success), so a
|
||||
# deferred command -> skills toggle cleanup queued
|
||||
# above is now safe to apply: the replacement skill
|
||||
# registration is confirmed, so the stale
|
||||
# command-mode artifact can finally be removed
|
||||
# without risking a transient state where neither
|
||||
# artifact exists. Only retire a stale command
|
||||
# whose corresponding skill was actually returned
|
||||
# this call — `registered_skills` may be empty or
|
||||
# a partial subset (missing source file, safety
|
||||
# rejection, corrupted manifest), and unregistering
|
||||
# a command whose skill replacement never landed
|
||||
# would leave neither artifact (#2948).
|
||||
if deferred_stale_commands:
|
||||
replaced_skill_names = set(registered_skills or [])
|
||||
# Commands may carry aliases (CommandRegistrar.
|
||||
# register_commands_for_agent() tracks and
|
||||
# returns primary + alias names flattened
|
||||
# together into one list), but
|
||||
# _register_extension_skills() only ever
|
||||
# renders/returns the *primary* command name's
|
||||
# skill — running an alias's own name through
|
||||
# _skill_name_from_command() never matches
|
||||
# anything real, so an alias would stay
|
||||
# tracked/on-disk forever even after its
|
||||
# primary's skill replacement landed. Map each
|
||||
# stale name back to its manifest command's
|
||||
# primary so the whole primary+alias group is
|
||||
# retired or kept together, based solely on
|
||||
# whether the *primary*'s skill replacement
|
||||
# actually landed (#2948).
|
||||
alias_to_primary: Dict[str, str] = {}
|
||||
for cmd_info in manifest.commands:
|
||||
primary_name = cmd_info.get("name")
|
||||
if not isinstance(primary_name, str):
|
||||
continue
|
||||
for alias in cmd_info.get("aliases", []) or []:
|
||||
if isinstance(alias, str):
|
||||
alias_to_primary[alias] = primary_name
|
||||
|
||||
group_fully_replaced: Dict[str, bool] = {}
|
||||
for cmd_name in deferred_stale_commands:
|
||||
primary_name = alias_to_primary.get(cmd_name, cmd_name)
|
||||
if primary_name in group_fully_replaced:
|
||||
continue
|
||||
group_fully_replaced[primary_name] = (
|
||||
HookExecutor._skill_name_from_command(primary_name)
|
||||
in replaced_skill_names
|
||||
)
|
||||
|
||||
fully_replaced = [
|
||||
cmd_name for cmd_name in deferred_stale_commands
|
||||
if group_fully_replaced.get(
|
||||
alias_to_primary.get(cmd_name, cmd_name), False
|
||||
)
|
||||
]
|
||||
if fully_replaced:
|
||||
registrar.unregister_commands(
|
||||
{agent_name: fully_replaced}, self.project_root
|
||||
)
|
||||
registered_commands = metadata.get(
|
||||
"registered_commands", {}
|
||||
)
|
||||
if isinstance(registered_commands, dict) and (
|
||||
registered_commands.get(agent_name)
|
||||
):
|
||||
new_registered = copy.deepcopy(registered_commands)
|
||||
remaining_commands = [
|
||||
c for c in new_registered[agent_name]
|
||||
if c not in fully_replaced
|
||||
]
|
||||
if remaining_commands:
|
||||
new_registered[agent_name] = remaining_commands
|
||||
else:
|
||||
new_registered.pop(agent_name, None)
|
||||
if new_registered != registered_commands:
|
||||
updates["registered_commands"] = new_registered
|
||||
|
||||
if updates:
|
||||
self.registry.update(ext_id, updates)
|
||||
@@ -2567,6 +3019,7 @@ class CommandRegistrar:
|
||||
project_root: Path,
|
||||
link_outputs: bool = False,
|
||||
create_missing_active_skills_dir: bool = False,
|
||||
only_agent: Optional[str] = None,
|
||||
) -> Dict[str, List[str]]:
|
||||
"""Register extension commands for all detected agents."""
|
||||
context_note = f"\n<!-- Extension: {manifest.id} -->\n<!-- Config: .specify/extensions/{manifest.id}/ -->\n"
|
||||
@@ -2578,6 +3031,7 @@ class CommandRegistrar:
|
||||
context_note=context_note,
|
||||
link_outputs=link_outputs,
|
||||
create_missing_active_skills_dir=create_missing_active_skills_dir,
|
||||
only_agent=only_agent,
|
||||
extension_id=manifest.id,
|
||||
)
|
||||
|
||||
|
||||
@@ -395,19 +395,14 @@ def _register_extensions_for_agent(
|
||||
"""Register all enabled extensions' commands/skills for ``agent_key``.
|
||||
|
||||
``use`` / ``switch`` re-register enabled extensions for the agent they
|
||||
activate; ``upgrade`` backfills them for the refreshed agent. Plain
|
||||
``install`` deliberately does not call this helper so adding a secondary
|
||||
integration has no extension side effects until it is selected or upgraded.
|
||||
See issue #2886.
|
||||
activate (rescaffold); ``upgrade`` does so only for the *active*
|
||||
integration. Plain ``install`` and upgrade of a non-active integration
|
||||
deliberately skip this helper so a secondary integration has no extension
|
||||
side effects until it is selected. See issues #2886 and #2948.
|
||||
|
||||
Known limitation: extension *skill* rendering is scoped to the active
|
||||
agent (init-options track a single ``ai`` / ``ai_skills`` pair). A
|
||||
skills-mode agent registered while it is *not* the active agent (e.g.
|
||||
Copilot ``--skills`` registered while non-active) therefore
|
||||
receives command files rather than skills here — matching ``extension
|
||||
add``'s multi-agent behavior. ``use`` / ``switch`` avoid this because they
|
||||
make the target the active agent first. Per-agent skills parity is tracked in
|
||||
#2948.
|
||||
Callers always pass the active agent (use/switch activate the target
|
||||
before registering), so extension *skill* rendering — which is scoped to
|
||||
the active ``ai`` / ``ai_skills`` init-options — matches ``agent_key``.
|
||||
|
||||
Best-effort: never aborts the surrounding integration operation. Callers
|
||||
invoke it *after* the use/upgrade/switch transaction has committed so a
|
||||
@@ -443,6 +438,71 @@ def _unregister_extensions_for_agent(
|
||||
)
|
||||
|
||||
|
||||
def _register_presets_for_agent(
|
||||
project_root: Path,
|
||||
agent_key: str,
|
||||
*,
|
||||
continuing: str,
|
||||
) -> None:
|
||||
"""Register all enabled presets' command overrides/skills for ``agent_key``.
|
||||
|
||||
Presets follow the same single-active rule as extensions (#2948):
|
||||
``use`` / ``switch`` re-register enabled presets for the agent they
|
||||
activate (rescaffold), so a preset installed while a different
|
||||
integration was active is not left targeting that inactive integration.
|
||||
|
||||
Best-effort: never aborts the surrounding integration operation.
|
||||
"""
|
||||
try:
|
||||
from ..presets import PresetManager
|
||||
|
||||
preset_mgr = PresetManager(project_root)
|
||||
preset_mgr.register_enabled_presets_for_agent(agent_key)
|
||||
except Exception as preset_err:
|
||||
from .. import _print_cli_warning
|
||||
|
||||
_print_cli_warning(
|
||||
"register preset artifacts for",
|
||||
"integration",
|
||||
agent_key,
|
||||
preset_err,
|
||||
continuing=continuing,
|
||||
)
|
||||
|
||||
|
||||
def _unregister_presets_for_agent(
|
||||
project_root: Path,
|
||||
agent_key: str,
|
||||
*,
|
||||
continuing: str,
|
||||
) -> None:
|
||||
"""Best-effort removal of ``agent_key``'s preset command/skill artifacts.
|
||||
|
||||
Mirrors ``_unregister_extensions_for_agent``: used by ``switch`` when
|
||||
uninstalling the previous integration so its preset command overrides
|
||||
and skill mirrors don't linger as orphans in the old agent's directory
|
||||
once a different (possibly not-yet-installed) integration becomes
|
||||
active (#2948).
|
||||
|
||||
Best-effort: never aborts the surrounding integration operation.
|
||||
"""
|
||||
try:
|
||||
from ..presets import PresetManager
|
||||
|
||||
preset_mgr = PresetManager(project_root)
|
||||
preset_mgr.unregister_agent_artifacts(agent_key)
|
||||
except Exception as preset_err:
|
||||
from .. import _print_cli_warning
|
||||
|
||||
_print_cli_warning(
|
||||
"clean up preset artifacts for",
|
||||
"integration",
|
||||
agent_key,
|
||||
preset_err,
|
||||
continuing=continuing,
|
||||
)
|
||||
|
||||
|
||||
def _unregister_enabled_extension_commands_for_agent(
|
||||
project_root: Path,
|
||||
agent_key: str,
|
||||
|
||||
@@ -29,6 +29,7 @@ from ._helpers import (
|
||||
_read_integration_json,
|
||||
_refresh_init_options_speckit_version,
|
||||
_register_extensions_for_agent,
|
||||
_register_presets_for_agent,
|
||||
_remove_integration_json,
|
||||
_resolve_integration_options,
|
||||
_resolve_integration_script_type,
|
||||
@@ -37,6 +38,7 @@ from ._helpers import (
|
||||
_set_default_integration_or_exit,
|
||||
_unregister_enabled_extension_commands_for_agent,
|
||||
_unregister_extensions_for_agent,
|
||||
_unregister_presets_for_agent,
|
||||
_update_init_options_for_integration,
|
||||
_write_integration_json,
|
||||
)
|
||||
@@ -133,13 +135,13 @@ def _installed_presets_affecting_agent(
|
||||
) -> list[str]:
|
||||
"""Return IDs of installed presets with artifacts registered for *agent_key*.
|
||||
|
||||
Presets register command overrides for every detected agent and mirror
|
||||
skills for the active skills agent, tracking the result in each preset's
|
||||
``registered_commands`` / ``registered_skills`` metadata. There is no
|
||||
agent-scoped preset re-registration mechanism, so a command↔skills *layout
|
||||
change* cannot reconcile those artifacts (see ``integration_upgrade``).
|
||||
Callers use this to detect the unsafe case and reject the migration rather
|
||||
than silently orphaning preset files / leaving stale registry entries.
|
||||
Preset registration is active-agent-only (#2948): command overrides are
|
||||
written for the active non-skills agent and skills for the active skills
|
||||
agent, tracked per preset in ``registered_commands`` /
|
||||
``registered_skills``. Entries for *other* agents may still exist from
|
||||
when those agents were active. Callers use this to reject command-root or
|
||||
command↔skills layout migrations before mutation: preset rescaffolding is
|
||||
best-effort and cannot guarantee every tracked artifact has a replacement.
|
||||
|
||||
Fails **closed**: a genuinely absent registry (no presets ever installed)
|
||||
returns an empty list, but if the registry file exists and cannot be read
|
||||
@@ -178,18 +180,36 @@ def _installed_presets_affecting_agent(
|
||||
f"preset '{preset_id}' entry is malformed"
|
||||
)
|
||||
registered_commands = meta.get("registered_commands", {})
|
||||
if not isinstance(registered_commands, dict):
|
||||
if not isinstance(registered_commands, dict) or not all(
|
||||
isinstance(names, list) for names in registered_commands.values()
|
||||
):
|
||||
raise _PresetRegistryUnreadableError(
|
||||
f"preset '{preset_id}' registered_commands is malformed"
|
||||
)
|
||||
registered_skills = meta.get("registered_skills", [])
|
||||
if include_skills:
|
||||
if not isinstance(registered_skills, (list, tuple)):
|
||||
if isinstance(registered_skills, dict):
|
||||
# Per-agent provenance ({agent: [skill names]}): only entries for
|
||||
# *this* agent make the preset affect it. Values must be lists —
|
||||
# anything else (e.g. null) leaves ownership undecidable, so fail
|
||||
# closed rather than read it as "no artifacts".
|
||||
if not all(
|
||||
isinstance(names, list) for names in registered_skills.values()
|
||||
):
|
||||
raise _PresetRegistryUnreadableError(
|
||||
f"preset '{preset_id}' registered_skills is malformed"
|
||||
)
|
||||
has_skills = include_skills and bool(
|
||||
registered_skills.get(agent_key)
|
||||
)
|
||||
elif isinstance(registered_skills, (list, tuple)):
|
||||
# Legacy flat list: not agent-scoped, so any recorded skill may
|
||||
# belong to this agent — fail closed and count it as affecting.
|
||||
has_skills = include_skills and bool(registered_skills)
|
||||
else:
|
||||
raise _PresetRegistryUnreadableError(
|
||||
f"preset '{preset_id}' registered_skills is malformed"
|
||||
)
|
||||
has_commands = bool(registered_commands.get(agent_key))
|
||||
has_skills = include_skills and bool(registered_skills)
|
||||
if has_commands or has_skills:
|
||||
affected.append(preset_id)
|
||||
return affected
|
||||
@@ -297,6 +317,14 @@ def integration_switch(
|
||||
"need re-registration."
|
||||
),
|
||||
)
|
||||
_register_presets_for_agent(
|
||||
project_root,
|
||||
target,
|
||||
continuing=(
|
||||
"The integration switch succeeded, but installed presets may "
|
||||
"need re-registration."
|
||||
),
|
||||
)
|
||||
console.print(f"\n[green]✓[/green] Default integration set to [bold]{target}[/bold].")
|
||||
raise typer.Exit(0)
|
||||
|
||||
@@ -354,6 +382,19 @@ def integration_switch(
|
||||
continuing="Continuing with integration switch; old extension artifacts may need manual cleanup.",
|
||||
)
|
||||
|
||||
# Unregister preset commands/skills for the old agent for the same
|
||||
# reason: without this, a preset's command overrides (including
|
||||
# custom preset commands) and skill mirrors rendered for
|
||||
# installed_key would remain orphaned in its directory once a
|
||||
# different, possibly not-yet-installed integration becomes active
|
||||
# (#2948). Scoped strictly to installed_key; other agents' files,
|
||||
# tracking, and the preset packs themselves are untouched.
|
||||
_unregister_presets_for_agent(
|
||||
project_root,
|
||||
installed_key,
|
||||
continuing="Continuing with integration switch; old preset artifacts may need manual cleanup.",
|
||||
)
|
||||
|
||||
# Clear metadata so a failed Phase 2 doesn't leave stale references
|
||||
installed_keys = [installed for installed in installed_keys if installed != installed_key]
|
||||
_clear_init_options_for_integration(project_root, installed_key)
|
||||
@@ -475,6 +516,24 @@ def integration_switch(
|
||||
f"[yellow]Warning:[/yellow] Failed to restore default "
|
||||
f"integration '{fallback_key}': {restore_err}"
|
||||
)
|
||||
else:
|
||||
# Under active-only registration the fallback may never
|
||||
# have received any extension/preset artifacts (it was
|
||||
# installed while another integration was active), and
|
||||
# Phase 1 already unregistered the outgoing agent's
|
||||
# artifacts. Rescaffold so the restored default is
|
||||
# actually usable. Both helpers are best-effort and
|
||||
# cannot raise past this point.
|
||||
_register_extensions_for_agent(
|
||||
project_root,
|
||||
fallback_key,
|
||||
continuing="The switch was rolled back; installed extensions may need re-registration.",
|
||||
)
|
||||
_register_presets_for_agent(
|
||||
project_root,
|
||||
fallback_key,
|
||||
continuing="The switch was rolled back; installed presets may need re-registration.",
|
||||
)
|
||||
else:
|
||||
_write_integration_json(
|
||||
project_root, fallback_key, installed_keys, _integration_settings(current)
|
||||
@@ -495,6 +554,11 @@ def integration_switch(
|
||||
target,
|
||||
continuing="The integration switch succeeded, but installed extensions may need re-registration.",
|
||||
)
|
||||
_register_presets_for_agent(
|
||||
project_root,
|
||||
target,
|
||||
continuing="The integration switch succeeded, but installed presets may need re-registration.",
|
||||
)
|
||||
|
||||
name = (target_integration.config or {}).get("name", target)
|
||||
console.print(f"\n[green]✓[/green] Switched to integration '{name}'")
|
||||
@@ -572,12 +636,11 @@ def integration_upgrade(
|
||||
)
|
||||
|
||||
# Guard: Kilo's legacy command root moves from .kilocode/workflows to
|
||||
# .kilo/commands. Preset command artifacts are registered only during
|
||||
# preset install/remove, with no agent-scoped re-registration hook to
|
||||
# recreate them at the new command root while preserving priority and
|
||||
# composition semantics. Refuse before setup writes .kilo/commands rather
|
||||
# than leaving legacy preset files orphaned or registry-tracked overrides
|
||||
# missing from the canonical directory.
|
||||
# .kilo/commands. Preset command artifacts are tracked outside the
|
||||
# integration manifest, and their agent-scoped rescaffold is best-effort,
|
||||
# not transactional with command-root cleanup. Refuse before setup writes
|
||||
# .kilo/commands rather than risking orphaned legacy files or missing
|
||||
# registry-tracked overrides in the canonical directory.
|
||||
if key == "kilocode" and legacy_command_root_upgrade_pending:
|
||||
config = integration.registrar_config or {}
|
||||
legacy = config.get("legacy_dir", "legacy command directory")
|
||||
@@ -620,18 +683,12 @@ def integration_upgrade(
|
||||
)
|
||||
raise typer.Exit(1)
|
||||
|
||||
# Guard: reject a command↔skills layout change while preset overrides are
|
||||
# installed for this agent (review #3415). A dual-mode agent (e.g. Bob)
|
||||
# can flip layout across an upgrade (``--skills`` / ``--legacy-commands``).
|
||||
# Extension artifacts are reconciled after the flip (see below), but preset
|
||||
# artifacts cannot be: there is no agent-scoped preset re-registration
|
||||
# anywhere in the CLI, so migrating would delete a preset's old-layout
|
||||
# files without recreating them in the new layout and leave the preset
|
||||
# registry claiming artifacts that no longer exist. Detect the intended
|
||||
# layout (``is_skills_mode`` reflects the resolved flags/disk state, so a
|
||||
# plain same-layout upgrade is unaffected) and bail out *before* any
|
||||
# mutation with an actionable error so the project is never left in a
|
||||
# half-migrated, inconsistent state.
|
||||
# Reject command↔skills layout changes while preset artifacts are tracked
|
||||
# for the integration (review #3415). Preset rescaffolding is best-effort:
|
||||
# an enabled preset can still have a missing/corrupt manifest or command
|
||||
# source, or fail during a write. Phase 2 would otherwise delete the
|
||||
# old-layout file before a replacement is known to exist. Refuse before
|
||||
# any mutation; same-layout upgrades still rescaffold the active agent.
|
||||
if _manifest_tracks_skill_layout(old_manifest) != integration.is_skills_mode(
|
||||
parsed_options, project_root
|
||||
):
|
||||
@@ -657,9 +714,9 @@ def integration_upgrade(
|
||||
f"preset override(s) are installed: [bold]{preset_list}[/bold]."
|
||||
)
|
||||
console.print(
|
||||
"Preset artifacts cannot yet be reconciled across a command↔skills "
|
||||
"layout change, so the migration would orphan their files and leave "
|
||||
"the preset registry inconsistent."
|
||||
"Preset artifacts cannot be safely reconciled across a "
|
||||
"command↔skills layout change, so the migration is refused "
|
||||
"before changing files."
|
||||
)
|
||||
console.print(
|
||||
"Remove the preset(s), run the upgrade, then reinstall them:\n"
|
||||
@@ -795,66 +852,21 @@ def integration_upgrade(
|
||||
),
|
||||
)
|
||||
|
||||
# Re-register enabled extensions for the upgraded agent so its extension
|
||||
# commands are (re)created — including agents installed before this
|
||||
# back-fill existed. Mirrors switch for command registration; see #2886.
|
||||
# Done after the upgrade has fully settled (Phase 2 included) and outside
|
||||
# the try/except above so this best-effort step cannot affect upgrade
|
||||
# success.
|
||||
#
|
||||
# Layout-change reconciliation: a dual-mode agent (e.g. Bob) can flip
|
||||
# between the legacy commands layout and the skills layout across an
|
||||
# upgrade (``upgrade bob --integration-options "--skills"`` / reverse
|
||||
# ``--legacy-commands``). Phase 2 above only removes stale files tracked by
|
||||
# the *integration* manifest (core commands); extension artifacts are
|
||||
# tracked separately in the extension registry, so the old layout's
|
||||
# extension command/skill files would otherwise linger as orphans. When the
|
||||
# layout actually changed, first unregister the agent's extension artifacts
|
||||
# (removing old-layout files and clearing per-agent registry entries) so the
|
||||
# re-registration below recreates them in the new layout. ``upgrade``s that
|
||||
# don't change layout skip this to avoid needless remove/re-add churn.
|
||||
#
|
||||
# Only the *active* integration is reconciled this way (``installed_key ==
|
||||
# key``). ``ExtensionManager.unregister_agent_artifacts`` treats the
|
||||
# per-extension ``registered_skills`` list as belonging to the passed agent
|
||||
# and, when that agent's skills directory is absent, falls back to scanning
|
||||
# every agent's skills directory — so running it for a *secondary*
|
||||
# (non-active) agent could delete or untrack the *active* agent's extension
|
||||
# skills. The subsequent re-registration cannot repair that because
|
||||
# extension skill rendering is intentionally scoped to the active agent
|
||||
# (#2948). Extension skills only ever exist for the active agent, so
|
||||
# skipping the unregister for a secondary agent orphans nothing new: a
|
||||
# secondary agent only has extension *command* files, which the
|
||||
# re-registration below rewrites in place regardless of layout.
|
||||
#
|
||||
# Known limitation: preset command/skill artifacts are NOT reconciled on a
|
||||
# layout change. There is no agent-scoped preset re-registration mechanism
|
||||
# anywhere in the CLI — ``use`` / ``switch`` / ``upgrade`` never reconcile
|
||||
# presets for any agent (presets are only (un)registered at preset
|
||||
# install/remove time). Rather than silently orphan them, the guard near
|
||||
# the top of this function rejects a layout-changing upgrade while preset
|
||||
# overrides are installed, so control only reaches here (with a changed
|
||||
# layout) when no preset artifacts are at stake. Full preset reconciliation
|
||||
# would require a new cross-cutting PresetManager subsystem affecting every
|
||||
# dual-layout agent, which is out of scope for this Bob migration.
|
||||
if (
|
||||
installed_key == key
|
||||
and _manifest_tracks_skill_layout(old_manifest)
|
||||
!= _manifest_tracks_skill_layout(new_manifest)
|
||||
):
|
||||
_unregister_extensions_for_agent(
|
||||
# Re-register enabled extensions and presets only when upgrading the
|
||||
# active integration. Inactive integrations remain untouched until
|
||||
# `use` or `switch` activates and rescaffolds them (#2948). This runs
|
||||
# after the core upgrade transaction, so failures remain best-effort.
|
||||
if key == installed_key:
|
||||
_register_extensions_for_agent(
|
||||
project_root,
|
||||
key,
|
||||
continuing=(
|
||||
"The integration layout changed, but old-layout extension "
|
||||
"artifacts may need manual cleanup."
|
||||
),
|
||||
continuing="The integration was upgraded, but installed extensions may need re-registration.",
|
||||
)
|
||||
_register_presets_for_agent(
|
||||
project_root,
|
||||
key,
|
||||
continuing="The integration was upgraded, but installed presets may need re-registration.",
|
||||
)
|
||||
_register_extensions_for_agent(
|
||||
project_root,
|
||||
key,
|
||||
continuing="The integration was upgraded, but installed extensions may need re-registration.",
|
||||
)
|
||||
|
||||
name = (integration.config or {}).get("name", key)
|
||||
console.print(f"\n[green]✓[/green] Integration '{name}' upgraded successfully")
|
||||
|
||||
@@ -18,6 +18,7 @@ from ._commands import integration_app, integration_catalog_app
|
||||
from ._helpers import (
|
||||
_read_integration_json,
|
||||
_register_extensions_for_agent,
|
||||
_register_presets_for_agent,
|
||||
_resolve_integration_options,
|
||||
_set_default_integration_or_exit,
|
||||
)
|
||||
@@ -248,6 +249,11 @@ def integration_use(
|
||||
key,
|
||||
continuing="The integration was selected, but installed extensions may need re-registration.",
|
||||
)
|
||||
_register_presets_for_agent(
|
||||
project_root,
|
||||
key,
|
||||
continuing="The integration was selected, but installed presets may need re-registration.",
|
||||
)
|
||||
console.print(f"[green]✓[/green] Default integration set to [bold]{key}[/bold].")
|
||||
|
||||
|
||||
|
||||
File diff suppressed because it is too large
Load Diff
@@ -303,7 +303,7 @@ class TestClaudeIntegration:
|
||||
assert "disable-model-invocation: false" in content
|
||||
|
||||
metadata = manager.registry.get("claude-skill-command")
|
||||
assert "speckit-research" in metadata.get("registered_skills", [])
|
||||
assert "speckit-research" in metadata.get("registered_skills", {}).get("claude", [])
|
||||
|
||||
|
||||
class TestClaudeArgumentHints:
|
||||
|
||||
@@ -1407,6 +1407,195 @@ class TestIntegrationInstall:
|
||||
project / ".github" / "skills" / "speckit-git-feature" / "SKILL.md"
|
||||
).exists()
|
||||
|
||||
def test_extension_add_registers_active_integration_only(self, tmp_path):
|
||||
"""``extension add`` registers commands for the active integration only.
|
||||
|
||||
Maintainer-requested behavior for #2948: with multiple integrations
|
||||
installed, ``extension add`` must treat the project as single-active —
|
||||
only the current integration gets the new extension's commands.
|
||||
Non-active integrations receive them when selected via
|
||||
``integration use`` / ``switch`` (rescaffold).
|
||||
"""
|
||||
project = _init_project(tmp_path, "claude")
|
||||
|
||||
result = _run_in_project(project, [
|
||||
"integration", "install", "codex",
|
||||
"--script", "sh",
|
||||
])
|
||||
assert result.exit_code == 0, result.output
|
||||
|
||||
result = _run_in_project(project, ["extension", "add", "git"])
|
||||
assert result.exit_code == 0, f"extension add failed: {result.output}"
|
||||
|
||||
registry_path = project / ".specify" / "extensions" / ".registry"
|
||||
registered = json.loads(registry_path.read_text(encoding="utf-8"))[
|
||||
"extensions"
|
||||
]["git"]["registered_commands"]
|
||||
assert "claude" in registered, "active integration gets the extension"
|
||||
assert "codex" not in registered, (
|
||||
"non-active integration must not be registered on add (#2948)"
|
||||
)
|
||||
assert (
|
||||
project / ".claude" / "skills" / "speckit-git-feature" / "SKILL.md"
|
||||
).exists()
|
||||
assert not (
|
||||
project / ".agents" / "skills" / "speckit-git-feature" / "SKILL.md"
|
||||
).exists()
|
||||
|
||||
# Selecting the other integration rescaffolds it with the extension.
|
||||
result = _run_in_project(project, ["integration", "use", "codex"])
|
||||
assert result.exit_code == 0, result.output
|
||||
|
||||
registered = json.loads(registry_path.read_text(encoding="utf-8"))[
|
||||
"extensions"
|
||||
]["git"]["registered_commands"]
|
||||
assert "codex" in registered, "use registers extensions for the new active agent"
|
||||
assert (
|
||||
project / ".agents" / "skills" / "speckit-git-feature" / "SKILL.md"
|
||||
).exists()
|
||||
|
||||
def test_extension_add_generic_active_does_not_backfill_other_agents(self, tmp_path):
|
||||
"""A recorded but unsupported active key (``generic``) must not
|
||||
fall back to registering every detected agent.
|
||||
|
||||
``generic`` is deliberately excluded from ``AGENT_CONFIGS`` because
|
||||
its output directory is only known via ``--commands-dir``, not a
|
||||
static config. Before the fix, treating that active key like "no
|
||||
active integration recorded" made the fallback register the
|
||||
extension for every other detected agent — exactly the multi-target
|
||||
behavior #2948 is meant to stop.
|
||||
"""
|
||||
project = _init_project(
|
||||
tmp_path, "generic",
|
||||
integration_options="--commands-dir .myagent/commands",
|
||||
)
|
||||
|
||||
result = _run_in_project(project, [
|
||||
"integration", "install", "codex",
|
||||
"--script", "sh",
|
||||
"--force",
|
||||
])
|
||||
assert result.exit_code == 0, result.output
|
||||
|
||||
result = _run_in_project(project, ["extension", "add", "git"])
|
||||
assert result.exit_code == 0, f"extension add failed: {result.output}"
|
||||
|
||||
registry_path = project / ".specify" / "extensions" / ".registry"
|
||||
registered = json.loads(registry_path.read_text(encoding="utf-8"))[
|
||||
"extensions"
|
||||
]["git"]["registered_commands"]
|
||||
assert "codex" not in registered, (
|
||||
"a recorded but unsupported active key must not target other "
|
||||
"detected agents (#2948)"
|
||||
)
|
||||
|
||||
def test_extension_add_malformed_ai_value_fails_closed(self, tmp_path):
|
||||
"""A recorded but malformed ``ai`` value (e.g. a list) must not be
|
||||
treated as "no active integration recorded" and must not crash.
|
||||
|
||||
Before the fix, ``init_options.get("ai")`` being falsy (``[]``,
|
||||
``""``, ``0``) triggered the same all-agents fallback as a missing
|
||||
key, and a *truthy* non-string value (e.g. a non-empty list) would
|
||||
reach ``AGENT_CONFIGS.get(active_agent)`` and raise ``TypeError``
|
||||
because a list is unhashable. Corrupted init-options must instead
|
||||
fail closed: register nothing rather than crash or back-fill every
|
||||
detected agent.
|
||||
"""
|
||||
project = _init_project(tmp_path, "claude")
|
||||
|
||||
result = _run_in_project(project, [
|
||||
"integration", "install", "codex",
|
||||
"--script", "sh",
|
||||
])
|
||||
assert result.exit_code == 0, result.output
|
||||
|
||||
init_options_path = project / ".specify" / "init-options.json"
|
||||
init_options = json.loads(init_options_path.read_text(encoding="utf-8"))
|
||||
init_options["ai"] = []
|
||||
init_options_path.write_text(json.dumps(init_options), encoding="utf-8")
|
||||
|
||||
result = _run_in_project(project, ["extension", "add", "git"])
|
||||
assert result.exit_code == 0, f"extension add failed: {result.output}"
|
||||
|
||||
registry_path = project / ".specify" / "extensions" / ".registry"
|
||||
registered = json.loads(registry_path.read_text(encoding="utf-8"))[
|
||||
"extensions"
|
||||
]["git"]["registered_commands"]
|
||||
assert registered == {}, (
|
||||
"a malformed recorded 'ai' value must fail closed, not "
|
||||
"back-fill every detected agent (#2948)"
|
||||
)
|
||||
|
||||
def test_extension_add_corrupted_init_options_file_fails_closed(self, tmp_path):
|
||||
"""A present-but-unparseable init-options.json must fail closed too,
|
||||
not be treated the same as "no file at all".
|
||||
|
||||
``load_init_options`` returns ``{}`` for a corrupted/unreadable
|
||||
file just like it does for a missing file, so a naive "no active
|
||||
agent recorded" check based on ``load_init_options`` alone can't
|
||||
tell a legacy pre-init-options project (legitimate all-agent
|
||||
fallback) apart from a corrupted-but-present file for a #2948
|
||||
project (must fail closed). Corrupting the file after a normal
|
||||
init must not reintroduce the all-agent fallback.
|
||||
"""
|
||||
project = _init_project(tmp_path, "claude")
|
||||
|
||||
result = _run_in_project(project, [
|
||||
"integration", "install", "codex",
|
||||
"--script", "sh",
|
||||
])
|
||||
assert result.exit_code == 0, result.output
|
||||
|
||||
init_options_path = project / ".specify" / "init-options.json"
|
||||
init_options_path.write_text("{not valid json", encoding="utf-8")
|
||||
|
||||
result = _run_in_project(project, ["extension", "add", "git"])
|
||||
assert result.exit_code == 0, f"extension add failed: {result.output}"
|
||||
|
||||
registry_path = project / ".specify" / "extensions" / ".registry"
|
||||
registered = json.loads(registry_path.read_text(encoding="utf-8"))[
|
||||
"extensions"
|
||||
]["git"]["registered_commands"]
|
||||
assert registered == {}, (
|
||||
"a corrupted init-options.json must fail closed, not be "
|
||||
"treated like a legacy project missing the file entirely (#2948)"
|
||||
)
|
||||
|
||||
def test_extension_add_dangling_init_options_symlink_fails_closed(self, tmp_path):
|
||||
"""A dangling init-options.json symlink must fail closed too, not be
|
||||
treated the same as "no file at all".
|
||||
|
||||
``Path.exists()`` follows symlinks and returns False for a broken
|
||||
symlink whose target doesn't exist, so a naive presence check based
|
||||
on ``Path.exists()`` alone mistakes a dangling symlink for "no file"
|
||||
and falls back to registering every detected agent.
|
||||
"""
|
||||
project = _init_project(tmp_path, "claude")
|
||||
|
||||
result = _run_in_project(project, [
|
||||
"integration", "install", "codex",
|
||||
"--script", "sh",
|
||||
])
|
||||
assert result.exit_code == 0, result.output
|
||||
|
||||
init_options_path = project / ".specify" / "init-options.json"
|
||||
init_options_path.unlink()
|
||||
init_options_path.symlink_to(project / ".specify" / "does-not-exist.json")
|
||||
assert not init_options_path.exists() # sanity: dangling
|
||||
assert init_options_path.is_symlink()
|
||||
|
||||
result = _run_in_project(project, ["extension", "add", "git"])
|
||||
assert result.exit_code == 0, f"extension add failed: {result.output}"
|
||||
|
||||
registry_path = project / ".specify" / "extensions" / ".registry"
|
||||
registered = json.loads(registry_path.read_text(encoding="utf-8"))[
|
||||
"extensions"
|
||||
]["git"]["registered_commands"]
|
||||
assert registered == {}, (
|
||||
"a dangling init-options.json symlink must fail closed, not be "
|
||||
"treated like a legacy project missing the file entirely (#2948)"
|
||||
)
|
||||
|
||||
|
||||
# ── uninstall ────────────────────────────────────────────────────────
|
||||
|
||||
@@ -1650,6 +1839,156 @@ class TestIntegrationUse:
|
||||
assert result.exit_code != 0
|
||||
assert "not installed" in result.output
|
||||
|
||||
def test_use_registers_presets_for_the_newly_active_agent(self, tmp_path):
|
||||
"""``integration use`` is the single rescaffold point for presets too.
|
||||
|
||||
Mirrors the extension single-active rule (#2948): a preset command
|
||||
override installed while ``claude`` was active must not target the
|
||||
inactive ``codex`` integration, and switching via ``integration use``
|
||||
must rescaffold it there.
|
||||
"""
|
||||
project = _init_project(tmp_path, "claude")
|
||||
|
||||
result = _run_in_project(project, [
|
||||
"integration", "install", "codex",
|
||||
"--script", "sh",
|
||||
])
|
||||
assert result.exit_code == 0, result.output
|
||||
|
||||
preset_src = tmp_path / "cmd-preset"
|
||||
(preset_src / "commands").mkdir(parents=True)
|
||||
(preset_src / "commands" / "speckit.specify.md").write_text(
|
||||
"---\ndescription: Overridden specify\n---\nOverridden content\n",
|
||||
encoding="utf-8",
|
||||
)
|
||||
manifest_data = {
|
||||
"schema_version": "1.0",
|
||||
"preset": {
|
||||
"id": "cmd-preset",
|
||||
"name": "Command Preset",
|
||||
"version": "1.0.0",
|
||||
"description": "Test preset with a command override",
|
||||
},
|
||||
"requires": {"speckit_version": ">=0.1.0"},
|
||||
"provides": {
|
||||
"templates": [
|
||||
{
|
||||
"type": "command",
|
||||
"name": "speckit.specify",
|
||||
"file": "commands/speckit.specify.md",
|
||||
}
|
||||
]
|
||||
},
|
||||
}
|
||||
import yaml
|
||||
|
||||
(preset_src / "preset.yml").write_text(yaml.dump(manifest_data), encoding="utf-8")
|
||||
|
||||
result = _run_in_project(project, ["preset", "add", "--dev", str(preset_src)])
|
||||
assert result.exit_code == 0, f"preset add failed: {result.output}"
|
||||
|
||||
registry_path = project / ".specify" / "presets" / ".registry"
|
||||
registered = json.loads(registry_path.read_text(encoding="utf-8"))[
|
||||
"presets"
|
||||
]["cmd-preset"]["registered_commands"]
|
||||
assert "claude" in registered, "active integration gets the preset command override"
|
||||
assert "codex" not in registered, (
|
||||
"non-active integration must not be registered on preset add (#2948)"
|
||||
)
|
||||
|
||||
result = _run_in_project(project, ["integration", "use", "codex"])
|
||||
assert result.exit_code == 0, result.output
|
||||
|
||||
registered = json.loads(registry_path.read_text(encoding="utf-8"))[
|
||||
"presets"
|
||||
]["cmd-preset"]["registered_commands"]
|
||||
assert "codex" in registered, "use registers presets for the new active agent"
|
||||
assert "claude" in registered, "the previous agent's registration is preserved"
|
||||
|
||||
def test_use_reregisters_presets_highest_precedence_last(self, tmp_path):
|
||||
"""When two enabled presets override the same command, the
|
||||
higher-precedence preset (lower priority number) must win the
|
||||
materialized file after ``integration use`` rescaffolds them.
|
||||
|
||||
``register_enabled_presets_for_agent`` iterates presets and each
|
||||
pass overwrites the same target file, so the write order matters.
|
||||
Before the fix, presets were processed lowest-number-first (highest
|
||||
precedence first), so the lower-precedence preset was written last
|
||||
and won -- reversing the documented priority stack (#2948).
|
||||
"""
|
||||
project = _init_project(tmp_path, "claude")
|
||||
|
||||
result = _run_in_project(project, [
|
||||
"integration", "install", "codex",
|
||||
"--script", "sh",
|
||||
])
|
||||
assert result.exit_code == 0, result.output
|
||||
|
||||
import yaml
|
||||
|
||||
def _make_preset(pack_id: str, content: str) -> Path:
|
||||
src = tmp_path / pack_id
|
||||
(src / "commands").mkdir(parents=True)
|
||||
(src / "commands" / "speckit.specify.md").write_text(
|
||||
f"---\ndescription: {pack_id}\n---\n{content}\n",
|
||||
encoding="utf-8",
|
||||
)
|
||||
manifest_data = {
|
||||
"schema_version": "1.0",
|
||||
"preset": {
|
||||
"id": pack_id,
|
||||
"name": pack_id,
|
||||
"version": "1.0.0",
|
||||
"description": f"Test preset {pack_id}",
|
||||
},
|
||||
"requires": {"speckit_version": ">=0.1.0"},
|
||||
"provides": {
|
||||
"templates": [
|
||||
{
|
||||
"type": "command",
|
||||
"name": "speckit.specify",
|
||||
"file": "commands/speckit.specify.md",
|
||||
}
|
||||
]
|
||||
},
|
||||
}
|
||||
(src / "preset.yml").write_text(yaml.dump(manifest_data), encoding="utf-8")
|
||||
return src
|
||||
|
||||
# Lower-precedence preset (higher priority number), installed first.
|
||||
low_precedence_src = _make_preset("low-precedence-preset", "LOW PRECEDENCE CONTENT")
|
||||
result = _run_in_project(project, [
|
||||
"preset", "add", "--dev", str(low_precedence_src), "--priority", "20",
|
||||
])
|
||||
assert result.exit_code == 0, f"preset add (low) failed: {result.output}"
|
||||
|
||||
# Higher-precedence preset (lower priority number), installed second.
|
||||
high_precedence_src = _make_preset("high-precedence-preset", "HIGH PRECEDENCE CONTENT")
|
||||
result = _run_in_project(project, [
|
||||
"preset", "add", "--dev", str(high_precedence_src), "--priority", "1",
|
||||
])
|
||||
assert result.exit_code == 0, f"preset add (high) failed: {result.output}"
|
||||
|
||||
# Sanity: the priority stack already picks the high-precedence
|
||||
# preset's content for the active (claude) integration.
|
||||
claude_skill = project / ".claude" / "skills" / "speckit-specify" / "SKILL.md"
|
||||
assert "HIGH PRECEDENCE CONTENT" in claude_skill.read_text(encoding="utf-8")
|
||||
assert "LOW PRECEDENCE CONTENT" not in claude_skill.read_text(encoding="utf-8")
|
||||
|
||||
result = _run_in_project(project, ["integration", "use", "codex"])
|
||||
assert result.exit_code == 0, result.output
|
||||
|
||||
# After rescaffolding for the newly active codex integration, the
|
||||
# high-precedence preset must still win -- not whichever preset
|
||||
# register_enabled_presets_for_agent happened to write last.
|
||||
codex_skill = project / ".agents" / "skills" / "speckit-specify" / "SKILL.md"
|
||||
content = codex_skill.read_text(encoding="utf-8")
|
||||
assert "HIGH PRECEDENCE CONTENT" in content, (
|
||||
"highest-precedence preset must win after `use` rescaffolds "
|
||||
"presets for the newly active integration (#2948)"
|
||||
)
|
||||
assert "LOW PRECEDENCE CONTENT" not in content
|
||||
|
||||
def test_use_refreshes_shared_templates_between_command_styles(self, tmp_path):
|
||||
project = _init_project(tmp_path, "claude")
|
||||
template = project / ".specify" / "templates" / "plan-template.md"
|
||||
@@ -2015,6 +2354,83 @@ class TestIntegrationSwitch:
|
||||
assert "opencode" in git_meta["registered_commands"]
|
||||
assert "copilot" not in git_meta["registered_commands"]
|
||||
|
||||
def test_switch_to_not_yet_installed_unregisters_old_preset_artifacts(self, tmp_path):
|
||||
"""Switching to a not-yet-installed integration must also clean up
|
||||
the old agent's preset command overrides, mirroring the existing
|
||||
extension cleanup on the same code path (#2948).
|
||||
|
||||
Without this, a preset's command override -- including a custom
|
||||
preset command -- rendered for the previous agent lingers as an
|
||||
orphan once a different, not-yet-installed integration becomes the
|
||||
new active agent.
|
||||
"""
|
||||
project = _init_project(tmp_path, "auggie")
|
||||
|
||||
preset_src = tmp_path / "switch-cleanup-preset"
|
||||
(preset_src / "commands").mkdir(parents=True)
|
||||
(preset_src / "commands" / "speckit.specify.md").write_text(
|
||||
"---\ndescription: Custom preset command\n---\nOverridden content\n",
|
||||
encoding="utf-8",
|
||||
)
|
||||
manifest_data = {
|
||||
"schema_version": "1.0",
|
||||
"preset": {
|
||||
"id": "switch-cleanup-preset",
|
||||
"name": "Switch Cleanup Preset",
|
||||
"version": "1.0.0",
|
||||
"description": "Test preset with a custom command override",
|
||||
},
|
||||
"requires": {"speckit_version": ">=0.1.0"},
|
||||
"provides": {
|
||||
"templates": [
|
||||
{
|
||||
"type": "command",
|
||||
"name": "speckit.specify",
|
||||
"file": "commands/speckit.specify.md",
|
||||
}
|
||||
]
|
||||
},
|
||||
}
|
||||
import yaml
|
||||
|
||||
(preset_src / "preset.yml").write_text(yaml.dump(manifest_data), encoding="utf-8")
|
||||
|
||||
result = _run_in_project(project, ["preset", "add", "--dev", str(preset_src)])
|
||||
assert result.exit_code == 0, f"preset add failed: {result.output}"
|
||||
|
||||
auggie_cmd = project / ".augment" / "commands" / "speckit.specify.md"
|
||||
assert auggie_cmd.exists(), "sanity: preset command registered for auggie"
|
||||
|
||||
registry_path = project / ".specify" / "presets" / ".registry"
|
||||
registered = json.loads(registry_path.read_text(encoding="utf-8"))[
|
||||
"presets"
|
||||
]["switch-cleanup-preset"]["registered_commands"]
|
||||
assert "auggie" in registered, "sanity: auggie tracked before switch"
|
||||
|
||||
# opencode is not yet installed in this project.
|
||||
result = _run_in_project(project, [
|
||||
"integration", "switch", "opencode",
|
||||
"--script", "sh",
|
||||
])
|
||||
assert result.exit_code == 0, result.output
|
||||
|
||||
assert not auggie_cmd.exists(), (
|
||||
"old agent's preset command override must be removed on switch "
|
||||
"to a not-yet-installed integration, mirroring the existing "
|
||||
"extension cleanup on this same code path (#2948)"
|
||||
)
|
||||
|
||||
opencode_cmd = project / ".opencode" / "commands" / "speckit.specify.md"
|
||||
assert opencode_cmd.exists(), "preset command should be registered for the new agent"
|
||||
|
||||
registered = json.loads(registry_path.read_text(encoding="utf-8"))[
|
||||
"presets"
|
||||
]["switch-cleanup-preset"]["registered_commands"]
|
||||
assert "auggie" not in registered, (
|
||||
"old agent's tracking must be dropped after switch cleanup"
|
||||
)
|
||||
assert "opencode" in registered
|
||||
|
||||
def test_switch_does_not_register_disabled_extensions(self, tmp_path):
|
||||
"""Disabled extensions should stay disabled and should not migrate commands."""
|
||||
project = _init_project(tmp_path, "opencode")
|
||||
@@ -2311,6 +2727,54 @@ class TestIntegrationSwitch:
|
||||
template = project / ".specify" / "templates" / "plan-template.md"
|
||||
assert "/speckit-plan" in template.read_text(encoding="utf-8")
|
||||
|
||||
def test_failed_switch_rescaffolds_fallback_extensions(self, tmp_path):
|
||||
"""Regression (review 3624184343).
|
||||
|
||||
When Phase 2 of a switch fails, rollback selects another installed
|
||||
integration as the new default. Under active-only registration that
|
||||
fallback may never have received extension artifacts (it was
|
||||
installed while another integration was active), and Phase 1 already
|
||||
unregistered the outgoing agent's artifacts — so the restored default
|
||||
must be rescaffolded, not just written to metadata.
|
||||
"""
|
||||
project = _init_project(tmp_path, "claude")
|
||||
result = _run_in_project(project, ["extension", "add", "git"])
|
||||
assert result.exit_code == 0, f"extension add failed: {result.output}"
|
||||
result = _run_in_project(project, [
|
||||
"integration", "install", "codex",
|
||||
"--script", "sh",
|
||||
])
|
||||
assert result.exit_code == 0, result.output
|
||||
|
||||
registry_path = project / ".specify" / "extensions" / ".registry"
|
||||
registered = json.loads(registry_path.read_text(encoding="utf-8"))[
|
||||
"extensions"
|
||||
]["git"]["registered_commands"]
|
||||
assert "codex" not in registered, (
|
||||
"precondition: secondary install has no extension artifacts"
|
||||
)
|
||||
|
||||
result = _run_in_project(project, [
|
||||
"integration", "switch", "generic",
|
||||
"--script", "sh",
|
||||
])
|
||||
assert result.exit_code != 0
|
||||
|
||||
data = json.loads(
|
||||
(project / ".specify" / "integration.json").read_text(encoding="utf-8")
|
||||
)
|
||||
assert data["integration"] == "codex", "precondition: fallback restored"
|
||||
|
||||
registered = json.loads(registry_path.read_text(encoding="utf-8"))[
|
||||
"extensions"
|
||||
]["git"]["registered_commands"]
|
||||
assert "codex" in registered, (
|
||||
"rollback must rescaffold extensions for the restored default"
|
||||
)
|
||||
assert (
|
||||
project / ".agents" / "skills" / "speckit-git-feature" / "SKILL.md"
|
||||
).exists()
|
||||
|
||||
|
||||
class TestIntegrationUpgrade:
|
||||
def test_upgrade_invalid_manifest_reports_cli_error(self, tmp_path):
|
||||
@@ -2652,10 +3116,10 @@ class TestIntegrationUpgrade:
|
||||
"disabled extensions must not be re-registered in the canonical dir"
|
||||
)
|
||||
|
||||
def test_upgrade_secondary_kilocode_legacy_dir_preserves_active_agent_skills(
|
||||
def test_upgrade_secondary_kilocode_legacy_dir_cleans_commands_without_backfill(
|
||||
self, tmp_path
|
||||
):
|
||||
"""Kilo command-root cleanup must not touch active agent extension skills."""
|
||||
"""Kilo cleanup stays agent-scoped without inactive extension backfill."""
|
||||
project = _init_project(tmp_path, "copilot", integration_options="--skills")
|
||||
result = _run_in_project(project, ["extension", "add", "git"])
|
||||
assert result.exit_code == 0, f"extension add failed: {result.output}"
|
||||
@@ -2698,12 +3162,17 @@ class TestIntegrationUpgrade:
|
||||
assert result.exit_code == 0, result.output
|
||||
|
||||
assert canonical.is_dir(), ".kilo/commands/ should exist after upgrade"
|
||||
assert sorted(canonical.glob("speckit.git.*.md")), (
|
||||
"secondary Kilo should regain enabled extension commands"
|
||||
assert not sorted(canonical.glob("speckit.git.*.md")), (
|
||||
"inactive Kilo must wait for use/switch before extension rescaffolding"
|
||||
)
|
||||
assert not legacy_git_command.exists(), (
|
||||
"secondary Kilo legacy extension commands should still be cleaned up"
|
||||
)
|
||||
registry = json.loads(registry_path.read_text(encoding="utf-8"))
|
||||
registered_commands = registry["extensions"]["git"].get(
|
||||
"registered_commands", {}
|
||||
)
|
||||
assert "kilocode" not in registered_commands
|
||||
assert skill.exists(), (
|
||||
"secondary Kilo legacy cleanup must not delete the active agent's "
|
||||
"extension skill"
|
||||
@@ -2842,24 +3311,241 @@ class TestIntegrationUpgrade:
|
||||
cmds_agents, skill_names = _git_registry()
|
||||
assert "bob" in cmds_agents and not skill_names
|
||||
|
||||
def test_upgrade_bob_layout_change_rejected_with_presets_installed(self, tmp_path):
|
||||
"""Regression (review #3415, 4726193915).
|
||||
def test_upgrade_layout_change_preserves_extension_artifacts_when_reregistration_fails(
|
||||
self, tmp_path
|
||||
):
|
||||
"""Regression (review 3624075109).
|
||||
|
||||
A command↔skills layout change cannot reconcile preset artifacts (no
|
||||
agent-scoped preset re-registration exists). Rather than silently
|
||||
orphaning preset files / leaving the registry inconsistent, a
|
||||
layout-changing ``upgrade`` must reject the migration with an
|
||||
actionable error *before any mutation* when preset overrides are
|
||||
installed for the agent. A same-layout upgrade must still succeed.
|
||||
A layout-changing upgrade must not eagerly unregister the agent's
|
||||
extension artifacts before re-registration: the retirement of each
|
||||
opposite-mode artifact belongs to
|
||||
``register_enabled_extensions_for_agent``'s deferred toggle cleanup,
|
||||
which retires an old artifact only after its replacement in the new
|
||||
layout is confirmed. If re-registration cannot rebuild an extension
|
||||
(here: its installed manifest is corrupted), the old artifact and its
|
||||
registry tracking must survive instead of leaving the extension with
|
||||
no artifacts at all.
|
||||
"""
|
||||
project = _init_project(
|
||||
tmp_path, "bob", integration_options="--legacy-commands"
|
||||
)
|
||||
result = _run_in_project(project, ["extension", "add", "git"])
|
||||
assert result.exit_code == 0, f"extension add failed: {result.output}"
|
||||
|
||||
commands = project / ".bob" / "commands"
|
||||
assert sorted(commands.glob("speckit.git.*.md")), (
|
||||
"precondition: git extension renders as legacy command files"
|
||||
)
|
||||
|
||||
# Corrupt the installed extension manifest so re-registration cannot
|
||||
# rebuild the artifacts in the new layout.
|
||||
(
|
||||
project / ".specify" / "extensions" / "git" / "extension.yml"
|
||||
).write_text("invalid: [", encoding="utf-8")
|
||||
|
||||
result = _run_in_project(project, [
|
||||
"integration", "upgrade", "bob",
|
||||
"--integration-options", "--skills",
|
||||
"--script", "sh", "--force",
|
||||
])
|
||||
assert result.exit_code == 0, (
|
||||
f"upgrade is best-effort about extensions: {result.output}"
|
||||
)
|
||||
|
||||
assert sorted(commands.glob("speckit.git.*.md")), (
|
||||
"old-layout extension artifacts must survive when their "
|
||||
"replacement could not be registered"
|
||||
)
|
||||
registry_path = project / ".specify" / "extensions" / ".registry"
|
||||
data = json.loads(registry_path.read_text(encoding="utf-8"))
|
||||
assert "bob" in data["extensions"]["git"].get("registered_commands", {}), (
|
||||
"extension registry must keep tracking the surviving artifacts"
|
||||
)
|
||||
|
||||
def test_upgrade_active_layout_change_rejected_before_missing_preset_source_can_lose_override(
|
||||
self, tmp_path
|
||||
):
|
||||
"""Regression (review 3623357447).
|
||||
|
||||
Layout-changing upgrades must fail closed even for the active
|
||||
integration. Preset rescaffolding is best-effort, so a missing source
|
||||
file could otherwise let stale integration cleanup delete the tracked
|
||||
old-layout override without creating its replacement.
|
||||
"""
|
||||
project = _init_project(
|
||||
tmp_path, "bob", integration_options="--legacy-commands"
|
||||
)
|
||||
commands = project / ".bob" / "commands"
|
||||
skills = project / ".bob" / "skills"
|
||||
|
||||
preset_src = tmp_path / "cmd-preset"
|
||||
(preset_src / "commands").mkdir(parents=True)
|
||||
(preset_src / "commands" / "speckit.plan.md").write_text(
|
||||
"---\ndescription: Overridden plan\n---\nOverridden plan content\n",
|
||||
encoding="utf-8",
|
||||
)
|
||||
manifest_data = {
|
||||
"schema_version": "1.0",
|
||||
"preset": {
|
||||
"id": "cmd-preset",
|
||||
"name": "Command Preset",
|
||||
"version": "1.0.0",
|
||||
"description": "Test preset with a command override",
|
||||
},
|
||||
"requires": {"speckit_version": ">=0.1.0"},
|
||||
"provides": {
|
||||
"templates": [
|
||||
{
|
||||
"type": "command",
|
||||
"name": "speckit.plan",
|
||||
"file": "commands/speckit.plan.md",
|
||||
}
|
||||
]
|
||||
},
|
||||
}
|
||||
import yaml
|
||||
|
||||
(preset_src / "preset.yml").write_text(
|
||||
yaml.dump(manifest_data), encoding="utf-8"
|
||||
)
|
||||
result = _run_in_project(project, ["preset", "add", "--dev", str(preset_src)])
|
||||
assert result.exit_code == 0, f"preset add failed: {result.output}"
|
||||
|
||||
cmd_file = commands / "speckit.plan.md"
|
||||
assert "Overridden plan content" in cmd_file.read_text(encoding="utf-8")
|
||||
|
||||
installed_source = (
|
||||
project
|
||||
/ ".specify"
|
||||
/ "presets"
|
||||
/ "cmd-preset"
|
||||
/ "commands"
|
||||
/ "speckit.plan.md"
|
||||
)
|
||||
assert installed_source.exists(), "precondition: preset source was installed"
|
||||
installed_source.unlink()
|
||||
|
||||
result = _run_in_project(project, [
|
||||
"integration", "upgrade", "bob",
|
||||
"--integration-options", "--skills",
|
||||
"--script", "sh", "--force",
|
||||
])
|
||||
assert result.exit_code != 0, (
|
||||
"layout change with tracked preset artifacts must be rejected"
|
||||
)
|
||||
assert "cmd-preset" in result.output
|
||||
assert not skills.exists(), "no skills layout must be scaffolded on rejection"
|
||||
assert "Overridden plan content" in cmd_file.read_text(encoding="utf-8"), (
|
||||
"tracked old-layout override must remain untouched"
|
||||
)
|
||||
|
||||
def test_upgrade_active_layout_change_rejected_with_disabled_preset(
|
||||
self, tmp_path
|
||||
):
|
||||
"""Regression (review 3623779277).
|
||||
|
||||
The post-upgrade rescaffold iterates *enabled* presets only, and a
|
||||
disabled preset's artifacts are deliberately frozen until removal
|
||||
(``preset disable``). An active-agent layout change must therefore be
|
||||
rejected while a disabled preset still owns artifacts for the agent —
|
||||
proceeding would delete its old-layout files in stale-manifest
|
||||
cleanup, skip recreating them, and leave its registry entries stale.
|
||||
Re-enabling does not make a non-transactional layout migration safe.
|
||||
"""
|
||||
project = _init_project(
|
||||
tmp_path, "bob", integration_options="--legacy-commands"
|
||||
)
|
||||
commands = project / ".bob" / "commands"
|
||||
skills = project / ".bob" / "skills"
|
||||
|
||||
preset_src = tmp_path / "cmd-preset"
|
||||
(preset_src / "commands").mkdir(parents=True)
|
||||
(preset_src / "commands" / "speckit.plan.md").write_text(
|
||||
"---\ndescription: Overridden plan\n---\nOverridden plan content\n",
|
||||
encoding="utf-8",
|
||||
)
|
||||
manifest_data = {
|
||||
"schema_version": "1.0",
|
||||
"preset": {
|
||||
"id": "cmd-preset",
|
||||
"name": "Command Preset",
|
||||
"version": "1.0.0",
|
||||
"description": "Test preset with a command override",
|
||||
},
|
||||
"requires": {"speckit_version": ">=0.1.0"},
|
||||
"provides": {
|
||||
"templates": [
|
||||
{
|
||||
"type": "command",
|
||||
"name": "speckit.plan",
|
||||
"file": "commands/speckit.plan.md",
|
||||
}
|
||||
]
|
||||
},
|
||||
}
|
||||
import yaml
|
||||
|
||||
(preset_src / "preset.yml").write_text(
|
||||
yaml.dump(manifest_data), encoding="utf-8"
|
||||
)
|
||||
result = _run_in_project(project, ["preset", "add", "--dev", str(preset_src)])
|
||||
assert result.exit_code == 0, f"preset add failed: {result.output}"
|
||||
result = _run_in_project(project, ["preset", "disable", "cmd-preset"])
|
||||
assert result.exit_code == 0, f"preset disable failed: {result.output}"
|
||||
|
||||
cmd_file = commands / "speckit.plan.md"
|
||||
assert "Overridden plan content" in cmd_file.read_text(encoding="utf-8")
|
||||
|
||||
result = _run_in_project(project, [
|
||||
"integration", "upgrade", "bob",
|
||||
"--integration-options", "--skills",
|
||||
"--script", "sh", "--force",
|
||||
])
|
||||
assert result.exit_code != 0, (
|
||||
"layout change with a disabled preset must be rejected"
|
||||
)
|
||||
assert "cmd-preset" in result.output
|
||||
assert not skills.exists(), "no skills layout must be scaffolded on rejection"
|
||||
assert "Overridden plan content" in cmd_file.read_text(encoding="utf-8"), (
|
||||
"the disabled preset's command file must be left untouched"
|
||||
)
|
||||
|
||||
# Enabled presets are also rejected: rescaffolding can still fail.
|
||||
result = _run_in_project(project, ["preset", "enable", "cmd-preset"])
|
||||
assert result.exit_code == 0, f"preset enable failed: {result.output}"
|
||||
result = _run_in_project(project, [
|
||||
"integration", "upgrade", "bob",
|
||||
"--integration-options", "--skills",
|
||||
"--script", "sh", "--force",
|
||||
])
|
||||
assert result.exit_code != 0
|
||||
assert "cmd-preset" in result.output
|
||||
assert not skills.exists()
|
||||
assert "Overridden plan content" in cmd_file.read_text(encoding="utf-8")
|
||||
|
||||
def test_upgrade_secondary_layout_change_rejected_with_presets_installed(
|
||||
self, tmp_path
|
||||
):
|
||||
"""Regression (review #3415, 4726193915; updated for review 3623357447).
|
||||
|
||||
Preset rescaffolding is active-agent-only, so a layout-changing
|
||||
``upgrade`` of a *non-active* integration still cannot reconcile that
|
||||
agent's preset artifacts. It must reject the migration with an
|
||||
actionable error *before any mutation* when preset overrides are
|
||||
installed for that agent. A same-layout upgrade must still succeed.
|
||||
"""
|
||||
project = _init_project(tmp_path, "copilot")
|
||||
result = _run_in_project(project, [
|
||||
"integration", "install", "bob",
|
||||
"--integration-options", "--legacy-commands",
|
||||
"--script", "sh", "--force",
|
||||
])
|
||||
assert result.exit_code == 0, result.output
|
||||
commands = project / ".bob" / "commands"
|
||||
skills = project / ".bob" / "skills"
|
||||
assert sorted(commands.glob("speckit.*.md"))
|
||||
|
||||
# Simulate an installed preset that registered command overrides for bob.
|
||||
# Simulate a historical preset registration for the non-active bob.
|
||||
presets_dir = project / ".specify" / "presets"
|
||||
presets_dir.mkdir(parents=True, exist_ok=True)
|
||||
(presets_dir / ".registry").write_text(
|
||||
@@ -2869,20 +3555,22 @@ class TestIntegrationUpgrade:
|
||||
"version": "1.0.0",
|
||||
"enabled": True,
|
||||
"registered_commands": {"bob": ["speckit.plan"]},
|
||||
"registered_skills": [],
|
||||
"registered_skills": {},
|
||||
}
|
||||
}
|
||||
}),
|
||||
encoding="utf-8",
|
||||
)
|
||||
|
||||
# Layout-changing upgrade is rejected, and nothing is mutated.
|
||||
# Layout-changing upgrade of the secondary agent is rejected untouched.
|
||||
result = _run_in_project(project, [
|
||||
"integration", "upgrade", "bob",
|
||||
"--integration-options", "--skills",
|
||||
"--script", "sh", "--force",
|
||||
])
|
||||
assert result.exit_code != 0, "layout change with presets must be rejected"
|
||||
assert result.exit_code != 0, (
|
||||
"secondary layout change with presets must be rejected"
|
||||
)
|
||||
assert "preset" in result.output.lower()
|
||||
assert "my-preset" in result.output
|
||||
assert not skills.exists(), "no skills layout must be scaffolded on rejection"
|
||||
@@ -3059,13 +3747,13 @@ class TestIntegrationUpgrade:
|
||||
"shared .sh scripts must be executable after upgrade"
|
||||
)
|
||||
|
||||
def test_upgrade_backfills_extension_commands_for_agent(self, tmp_path):
|
||||
"""Upgrade re-registers enabled extensions for the upgraded agent.
|
||||
def test_upgrade_does_not_backfill_non_active_integration(self, tmp_path):
|
||||
"""Upgrading a non-active integration must not register extensions for it.
|
||||
|
||||
Regression for #2886: agents installed before extension back-fill
|
||||
existed (or whose extension artifacts went missing) should regain the
|
||||
enabled extensions' commands on ``upgrade``, reaching parity with
|
||||
``switch``.
|
||||
Maintainer-requested behavior for #2948 (reverses the #2886 upgrade
|
||||
back-fill): non-active integrations only receive extension artifacts
|
||||
when selected via ``integration use`` / ``switch``. Upgrade of a
|
||||
non-active integration refreshes its own files and nothing else.
|
||||
"""
|
||||
project = _init_project(tmp_path, "claude")
|
||||
|
||||
@@ -3078,21 +3766,10 @@ class TestIntegrationUpgrade:
|
||||
])
|
||||
assert result.exit_code == 0, result.output
|
||||
|
||||
# Simulate a project created before the install/upgrade back-fill: drop
|
||||
# codex's extension registration and its rendered artifacts.
|
||||
registry_path = project / ".specify" / "extensions" / ".registry"
|
||||
registry = json.loads(registry_path.read_text(encoding="utf-8"))
|
||||
registry["extensions"]["git"]["registered_commands"].pop("codex", None)
|
||||
registry_path.write_text(json.dumps(registry), encoding="utf-8")
|
||||
agents_skills = project / ".agents" / "skills"
|
||||
for skill_dir in agents_skills.glob("speckit-git-*"):
|
||||
shutil.rmtree(skill_dir)
|
||||
|
||||
# Precondition: codex is now missing the git extension.
|
||||
assert "codex" not in json.loads(registry_path.read_text(encoding="utf-8"))[
|
||||
"extensions"
|
||||
]["git"]["registered_commands"]
|
||||
assert not (agents_skills / "speckit-git-feature" / "SKILL.md").exists()
|
||||
|
||||
result = _run_in_project(project, [
|
||||
"integration", "upgrade", "codex",
|
||||
@@ -3100,12 +3777,94 @@ class TestIntegrationUpgrade:
|
||||
])
|
||||
assert result.exit_code == 0, result.output
|
||||
|
||||
# Upgrade back-filled the git extension for codex.
|
||||
registered = json.loads(registry_path.read_text(encoding="utf-8"))[
|
||||
"extensions"
|
||||
]["git"]["registered_commands"]
|
||||
assert "codex" in registered, "upgrade should re-register extension commands (#2886)"
|
||||
assert (agents_skills / "speckit-git-feature" / "SKILL.md").exists()
|
||||
assert "codex" not in registered, (
|
||||
"upgrade must not back-fill non-active integrations (#2948)"
|
||||
)
|
||||
assert not (
|
||||
project / ".agents" / "skills" / "speckit-git-feature" / "SKILL.md"
|
||||
).exists()
|
||||
|
||||
def test_upgrade_active_integration_reregisters_extensions(self, tmp_path):
|
||||
"""Upgrading the active integration restores its extension commands.
|
||||
|
||||
The active integration keeps the re-registration pass on upgrade so
|
||||
missing or stale extension command files are recreated (#2948 scopes
|
||||
the pass to the active integration; #2886 introduced it).
|
||||
"""
|
||||
project = _init_project(tmp_path, "claude")
|
||||
|
||||
result = _run_in_project(project, ["extension", "add", "git"])
|
||||
assert result.exit_code == 0, f"extension add failed: {result.output}"
|
||||
|
||||
cmd_file = project / ".claude" / "skills" / "speckit-git-feature" / "SKILL.md"
|
||||
assert cmd_file.exists(), "precondition: extension command registered"
|
||||
cmd_file.unlink()
|
||||
|
||||
result = _run_in_project(project, [
|
||||
"integration", "upgrade", "claude",
|
||||
"--script", "sh",
|
||||
])
|
||||
assert result.exit_code == 0, result.output
|
||||
|
||||
assert cmd_file.exists(), (
|
||||
"upgrade of the active integration re-registers extension commands"
|
||||
)
|
||||
|
||||
def test_upgrade_active_integration_reregisters_presets(self, tmp_path):
|
||||
"""Upgrading the active integration restores missing preset artifacts."""
|
||||
import yaml
|
||||
|
||||
project = _init_project(tmp_path, "claude")
|
||||
preset_src = tmp_path / "upgrade-preset"
|
||||
(preset_src / "commands").mkdir(parents=True)
|
||||
(preset_src / "commands" / "speckit.upgrade-check.md").write_text(
|
||||
"---\ndescription: Upgrade check\n---\nPreset upgrade body\n",
|
||||
encoding="utf-8",
|
||||
)
|
||||
manifest = {
|
||||
"schema_version": "1.0",
|
||||
"preset": {
|
||||
"id": "upgrade-preset",
|
||||
"name": "Upgrade Preset",
|
||||
"version": "1.0.0",
|
||||
"description": "Upgrade preset test",
|
||||
},
|
||||
"requires": {"speckit_version": ">=0.1.0"},
|
||||
"provides": {
|
||||
"templates": [
|
||||
{
|
||||
"type": "command",
|
||||
"name": "speckit.upgrade-check",
|
||||
"file": "commands/speckit.upgrade-check.md",
|
||||
}
|
||||
]
|
||||
},
|
||||
}
|
||||
(preset_src / "preset.yml").write_text(
|
||||
yaml.dump(manifest), encoding="utf-8"
|
||||
)
|
||||
|
||||
result = _run_in_project(
|
||||
project, ["preset", "add", "--dev", str(preset_src)]
|
||||
)
|
||||
assert result.exit_code == 0, result.output
|
||||
|
||||
skill_dir = (
|
||||
project / ".claude" / "skills" / "speckit-upgrade-check"
|
||||
)
|
||||
skill_file = skill_dir / "SKILL.md"
|
||||
assert "Preset upgrade body" in skill_file.read_text(encoding="utf-8")
|
||||
shutil.rmtree(skill_dir)
|
||||
|
||||
result = _run_in_project(project, [
|
||||
"integration", "upgrade", "claude",
|
||||
"--script", "sh",
|
||||
])
|
||||
assert result.exit_code == 0, result.output
|
||||
assert "Preset upgrade body" in skill_file.read_text(encoding="utf-8")
|
||||
|
||||
def test_upgrade_non_active_agent_preserves_active_agent_skills(self, tmp_path):
|
||||
"""Upgrading a non-active agent must not touch the active agent's skills.
|
||||
@@ -3195,9 +3954,28 @@ class TestIntegrationUpgrade:
|
||||
with pytest.raises(_PresetRegistryUnreadableError):
|
||||
_installed_presets_affecting_agent(project, "bob")
|
||||
|
||||
# Malformed registered_skills (not a list) → raise.
|
||||
# Malformed registered_skills (neither list nor dict) → raise.
|
||||
registry.write_text(
|
||||
json.dumps({"presets": {"p1": {"registered_skills": {}}}}),
|
||||
json.dumps({"presets": {"p1": {"registered_skills": "oops"}}}),
|
||||
encoding="utf-8",
|
||||
)
|
||||
with pytest.raises(_PresetRegistryUnreadableError):
|
||||
_installed_presets_affecting_agent(project, "bob")
|
||||
|
||||
# Dict-shaped fields with non-list values (ownership undecidable)
|
||||
# must also fail closed, not read as "no artifacts".
|
||||
registry.write_text(
|
||||
json.dumps(
|
||||
{"presets": {"p1": {"registered_skills": {"bob": None}}}}
|
||||
),
|
||||
encoding="utf-8",
|
||||
)
|
||||
with pytest.raises(_PresetRegistryUnreadableError):
|
||||
_installed_presets_affecting_agent(project, "bob")
|
||||
registry.write_text(
|
||||
json.dumps(
|
||||
{"presets": {"p1": {"registered_commands": {"bob": ""}}}}
|
||||
),
|
||||
encoding="utf-8",
|
||||
)
|
||||
with pytest.raises(_PresetRegistryUnreadableError):
|
||||
@@ -3207,13 +3985,24 @@ class TestIntegrationUpgrade:
|
||||
registry.write_text(json.dumps({"presets": {}}), encoding="utf-8")
|
||||
assert _installed_presets_affecting_agent(project, "bob") == []
|
||||
|
||||
# Valid registry with a preset registered for bob → reported.
|
||||
# Valid registry with a preset registered for bob → report its ID.
|
||||
# registered_skills comes in two shapes: a legacy flat list (not
|
||||
# agent-scoped → fail closed, any entry affects) and the per-agent
|
||||
# dict written by preset registration ({agent: [skill names]} → only
|
||||
# this agent's entries affect it).
|
||||
registry.write_text(
|
||||
json.dumps({
|
||||
"presets": {
|
||||
"p1": {"registered_commands": {"bob": ["speckit.plan"]}},
|
||||
"p2": {"registered_commands": {"codex": ["speckit.plan"]}},
|
||||
"p3": {"registered_skills": ["speckit-x"]},
|
||||
"p4": {"registered_skills": {"bob": ["speckit-y"]}},
|
||||
"p5": {"registered_skills": {"codex": ["speckit-z"]}},
|
||||
"p6": {"registered_skills": {"bob": []}},
|
||||
"p7": {
|
||||
"enabled": False,
|
||||
"registered_commands": {"bob": ["speckit.tasks"]},
|
||||
},
|
||||
}
|
||||
}),
|
||||
encoding="utf-8",
|
||||
@@ -3221,9 +4010,12 @@ class TestIntegrationUpgrade:
|
||||
assert sorted(_installed_presets_affecting_agent(project, "bob")) == [
|
||||
"p1",
|
||||
"p3",
|
||||
"p4",
|
||||
"p7",
|
||||
]
|
||||
assert _installed_command_presets_affecting_agent(project, "bob") == [
|
||||
"p1"
|
||||
"p1",
|
||||
"p7",
|
||||
]
|
||||
|
||||
|
||||
|
||||
File diff suppressed because it is too large
Load Diff
@@ -1197,6 +1197,7 @@ class TestExtensionManager:
|
||||
link_outputs=False,
|
||||
create_missing_active_skills_dir=False,
|
||||
extension_id=None,
|
||||
only_agent=None,
|
||||
):
|
||||
captured["create_missing_active_skills_dir"] = (
|
||||
create_missing_active_skills_dir
|
||||
@@ -9024,7 +9025,7 @@ $ARGUMENTS
|
||||
from specify_cli import app
|
||||
from specify_cli.agents import CommandRegistrar
|
||||
|
||||
project_dir, ext_dir, claude_commands_dir = self._setup_mock_extension(tmp_path, "claude")
|
||||
project_dir, ext_dir, agents_commands_dir = self._setup_mock_extension(tmp_path, "amp")
|
||||
|
||||
# 3. Run specify extension add
|
||||
runner = CliRunner()
|
||||
@@ -9041,8 +9042,8 @@ $ARGUMENTS
|
||||
assert "speckit-mock-ext-hello" not in result.output
|
||||
|
||||
# Verify on-disk command names are dotted
|
||||
hello_file = claude_commands_dir / "speckit.mock-ext.hello.md"
|
||||
greet_file = claude_commands_dir / "speckit.mock-ext.greet.md"
|
||||
hello_file = agents_commands_dir / "speckit.mock-ext.hello.md"
|
||||
greet_file = agents_commands_dir / "speckit.mock-ext.greet.md"
|
||||
|
||||
assert hello_file.exists()
|
||||
assert greet_file.exists()
|
||||
|
||||
File diff suppressed because it is too large
Load Diff
Reference in New Issue
Block a user