48 Commits

Author	SHA1	Message	Date
Quratulain-bilal	0117a7b977	fix: correct Optional type annotation for context_note parameter (#3765) ... The context_note parameter in CommandRegistrar methods was annotated as \str = None\ which is a type lie — the default is None but the type hint says str. Static type checkers (mypy/pyright) would flag this as an error. Changed to \Optional[str] = None\ for correctness, consistent with how extension_id (same class) is already typed.	2026-07-27 16:12:20 -05:00
Marsel Safin	683bfd00c9	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 after 2486c08, 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 HEAD b9d9053 (#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(). After 1d8f9e3, 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 HEAD 31c9b97 (#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 HEAD ab6c28c), 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 HEAD d0d152e: 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>	2026-07-27 14:09:40 -05:00
Marsel Safin	2a0ada9a6a	feat(scripts): port create-new-feature, setup-plan and setup-tasks to Python (#3386) ... * feat(scripts): port create-new-feature, setup-plan and setup-tasks to Python Ports the three core workflow scripts to Python as part of #3280, following the check-prerequisites PoC pattern from #3302. Adds resolve_template() to the shared common.py module and parity tests that run bash and Python side by side. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(tests): treat only None env as unset in parity run helper Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(scripts): fall back to directory scan on any registry error, skip hidden preset dirs Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * feat(templates): add py: lines for setup_plan and setup_tasks Ships with the scripts they reference; the remaining templates got their py: lines in #3403. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: support py variant in skills placeholder resolver resolve_skill_placeholders only accepted sh/ps, so a py init option fell into the fallback path and {SCRIPT} rendered without an interpreter prefix. Accept py and prefix the resolved interpreter, matching process_template. Also guard ps_cmd against a missing PowerShell with a clear assert. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * test: pin clean-error behavior for invalid --number Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs(scripts): reword unused-arg comment to match implementation The loop accepts and silently ignores extra positional args (it doesn't build a collected list); match the wording to what the code and setup-plan.sh actually do. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: fall back when configured script variant is missing from frontmatter Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(scripts): reject signed/whitespace --number values to match bash 10# parity The bash twin uses $((10#$BRANCH_NUMBER)), which rejects signed and whitespace-padded values. Python's int() accepted them (e.g. -1), producing a malformed -01-... prefix that sequential scans ignore. Restrict --number to unsigned decimal digits before conversion, and pin the parity with a bash-comparison test. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(scripts): complete Python port installation Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(integrations): fall back for missing script variants Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * test: make Python script checks platform-aware Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix Windows Python command invocation parity Use PowerShell's call operator for spaced Python interpreter paths and align setup-tasks missing-template errors across script variants. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(scripts): preserve cross-platform Python parity Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: reject signed PowerShell feature numbers Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(scripts): align feature number range Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(scripts): reject exhausted feature numbers Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(scripts): complete create feature parity Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(scripts): align create feature outputs Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(scripts): harden cross-platform parity Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(scripts): keep truncation JSON clean Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(scripts): align setup failure parity Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(scripts): close parity edge cases Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(scripts): propagate PowerShell setup errors Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(scripts): harden fallback resolution Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(scripts): stabilize PowerShell fallbacks Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(scripts): complete setup-plan parity Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(cli): require runnable script fallbacks Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(cli): preserve shell fallback without preference Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(scripts): restore help and symlink parity - setup-tasks.ps1: check -Help before unknown-argument validation so '-Help --bogus' exits 0 like the Bash/Python variants - common.py: strip the repo root prefix lexically in persist_feature_json instead of resolve(), so a symlinked specs/ still persists the relative 'specs/NNN-name' path the Bash/PowerShell helpers store Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(scripts): align persist-hint quoting with shlex.quote - create-new-feature.sh: replace printf %q with a shell_quote helper that emits shlex.quote-identical output, so the persistence hints stay byte-identical between the Bash and Python variants (printf %q output also varies between bash versions) - promote the negative --number test to an all-variants parity test now that Bash and PowerShell reject signed values consistently - add a spaced-repo-path parity test for the persistence hints Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>	2026-07-21 11:40:15 -05:00
Andrew Chen	7873c447bd	fix(agents): parse frontmatter on the --- delimiter line, not any --- substring (#3590) ... CommandRegistrar.parse_frontmatter located the closing delimiter with content.find("---", 3), a raw substring search. It stopped at the first "---" anywhere after the opening — including one embedded in a frontmatter value (e.g. a description "Separate sections with --- markers") or inside an indented literal block — which truncated the frontmatter and spilled the remainder into the body, silently corrupting both the parsed metadata and the rendered command body. Match the closing "---" on line boundaries, mirroring the line-anchored scan already used by VibeIntegration._inject_frontmatter_flag.	2026-07-21 10:22:52 -05:00
Davide Barletta	74662cffad	feat: update Bob integration to skills-based layout for Bob 2.0 (#3415) ... * feat: update Bob integration to skills-based layout for Bob 2.0 Bob 2.0 replaces the command-based workflow (.bob/commands/*.md) with a skills-based layout (.bob/skills/speckit-<name>/SKILL.md), matching the pattern used by Claude Code, Codex, and other skills-first agents. - Switch BobIntegration from MarkdownIntegration to SkillsIntegration - Update folder/dir from .bob/commands to .bob/skills - Change extension from .md to /SKILL.md (skills layout) - Add --skills option (default: True) consistent with Codex pattern - Update tests to inherit from SkillsIntegrationTests (28 tests pass) - Bump catalog entry to version 2.0.0 with updated description Assisted-by: IBM Bob (model: claude-sonnet-4-5, autonomous) * PR comments fix: keep old Bob 1 commands till next release * Copilot suggested change Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * feat(bob): address copilot comments, make skills layout default, demote legacy commands to opt-in * fix(bob): honor legacy_commands in ai_skills persistence and add bob to ALWAYS_SLASH_AGENTS - init.py: suppress ai_skills=True when --legacy-commands is passed so extensions and presets target .bob/commands, not .bob/skills - _invocation_style.py: add 'bob' to ALWAYS_SLASH_AGENTS so init next-steps and hook invocations always show /speckit-<name> (skills is the default layout; no ai_skills flag required) * Copilot suggestion Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * fix(bob): extend IntegrationBase directly to avoid false isinstance(SkillsIntegration) - bob/__init__.py: switch BobIntegration base from SkillsIntegration to IntegrationBase; add _BobSkillsHelper for skills-mode delegation; set invoke_separator='-' explicitly; set _skills_mode flag in setup() so consumers can derive the effective mode without isinstance checks - _helpers.py: replace isinstance(integration, SkillsIntegration) guard with getattr(_skills_mode) so legacy-commands mode does not persist ai_skills=True - _invocation_style.py: remove 'bob' from ALWAYS_SLASH_AGENTS — Bob 2.0 skills are invoked via natural language, not /skill-name slash commands - integrations/catalog.json: advance updated_at to 2026-07-15 * fix(lint): remove unused SkillsIntegration import from _helpers.py * Copilot suggested change Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * feat(bob): add bob skills integration with registrar-based mode detection * address 3 comments from copilot * feat(bob): update registrar config to use legacy commands layout * fix lint * Suggested fix from Copilot Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * fix pr comment * fix pr comment * fix pr comment * refactor(bob): resolve skills mode via base-class hooks + fix command-ref separators Rework the dual-mode handling introduced for Bob 2.0 so an integration's internal representation never leaks into shared init/install/upgrade code, and fix the legacy command-reference separator surfaced in review. Base-class contract: - Add IntegrationBase.is_skills_mode(parsed_options) — the single hook the shared machinery consults to decide whether to persist ai_skills and render skill invocations. SkillsIntegration returns True; Copilot honors --skills / self._skills_mode; Bob returns `not legacy_commands`. - Add IntegrationBase.invoke_separator_for_mode(skills_enabled) — resolves the command-ref separator from a project's persisted mode for registration paths that only have the ai_skills flag (no CLI parsed_options). Default is behavior-preserving; Bob maps skills->"-", legacy->".". - BobIntegration stays on IntegrationBase (mirroring Copilot, the other dual-mode agent) and delegates setup() to internal _BobSkillsHelper / _BobMarkdownHelper. Removes the _skills_mode method and all isinstance(SkillsIntegration) / callable(_skills_mode) probing from _helpers.py and init.py. Fix legacy separator (review feedback): CommandRegistrar.register_commands and PresetManager._resolve_skill_command_refs previously read the single static AGENT_CONFIGS[key]["invoke_separator"], so legacy .bob/commands/ extension and preset command refs rendered /speckit-<cmd> instead of Bob 1.x /speckit.<cmd>. Both now resolve the separator per project mode via invoke_separator_for_mode. Tests: add regression coverage for the is_skills_mode / invoke_separator_for_mode hooks and legacy extension command-ref separators; normalize a width-sensitive workflow assertion to match its siblings. Full suite green. Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 63f93544-a77f-4f01-bf04-c88806a97dbf * fix(bob,copilot): address review — preserve legacy layout, dual-mode separators, extension-skill token resolution Addresses PR review 4716036212 (3 comments): 1. Bob legacy-install regression: `use`/`switch`/`upgrade` on an existing Bob 1.x project (only `.bob/commands/` on disk, no stored `legacy_commands`) called `is_skills_mode(None)` -> True and rewrote `ai_skills=True`, silently switching extension/command-reference handling to the skills layout. `is_skills_mode` now takes an optional `project_root`; Bob preserves an already-installed legacy layout until an explicit upgrade creates `.bob/skills/`. A fresh project still defaults to skills. 2. Copilot dual-mode separator: `invoke_separator_for_mode` was inherited from the base (mode-independent) and returned Copilot's static `.`, so preset/extension command refs in a Copilot skills project rendered `/speckit.<name>` instead of `/speckit-<name>`. Override it on Copilot to track the persisted `ai_skills` state, consistent with `build_command_invocation` and `effective_invoke_separator`. 3. Bob extension-skill command-ref tokens: verified that merging main's generic `_resolve_command_ref_tokens` (#3544) resolves Bob's tokens via the `CONDITIONAL_SLASH_AGENTS` path (`/speckit-<name>`); added Bob to the command-ref regression parametrize plus dedicated Bob use-path tests. All tests pass (full suite green; merged with current main incl. #3544). Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 63f93544-a77f-4f01-bf04-c88806a97dbf * fix(bob): resolve command-ref separator with project-aware mode before shared-infra refresh (review #3415) The `use`/`switch` paths refresh shared infrastructure via `_with_integration_setting()` / `_invoke_separator_for_integration()`, which previously resolved the invoke separator through `effective_invoke_separator` / `is_skills_mode` WITHOUT a project_root. For a pre-PR Bob 1.x project (.bob/commands/ on disk, no stored options), this defaulted to the skills "-" separator and rewrote rendered shared-template command refs to /speckit-*, even though ai_skills stayed false. Thread project_root through effective_invoke_separator, the two runtime helpers, and every call site so Bob's on-disk legacy detection governs the separator before shared infra is refreshed. Add a rendered-shared-template regression test covering `use --force`. Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 63f93544-a77f-4f01-bf04-c88806a97dbf * fix(bob): scope persisted ai_skills flag to active agent when resolving command-ref separator (review #3415) `register_commands` runs once per detected agent, but the persisted `ai_skills` flag describes only the active integration (`opts["ai"]`). When another agent (e.g. Copilot) is active in skills mode while a legacy `.bob/commands` layout is also present, the previous code passed that global `True` to Bob's `invoke_separator_for_mode`, rewriting Bob 1.x command refs to `/speckit-*` instead of `/speckit.*`. Only consult the persisted flag for the agent it describes (`opts["ai"] == agent_name`); otherwise resolve the separator from the agent's own project-aware `effective_invoke_separator(None, project_root)`. Add regression tests covering the mismatched-active-agent case and a control for Bob-active skills mode. Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 63f93544-a77f-4f01-bf04-c88806a97dbf * fix(bob): detect Spec Kit layout from managed artifacts, not any skills dir (review #3415) Two related mis-detections from review 4723246468: 1. `BobIntegration.is_skills_mode` treated the mere presence of a `.bob/skills/` directory as proof the project is skills-based. A legacy Spec Kit install (managed `.bob/commands/speckit.*.md`) that also carried unrelated Bob 2 skills would be misclassified as skills, so `integration use bob` persisted `ai_skills` and rewrote shared refs. Now the layout is inferred from managed Spec Kit artifacts: legacy/command mode only when managed `speckit.*.md` command files exist and no managed `speckit-*` skill dirs do. 2. The `register_commands` separator for an inactive agent used a disk-based `effective_invoke_separator(None, project_root)` fallback that could pick the skills separator even though the registrar writes the static command layout (`.bob/commands/*.md`). Inactive agents now resolve the separator from the registrar's actual output layout (`extension == "/SKILL.md"`), so command-layout files keep `/speckit.*` refs regardless of sibling dirs. Update the affected hook/E2E tests to use managed artifacts and add regression tests for the mixed-layout and inactive-registrar scenarios. Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 63f93544-a77f-4f01-bf04-c88806a97dbf * fix(bob): apply managed-artifact detection on upgrade + consistent skill post-processing (review #3415) Two issues from review 4723782860: 1. `BobIntegration.setup()` resolved the layout via `is_skills_mode(parsed_options)` WITHOUT `project_root`, so `integration upgrade bob` on a Bob 1.x install (managed `.bob/commands/speckit.*.md`, no stored options) ignored the existing command files, generated skills, and stale-deleted the legacy commands — silently migrating the project. Pass `project_root` so the same managed-artifact detection used by `use` also governs upgrades. 2. Only `_BobSkillsHelper` overrode `post_process_skill_content` to suppress the shared slash-command hook note. Preset/extension skill generators call that hook on the registered `BobIntegration`, which inherited `IntegrationBase`'s note-injecting default. Repeat the no-op (delegating to the skills helper) on the registered class so every Bob skill-generation path is consistent with intent-activated core Bob skills. Add regression tests for the upgrade-preservation and post-processing paths. Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 63f93544-a77f-4f01-bf04-c88806a97dbf * feat(bob): add --skills migration opt-in; fix separator + manifest loss (review #3415) Address review #3415 (4724160183): - Comment 1: Add an explicit `--skills` opt-in to BobIntegration. It forces the skills layout over on-disk auto-detection, giving legacy Bob 1.x installs a supported migration path (`integration upgrade bob --integration-options="--skills"`). `--skills` and `--legacy-commands` are mutually exclusive (clean exit-1 error). - Comment 2: In CommandRegistrar.register_commands, derive the command-ref separator from the output layout (agent_config["extension"]) for the active agent too, not the persisted ai_skills flag. A command-layout file (.bob/commands/*.md, .github/agents/*.agent.md) always renders /speckit.*; only a /SKILL.md scaffold uses /speckit-*. Dual-layout agents (Bob, Copilot) write skills via their own setup()/skills path, so register_commands only ever emits their command-layout files. - Comment 3: Update docs/reference/integrations.md Bob entry to document the skills-based default (.bob/skills/), the deprecated --legacy-commands opt-out, and the --skills migration path. Also fix a latent manifest-loss bug surfaced by the migration path: the upgrade Phase 2 stale-file cleanup built a throwaway manifest sharing the integration key and called uninstall(), which always deleted {key}.manifest.json. Any layout-shrinking upgrade (e.g. legacy->skills) thus wiped the freshly-saved manifest, leaving the project untracked and un-upgradeable. uninstall() now takes remove_manifest (default True); the stale-cleanup pass passes False. Adds regression tests for the --skills opt-in, mutual exclusion, corrected active-agent separator, remove_manifest=False, and an end-to-end legacy->skills migration that verifies the manifest survives and the project remains upgradeable. Full suite: 4555 passed, 5 skipped. Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 63f93544-a77f-4f01-bf04-c88806a97dbf * docs(agents): align token-resolution comment with output-layout separator rule (review #3415) Address review #3415 (4725516805). The comment above resolve_command_refs still described the removed state-based behavior ("resolve it from the integration using the project's persisted skills state"). Update it to describe the output-layout rule that register_commands now uses: _sep is derived from the layout this registrar writes (a /SKILL.md scaffold uses the skills separator; a command-layout file uses the command separator), not the persisted ai_skills state. Comment-only change; no behavior change. Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 63f93544-a77f-4f01-bf04-c88806a97dbf * fix(bob): reconcile extension artifacts on layout change (review #3415) When a dual-mode agent (Bob) flips between the legacy commands layout and the skills layout during `integration upgrade` (via `--skills` / `--legacy-commands`), the old layout's extension command/skill files were left orphaned: Phase 2 stale cleanup only removes files tracked by the *integration* manifest, while extension artifacts are tracked in the extension registry. Detect the layout flip by comparing whether the old vs new manifest tracks a `/SKILL.md` scaffold, and when it changed, unregister the agent's extension artifacts before the existing re-registration so they are recreated in the new layout (and the per-agent registry is updated). Preset artifacts are documented as a known, pre-existing cross-cutting gap: no agent-scoped preset re-registration exists in use/switch/upgrade for any agent, so reconciling them is out of scope for this Bob migration. Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 63f93544-a77f-4f01-bf04-c88806a97dbf * fix(bob): reject layout migration when preset overrides are installed (review #3415) A command↔skills layout change during `integration upgrade` cannot reconcile preset artifacts: presets track their command/skill files in per-preset `registered_commands`/`registered_skills` metadata, and there is no agent-scoped preset re-registration anywhere in the CLI. 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 via `is_skills_mode` (so a plain same-layout upgrade is unaffected) and, when it flips while preset overrides are installed for the agent, reject the upgrade *before any mutation* with an actionable error pointing at the remove → upgrade → reinstall workaround. Extension artifacts are still reconciled for the safe (no-preset) case. Adds a regression test and documents the migration caveat in the Bob integration reference entry. Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 63f93544-a77f-4f01-bf04-c88806a97dbf * fix(bob): restrict layout reconciliation to the active integration (review #3415) `integration_upgrade` supports upgrading a secondary (non-active) integration, but the layout-change extension reconciliation was unsafe there. `ExtensionManager.unregister_agent_artifacts()` treats the unscoped 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 reconciling a secondary Bob layout flip 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). Gate the unregister-before-register reconciliation on `installed_key == key` so it only runs for the active integration. Secondary agents only ever have extension command files (skills are active-agent-only), which the existing re-registration rewrites in place, so skipping the unregister orphans nothing new. Adds a regression test asserting a secondary Bob layout change leaves the active agent's extension skill intact on disk and in the registry. Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 63f93544-a77f-4f01-bf04-c88806a97dbf * fix(bob): fail closed when preset registry is unreadable (review #3415) Address review 4744636079: - _migrate_commands: the preset guard previously failed *open* — a registry read/parse error returned an empty "no presets" list, so a --force layout-changing upgrade could delete preset-overridden command files while their registry state was unknown. Read the registry file directly and raise _PresetRegistryUnreadableError on any read/parse failure or malformed structure, rejecting the migration before any mutation. A genuinely absent registry still returns [] (safe). - bob: correct the is_skills_mode docstring — upgrade *does* run setup(); disk detection is needed because legacy Bob 1.x installs never persisted a legacy_commands option, so the stored mode is unavailable. - tests: add fail-closed E2E (corrupted registry rejected, valid-empty allowed) plus a unit test for _installed_presets_affecting_agent covering absent / corrupted / malformed / valid / affecting-agent cases. Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 63f93544-a77f-4f01-bf04-c88806a97dbf * fix(bob): fail closed on malformed preset entries too (review #3415) Address review 4745191015: the preset guard read a parseable registry but silently skipped malformed per-preset metadata and treated a malformed registered_commands value as "no matching artifacts". A registry such as {"presets":{"p1":[]}} therefore allowed a layout migration even though p1's ownership is unknown, risking deletion of preset-managed files. Now raise _PresetRegistryUnreadableError for a non-dict preset entry, a non-dict registered_commands, or a non-list registered_skills. Extend the unit test to cover these malformed shapes. Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 63f93544-a77f-4f01-bf04-c88806a97dbf --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: Manfred Riem <15701806+mnriem@users.noreply.github.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>	2026-07-21 09:20:20 -05:00
Marsel Safin	3b7d95a408	fix: rewrite extension-relative subdir paths in generated command bodies (#3444) ... * fix: rewrite extension-relative subdir paths in generated command bodies Extension command bodies reference bundled files relative to the extension root (agents/, knowledge-base/, templates/, ...). Generated SKILL.md and command files emitted those paths verbatim, so agents resolved them against the workspace root where they do not exist. Add CommandRegistrar.rewrite_extension_paths, which rewrites references to subdirectories that actually exist in the installed extension to .specify/extensions/<id>/..., and call it once in register_commands so every output format and alias gets the fix. commands/, specs/ and dot-directories are never rewritten. Fixes #2101 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: only rewrite relative extension path references Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: use callable re.sub replacement for extension subdir rewrite subdir and extension_id come from filesystem directory names and were interpolated into a re.sub string replacement template. A directory name containing a backslash (e.g. assets\q) would raise re.error: bad escape, aborting command registration even when the body didn't reference it. Use a callable replacement so these values are treated literally. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: make subdir rewrite regression test cross-platform Renamed the test's subdir fixture from "assets\\q" to "assets[q]": on Windows, backslash is a path separator, so mkdir would create nested "assets/q" dirs instead of one literally-named directory, and iterdir() would only discover "assets", never exercising the rewrite. extension_id keeps a real backslash/"\\1" since it isn't used to create a directory, still verifying the callable replacement handles it literally. Added a sanity assertion for this assumption. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: apply extension subdir path rewrite in skills-mode renderer register_commands() rewrote extension-relative subdir references (agents/, knowledge-base/, etc.) via rewrite_extension_paths(), but _register_extension_skills() - the separate renderer used for active non-native skills agents (e.g. Claude with ai_skills: true) - never called it. Generated SKILL.md files left agents/... and knowledge-base/... unresolved, and mapped the extension's own templates/ through the generic project-level rewrite instead of its installed .specify/extensions/<id>/templates/ location. Reuse the existing rewrite_extension_paths() helper in _register_extension_skills() at the same point register_commands() applies it (before resolve_skill_placeholders' generic rewrite), and add a skills-mode regression test. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: apply extension subdir path rewrite on preset restore/reconcile paths _unregister_skills() restored extension-backed SKILL.md content via resolve_skill_placeholders() without first calling rewrite_extension_paths(), so removing a preset override that shadowed an extension command restored the bare, unresolvable agents/... and knowledge-base/... references. Carried extension_id/extension_dir through _build_extension_skill_restore_index() and applied the same rewrite used at initial registration before restoring. Found the identical gap in _reconcile_composed_commands()'s non-skill agent path: when a removed preset's command reverts to an extension winner, register_commands_for_non_skill_agents() was called without extension_id, so the rewrite never ran for plain command-file agents either. Passed extension_id through there too. Added regression tests for both restore paths (skills-mode and non-skill-agent command files) in tests/test_presets.py. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: apply extension subdir path rewrite when composing over extension base PresetResolver.resolve_content() read the effective base layer's raw content directly via path.read_text() before composing append/prepend/ wrap overlays on top of it, and its outright-replace shortcut did the same. When that base layer was extension-provided, neither read path applied rewrite_extension_paths(), so composing a preset over an extension command (or an extension winning outright through resolve_content) left bare, unresolvable agents/... and knowledge-base/... references in the composed output. All three call sites (PresetManager._register_commands()'s composed path, _reconcile_composed_commands()'s composed path, and skills-mode reading the .composed file written by either) consume resolve_content's return value, so fixing the read at its source covers command output, skill output, and both initial-install and reconcile flows without threading extension identity through each caller. Tagged extension layers in collect_all_layers() with extension_id/ extension_dir, and added a _read_layer_content() helper in resolve_content() that applies rewrite_extension_paths() whenever a layer carries that extension identity — used at both raw-read sites (outright-replace shortcut and composition base). Composing (append/prepend/wrap) layers are never extension-provided (extensions are always inserted with strategy "replace"), so no other read site needs the rewrite. Added regression tests: a parametrized resolve_content() test covering append/prepend/wrap composing over an extension base, a skills-mode test asserting the composed SKILL.md resolves the extension's subdir references, and a non-skill-agent (Gemini) install-time test matching the reported live repro. Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>	2026-07-13 10:50:59 -05:00
Ali jawwad	54ed736479	fix(agents): resolve skill placeholders in Goose (yaml) command output (#3374) ... * fix(agents): resolve skill placeholders in Goose (yaml) command output CommandRegistrar.register_commands resolves {SCRIPT}/__AGENT__ and the $ARGUMENTS placeholder in the markdown and toml branches, but the yaml branch (Goose recipes) called render_yaml_command directly, skipping both. So extension/preset command bodies installed for Goose kept literal {SCRIPT}, __AGENT__, and repo-relative script paths in the generated .goose/recipes/*.yaml prompt. Mirror the markdown/toml branches: run resolve_skill_placeholders + _convert_argument_placeholder on the body before render_yaml_command. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(goose): assert positive placeholder replacements in recipe prompt Per review: parse the generated recipe with yaml.safe_load and assert the prompt contains the resolved values (.specify/scripts/, 'agent goose', {{args}}), not merely that the literal tokens are absent — a wrong output that happens to omit the exact strings would otherwise pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>	2026-07-09 07:48:38 -05:00
Quratulain-bilal	94c7ec288f	fix(toml): escape control characters so generated command files parse (#3341) ... * fix(toml): escape control characters so generated command files parse both toml renderers (TomlIntegration._render_toml_string for gemini/tabnine and CommandRegistrar.render_toml_command for extension/preset commands) wrote control characters raw into a multiline or basic string. toml forbids literal control chars (U+0000-U+001F except tab/newline, and U+007F) in every string form, and a bare CR that is not part of a CRLF pair, so a description or body containing one produced a .toml file that fails to parse. route any value with such a character to a fully-escaped basic string that emits the leftover control chars as \uXXXX. added regression tests that round-trip through tomllib. * refactor(toml): centralize control-char escaping in one shared helper the control-char detection and basic-string escaping added for both toml renderers were copy-pasted into agents.py and integrations/base.py. move the two functions into specify_cli/_toml_string.py and have both renderers delegate to it, so the escaping rules can't drift apart later. no behavior change; both renderers now reference the same implementation.	2026-07-08 07:42:34 -05:00
Roland Huss	d4e7d2b888	feat(integrations): generalize post-processing to all format types (#3311) ... * feat(integrations): add post_process_command_content() hook for all format types Add post_process_command_content(self, content: str) -> str to IntegrationBase with a no-op default. Wire it into register_commands() for non-skills format types (Markdown, TOML, YAML) after format rendering, before writing to disk. Also applies to aliases rendered via the inject_name path (cline, forge). Skills-format agents are excluded to preserve the existing post_process_skill_content() path and avoid double-processing. This gives extension authors a clean per-agent content transformation seam for all 21 non-skills integrations that previously had no post-processing hook. Ref: #3303 Assisted-By: 🤖 Claude Code * fix: initialize _integration before conditional branch Prevents potential UnboundLocalError if the non-skills guard is refactored without updating the alias path reference. Assisted-By: 🤖 Claude Code	2026-07-07 11:35:43 -05:00
Zhiyao Wen	1930f89d17	fix extension-local script path rewriting (#3364) ... Co-authored-by: Zhiyao <zhiyao@ZhiyaodeMacBook-Air.local>	2026-07-07 10:51:50 -05:00
Manfred Riem	53d9543355	feat: make agent-context extension a full opt-in (#3097) ... * docs: add Spec Kit spec for agent-context full opt-in Use Spec Kit's own specify workflow to author the spec that makes the agent-context extension a full opt-in, removing all agent-context configuration/support from the Python codebase and removing the deprecation message. Force-added despite specs/ being gitignored; the generated artifact will be purged prior to merge. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs: add Spec Kit plan artifacts for agent-context full opt-in Phase 0/1 of the SDD plan workflow: plan.md, research.md, data-model.md, quickstart.md, and contracts/cli-behavior.md. Constitution Check is a documented no-op (repo has no ratified constitution). Force-added despite specs/ being gitignored; generated artifacts will be purged prior to merge. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs: correct Constitution Check against ratified v1.0.0 Earlier draft wrongly treated the gate as a no-op; the fork's main is 16 commits behind upstream/main, which carries .specify/memory/constitution.md. Re-evaluate the feature against Principles I-V (all PASS) and note that Principle I mandates keeping context_file as a declared class attribute, validating the R1 metadata decision. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs: refresh plan artifacts against synced upstream/main After syncing fork main to upstream and rebasing, re-scan the current agent-context surface. Upstream generalized the single context_file into a plural context_files concept with new resolver helpers (_resolve_context_files, _resolve_context_file_values, _format_context_file_values) and upsert/remove now loop over multiple files. Update research.md, data-model.md, contracts, quickstart grep guards, and the plan summary to cover the expanded removal scope. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs: add Spec Kit tasks for agent-context full opt-in Phase 2 of SDD: dependency-ordered tasks.md (30 tasks) organized by the three user stories, with mandatory test tasks (Constitution Principle II) and a foundational phase decoupling __CONTEXT_FILE__ resolution from the extension config. Includes the extension self-seeding task (T015) and a static guard test (T002) enforcing zero agent-context references in the CLI. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * feat!: remove agent-context lifecycle from the Specify CLI Make the agent-context extension a full opt-in. The CLI no longer installs the extension during init, writes agent-context-config.yml, or creates/updates/removes the managed Spec Kit section in agent context files. Context-section upsert/remove, marker resolution, extension-enabled gating, the config helpers, and the obsolete inline deprecation warning are all removed. Integration context_file stays as inert metadata; __CONTEXT_FILE__ now resolves from registry metadata. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * feat(agent-context): self-seed context file from the active integration When agent-context-config.yml has no context_file/context_files, the bundled bash and PowerShell update scripts now resolve the context file from the active integration in .specify/init-options.json via the integration registry, so the extension no longer depends on the CLI writing its config. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * test+docs: update suite and docs for agent-context opt-in Update integration/extension tests to expect no agent-context install, config, or context-section writes during init. Add a static guard test (test_agent_context_cli_free.py) asserting the CLI source is free of agent-context lifecycle symbols, plus backward-compatibility tests for legacy projects. Refresh AGENTS.md, the extension README, and add a CHANGELOG entry describing the opt-in behavior change. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(agent-context): warn on self-seed failure, correct docs, speed up guard test Address PR review feedback: - Self-seed scripts (bash + PowerShell) now emit an actionable warning when an active integration is configured but specify_cli cannot be imported by the chosen Python (e.g. pipx installs), or when the integration declares no context file, instead of silently falling through to 'nothing to do'. - Correct the extension README disable note: command rendering never reads the extension config; __CONTEXT_FILE__ is always substituted from integration metadata, so a stale context_files value cannot affect rendering. - Cache CLI source reads in the static guard test via a module-scoped fixture so the directory walk happens once instead of once per forbidden symbol. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * feat(agent-context): ship self-owned per-agent context-file defaults The extension now bundles agent-context-defaults.json (key→context_file map) and self-seeds from it, dropping any dependency on the Specify CLI registry. Both the bash and PowerShell update scripts read the bundled JSON map keyed by the active integration from init-options.json. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * feat!: remove all agent-context state from the Specify CLI Strip every context_file reference from the CLI: the field on all 35 integration classes, the IntegrationBase plumbing (process_template param/step, _context_file_display, docstrings), the __CONTEXT_FILE__ resolution in agents.py, the legacy context_file/context_markers popping in _helpers.py, and the context_file template in integration_scaffold.py. Also drop the Agent context update step and __CONTEXT_FILE__ placeholder from templates/commands/plan.md. The agent-context extension now solely owns all context-file knowledge, including the per-agent default mapping. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * test: drop context_file coverage and guard against CLI reintroduction Remove CONTEXT_FILE attrs and context_file assertions across the base mixins, all 35 per-integration test files, shared integration tests, and conftest stubs. Rewrite the base-mixin context tests to assert no managed section is written and no __CONTEXT_FILE__ placeholder survives. Extend the CLI-free static guard to forbid context_file, __CONTEXT_FILE__, and _context_file_display in src/specify_cli, and have the extension tests copy the bundled defaults JSON so self-seed runs without the CLI. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs: reflect full removal of agent-context state from the CLI Update AGENTS.md (integration examples, required-fields table, context behavior section, pitfalls), CHANGELOG, and the SDD spec artifacts (FR-007, SC-002, data-model) to state that the CLI carries no context_file and the extension fully owns the per-agent default mapping via agent-context-defaults.json. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs: align SDD artifacts with full context_file removal Update research.md (R1, R2, R4, summary table), contracts/cli-behavior.md (C3, C5), tasks.md (Phase 2, T026, notes), plan.md (Principle I, source map), and checklists/requirements.md so the spec artifacts reflect the implemented decision: the CLI carries no context_file attribute or __CONTEXT_FILE__ resolution, and the per-agent defaults map lives in the extension. Resolves PR review #4548130110. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs: scrub stale context-file mentions from CLI docstrings Update the multi_install_safe docstring (drop the removed "context file" invariant), the RovoDev setup docstring (no longer upserts a context section), the Copilot module docstring (drop the context-file line), and tighten the _update_init_options_for_integration note. Pure docstring changes — no behavioral impact. Resolves PR review #4548237085. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * test+docs: harden agent-context test helper and fix stale docs - base.py: document multi_install_safe as an optional subclass attribute in the IntegrationBase docstring. - test_cli.py: clarify the init-options assertion is guarding against leftover legacy agent-context keys, not relocation. - test_extension_agent_context.py: _install_agent_context_config now asserts the bundled agent-context-defaults.json exists and always copies it, so self-seeding tests fail loudly instead of silently skipping when the map is missing. - test_integration_cursor_agent.py: drop Path/IntegrationManifest imports left unused after removing the context-section frontmatter tests. Resolves PR review #4548293116. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * chore: remove gitignored SDD artifacts from specs/ The specs/001-agent-context-full-optin/ artifacts were force-added for dogfooding visibility, but specs/ is gitignored and these were always intended to be purged before merge. Remove them so merging does not add an intentionally-untracked directory to repo history. Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * chore: keep CHANGELOG.md identical to upstream CHANGELOG.md is auto-generated at release time, so the branch should not carry a manual entry. Restore it to match upstream/main exactly. Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: preserve Cursor .mdc frontmatter in agent-context updater scripts The bundled agent-context updater scripts wrote the managed section as plain text. For Cursor-style `.mdc` targets this dropped the required `---\nalwaysApply: true\n---` frontmatter, reintroducing the rule-loading bug originally fixed in #1699. Port the `_ensure_mdc_frontmatter` logic into both the bash and PowerShell updaters: prepend frontmatter when missing, repair `alwaysApply` when set to the wrong value, and leave non-`.mdc` targets untouched. Add regression tests covering both shells. Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * test: scope CLI-free guard to agent-context-specific symbols Drop the bare "context_file" substring from FORBIDDEN_SYMBOLS so the guard no longer fails on unrelated future CLI fields named context_file. The list still covers agent-context-specific identifiers (__CONTEXT_FILE__, _context_file_display, _resolve_context_files, _resolve_context_file_values). Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: harden agent-context bash self-seed against malformed init JSON Two robustness fixes in the embedded Python self-seed logic: - Coerce the integration value from init-options.json to a string only when it is actually a string; otherwise treat it as unset so a corrupted dict/list value degrades to the existing nothing-to-do behavior instead of breaking the agents-map lookup. - Normalize agent-context-defaults.json: only use 'agents' when both the JSON root and the 'agents' value are dicts, so a wrong-shaped (but valid) JSON falls back to the warning path instead of raising on .get. Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: correct PowerShell hyphenated key lookup and regex replace count - Self-seed now reads the defaults mapping via $defaults.agents.PSObject.Properties[$integrationKey].Value instead of member access ($defaults.agents.$integrationKey), which parsed hyphenated keys like 'cursor-agent'/'kiro-cli' as subtraction and failed to resolve. - Replace the static [regex]::Replace(..., 1) call, whose trailing 1 was interpreted as RegexOptions.IgnoreCase rather than a replacement count, with an instance Regex whose Replace(input, replacement, 1) limits to the first match as intended. Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: make bash .mdc frontmatter guard case-insensitive The bash updater only injected Cursor .mdc frontmatter when ctx_path ended in lowercase '.mdc', so a mixed/upper-case extension (e.g. specify-rules.MDC) was skipped and Cursor would not auto-load the rule file. Compare against the casefolded path. The PowerShell variant already uses -match, which is case-insensitive by default, so no change is needed there. Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs: document separator-agnostic agent-context update invocation The README hard-coded the dot-notation slash command (/speckit.agent-context.update), which hyphen-separator agents like Forge and Cline do not recognize. Document the canonical command ID plus both slash invocations so users copy the form their agent accepts. Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>	2026-06-29 15:27:26 -05:00
Ali jawwad	034fbfcbb4	fix: render valid TOML when a command body contains backslashes (#3135) ... render_toml_command() emitted the body inside a multiline *basic* TOML string ("""..."""), which processes backslash escape sequences. A command body containing a backslash — e.g. a Windows path like C:\Users\... whose \U reads as an invalid unicode escape — therefore produced unparseable TOML ("Invalid hex value"), so the generated Gemini/Tabnine command file failed to load. A body ending in a backslash also silently ate the closing newline via TOML line-continuation. Route bodies containing a backslash to the multiline *literal* form ('''...'''), which does not process escapes, or to the escaped basic string when both triple-quote styles are present. Mirrors the escaping already done by base.py's TomlIntegration. Add tests covering a Windows path, a trailing backslash, and the backslash + both-triple-quote-styles fallback. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-06-24 13:13:44 -05:00
JinHyuk Sung	0c975bbef7	fix: write Codex dev skills as files (#2988) ... * fix: write Codex dev skills as files * fix: route codex dev symlink policy through metadata * fix: replace codex dev symlinks on refresh * fix: migrate codex dev skill symlinks * fix: avoid inactive shared skill dev symlinks * fix: preserve unrelated dev skill symlinks	2026-06-23 10:55:38 -05:00
Austin Z.	bbdf1b8f40	fix(agent-context): support multiple context files safely (#2969) ... * fix(agent-context): support multiple context files safely * fix(agent-context): harden context file validation * fix(agent-context): preserve disabled context target * fix(agent-context): address review follow-ups * fix(agent-context): dedupe PowerShell context files * fix(agent-context): align context file dedupe * fix(agent-context): align bash context file dedupe * fix(agent-context): preserve disabled display target * fix(agent-context): require yaml-capable updater python * fix(agent-context): preserve context files config * fix(agent-context): align context file fallbacks * fix(agent-context): share context file resolution --------- Co-authored-by: AustinZ21 <AustinZ21@users.noreply.github.com>	2026-06-22 12:10:55 -05:00
Manfred Riem	902f5431f9	Harden command registration path handling (#3088) ... * fix: validate command 'file' field against path traversal in registrar CommandRegistrar.register_commands() read each command body from source_dir / cmd_file without validating the manifest 'file' field, unlike the parallel skill and preset readers which already reject absolute paths and '..' traversal. A malicious extension/preset/bundle manifest with file: ../../../etc/passwd (or an absolute path) could read arbitrary host files verbatim into a generated agent command at a predictable path (GHSA-w5fv-7w9x-7fc5, CWE-22). Add the same containment guard at the command read site and reject a traversal/absolute 'file' at manifest-load time in ExtensionManifest._validate() for defense-in-depth, plus regression tests for both the read path and the manifest validator. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * test/fix: address review — robust absolute-path test and tolerant reads - register_commands(): use is_file() instead of exists() and skip the command if read_text() raises (directory or non-UTF8 file), aligning with the other command/skill readers. - Traversal tests: point the absolute-path payload at the real temp secret.txt (guaranteed to exist on all platforms) instead of /etc/passwd, so the absolute-path guard is genuinely exercised and the test fails if it regresses, rather than passing because the target happens not to exist (e.g. on Windows runners). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * test: rename traversal fixtures to avoid CodeQL secret-storage false positive The regression fixtures named an out-of-tree file secret.txt with TOP-SECRET-CREDENTIAL content. CodeQL's clear-text-storage heuristic treated that read content as sensitive and followed the static path into the pre-existing write_text sinks in _write_registered_output, raising false 'clear-text storage of sensitive information' alerts on PR 3088. Rename the fixtures to neutral outside.txt / OUTSIDE-FILE-MARKER and drop /etc/passwd payloads; the test semantics (a file outside source_dir must never be read into a generated command) are unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: reject Windows drive-relative 'file' values in traversal guards is_absolute() is False for Windows drive-relative paths like C:outside.txt, which contain no '..' yet resolve against the process CWD on that drive — bypassing the containment guard on Windows. Evaluate the 'file' value under PureWindowsPath as well so both the registrar runtime guard and the manifest-load validator reject drive letters (and backslash '..' segments) cross-platform. Extend the regression tests with drive-relative cases. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: use anchor under both path flavors so POSIX-absolute is rejected on Windows On a Windows runner WindowsPath('/abs/outside.md').is_absolute() is False (no drive), so the prior native-Path check let a leading-slash 'file' value through and the manifest validator did not raise. Evaluate the value under both PurePosixPath and PureWindowsPath and reject any non-empty anchor — covering POSIX-absolute, Windows drive-relative, Windows absolute, and rooted-without-drive — in both the registrar guard and the manifest validator. The registrar join now uses the raw 'file' string so native separators are handled by the resolve()/relative_to() containment check. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: validate command 'file' field against path traversal in registrar CommandRegistrar.register_commands() read each command body from source_dir / cmd_file without validating the manifest 'file' field, unlike the parallel skill and preset readers which already reject absolute paths and '..' traversal. A malicious extension/preset/bundle manifest with file: ../../../etc/passwd (or an absolute path) could read arbitrary host files verbatim into a generated agent command at a predictable path (GHSA-w5fv-7w9x-7fc5, CWE-22). Add the same containment guard at the command read site and reject a traversal/absolute 'file' at manifest-load time in ExtensionManifest._validate() for defense-in-depth, plus regression tests for both the read path and the manifest validator. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * test/fix: address review — robust absolute-path test and tolerant reads - register_commands(): use is_file() instead of exists() and skip the command if read_text() raises (directory or non-UTF8 file), aligning with the other command/skill readers. - Traversal tests: point the absolute-path payload at the real temp secret.txt (guaranteed to exist on all platforms) instead of /etc/passwd, so the absolute-path guard is genuinely exercised and the test fails if it regresses, rather than passing because the target happens not to exist (e.g. on Windows runners). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * test: rename traversal fixtures to avoid CodeQL secret-storage false positive The regression fixtures named an out-of-tree file secret.txt with TOP-SECRET-CREDENTIAL content. CodeQL's clear-text-storage heuristic treated that read content as sensitive and followed the static path into the pre-existing write_text sinks in _write_registered_output, raising false 'clear-text storage of sensitive information' alerts on PR 3088. Rename the fixtures to neutral outside.txt / OUTSIDE-FILE-MARKER and drop /etc/passwd payloads; the test semantics (a file outside source_dir must never be read into a generated command) are unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: reject Windows drive-relative 'file' values in traversal guards is_absolute() is False for Windows drive-relative paths like C:outside.txt, which contain no '..' yet resolve against the process CWD on that drive — bypassing the containment guard on Windows. Evaluate the 'file' value under PureWindowsPath as well so both the registrar runtime guard and the manifest-load validator reject drive letters (and backslash '..' segments) cross-platform. Extend the regression tests with drive-relative cases. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: use anchor under both path flavors so POSIX-absolute is rejected on Windows On a Windows runner WindowsPath('/abs/outside.md').is_absolute() is False (no drive), so the prior native-Path check let a leading-slash 'file' value through and the manifest validator did not raise. Evaluate the value under both PurePosixPath and PureWindowsPath and reject any non-empty anchor — covering POSIX-absolute, Windows drive-relative, Windows absolute, and rooted-without-drive — in both the registrar guard and the manifest validator. The registrar join now uses the raw 'file' string so native separators are handled by the resolve()/relative_to() containment check. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * refactor: harden register_commands inputs and tighten manifest 'file' validation Address review feedback on #3088: - register_commands(): skip non-string/empty 'file' values instead of raising TypeError, and hoist source_dir.resolve() out of the per-command loop. - ExtensionManifest._validate(): reject 'file' values with leading/trailing whitespace with a clear ValidationError instead of a confusing missing-file failure later. - tests: add non-string 'file' and whitespace cases; use yaml.safe_dump with explicit utf-8 encoding in the manifest validation test. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * refactor: align runtime '..' policy, correct comment, dedupe test helper Address review feedback on #3088: - register_commands(): also reject '..' segments under both POSIX and Windows semantics, keeping runtime policy consistent with ExtensionManifest._validate() and the skill/preset readers (not just relying on the resolve()/relative_to() containment backstop). - Replace the version-dependent is_absolute() claim in the extensions.py comment with the actual portability rationale (native Path is OS- dependent; C:foo is anchored but not absolute). - Extract the duplicated leak-detection assertion into _assert_no_marker_leak() and add an in-bounds '..' payload that exercises the new runtime '..' rejection. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Extract shared path-safety policy and warn on unreadable command files Introduce relative_extension_path_violation() in _utils.py as the single source of truth for the extension-relative `file` path-safety policy, and use it from both the runtime registrar guard (agents.py) and the manifest-load validator (extensions.py) so the two cannot drift. Warn (instead of silently skipping) when an in-bounds command file exists but cannot be read/decoded, surfacing misconfigured extensions. Add unit tests for the shared helper, a read-skip warning test, and make the in-bounds `..` test create its target file so the skip is attributable to the `..` rejection rather than file absence. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Retrigger CI Empty commit to re-trigger code scanning / CodeQL analysis on the PR merge ref. Assisted-by: GitHub Copilot CLI (model: Claude Opus 4.8, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>	2026-06-22 10:25:29 -05:00
Ali jawwad	f9c6cf83e5	fix(presets): preserve argument-hint in preset SKILL.md generation (#2978) ... * fix(presets): preserve argument-hint in preset SKILL.md generation Preset-provided and extension-override commands that declare `argument-hint:` in their frontmatter had it dropped from the generated Claude SKILL.md, and it was re-dropped when a preset was removed and its overridden skill restored. This is the preset-side analog of the extension fix in #2903 / #2916. Factor the argument-hint carry-over into a shared CommandRegistrar.apply_argument_hint() helper and apply it at the four preset skill-generation sites (register, reconcile override-restore, and the core/extension unregister-restore paths). The extension path from The helper writes argument-hint into the frontmatter dict before serialization (so a folded multi-line description cannot be split into invalid YAML) and only for integrations that support it (those exposing inject_argument_hint -- currently Claude), leaving build_skill_frontmatter's shared shape unchanged for every other agent. Core templates carry no argument-hint, so the core-restore path is a no-op. No behavior change for non-Claude agents or the core path. Add regression tests covering a folding description (Claude) and the non-Claude gate (codex). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(presets): address review - guard skill_frontmatter type and tighten apply_argument_hint annotations Add a symmetric isinstance(skill_frontmatter, dict) guard so the helper stays a safe no-op if a caller passes a non-dict, and annotate the parameters as Dict[str, Any] with an optional integration to match real call-site usage. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-06-22 09:52:13 -05:00
minbang	a9a759450d	fix: recover active skills registration for extensions (#2803) ... Extension command registration now resolves the active skills directory before writing command artifacts. This lets initialized skills-backed agents recover a missing active skills directory while preserving the existing preset registration behavior. Add regression coverage for missing active skills directories, shared skills directories, and symlinked parent guards. Fixes #2769. Co-authored-by: OpenAI Codex <codex@openai.com>	2026-06-04 09:53:31 -05:00
Pedro Barbosa	44aac9f6e4	feat: add native Cline integration (#2508) ... * test: strip ansi to make asserts work * feat: add native Cline integration	2026-06-01 11:20:48 -05:00
Copilot	50da3a0f77	Extract agent context updates into bundled agent-context extension (#2546) ... * Initial plan * Extract agent context updates into bundled agent-context extension * Potential fix for pull request finding 'Unused import' Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com> * Potential fix for pull request finding 'Unused import' Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com> * fix: address review comments on agent-context extension - bash: parse init-options.json with a single python3 invocation instead of three separate read_json_field calls, for parity with the PowerShell ConvertFrom-Json approach and to avoid divergent error semantics - bash: use parameter expansion to strip PROJECT_ROOT prefix from plan path instead of sed interpolation, avoiding special-character fragility - powershell: limit Get-ChildItem to -Depth 1 so plan.md discovery matches the bash glob specs/*/plan.md (one level deep) — fixes cross-platform inconsistency with nested plan.md files - powershell: replace Substring+Length relative-path with [System.IO.Path]::GetRelativePath for robustness across case/PSDrive differences - __init__.py: move agent-context extension install to after save_init_options so init-options.json is present when hooks run - __init__.py: seed context_markers in init-options only when context_file is truthy; avoids noise for integrations without a context file - integrations/base.py: narrow blanket except Exception in _resolve_context_markers to ImportError / (OSError, ValueError) so unexpected bugs surface instead of being silently swallowed * fix: gate context_markers in _update_init_options_for_integration on context_file Apply the same gating logic used during `specify init`: only write context_markers to init-options.json when the integration actually has a context_file set. When switching to an integration without a context file the stale markers are removed, keeping the two init paths consistent. * fix: move context_file/context_markers from init-options.json to agent-context extension config * Potential fix for pull request finding 'Unused global variable' Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com> * fix: clarify local import comment in agents.py * Fix remaining agent-context review findings * Fix follow-up agent-context review issues * Address review feedback: narrow except, improve PyYAML messaging, surface config-written note * Fix double-space in PyYAML install hint message * Potential fix for pull request finding 'Empty except' Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com> * Potential fix for pull request finding 'Empty except' Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com> * Address latest agent-context review feedback * Harden bash config parse output handling * Clarify ImportError-only fallback comment * Apply review feedback: drop dead try/except, guard ext-config creation, explicit ConvertFrom-Yaml check * Remove redundant $Options = $null in PS1 catch block * Add constitution directives, deprecation warning, agent-context auto-install, and init flow fix - Add constitution-loading directive to specify, clarify, tasks, checklist, taskstoissues commands - Add deprecation warning (v0.12.0) in upsert_context_section() - Auto-install agent-context extension during specify init - Move context_file from init-options.json to agent-context extension config - Add tests: deprecation warning, corrupt config, constitution directives - Update file inventories across all integration tests * Address review: fix init ordering, test coverage, and hermes inventory - Move agent-context extension install after init-options.json is saved so skill registration can read ai_skills + integration key - Write extension config after install (avoids template overwriting context_file) - Fix test_defaults_when_markers_field_missing to truly test missing markers key - Update hermes tests to allow extension-installed agent-context skill * Address review: chmod ordering, preserve markers, PS1 Python check, YAML key order - Move ensure_executable_scripts after agent-context extension install so extension scripts get execute bits set - Use preserve_markers=True on reinit to keep user-customized markers - Add Python 3 version check in PowerShell fallback (matching bash behavior) - Add sort_keys=False to yaml.safe_dump for stable config output * Address review: path traversal guards and docstring fix - Reject absolute paths and '..' segments in context_file in both bash and PowerShell scripts to prevent writes outside the project root - Fix docstring in _update_init_options_for_integration to accurately describe marker preservation behavior * Address review: strict enabled check, docstring, segment-level path traversal - Use 'is not False' for enabled check so only literal False disables - Update upsert_context_section docstring to mention disabled-extension return - Fix path traversal guards to check actual path segments, not substrings (allows filenames like 'notes..md' while rejecting '../' traversal) * Address review: UnicodeError handling, missing extension warning - Add UnicodeError to exception tuples in _load_agent_context_config and _resolve_context_markers so garbled UTF-8 config files fall back to defaults - Emit error (with reinstall command) instead of silent skip when bundled agent-context extension is not found during init * Address review: bash backslash traversal guard, wheel packaging - Reject backslash separators and Windows drive-letter paths in bash context_file validation (prevents traversal on Git-Bash/Windows) - Add extensions/agent-context to pyproject.toml force-include so the bundled extension is included in wheel builds * Address review: write extension config before init-options.json - Reorder writes in _update_init_options_for_integration so the agent-context extension config is updated first; if it fails, init-options.json remains consistent with the previous state --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Manfred Riem <15701806+mnriem@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>	2026-05-30 06:37:18 -05:00
NgoQuocViet2001	9307093d8a	Fix --dev extension agent symlinks (#2554) ... * Fix dev extension agent symlinks * Address dev symlink review feedback * fix: handle dev symlink relpath failures * fix: fall back when dev cache writes fail * test: cover dev symlink fallback without privileges	2026-05-28 09:29:17 -05:00
Dave Majors Stark	5a50b75adb	feat: add Hermes Agent integration (with review fixes) (#2651) ... * feat: add Hermes Agent integration * feat: add Hermes Agent integration * feat: add Hermes Agent integration * feat: add Hermes Agent integration (with review fixes) - Full SkillsIntegration subclass with dual install strategy (project-local .hermes/skills/ + global ~/.hermes/skills/) - CLI fix: integration_uninstall now calls integration.teardown() instead of manifest.uninstall() directly, allowing custom cleanup - Fix Copilot review issues: - Docstring now reflects both -Q (quiet) and -q (query) flags - Empty command guard prevents passing empty skill names - Add catalog entry for hermes in integrations/catalog.json Co-authored-by: Zhaoxiaoguang001 <3357983213@qq.com> * feat: write Hermes skills directly to global ~/.hermes/skills/ Hermes loads skills from the global ~/.hermes/skills/ directory, not from project-local paths. The old dual-install strategy copied SKILL.md files to both locations — project-local (for manifest tracking) and global (for Hermes discovery). This change removes the project-local copies entirely: - setup() writes directly to ~/.hermes/skills/speckit-*/SKILL.md - An empty .hermes/skills/ marker directory is created in the project so extension commands (e.g. git) can detect Hermes as an active integration via register_commands_for_all_agents() - teardown() cleans both the global speckit-* dirs and the local marker - import yaml moved to local import inside setup() Tests updated: Hermes-specific tests now assert global skill location, and shared SkillsIntegrationTests that assumed project-local files are overridden with Hermes-appropriate assertions. Co-authored-by: Zhaoxiaoguang001 <3357983213@qq.com> * fix: address Copilot review feedback on Hermes integration Addresses all 6 review comments from copilot-pull-request-reviewer: 1. Hard-fail on missing integration key → fall back to manifest.uninstall() with a warning instead of raising an error. Allows users to always remove stale integration files even when the integration class is missing from the registry. 2. HOME isolation in tests → every test that calls setup() or CliRunner now monkeypatches Path.home() to a temp directory, keeping the test suite hermetic and non-destructive. 3. HermesIntegration.teardown() now delegates to manifest.uninstall() for project-local tracked files (scripts, manifest), merging results with global cleanup. 4. Global skills cleanup gated behind force=True to avoid destroying speckit-* skills shared across multiple Spec Kit projects when running 'specify integration uninstall hermes' without --force. 5. Line 160 isolation (CLI test test_complete_file_inventory_sh). 6. Line 258 isolation (Path.home assertion in test_ai_hermes_without_ai_skills_auto_promotes). * fix: address second Copilot review round — 6 remaining observations - Move to module scope (was inside per-template loop) - Add safety checks in setup() matching standard - Fix docstrings: global skills always removed on uninstall (standard) - Fix removal tracking: only report after successful rmtree - Override shared test_modified_file_survives_uninstall with Hermes-appropriate behaviour (global skills always removed, no hash tracking) - Update PR description to match implementation (global-only skills + marker) * fix: add first-class global/home-based agent dir support in CommandRegistrar Resolves Copilot HIGH concern (discussion_r3312194525): HermesIntegration.registrar_config.dir was '.hermes/skills' (project- relative), but skills live in ~/.hermes/skills/ (global). Extensions and presets registering commands for the 'hermes' agent via CommandRegistrar would write to the project-local marker directory instead of the real global skills directory, making those commands invisible to Hermes. Fix consists of three parts: 1. CommandRegistrar._resolve_agent_dir now supports '~/'-prefixed and absolute paths in agent_config['dir']. Relative paths still resolve against project_root as before — zero change for existing agents (Claude, Codex, Gemini, etc.). 2. HermesIntegration.registrar_config.dir changed from '.hermes/skills' to '~/.hermes/skills', so extensions/presets write directly to the global directory Hermes searches at runtime. 3. Two inline project_root / agent_config['dir'] calls in the extension update backup/restore paths (src/specify_cli/__init__.py) now delegate to _resolve_agent_dir, giving them the same global-dir support plus the legacy_dir fallback they were missing (improvement for all agents). Test side-effect: test_update_failure_rolls_back_registry_hooks_and_commands was constructing verification paths with project_dir / '~/.hermes/skills' (literal tilde) — fixed to use _resolve_agent_dir and monkeypatch Path.home() so Hermes' global dir doesn't leak into the real filesystem. * fix: address remaining 3 Copilot review observations (round 3) - teardown docstring: clarify marker removal is conditional (if empty) - test_pre_existing_skills_not_removed: now actually calls teardown() to verify foreign skills survive uninstall (was only running setup) - integration_switch Phase 1: replaced old_manifest.uninstall() + remove_context_section() with current_integration.teardown(), matching the pattern already used in integration_uninstall. This ensures custom teardown logic (e.g. Hermes global skills cleanup) runs during switches. * fix: address Copilot round 4 — home-relative dir resolution + project-local detection 1. _resolve_agent_dir(): expand ~/... via Path.home() + slice instead of expanduser(), so tests that monkeypatch Path.home() properly isolate the home directory (Copilot r3312731595, r3312731729) 2. Add detect_dir field to registrar_config: Hermes declares detect_dir='.hermes/skills' (project-local marker). CommandRegistrar checks detect_dir before resolving the output dir, preventing global dirs like ~/.hermes/skills from causing false detection in every project (Copilot r3312731682) 3. test_update_failure_rolls_back: no additional changes needed — the _resolve_agent_dir fix makes the existing Path.home() monkeypatch effective, so ~/.hermes/skills is not found in the fake home and Hermes is properly skipped. Tests: 2236 passed (2009 integration + 195 extension + 32 hermes) --------- Co-authored-by: Zhaoxiaoguang001 <3357983213@qq.com> Co-authored-by: majordave <majordave@users.noreply.github.com>	2026-05-28 09:04:03 -05:00
Marcus Burghardt	9732a4d092	fix(opencode): use commands/ directory (plural) to match OpenCode docs (#2453) ... * fix(opencode): use commands/ directory (plural) to match OpenCode docs OpenCode documentation (https://opencode.ai/docs/commands/) uses .opencode/commands/ (plural) as the canonical command directory. The OpenCode runtime supports both .opencode/command/ and .opencode/commands/ via a {command,commands} glob, but the singular form was the original convention and is now outdated. Update the OpenCode integration to write to .opencode/commands/ instead of .opencode/command/, aligning with the documented standard and the OpenSpec fix (Fission-AI/OpenSpec#748). Signed-off-by: Marcus Burghardt <maburgha@redhat.com> Assisted-by: OpenCode (claude-opus-4-6) * feat(registrar): add legacy_dir fallback for backward-compatible directory migration Add _resolve_agent_dir() to CommandRegistrar that checks a legacy_dir fallback when the canonical directory does not exist. When legacy_dir is found, a deprecation warning directs users to run "specify integration upgrade" to migrate. The OpenCode integration declares legacy_dir: ".opencode/command" so that extension and preset registration, as well as command cleanup, continue working for projects that have not yet migrated to .opencode/commands/. The legacy_dir mechanism is opt-in: integrations that do not declare it get no fallback and no behavioral change. Add end-to-end test verifying that "specify integration upgrade opencode" migrates commands from legacy .opencode/command/ to canonical .opencode/commands/ and removes stale files. Signed-off-by: Marcus Burghardt <maburgha@redhat.com> Assisted-by: OpenCode (claude-opus-4-6) * fix(registrar): address PR review feedback on legacy_dir handling - Fix deprecation warning formatting: quote paths and remove trailing '/.' that produced confusing '.opencode/commands/.' output - Eliminate duplicate warnings: pass pre-resolved directory to register_commands() via _resolved_dir parameter so _resolve_agent_dir() is only called once per agent - Fix unregister_commands() to clean both canonical and legacy dirs when both exist, preventing orphaned command files after upgrade - Add test_unregister_cleans_legacy_when_both_dirs_exist regression test and tighten warning count assertion to exactly 1 Assisted-by: OpenCode (claude-opus-4-6) Signed-off-by: Marcus Burghardt <maburgha@redhat.com> --------- Signed-off-by: Marcus Burghardt <maburgha@redhat.com>	2026-05-13 09:55:56 -05:00
Eric Rodriguez Suazo	793632089a	fix(forge): use hyphen notation for command refs in Forge integration (#2462) ... * fix(forge): use hyphen notation for command refs in Forge integration - Add invoke_separator = "-" class attribute to ForgeIntegration so effective_invoke_separator() returns "-" for shared-template installs - Add "invoke_separator": "-" to ForgeIntegration.registrar_config so agents.py CommandRegistrar can resolve refs with the correct separator - Pass invoke_separator to process_template() in ForgeIntegration.setup() so all .forge/commands/*.md bodies use /speckit-foo notation - Replace literal /speckit.specify with __SPECKIT_COMMAND_SPECIFY__ in extensions/git/commands/speckit.git.feature.md so every agent resolves the reference through its own separator - Apply resolve_command_refs re.sub in agents.py register_commands() after argument-placeholder substitution so extension commands registered for Forge get /speckit-foo refs; all other agents continue to get /speckit.foo Fixes ZSH compatibility: dot-notation command invocations (/speckit.specify) are misinterpreted by ZSH as file-path operations; hyphen notation (/speckit-specify) works correctly in all shells. * fix(agents): propagate invoke_separator from integration class into AGENT_CONFIGS Skills-based agents (claude, codex, kimi, …) inherit invoke_separator="-" from SkillsIntegration but do not repeat it in their registrar_config dicts. _build_agent_configs() was copying registrar_config verbatim, so register_commands() fell back to "." when resolving __SPECKIT_COMMAND_*__ tokens for those agents — emitting /speckit.specify instead of the correct /speckit-specify for extension commands like speckit.git.feature. Fix: after copying registrar_config, inject invoke_separator from the integration's class attribute when it is not already declared explicitly. This makes the integration class the single source of truth for all agents, without requiring each SkillsIntegration subclass to duplicate the field. Also replace the inline re.sub in register_commands() with a call to IntegrationBase.resolve_command_refs() (deferred import to avoid the existing circular dependency) so token-resolution logic is not duplicated. Adds two tests in test_agent_config_consistency.py: - test_skills_agents_have_hyphen_invoke_separator_in_agent_configs: asserts every /SKILL.md agent has invoke_separator="-" in AGENT_CONFIGS. - test_skills_agent_command_token_resolves_with_hyphen: end-to-end check via CommandRegistrar that the git extension's speckit.git.feature command is installed for Claude with /speckit-specify (not /speckit.specify). Addresses review comment on PR #2462.	2026-05-06 12:19:10 -05:00
Copilot	a067d4c2e3	feat(presets): Composition strategies (prepend, append, wrap) for templates, commands, and scripts (#2133) ... * fix: rebase onto upstream/main, resolve conflicts with PR #2189 upstream/main merged PR #2189 (wrap-only strategy) which overlaps with our comprehensive composition strategies (prepend/append/wrap). Resolved conflicts keeping our implementation as source of truth: - README: keep our future considerations (composition is now fully implemented, not a future item) - presets.py: keep our composition architecture (_reconcile_composed_commands, collect_all_layers, resolve_content) while preserving #2189's _substitute_core_template which is used by agents.py for skill generation - tests: keep both test sets (our composition tests + #2189's wrap tests), removed TestReplayWrapsForCommand and TestInstallRemoveWrapLifecycle which test the superseded _replay_wraps_for_command API; our composition tests cover equivalent scenarios - Restored missing _unregister_commands call in remove() that was lost during #2189 merge * fix: re-create skill directory in _reconcile_skills after removal After _unregister_skills removes a skill directory, _register_skills skips writing because the dir no longer passes the is_dir() check. Fix by ensuring the skill subdirectory exists before calling _register_skills so the next winning preset's content gets registered. Fixes the Claude E2E failure where removing a top-priority override preset left skill-based agents without any SKILL.md file. * fix: address twenty-third round of Copilot PR review feedback - Protect reconciliation in remove(): wrap _reconcile_composed_commands and _reconcile_skills in try/except so failures emit a warning instead of leaving the project in an inconsistent state - Protect reconciliation in install(): same pattern for post-install reconciliation so partial installs don't lack cleanup - Inherit scripts/agent_scripts from base frontmatter: when composing commands, merge scripts and agent_scripts keys from the base command's frontmatter into the top layer's frontmatter if missing, preventing composed commands from losing required script references - Add tier-5 bundled core fallback to collect_all_layers(): check the bundled core_pack (wheel) or repo-root templates (source checkout) when .specify/templates/ doesn't contain the core file, matching resolve()'s tier-5 fallback so composition can always find a base layer * fix: address twenty-fourth round of Copilot PR review feedback - Use yaml.safe_load for frontmatter parsing in resolve_content instead of CommandRegistrar.parse_frontmatter which uses naive find('---',3); strip strategy key from final frontmatter to prevent leaking internal composition directives into rendered agent command files - Filter _reconcile_skills to specific commands: use _FilteredManifest wrapper so only the commands being reconciled get their skills updated, preventing accidental overwrites of other commands' skills that may be owned by higher-priority presets * fix: address twenty-fifth round of Copilot PR review feedback - Support legacy command-frontmatter strategy: when preset.yml doesn't declare a strategy, check the command file's YAML frontmatter for strategy: wrap as a fallback so legacy wrap presets participate in composition and multi-preset chaining - Guard skill dir creation in _reconcile_skills: only re-create the skill directory if the skill was previously managed (listed in some preset's registered_skills), avoiding creation of new skill dirs that _register_skills would normally skip * fix: add explanatory comment to empty except in legacy frontmatter parsing * fix: address twenty-sixth round of Copilot PR review feedback - Unregister stale commands when composition fails: when resolve_content returns None during reconciliation (base layer removed), unregister the command from non-skill agents and emit a warning - Load extension aliases during reconciliation: _register_command_from_path now checks extension.yml for aliases when the winning layer is an extension, so alias files are restored after preset removal - Use line-based fence detection for legacy frontmatter strategy fallback: scan for --- on its own line instead of split('---',2) to avoid mis-parsing YAML values containing --- * fix: address twenty-seventh round of Copilot PR review feedback - Handle non-preset winners in _reconcile_skills: when the winning layer is core/extension/project-override, restore skills via _unregister_skills so skill-based agents stay consistent with the priority stack - Update base_frontmatter_text on replace layers: when a higher-priority replace layer occurs during composition, update both top and base frontmatter so scripts/agent_scripts inheritance reflects the effective base beneath the top composed layer * fix: address twenty-eighth round of Copilot PR review feedback - Parse only interior lines in _parse_fm_yaml: use lines[1:-1] instead of filtering all --- lines, preventing corruption when YAML values contain a line that is exactly --- - Omit empty frontmatter: skip re-rendering when top_fm is empty dict to avoid emitting ---/{}/--- for intentionally empty frontmatter - Update scaffold wrap comment: mention both {CORE_TEMPLATE} and $CORE_SCRIPT placeholders for templates/commands vs scripts - Clarify shell composition scope in ARCHITECTURE.md: note that bash/PS1 resolve_template_content only handles templates; command/script composition is handled by the Python resolver * fix: address twenty-ninth round of Copilot PR review feedback - Fix TestCollectAllLayers docstring: reference collect_all_layers() - Add default/unknown strategy handling in bash/PS1 composition: error on unrecognized strategy values instead of silently skipping - Fix comment: .composed/ is a persistent dir, not temporary - Fix comment: legacy fallback checks all valid strategies, not just wrap - Cache PresetRegistry in _reconcile_skills: build presets_by_priority once instead of constructing registry per-command * fix: address thirtieth round of Copilot PR review feedback - Guard legacy frontmatter fallback: only check command file frontmatter for strategy when the manifest entry doesn't explicitly include the strategy key, preventing override of manifest-declared strategies - Document rollback limitation: note that mid-registration failures may leave orphaned agent command files since partial progress isn't captured by the local vars * fix: handle project override skills and extension context in reconciliation * fix: add comment to empty except in extension registration fallback * fix: filter extension commands in reconciliation and fix type annotation * fix: filter extension commands from post-install reconciliation Apply the same extension-installed check used in _register_commands to the reconciliation command list, preventing reconciliation from registering commands for extensions that are not installed. * fix: skip convention fallback for explicit file paths and add stem fallback to tier-5 When a preset manifest provides an explicit file path that does not exist, skip the convention-based fallback to avoid masking typos. Also add speckit.<stem> to <stem>.md fallback in tier-5 bundled/source core lookup for consistency with tier-4. * fix: scan past non-replace layers to find base in resolve_content The base-finding scan now skips non-replace layers below a replace layer instead of stopping at the first non-replace. This fixes the case where a low-priority append/prepend layer sits below a replace that should serve as the base for composition. * fix: add context_note to non-skill agent registration for extensions Add context_note parameter to register_commands_for_non_skill_agents and pass extension name/id during reconciliation so rendered command files preserve the extension-specific context markers. * fix: Optional type, rollback safety, and override skill restoration - Fix context_note type to Optional[str] - Wrap shutil.rmtree in try/except during install rollback - Separate override-backed skills from core/extension in _reconcile_skills * fix: align bash/PS1 base-finding with Python resolver Rewrite bash and PowerShell composition loops to find the effective base replace layer first (scanning bottom-up, skipping non-replace layers below it), then compose only from the base upward. This prevents evaluation of irrelevant lower layers (e.g. a wrap with no placeholder below a replace) and matches resolve_content behavior. * fix: PS1 no-python warning, integration hook for override skills, alias cleanup - Warn when no Python 3 found in PS1 and presets use composition strategies - Apply post_process_skill_content integration hook when restoring override-backed skills so agent-specific flags are preserved - Unregister command aliases alongside primary name when composition fails to prevent orphaned alias files * fix: include aliases in removed_cmd_names during preset removal Read aliases from preset manifest before deleting pack_dir so alias command files are included in unregistration and reconciliation. * fix: add comment to empty except in alias extraction during removal * fix: scan top-down for effective base in all resolvers Change base-finding to scan from highest priority downward to find the nearest replace layer, then compose only layers above it. Prevents evaluation of irrelevant lower layers (e.g. a wrap without placeholder below a higher-priority replace) across Python, bash, and PowerShell. * fix: align CLI composition chain display with top-down base-finding Show only contributing layers (base and above) in preset resolve output, matching resolve_content top-down semantics. Layers below the effective base are omitted since they do not contribute. * fix: guard corrupted registry entries and make manifest authoritative - Add isinstance(meta, dict) guard in bash registry parsing so corrupted entries are skipped instead of breaking priority ordering - Only use convention-based file lookup when the manifest does not list the requested template, making preset.yml authoritative and preventing stray on-disk files from creating unintended layers * fix: align resolve() with manifest file paths and match extension context_note - Update resolve() preset tier to consult manifest file paths before convention-based lookup, matching collect_all_layers behavior - Use exact extension context_note format matching extensions.CommandRegistrar - Update test to declare template in manifest (authoritative manifest) * revert: restore resolve() convention-based behavior for backwards compatibility resolve() is the existing public API used by shell scripts and other callers. Changing it to manifest-authoritative breaks backward compat for presets that rely on convention-based file lookup. Only the new collect_all_layers/resolve_content path uses manifest-authoritative logic. * fix: only pre-compose when this preset is the top composing layer Skip composition in _register_commands when a higher-priority replace layer already wins for the command. Register the raw file instead and let reconciliation write the correct final content. * fix: deduplicate PyYAML warnings and use self.registry in reconciliation - Emit PyYAML-missing warning once per function call in bash/PS1 instead of per-preset to avoid spamming stderr - Use self.registry.list_by_priority() in reconciliation methods instead of constructing new PresetRegistry instances to avoid redundant I/O and potential consistency issues * fix: document strategy handling consistency between layers and registrar Composed output already strips strategy from frontmatter (resolve_content pops it). Raw file registration preserves legacy frontmatter strategy for backward compat; reconciliation corrects the final state. * fix: correct stale comments for alias tracking and base-finding algorithm * security: validate manifest file paths in bash/PowerShell resolvers Reject absolute paths and parent directory traversal (..) in the manifest-declared file field before joining with the preset directory. Matches the Python-side validation in PresetManifest._validate(). --------- Co-authored-by: Manfred Riem <15701806+mnriem@users.noreply.github.com>	2026-04-23 10:07:52 -05:00
swithek	ecb3b94b43	fix: resolve skill placeholders for all SKILL.md agents, not just codex/kimi (#2313) ... * fix: resolve skill placeholders for all SKILL.md agents, not just codex/kimi * chore: remove unused NATIVE_SKILLS_AGENTS constant	2026-04-22 15:12:44 -05:00
Kennedy	22e76995c7	feat: implement preset wrap strategy (#2189) ... * feat: implement strategy: wrap * fix: resolve merge conflict for strategy wrap correctness * feat: multi-preset composable wrapping with priority ordering Implements comment #4 from PR review: multiple installed wrap presets now compose in priority order rather than overwriting each other. Key changes: - PresetResolver.resolve() gains skip_presets flag; resolve_core() wraps it to skip tier 2, preventing accidental nesting during replay - _replay_wraps_for_command() recomposed all enabled wrap presets for a command in ascending priority order (innermost-first) after any install or remove - _replay_skill_override() keeps SKILL.md in sync with the recomposed command body for ai-skills-enabled projects - install_from_directory() detects strategy: wrap commands, stores wrap_commands in the registry entry, and calls replay after install - remove() reads wrap_commands before deletion, removes registry entry before rmtree so replay sees post-removal state, then replays remaining wraps or unregisters when none remain Tests: TestResolveCore (5), TestReplayWrapsForCommand (5), TestInstallRemoveWrapLifecycle (5), plus 2 skill/alias regression tests * fix: resolve extension commands via manifest file mapping PresetResolver.resolve_extension_command_via_manifest() consults each installed extension.yml to find the actual file declared for a command name, rather than assuming the file is named <cmd_name>.md. This fixes _substitute_core_template for extensions like selftest where the manifest maps speckit.selftest.extension → commands/selftest.md. Resolution order in _substitute_core_template is now: 1. resolve_core(cmd_name) — project overrides win, then name-based lookup 2. resolve_extension_command_via_manifest(cmd_name) — manifest fallback 3. resolve_core(short_name) — core template short-name fallback Path traversal guard mirrors the containment check already present in ExtensionManager to reject absolute paths or paths escaping the extension root. * fix: add bundled core_pack as Priority 5 in PresetResolver.resolve() resolve_core() was returning None for built-in commands (implement, specify, etc.) because PresetResolver only checked .specify/templates/ commands/ (Priority 4), which is never populated for commands in a normal project. strategy:wrap presets rely on resolve_core() to fetch the {CORE_TEMPLATE} body, so the wrap was silently skipped and SKILL.md was never updated. Priority 5 now checks core_pack/commands/ (wheel install) or repo_root/templates/commands/ (source checkout), mirroring the pattern used by _locate_core_pack() elsewhere. Updated two tests whose assertions assumed resolve_core() always returned None when .specify/templates/commands/ was absent. * fix: harden preset wrap replay removal * fix: stabilize existing directory error output * fix: track outermost_pack_id from contributing preset; use Path.parts in tests - outermost_pack_id now updates alongside outermost_frontmatter inside the wrap loop, so it reflects the actual last contributing preset rather than always taking wrap_presets[0] (which may have been skipped) - Replace str(path) substring checks in TestResolveCore with Path.parts tuple comparisons for correct behaviour on Windows (CI runs windows-latest) * fix: guard against non-mapping YAML manifests; apply integration post-processing in replay - ExtensionManifest._load raises ValidationError for non-dict YAML roots instead of TypeError - PresetManager._replay_wraps_for_command calls integration.post_process_skill_content, matching _register_skills behaviour - PresetResolver skips extensions that raise OSError/TypeError/AttributeError on manifest load - Tests: non-mapping YAML, OSError manifest skip, and replay integration post-processing	2026-04-21 15:02:31 -05:00
김준호	569d18a59d	fix(agents): block directory traversal in command write paths (#2229) (#2296) ... Extend the alias containment guard from b67b285 to the two remaining write paths that derive filenames from free-form command/alias names: - Primary command write in CommandRegistrar.register_commands() - CommandRegistrar.write_copilot_prompt() Consolidate the check into a shared _ensure_inside() helper. Per maintainer guidance on #2229, use a lexical (os.path.normpath + Path.is_relative_to) containment check rather than resolve() so `..` / absolute-path traversal is rejected while intentionally symlinked sub-directories under an agent's commands directory (e.g. .claude/skills/shared -> /team/shared-skills) keep working for existing extension setups. Add 22 parametrised regression cases covering traversal payloads on primary commands, aliases, and the Copilot companion prompt, plus a positive case that confirms symlinked sub-directories remain supported.	2026-04-21 12:06:09 -05:00
Manfred Riem	fc3d1244c0	fix: replace shell-based context updates with marker-based upsert (#2259) ... * Replace shell-based context updates with marker-based upsert Replace ~3500 lines of bash/PowerShell agent context update scripts with a Python-based approach using <!-- SPECKIT START/END --> markers. IntegrationBase now manages the agent context file directly: - upsert_context_section(): creates or updates the marked section at init/install/switch time with a directive to read the current plan - remove_context_section(): removes the section at uninstall, deleting the file only if it becomes empty - __CONTEXT_FILE__ placeholder in command templates is resolved per integration so the plan command references the correct agent file - context_file is persisted in init-options.json for extension access The plan command template instructs the LLM to update the plan reference between the markers in the agent context file. Removed: - scripts/bash/update-agent-context.sh (857 lines) - scripts/powershell/update-agent-context.ps1 (515 lines) - 56 integration wrapper scripts (update-context.sh/.ps1) - templates/agent-file-template.md - agent_scripts frontmatter key and {AGENT_SCRIPT} replacement logic - update-context reference from integration.json - tests/test_cursor_frontmatter.py (tested deleted scripts) Added: - upsert/remove context section methods on IntegrationBase - __CONTEXT_FILE__ placeholder support in process_template() - context_file field in init-options.json (init/switch/uninstall) - Per-integration tests: context file correctness, plan reference, init-options persistence (78 new context_file tests) - End-to-end CLI validation across all 28 integrations * fix: search for end marker after start marker in context section methods Address Copilot review: content.find(CONTEXT_MARKER_END) searched from the start of the file rather than after the located start marker. If the file contained a stray end marker before the start marker, the wrong slice could be replaced. Now both upsert_context_section() and remove_context_section() pass start_idx as the second argument to find() and validate end_idx > start_idx before performing the replacement. * fix: address Copilot review feedback on context section handling 1. Fix grammar in _build_context_section() directive text — add commas for a complete sentence. 2. Resolve __CONTEXT_FILE__ in resolve_skill_placeholders() — skills generated via extensions/presets for codex/kimi now replace the placeholder using the context_file value from init-options.json. 3. Handle Cursor .mdc frontmatter — when creating a new .mdc context file, prepend alwaysApply: true YAML frontmatter so Cursor auto-loads the rules. 4. Fix empty-file leading newline — when the context file exists but is empty, write the section directly instead of prepending a blank line. * fix: address second round of Copilot review feedback 1. Ensure .mdc frontmatter on existing files — upsert_context_section() now checks for missing YAML frontmatter on .mdc files during updates (not just creation), so pre-existing Cursor files get alwaysApply. 2. Guard against context_file=None — use 'or ""' instead of a default arg so explicit null values in init-options.json don't cause a TypeError in str.replace(). 3. Clean up .mdc files on removal — remove_context_section() treats files containing only the Speckit-generated frontmatter block as empty, deleting them rather than leaving orphaned frontmatter. * fix: address third round of Copilot review feedback 1. CRLF-safe .mdc frontmatter check — use lstrip().startswith('---') instead of startswith('---\n') so CRLF files don't get duplicate frontmatter. 2. CRLF-safe .mdc removal check — normalize line endings before comparing against the sentinel frontmatter string. 3. Call remove_context_section() during integration_uninstall() — the manifest-only uninstall was leaving the managed SPECKIT markers behind in the agent context file. 4. Fix stale docstring — remove 'agent_scripts' mention from test_lean_commands_have_no_scripts(). * fix: address fourth round of Copilot review feedback 1. Remove unused script_type parameter from _write_integration_json() and all 3 call sites — the parameter was no longer referenced after the update-context script removal. 2. Fix _build_context_section() docstring — correct example path from '.specify/plans/plan.md' to 'specs/<feature>/plan.md'. 3. Improve .mdc frontmatter-only detection in remove_context_section() — use regex to match any YAML frontmatter block (not just the exact Speckit-generated one), so .mdc files with additional frontmatter keys are also cleaned up when no body content remains. * fix: handle corrupted markers and parse .mdc frontmatter robustly 1. Handle partial/corrupted markers in upsert_context_section() — if only the START marker exists (no END), replace from START through EOF. If only the END marker exists, replace from BOF through END. This keeps upsert idempotent even when a user accidentally deletes one marker. 2. Parse .mdc YAML frontmatter properly — new _ensure_mdc_frontmatter() helper parses existing frontmatter and ensures alwaysApply: true is set, rather than just checking for the --- delimiter. Handles missing frontmatter, existing frontmatter without alwaysApply, and already-correct frontmatter. * fix: preserve .mdc frontmatter, add tests, clean up on switch 1. Rewrite _ensure_mdc_frontmatter() with regex — preserves comments, formatting, and custom keys in existing frontmatter instead of destructively re-serializing via yaml.safe_dump(). Inserts or fixes alwaysApply: true in place. 2. Add 6 focused .mdc frontmatter tests to cursor-agent test file: new file creation, missing frontmatter, preserved custom keys, wrong alwaysApply value, idempotent upserts, removal cleanup. 3. Call remove_context_section() during integration switch Phase 1 — prevents stale SPECKIT markers from being left in the old integration's context file. Also clear context_file from init-options during the metadata reset. * fix: remove unused MDC_FRONTMATTER, preserve inline comments, normalize bare CR 1. Remove unused MDC_FRONTMATTER class variable — dead code after _ensure_mdc_frontmatter() was rewritten with regex. 2. Preserve inline comments when fixing alwaysApply — the regex substitution now captures trailing '# comment' text and keeps it. 3. Normalize bare CR in upsert_context_section() — match the behavior of remove_context_section() which already normalizes both CRLF and bare CR. 4. Clarify .mdc removal comment — 'treat frontmatter-only as empty' instead of misleading 'strip frontmatter'. * fix: handle corrupted markers in remove, CRLF-safe end-marker consumption 1. Handle corrupted markers in remove_context_section() — mirror upsert's behavior: start-only removes start→EOF, end-only removes BOF→end. Previously bailed out leaving partial markers behind. 2. CRLF-safe end-marker consumption — both upsert and remove now handle \r\n after the end marker, not just \n. Prevents extra blank lines at replacement boundaries in CRLF files. 3. Clarify path rule in plan template — distinguish filesystem operations (absolute paths) from documentation/agent context references (project-relative paths). * fix: only remove context section when both markers are well-ordered remove_context_section() previously treated mismatched markers as corruption and aggressively removed from BOF→end-marker or start-marker→EOF, which could delete user-authored content if only one marker remained. Now it only removes when both START and END markers exist and are properly ordered, returning False otherwise.	2026-04-17 13:57:51 -05:00
Manfred Riem	8fc2bd3489	fix: allow Claude to chain skills for hook execution (#2227) ... * fix: allow Claude to chain skills for hook execution (#2178) - Set disable-model-invocation to false so Claude can invoke extension skills (e.g. speckit-git-feature) from within workflow skills - Inject dot-to-hyphen normalization note into Claude SKILL.md hook sections so the model maps extension.yml command names to skill names - Replace Unicode checkmark with ASCII [OK] in auto-commit scripts to fix PowerShell encoding errors on Windows - Move Claude-specific frontmatter injection to ClaudeIntegration via post_process_skill_content() hook on SkillsIntegration, wired through presets and extensions managers - Add positive and negative tests for all changes Fixes #2178 * refactor: address PR review feedback - Preserve line-ending style (CRLF/LF) in _inject_hook_command_note instead of always inserting \n, matching the convention used by other injection helpers in the same module. - Extract duplicated _post_process_skill() from extensions.py and presets.py into a shared post_process_skill() function in agents.py. Both modules now import and call the shared helper. * fix: match full hook instruction line in regex The regex in _inject_hook_command_note only matched lines ending immediately after 'output the following', but the actual template lines continue with 'based on its `optional` flag:'. Use [^\r\n]* to capture the rest of the line before the EOL. * refactor: use integration object directly for post_process_skill_content Instead of a free function in agents.py that re-resolves the integration by key, callers in extensions.py and presets.py now resolve the integration once via get_integration() and call integration.post_process_skill_content() directly. The base identity method lives on SkillsIntegration.	2026-04-15 14:35:05 -05:00
Michal Bachorik	33a28ec8f7	fix: unofficial PyPI warning (#1982) and legacy extension command name auto-correction (#2017) (#2027) ... * docs: warn about unofficial PyPI packages and recommend version verification (#1982) Clarify that only packages from github/spec-kit are official, and add `specify version` as a post-install verification step to help users catch accidental installation of an unrelated package with the same name. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(extensions): auto-correct legacy command names instead of hard-failing (#2017) Community extensions that predate the strict naming requirement use two common legacy formats ('speckit.command' and 'extension.command'). Instead of rejecting them outright, auto-correct to the required 'speckit.{extension}.{command}' pattern and emit a compatibility warning so authors know they need to update their manifest. Names that cannot be safely corrected (e.g. single-segment names) still raise ValidationError. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(tests): isolate preset catalog search test from community catalog network calls test_search_with_cached_data asserted exactly 2 results but was getting 4 because _get_merged_packs() queries the full built-in catalog stack (default + community). The community catalog had no local cache and hit the network, returning real presets. Writing a project-level preset-catalogs.yml that pins the test to the default URL only makes the count assertions deterministic. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(extensions): extend auto-correction to aliases (#2017) The upstream #1994 added alias validation in _collect_manifest_command_names, which also rejected legacy 2-part alias names (e.g. 'speckit.verify'). Extend the same auto-correction logic from _validate() to cover aliases, so both 'speckit.command' and 'extension.command' alias formats are corrected to 'speckit.{ext_id}.{command}' with a compatibility warning instead of hard-failing. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(extensions): address PR review feedback (#2017) - _try_correct_command_name: only correct 'X.Y' to 'speckit.ext_id.Y' when X matches ext_id, preventing misleading warnings followed by install failure due to namespace mismatch - _validate: add aliases type/string guards matching _collect_manifest _command_names defensive checks - _validate: track command renames and rewrite any hook.*.command references that pointed at a renamed command, emitting a warning - test: fix test_command_name_autocorrect_no_speckit_prefix to use ext_id matching the legacy namespace; add namespace-mismatch test - test: replace redundant preset-catalogs.yml isolation with monkeypatch.delenv("SPECKIT_PRESET_CATALOG_URL") so the env var cannot bypass catalog restriction in CI environments Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Update docs/installation.md Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * fix(extensions): warn when hook command refs are silently canonicalized; fix grammar - Hook rewrites (alias-form or rename-map) now always emit a warning so extension authors know to update their manifests. Previously only rename-map rewrites produced a warning; pure alias-form lifts were silent. - Pluralize "command/commands" in the uninstall confirmation message so single-command extensions no longer print "1 commands". Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(extensions): raise ValidationError for non-dict hook entries Silently skipping non-dict hook entries left them in manifest.hooks, causing HookExecutor.register_hooks() to crash with AttributeError when it called hook_config.get() on a non-mapping value. Also updates PR description to accurately reflect the implementation (no separate _try_correct_alias_name helper; aliases use the same _try_correct_command_name path). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(extensions): derive remove cmd_count from registry, fix wording Previously cmd_count used len(ext_manifest.commands) which only counted primary commands and missed aliases. The registry's registered_commands already tracks every command name (primaries + aliases) per agent, so max(len(v) for v in registered_commands.values()) gives the correct total. Also changes "from AI agent" → "across AI agents" since remove() unregisters commands from all detected agents. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(extensions): distinguish missing vs empty registered_commands in remove prompt Using get() without a default lets us tell apart: - key missing (legacy registry entry) → fall back to manifest count - key present but empty dict (installed with no agent dirs) → show 0 Previously the truthiness check `if registered_commands and ...` treated both cases the same, so an empty dict fell back to len(manifest.commands) and overcounted commands that would actually be removed. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(extensions): clarify removal prompt wording to 'per agent' 'across AI agents' implied a total count, but cmd_count uses max() across agents (per-agent count). Using sum() would double-count since users think in logical commands, not per-agent files. 'per agent' accurately describes what the number represents. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(extensions): clarify cmd_count comment — per-agent max, not total The comment said 'covers all agents' implying a total, but cmd_count uses max() across agents (per-agent count). Updated comment to explain the max() choice and why sum() would double-count. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * test(extensions): add CLI tests for remove confirmation pluralization Adds TestExtensionRemoveCLI with two CliRunner tests: - singular: 1 registered command → '1 command per agent' - plural: 2 registered commands → '2 commands per agent' These prevent regressions on the cmd_count pluralization logic and the 'per agent' wording introduced in this PR. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(agents): remove orphaned SKILL.md parent dirs on unregister For SKILL.md-based agents (codex, kimi), each command lives in its own subdirectory (e.g. .agents/skills/speckit-ext-cmd/SKILL.md). The previous unregister_commands() only unlinked the file, leaving an empty parent dir. Now attempts rmdir() on the parent when it differs from the agent commands dir. OSError is silenced so non-empty dirs (e.g. user files) are safely left. Adds test_unregister_skill_removes_parent_directory to cover this. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(extensions): drop alias pattern enforcement from _validate() Aliases are intentionally free-form to preserve community extension compatibility (e.g. 'speckit.verify' short aliases used by spec-kit-verify and other existing extensions). This aligns _validate() with the intent of upstream commit 4deb90f (fix: restore alias compatibility, #2110/#2125). Only type and None-normalization checks remain for aliases. Pattern enforcement continues for primary command names only. Updated tests to verify free-form aliases pass through unchanged with no warnings instead of being auto-corrected. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(extensions): guard against non-dict command entries in _validate() If provides.commands contains a non-mapping entry (e.g. an int or string), 'name' not in cmd raises TypeError instead of a user-facing ValidationError. Added isinstance(cmd, dict) check at the top of the loop. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: iamaeroplane <michal.bachorik@gmail.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>	2026-04-15 07:35:49 -05:00
Furkan Köykıran	b67b2856b1	feat(agents): add Goose AI agent support (#2015) ... * feat(integrations): add YamlIntegration base class for YAML recipe agents Signed-off-by: Furkan Köykıran <furkankoykiran@gmail.com> * feat(integrations): add Goose integration subpackage with YAML recipe support Signed-off-by: Furkan Köykıran <furkankoykiran@gmail.com> * feat(integrations): register GooseIntegration in the integration registry Signed-off-by: Furkan Köykıran <furkankoykiran@gmail.com> * feat(agents): add YAML format support to CommandRegistrar for extension/preset commands Signed-off-by: Furkan Köykıran <furkankoykiran@gmail.com> * feat(scripts): add goose agent type to bash update-agent-context script Signed-off-by: Furkan Köykıran <furkankoykiran@gmail.com> * feat(scripts): add goose agent type to PowerShell update-agent-context script Signed-off-by: Furkan Köykıran <furkankoykiran@gmail.com> * docs(agents): add Goose to supported agents table and integration notes Signed-off-by: Furkan Köykıran <furkankoykiran@gmail.com> * docs(readme): add Goose to supported agents table Signed-off-by: Furkan Köykıran <furkankoykiran@gmail.com> * test(integrations): add YamlIntegrationTests base mixin for YAML agent testing Signed-off-by: Furkan Köykıran <furkankoykiran@gmail.com> * test(integrations): add Goose integration tests Signed-off-by: Furkan Köykıran <furkankoykiran@gmail.com> * test(consistency): add Goose consistency checks for config, registrar, and scripts Signed-off-by: Furkan Köykıran <furkankoykiran@gmail.com> * docs(agents): move Goose to YAML Format section in Command File Formats Goose uses YAML recipes, not Markdown. Remove it from the Markdown Format list and add a dedicated YAML Format subsection with a representative recipe example showing prompt: | and {{args}} placeholders. * refactor(agents): delegate render_yaml_command to YamlIntegration Remove the duplicate header dict, yaml.safe_dump call, body indentation, and _human_title logic from CommandRegistrar.render_yaml_command(). Delegate to YamlIntegration._render_yaml() and _human_title() so YAML recipe output stays consistent across the init-time generation and command-registration code paths. * fix(agents): guard alias output path against directory traversal Validate that alias_file resolves within commands_dir before writing. Uses the same resolve().relative_to() pattern already established in extensions.py for ZIP path containment checks. * docs(agents): add Goose to Multi-Agent Support comment list in update-agent-context.sh * fix(agents): add goose to print_summary Usage line in bash context script The print_summary() function listed all supported agents in its Usage output but omitted goose, making it inconsistent with the header docs and the error message in update_specific_agent(). Signed-off-by: Furkan Köykıran <furkankoykiran@gmail.com> * fix(agents): add goose to Print-Summary Usage line in PowerShell context script The Print-Summary function listed all supported agents in its Usage output but omitted goose, making it inconsistent with the ValidateSet and the header documentation. Signed-off-by: Furkan Köykıran <furkankoykiran@gmail.com> * fix(agents): normalize description and title types in YamlIntegration.setup() YAML frontmatter can contain non-string types (null, list, int). Add isinstance checks matching TomlIntegration._extract_description() to ensure Goose recipes always receive valid string fields. Signed-off-by: Furkan Köykıran <furkankoykiran@gmail.com> * fix(agents): validate shared script exists before exec in Goose bash wrapper Add Forge-style check that the shared update-agent-context.sh is present and executable, producing a clear error instead of a cryptic shell exec failure when the shared script is missing. Signed-off-by: Furkan Köykıran <furkankoykiran@gmail.com> * fix(agents): validate shared script exists before invoke in Goose PowerShell wrapper Add Forge-style Test-Path check that the shared update-agent-context.ps1 exists, producing a clear error instead of a cryptic PowerShell failure when the shared script is missing. Signed-off-by: Furkan Köykıran <furkankoykiran@gmail.com> * fix(agents): normalize title and description types in render_yaml_command() Extension/preset frontmatter can contain non-string types. Add isinstance checks matching the normalization in YamlIntegration.setup() so both code paths produce valid Goose recipe fields. Signed-off-by: Furkan Köykıran <furkankoykiran@gmail.com> * fix(agents): replace $ARGUMENTS with arg_placeholder in process_template() Signed-off-by: Furkan Köykıran <furkankoykiran@gmail.com> * test(agents): assert $ARGUMENTS absent from generated YAML recipes Signed-off-by: Furkan Köykıran <furkankoykiran@gmail.com> * test(agents): assert $ARGUMENTS absent from generated TOML commands Signed-off-by: Furkan Köykıran <furkankoykiran@gmail.com> * fix(tests): rewrite docstring to avoid embedded triple-quote in TOML test Signed-off-by: Furkan Köykıran <furkankoykiran@gmail.com> --------- Signed-off-by: Furkan Köykıran <furkankoykiran@gmail.com>	2026-04-13 07:55:44 -05:00
toxicafunk	71143598be	fix(forge): use hyphen notation in frontmatter name field (#2075) ... * fix(forge): use hyphen notation in frontmatter name field - Changed injected name field from 'speckit.{command}' to 'speckit-{command}' - Keeps standard filename format 'speckit.{command}.md' - Aligns with Forge's command naming convention requirements - All tests pass * feat(forge): centralize name formatting to fix extension/preset command names Address PR feedback by centralizing Forge command name formatting to ensure consistent hyphenated names across both core template setup and extension/preset command registration. Changes: - Add format_forge_command_name() utility function in forge integration - Update ForgeIntegration._apply_forge_transformations() to use centralized formatter - Add _format_name_for_agent() helper in CommandRegistrar to apply agent-specific formatting - Update CommandRegistrar.register_commands() to format names for Forge (both primary commands and aliases) - Add comprehensive test coverage for the formatter and registrar behavior Impact: - Extension commands installed for Forge now use 'name: speckit-my-extension-example' instead of 'name: speckit.my-extension.example' - Fixes ZSH/shell compatibility issues with dot notation in command names - Maintains backward compatibility for all other agents (they continue using dot notation) - Eliminates duplication between integration setup and registrar paths Example transformation: Before: name: speckit.jira.sync-status (breaks in ZSH/Forge) After: name: speckit-jira-sync-status (works everywhere) Fixes inconsistency where core templates used hyphens but extension/preset commands preserved dots, breaking Forge's naming requirements. * refactor(forge): move name formatting logic to integration module Move _format_name_for_agent function logic into Forge integration's registrar_config as a 'format_name' callback, improving separation of concerns and keeping Forge-specific logic within its integration module. Changes: - Remove _format_name_for_agent() from agents.py (shared module) - Add 'format_name' callback to Forge's registrar_config pointing to format_forge_command_name - Update CommandRegistrar to use format_name callback when available - Maintains same behavior: Forge commands use hyphenated names, others use dot notation Benefits: - Better encapsulation: Forge-specific logic lives in forge integration - More extensible: Other integrations can provide custom formatters via registrar_config - Cleaner separation: agents.py doesn't need to know about specific agent requirements * fix(forge): make format_forge_command_name idempotent Handle already-hyphenated names (speckit-foo) to prevent double-prefixing (speckit-speckit-foo). The function now returns already-formatted names unchanged, making it safe to call multiple times. Changes: - Add early return for names starting with 'speckit-' - Update docstring to clarify accepted input formats - Add examples showing idempotent behavior - Add test coverage for idempotent behavior Examples: format_forge_command_name('speckit-plan') -> 'speckit-plan' (unchanged) format_forge_command_name('speckit.plan') -> 'speckit-plan' (converted) format_forge_command_name('plan') -> 'speckit-plan' (prefixed) * test(forge): strengthen name field assertions and clarify comments Improve test_name_field_uses_hyphenated_format to fail loudly when the name field is missing instead of silently passing. Changes: - Add explicit assertion that name_match is not None before validating value - Ensures test fails if regex doesn't match (e.g., frontmatter rendering changes) - Clarify Claude comment: it doesn't use inject_name path but SKILL.md frontmatter still includes hyphenated name via build_skill_frontmatter() Before: Test would silently pass if 'name:' field was missing from frontmatter After: Test explicitly asserts field presence before validating format * docs(forge): clarify frontmatter name requirement and improve test isolation Fix misleading docstring and improve test to properly validate that the format_name callback is Forge-specific. Changes to src/specify_cli/integrations/forge/__init__.py: - Reword module docstring to clarify the requirement is specifically for the frontmatter 'name' field value, not command files or invocation - Before: 'Requires hyphenated command names ... instead of dot notation' (implied dot notation unsupported overall) - After: 'Uses a hyphenated frontmatter name value ... for shell compatibility' (clarifies it's the frontmatter field, and Forge still supports dot filenames) Changes to tests/integrations/test_integration_forge.py: - Replace Claude with Windsurf in test_registrar_does_not_affect_other_agents - Claude uses build_skill_frontmatter() which always includes hyphenated names, so testing it didn't validate that format_name callback is Forge-only - Windsurf is a standard markdown agent without inject_name - Now asserts NO 'name:' field is present, proving format_name isn't invoked - This properly validates the callback mechanism is isolated to Forge * test(forge): use parse_frontmatter for precise YAML validation Replace regex and string searches with CommandRegistrar.parse_frontmatter() to validate only YAML frontmatter, not entire file content. Prevents false positives if command body contains 'name:' lines. Changes: - test_forge_specific_transformations: Parse frontmatter dict instead of string search - test_name_field_uses_hyphenated_format: Replace regex with frontmatter parsing - test_registrar_formats_extension_command_names_for_forge: Use dict validation - test_registrar_formats_alias_names_for_forge: Use dict validation Benefits: More precise, robust against body content, better error messages, consistent with existing codebase utilities. --------- Co-authored-by: ericnoam <eric.rodriguez@leovegas.com>	2026-04-08 14:37:19 -05:00
Hamilton Snow	7f08f31286	[stage2] fix: serialize multiline descriptions in legacy TOML renderer (#2097) ... * fix: preserve multiline descriptions in legacy toml renderer * refactor: reuse toml escape helper for prompt fallback	2026-04-06 08:39:01 -05:00
Radu Chindris	535ddbe0d2	fix: add user-invocable: true to skill frontmatter (#2077) ... Skills were missing this field, causing them to be treated as "managed" instead of user-invocable via /speckit-* commands.	2026-04-03 09:33:10 -05:00
Eric Rodriguez Suazo	b8e7851234	feat: add Forgecode agent support (#2034) ... * feat: add Forgecode (forge) agent support - Add 'forgecode' to AGENT_CONFIGS in agents.py with .forge/commands directory, markdown format, and {{parameters}} argument placeholder - Add 'forgecode' to AGENT_CONFIG in __init__.py with .forge/ folder, install URL, and requires_cli=True - Add forgecode binary check in check_tool() mapping agent key 'forgecode' to the actual 'forge' CLI binary - Add forgecode case to build_variant() in create-release-packages.sh generating commands into .forge/commands/ with {{parameters}} - Add forgecode to ALL_AGENTS in create-release-packages.sh * fix: strip handoffs frontmatter and replace $ARGUMENTS for forgecode The forgecode agent hangs when listing commands because the 'handoffs' frontmatter field (a Claude Code-specific feature) contains 'send: true' entries that forge tries to act on when indexing .forge/commands/ files. Additionally, $ARGUMENTS in command bodies was never replaced with {{parameters}}, so user input was not passed through to commands. Python path (agents.py): - Add strip_frontmatter_keys: [handoffs] to the forgecode AGENT_CONFIG entry so register_commands drops the key before rendering Bash path (create-release-packages.sh): - Add extra_strip_key parameter to generate_commands; pass 'handoffs' for the forgecode case in build_variant - Use regex prefix match (~ "^"extra_key":") instead of exact equality to handle trailing whitespace after the YAML key - Add sed replacement of $ARGUMENTS -> $arg_format in the body pipeline so {{parameters}} is substituted in forgecode command files * feat: add name field injection for forgecode agent Forgecode requires both 'name' and 'description' fields in command frontmatter. This commit adds automatic injection of the 'name' field during command generation for forgecode. Changes: - Python (agents.py): Add inject_name: True to forgecode config and implement name injection logic in register_commands - Bash (create-release-packages.sh): Add post-processing step to inject name field into frontmatter after command generation This complements the existing handoffs stripping fix (d83be82) to fully support forgecode command requirements. * test: update test_argument_token_format for forgecode special case Forgecode uses {{parameters}} instead of the standard $ARGUMENTS placeholder. Updated test to check for the correct placeholder format for forgecode agent. - Added special case handling for forgecode in test_argument_token_format - Updated docstring to document forgecode's {{parameters}} format - Test now passes for all 26 agents including forgecode * docs: add forgecode to README documentation Added forgecode agent to all relevant sections: - Added to Supported AI Agents table - Added to --ai option description - Added to specify check command examples - Added initialization example - Added to CLI tools check list in detailed walkthrough Forgecode is now fully documented alongside other supported agents. * fix: show 'forge' binary name in user-facing messages for forgecode Addresses Copilot PR feedback: Users should see the actual executable name 'forge' in status and error messages, not the agent key 'forgecode'. Changes: - Added 'cli_binary' field to forgecode AGENT_CONFIG (set to 'forge') - Updated check_tool() to accept optional display_key parameter - Updated check_tool() to use cli_binary from AGENT_CONFIG when available - Updated check() command to display cli_binary in StepTracker - Updated init() error message to show cli_binary instead of agent key UX improvements: - 'specify check' now shows: '● forge (available/not found)' - 'specify init --ai forgecode' error shows: 'forge not found' (instead of confusing 'forgecode not found') This makes it clear to users that they need to install the 'forge' binary, even though they selected the 'forgecode' agent. * refactor: rename forgecode agent key to forge Aligns with AGENTS.md design principle: "Use the actual CLI tool name as the key, not a shortened version" (AGENTS.md:61-83). The actual CLI executable is 'forge', so the AGENT_CONFIG key should be 'forge' (not 'forgecode'). This follows the same pattern as other agents like cursor-agent and kiro-cli. Changes: - Renamed AGENT_CONFIG key: "forgecode" → "forge" - Removed cli_binary field (no longer needed) - Simplified check_tool() - removed cli_binary lookup logic - Simplified init() and check() - removed display_key mapping - Updated all tests: test_forge_name_field_in_frontmatter - Updated documentation: README.md Code simplification: - Removed 6 lines of workaround code - Removed 1 function parameter (display_key) - Eliminated all special-case logic for forge Note: No backward compatibility needed - forge is a new agent being introduced in this PR. * fix: ensure forge alias commands have correct name in frontmatter When inject_name is enabled (for forge), alias command files must have their own name field in frontmatter, not reuse the primary command's name. This is critical for Forge's command discovery and dispatch system. Changes: - For agents with inject_name, create a deepcopy of frontmatter for each alias and set the name to the alias name - Re-render the command content with the alias-specific frontmatter - Ensures each alias file has the correct name field matching its filename This fixes command discovery issues where forge would try to invoke aliases using the primary command's name. * feat: add forge to PowerShell script and fix test whitespace 1. PowerShell script (create-release-packages.ps1): - Added forge agent support for Windows users - Enables `specify init --ai forge --offline` on Windows - Enhanced Generate-Commands with ExtraStripKey parameter - Added frontmatter stripping for handoffs key - Added $ARGUMENTS replacement for {{parameters}} - Implemented forge case with name field injection - Complete parity with bash script 2. Test file (test_core_pack_scaffold.py): - Removed trailing whitespace from blank lines - Cleaner diffs and no linter warnings Addresses Copilot PR feedback on both issues. * fix: use .NET Regex.Replace for count-limited replacement in PowerShell Addresses Copilot feedback: PowerShell's -replace operator does not support a third argument for replacement count. Using it causes an error or mis-parsing that would break forge package generation on Windows. Changed from: $content -replace '(?m)^---$', "---`nname: $cmdName", 1 To: $regex = [regex]'(?m)^---$' $content = $regex.Replace($content, "---`nname: $cmdName", 1) The .NET Regex.Replace() method properly supports the count parameter, ensuring the name field is injected only after the first frontmatter delimiter (not the closing one). This fix is critical for Windows users running: specify init --ai forge --offline * Apply suggestion from @Copilot Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * feat: migrate Forge agent to Python integration system - Create ForgeIntegration class with custom processing for {{parameters}}, handoffs stripping, and name injection - Add update-context scripts (bash and PowerShell) for Forge - Register Forge in integration registry - Update AGENTS.md with Forge documentation and special processing requirements section - Add comprehensive test suite (11 tests, all passing) Closes migration from release packaging to Python-based scaffolding for Forge agent. * fix: replace $ARGUMENTS with {{parameters}} in Forge templates - Add replacement of $ARGUMENTS to {{parameters}} after template processing - Use arg_placeholder from config (Copilot's cleaner approach) - Remove unused 'import re' from _apply_forge_transformations() - Enhance tests to verify $ARGUMENTS replacement works correctly - All 11 tests pass Fixes template processing to ensure Forge receives user-supplied parameters correctly. * refactor: make ForgeIntegration extend MarkdownIntegration - Change base class from IntegrationBase to MarkdownIntegration - Eliminates ~30 lines of duplicated validation/setup boilerplate - Aligns with the pattern used by 20+ other markdown agents (Bob, Claude, Windsurf, etc.) - Update AGENTS.md to reflect new inheritance hierarchy - All Forge-specific processing retained ({{parameters}}, handoffs stripping, name injection) - All 535 integration tests pass This addresses reviewer feedback about using the MarkdownIntegration convenience base class. * style: remove trailing whitespace from test file - Strip trailing spaces from blank lines in test_integration_forge.py - Fixes W291 linting warnings - No functional changes * style: remove trailing whitespace from Forge integration - Strip trailing spaces from blank lines in __init__.py - Fixes whitespace on lines 20, 86, 90, 93, 139, 143 - Verified other files in forge/ directory have no trailing whitespace - No functional changes, all tests pass * test: derive expected commands from templates dynamically - Remove hard-coded command count (9) and command set from test_directory_structure - Use forge.list_command_templates() to derive expected commands - Test now auto-syncs when core command templates are added/removed - Prevents test breakage when template set changes - All 11 tests pass * fix: make Forge update-context scripts handle AGENTS.md directly - Add fallback logic to update/create AGENTS.md when shared script doesn't support forge yet - Check if shared dispatcher knows about 'forge' before delegating - If shared script doesn't support forge, handle AGENTS.md updates directly: - Add Forge section to existing AGENTS.md if not present - Create new AGENTS.md with Forge section if file doesn't exist - Both bash and PowerShell scripts implement same logic - Prevents 'Unknown agent type' errors until shared scripts add forge support - Future-compatible: automatically delegates when shared script supports forge Addresses reviewer feedback about update-context scripts failing without forge support. * feat: add Forge support to shared update-agent-context scripts - Add forge case to bash and PowerShell update-agent-context scripts - Add FORGE_FILE variable mapping to AGENTS.md (like opencode/codex/pi) - Add forge to all usage/help text and ValidateSet parameters - Include forge in update_all_existing_agents functions Wrapper script improvements: - Simplify Forge wrapper scripts to unconditionally delegate to shared script - Remove complex fallback logic that created stub AGENTS.md files - Add clear error messages if shared script is missing/not executable - Align with pattern used by other integrations (opencode, bob, etc.) Benefits: - Plan command's {AGENT_SCRIPT} now works for Forge users - No more incomplete/stub context files masking missing support - Cleaner, more maintainable code (-39 lines in wrappers) - Consistent architecture across all integrations Update AGENTS.md to document that Forge integration ensures shared scripts include forge support for context updates. Addresses reviewer feedback about Forge support being incomplete for workflow steps that run {AGENT_SCRIPT}. * fix: resolve unbound variable and duplicate file update issues - Fix undefined FORGE_FILE variable in bash update-agent-context.sh - Add missing FORGE_FILE definition pointing to AGENTS.md - Update comment to include Forge in list of agents sharing AGENTS.md - Prevents crash with 'set -u' when running without explicit agent type - Add deduplication logic to PowerShell update-agent-context.ps1 - Implement Update-IfNew helper to track processed files by real path - Prevents AGENTS.md from being rewritten multiple times - Matches existing deduplication behavior in bash script - Prevent duplicate YAML keys in Forge frontmatter injection - Check for existing 'name:' field before injection in both scripts - PowerShell: Parse frontmatter to detect existing name field - Bash: Enhanced awk script to check frontmatter state - Future-proofs against template changes that add name fields All scripts now have consistent behavior and proper error handling. * fix: import timezone from datetime for rate limit header parsing The _parse_rate_limit_headers() function uses timezone.utc on line 82 but timezone was never imported from datetime. This would raise a NameError the first time GitHub API rate-limit headers are parsed. Import timezone alongside datetime to fix the missing import. * fix: correct variable scope in PowerShell deduplication and update docs - Fix Update-IfNew in PowerShell update-agent-context.ps1 - Changed from $script: scope to Set-Variable -Scope 1 - Properly mutates parent function's local variables - Fixes deduplication tracking for shared AGENTS.md file - Prevents incorrect default Claude file creation - Update create-release-packages.sh documentation - Add missing 'forge' to AGENTS list in header comment - Documentation now matches actual ALL_AGENTS array Without this fix, AGENTS.md would be updated multiple times (once for each agent sharing it: opencode, codex, amp, kiro, bob, pi, forge) and the script would always create a default Claude file even when agent files exist. * fix: resolve missing scaffold_from_core_pack import in tests The test_core_pack_scaffold.py imports scaffold_from_core_pack from specify_cli, but that symbol does not exist in the current codebase. This causes an ImportError when the test module is loaded. Implement a resilient resolver that: - Tries scaffold_from_core_pack first (expected name) - Falls back to alternative names (scaffold_from_release_pack, etc.) - Gracefully skips tests if no compatible entrypoint exists This prevents import-time failures and makes the test future-proof for when the actual scaffolding function is added or restored. * fix: prevent duplicate path prefixes and consolidate shared file updates PowerShell release script: - Add deduplication pass to Rewrite-Paths function - Prevents .specify.specify/ double prefixes in generated commands - Matches bash script behavior with regex '(?:\.specify/){2,}' -> '.specify/' Bash update-agent-context script: - Consolidate AGENTS.md updates to single call - Remove redundant calls for $AMP_FILE, $KIRO_FILE, $BOB_FILE, $FORGE_FILE - Update label to 'Codex/opencode/Amp/Kiro/Bob/Pi/Forge' to reflect all agents - Prevents always-deduped $FORGE_FILE call that never executed Both fixes improve efficiency and correctness while maintaining parity between bash and PowerShell implementations. * refactor: remove unused rate-limit helpers and improve PowerShell scripts - Remove unused _parse_rate_limit_headers() and _format_rate_limit_error() from src/specify_cli/__init__.py (56 lines of dead code) - Add GENRELEASES_DIR override support to PowerShell release script with comprehensive safety checks (parity with bash script) - Remove redundant shared-file update calls from PowerShell agent context script (AMP_FILE, KIRO_FILE, BOB_FILE, FORGE_FILE all resolve to AGENTS.md) - Update test docstring to accurately reflect Forge's {{parameters}} token Changes align PowerShell scripts with bash equivalents and reduce maintenance burden by removing dead code. * fix: add missing 'forge' to PowerShell usage text and fix agent order - Add 'forge' to usage message in Print-Summary (was missing from list) - Reorder ValidateSet to match bash script order (vibe before qodercli) This ensures PowerShell script documentation matches bash script and includes all supported agents consistently. * refactor: remove old architecture files deleted in b1832c9 Remove files that were deleted in b1832c9 (Stage 6 migration) but remained on this branch due to merge conflicts: - Remove .github/workflows/scripts/create-release-packages.{sh,ps1} (replaced by inline release.yml + uv tool install) - Remove tests/test_core_pack_scaffold.py (scaffold system removed, tests no longer relevant) These files existed on the feature branch because they were modified before b1832c9 landed. The merge kept our versions, but they should be deleted to align with the new integration-only architecture. This PR now focuses purely on adding NEW Forge integration support, not restoring old architecture. * refactor: remove unused timezone import from __init__.py Remove unused timezone import that was added in 4a57f79 for rate-limit header parsing but became obsolete when rate-limit helper functions were removed in 59c4212 (and also removed in upstream b1832c9). No functional changes - purely cleanup of unused import. * docs: clarify that handoffs is a Claude Code feature, not Forge's Update docstrings to accurately explain that the 'handoffs' frontmatter key is from Claude Code (for multi-agent collaboration) and is stripped because it causes Forge to hang, not because it's a Forge-specific feature. Changes: - Module docstring: 'Forge-specific collaboration feature' → 'Claude Code feature that causes Forge to hang' - Class docstring: Add '(incompatible with Forge)' clarification - Method docstring: Add '(from Claude Code templates; incompatible with Forge)' context This avoids implying that handoffs belongs to Forge when it actually comes from spec-kit templates designed for Claude Code compatibility. --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>	2026-04-02 16:40:43 -05:00
Manfred Riem	b1832c9477	Stage 6: Complete migration — remove legacy scaffold path (#1924) (#2063) ... * Stage 6: Complete migration — remove legacy scaffold path (#1924) Remove the legacy GitHub download and offline scaffold code paths. All 26 agents now use the integration system exclusively. Code removal (~1073 lines from __init__.py): - download_template_from_github(), download_and_extract_template() - scaffold_from_core_pack(), _locate_release_script() - install_ai_skills(), _get_skills_dir (restored slim version for presets) - _has_bundled_skills(), _migrate_legacy_kimi_dotted_skills() - AGENT_SKILLS_MIGRATIONS, _handle_agent_skills_migration() - _parse_rate_limit_headers(), _format_rate_limit_error() - Three-way branch in init() collapsed to integration-only Config derivation (single source of truth): - AGENT_CONFIG derived from INTEGRATION_REGISTRY (replaced 180-line dict) - CommandRegistrar.AGENT_CONFIGS derived from INTEGRATION_REGISTRY (replaced 160-line dict) - Backward-compat constants kept for presets/extensions: SKILL_DESCRIPTIONS, NATIVE_SKILLS_AGENTS, DEFAULT_SKILLS_DIR Release pipeline cleanup: - Deleted create-release-packages.sh/.ps1 (948 lines of ZIP packaging) - Deleted create-github-release.sh, generate-release-notes.sh - Deleted simulate-release.sh, get-next-version.sh, update-version.sh - Removed .github/workflows/scripts/ directory entirely - release.yml is now self-contained: check, notes, release all inlined - Install instructions use uv tool install with version tag Test cleanup: - Deleted test_ai_skills.py (tested removed functions) - Deleted test_core_pack_scaffold.py (tested removed scaffold) - Cleaned test_agent_config_consistency.py (removed 19 release-script tests) - Fixed test_branch_numbering.py (removed dead monkeypatches) - Updated auto-promote tests (verify files created, not tip messages) 1089 tests pass, 0 failures, ruff clean. * fix: resolve merge conflicts with #2051 (claude as skills) - Fix circular import: move CommandRegistrar import in claude integration to inside method bodies (was at module level) - Lazy-populate AGENT_CONFIGS via _ensure_configs() to avoid circular import at class definition time - Set claude registrar_config to .claude/commands (extension/preset target) since the integration handles .claude/skills in setup() - Update tests from #2051 to match: registrar_config assertions, remove --integration tip assertions, remove install_ai_skills mocks 1086 tests pass. * fix: properly preserve claude skills migration from #2051 Restore ClaudeIntegration.registrar_config to .claude/skills (not .claude/commands) so extension/preset registrations write to the correct skills directory. Update tests that simulate claude setup to use .claude/skills and check for SKILL.md layout. Some tests still need updating for the full skills path — 10 remaining failures from the #2051 test expectations around the extension/preset skill registration flow. WIP: 1076/1086 pass. * fix: properly handle SKILL.md paths in extension update rollback and tests Fix extension update rollback using _compute_output_name() for SKILL.md agents (converts dots to hyphens in skill directory names). Previously the backup and cleanup code constructed paths with raw command names (e.g. speckit.test-ext.hello/SKILL.md) instead of the correct computed names (speckit-test-ext-hello/SKILL.md). Test fixes for claude skills migration: - Update claude tests to use .claude/skills paths and SKILL.md layout - Use qwen (not claude) for skills-guard tests since claude's agent dir IS the skills dir — creating it triggers command registration - Fix test_extension_command_registered_when_extension_present to check skills path format 1086 tests pass, 0 failures, ruff clean. * fix: address PR review — lazy init, assertions, deprecated flags - _ensure_configs(): catch ImportError (not Exception), don't set _configs_loaded on failure so retries work - Move _ensure_configs() before unregister loop (not inside it) - Module-level try/except catches ImportError specifically - Remove tautology assertion (or True) in test_extensions.py - Strengthen preset provenance assertion to check source: field - Mark --offline, --skip-tls, --debug, --github-token as hidden deprecated no-ops in init() 1086 tests pass. * fix: remove deleted release scripts from pyproject.toml force-include Removes force-include entries for create-release-packages.sh/.ps1 which were deleted but still referenced in [tool.hatch.build].	2026-04-02 12:34:34 -05:00
Andrii Furmanets	a858c1d6da	Install Claude Code as native skills and align preset/integration flows (#2051) ... * Use Claude skills for generated commands * Fix Claude integration and preset skill flows * Group Claude tests in integration suite * Align Claude skill frontmatter across generators * Fix native skill preset cleanup * Keep legacy AI skills test on legacy path * Move Claude here-mode test to CLI suite	2026-04-02 09:44:48 -05:00
Manfred Riem	4f9d966beb	Stage 5: Skills, Generic & Option-Driven Integrations (#1924) (#2052) ... * Stage 5: Skills, Generic & Option-Driven Integrations (#1924) Add SkillsIntegration base class and migrate codex, kimi, agy, and generic to the integration system. Integrations: - SkillsIntegration(IntegrationBase) in base.py — creates speckit-<name>/SKILL.md layout matching release ZIP output byte-for-byte - CodexIntegration — .agents/skills/, --skills default=True - KimiIntegration — .kimi/skills/, --skills + --migrate-legacy options, dotted→hyphenated skill directory migration - AgyIntegration — .agent/skills/, skills-only (commands deprecated v1.20.5) - GenericIntegration — user-specified --commands-dir, MarkdownIntegration - All four have update-context.sh/.ps1 scripts - All four registered in INTEGRATION_REGISTRY CLI changes: - --ai <agent> auto-promotes to integration path for all registered agents - Interactive agent selection also auto-promotes (bug fix) - --ai-skills and --ai-commands-dir show deprecation notices on integration path - Next-steps display shows correct skill invocation syntax for skills integrations - agy added to CommandRegistrar.AGENT_CONFIGS Tests: - test_integration_base_skills.py — reusable mixin with setup, frontmatter, directory structure, scripts, CLI auto-promote, and complete file inventory (sh+ps) tests - Per-agent test files: test_integration_{codex,kimi,agy,generic}.py - Kimi legacy migration tests, generic --commands-dir validation - Registry updated with Stage 5 keys - Removed 9 dead-mock tests, moved 4 integration tests to proper locations - Fixed all bare project-name tests to use tmp_path - Fixed 6 pre-existing ANSI escape code test failures in test_extensions.py and test_presets.py 1524 tests pass, 0 failures. * fix: remove unused variable flagged by ruff (F841) * fix: address PR review — integration-type-aware deprecation messages and early generic validation - --ai-skills deprecation message now distinguishes SkillsIntegration ("skills are the default") from command-based integrations ("has no effect") - --ai-commands-dir validation for generic runs even when auto-promoted, giving clear CLI error instead of late ValueError from setup() - Resolves review comments from #2052 * fix: address PR review round 2 - Remove unused SKILL_DESCRIPTIONS dict from base.py (dead code after switching to template descriptions for ZIP parity) - Narrow YAML parse catch from Exception to yaml.YAMLError - Remove unused shutil import from test_integration_kimi.py - Remove unused _REGISTRAR_EXEMPT class attr from test_registry.py - Reword --ai-commands-dir deprecation to be actionable - Update generic validation error to mention both --ai and --integration * fix: address PR review round 3 - Clarify parsed_options forwarding is intentional (all options passed, integrations decide what to use) - Extract _strip_ansi() helper in test_extensions.py and test_presets.py - Remove unused pytest import (test_cli.py), unused locals (test_integration_base_skills.py) - Reword --ai-commands-dir deprecation to be actionable without referencing the not-yet-implemented --integration-options * fix: address PR review round 4 - Reorder kimi migration: run super().setup() first so hyphenated targets exist, then migrate dotted dirs (prevents user content loss) - Move _strip_ansi() to shared tests/conftest.py, import from there in test_extensions.py, test_presets.py, test_ai_skills.py - Remove now-unused re imports from all three test files * fix: address PR review round 5 - Use write_bytes() for LF-only newlines (no CRLF on Windows) - Add --integration-options CLI parameter — raw string passed through to the integration via opts['raw_options']; the integration owns parsing of its own options - GenericIntegration.setup() reads --commands-dir from raw_options when not in parsed_options (supports --integration-options="...") - Skip early --ai-commands-dir validation when --integration-options is provided (integration validates in its own setup()) - Remove parse_integration_options from core — integrations parse their own options * fix: address PR review round 6 - GenericIntegration is now stateless: removed self._commands_dir instance state, overrides setup() directly to compute destination from parsed_options/raw_options on the stack - commands_dest() raises by design (stateless singleton) - _quote() in SkillsIntegration now escapes backslashes and double quotes to produce valid YAML even with special characters * fix: address PR review round 7 - Support --commands-dir=value form in raw_options parsing (not just --commands-dir value with space separator) - Normalize CRLF to LF in write_file_and_record() before encoding - Persist ai_skills=True in init-options.json when using a SkillsIntegration, so extensions/presets emit SKILL.md overrides correctly even without explicit --ai-skills flag	2026-04-02 08:00:12 -05:00
Manfred Riem	255371d367	Stage 3: Standard markdown integrations — 19 agents migrated to plugin architecture (#2038) ... * Stage 3: Standard markdown integrations — 19 agents migrated to plugin architecture Migrate all standard markdown integrations to self-contained subpackages under integrations/. Each subclasses MarkdownIntegration with config-only overrides (~10 lines per __init__.py). Integrations migrated (19): claude, qwen, opencode, junie, kilocode, auggie, roo, codebuddy, qodercli, amp, shai, bob, trae, pi, iflow, kiro-cli, windsurf, vibe, cursor-agent Changes: - Create integrations/<key>/ subpackage with __init__.py and scripts/ (update-context.sh, update-context.ps1) for each integration - Register all 19 in INTEGRATION_REGISTRY (20 total with copilot) - MarkdownIntegration.setup() processes templates (replaces {SCRIPT}, {ARGS}, __AGENT__; strips frontmatter blocks; rewrites paths) - Extract install_scripts() to IntegrationBase; refactor copilot to use it - Generalize --ai auto-promote from copilot-only to registry-driven: any integration registered in INTEGRATION_REGISTRY auto-promotes. Unregistered agents (gemini, tabnine, codex, kimi, agy, generic) continue through the legacy --ai path unchanged. - Fix cursor/cursor-agent key mismatch in CommandRegistrar.AGENT_CONFIGS - Add missing vibe entry to CommandRegistrar.AGENT_CONFIGS - Update kiro alias test to reflect auto-promote behavior Testing: - Per-agent test files (test_integration_<agent>.py) with shared mixin - 1316 tests passing, 0 failures - Complete file inventory tests for both sh and ps variants - Byte-for-byte validated against v0.4.3 release packages (684 files) * Address PR review: fix repo root detection and no-op test - Fix repo root fallback in all 20 update-context.sh scripts: walk up from script location to find .specify/ instead of falling back to pwd - Fix repo root fallback in all 20 update-context.ps1 scripts: walk up from script location to find .specify/ instead of falling back to $PWD - Add assertions to test_setup_writes_to_correct_directory: verify expected_dir exists and all command files reside under it * Fix REPO_ROOT priority: prefer .specify walk-up over git root In monorepos the git toplevel may differ from the project root that contains .specify/. The previous fix still preferred git rev-parse over the walk-up result. Bash scripts (20): prefer the discovered _root when it contains .specify/; only accept git root if it also contains .specify/. PowerShell scripts (20): validate git root contains .specify/ before using it; fall back to walking up from script directory otherwise. * Guard git call with try/catch in PowerShell scripts With $ErrorActionPreference = 'Stop', an unguarded git rev-parse throws a terminating CommandNotFoundException when git is not installed, preventing the .specify walk-up fallback from running. Wrap the git call in try/catch across all 20 update-context.ps1 scripts so the fallback works reliably without git. * Rename hyphenated package dirs to valid Python identifiers Rename kiro-cli → kiro_cli and cursor-agent → cursor_agent so the packages can be imported with normal Python syntax instead of importlib. The user-facing integration key (IntegrationBase.key) stays hyphenated to match the actual CLI tool / binary name. Also reorganize _register_builtins(): imports and registrations are now grouped alphabetically with clear section comments. * Reuse CommandRegistrar path rewriting in process_template() Replace the duplicated regex-based path rewriting in MarkdownIntegration.process_template() with a call to the shared CommandRegistrar._rewrite_project_relative_paths() implementation. This ensures extension-local paths are preserved and boundary rules stay consistent across the codebase. * Promote _rewrite_project_relative_paths to public API Rename CommandRegistrar._rewrite_project_relative_paths() to rewrite_project_relative_paths() (drop leading underscore) so integrations can call it without reaching into a private method across subsystem boundaries. Addresses PR review feedback: https://github.com/github/spec-kit/pull/2038#discussion_r3022105627 * Broaden TestRegistrarKeyAlignment to cover all integration keys Parametrize across ALL_INTEGRATION_KEYS instead of only checking cursor-agent and vibe. Keeps a separate negative test for the stale 'cursor' shorthand. Addresses PR review feedback: https://github.com/github/spec-kit/pull/2038#discussion_r3022269032	2026-04-01 09:17:21 -05:00
Hamilton Snow	ccc44dd00a	Unify Kimi/Codex skill naming and migrate legacy dotted Kimi dirs (#1971) ... * fix: unify hyphenated skills and migrate legacy kimi dotted dirs * fix: preserve legacy kimi dotted preset skill overrides * fix: migrate kimi legacy dotted skills without ai-skills flag * fix: harden kimi migration and cache hook init options * fix: apply kimi preset skill overrides without ai-skills flag * fix: keep sequential branch numbering beyond 999 * test: align kimi scaffold skill path with hyphen naming * chore: align hook typing and preset skill comment * fix: restore AGENT_SKILLS_DIR_OVERRIDES compatibility export * refactor: remove AGENT_SKILLS_DIR_OVERRIDES and update callers * fix(ps1): support sequential branch numbers above 999 * fix: resolve preset skill placeholders for skills agents * Fix legacy kimi migration safety and preset skill dir checks * Harden TOML rendering and consolidate preset skill restore parsing * Fix PowerShell overflow and hook message fallback for empty invocations * Restore preset skills from extensions * Refine preset skill restore helpers * Harden skill path and preset checks * Guard non-dict init options * Avoid deleting unmanaged preset skill dirs * Unify extension skill naming with hooks * Harden extension native skill registration * Normalize preset skill titles	2026-03-26 10:53:30 -05:00
Seiya Kojima	a351c826ee	fix(cli): add allow_unicode=True and encoding="utf-8" to YAML I/O (#1936) ... None of the yaml.dump() calls specify allow_unicode=True, causing non-ASCII characters in extension descriptions to be escaped to \uXXXX sequences in generated .agent.md frontmatter and config files. Add allow_unicode=True to all 6 yaml.dump() call sites, and encoding="utf-8" to all corresponding write_text() and read_text() calls to ensure consistent UTF-8 handling across platforms.	2026-03-23 08:49:10 -05:00
Hamilton Snow	191f33213c	fix: Align native skills frontmatter with install_ai_skills (#1920) ... * docs(sdk): align native skills frontmatter + document multi-generator drift * fix: clarify skills frontmatter contract and AGENTS sections	2026-03-20 13:28:11 -05:00
Irina Chichikova	34171efcef	Add support for Junie (#1831) ... * Add support for Junie * Add Junie agent configuration to specify-cli * Add support for iflow agent in context update scripts	2026-03-19 11:54:42 -05:00
Hamilton Snow	c8af730b14	feat: migrate Codex/agy init to native skills workflow (#1906) ... * feat: migrate codex and agy to native skills flow * fix: harden codex skill frontmatter and script fallback * fix: clarify skills separator default expansion * fix: rewrite agent_scripts paths for codex skills * fix: align kimi guidance and platform-aware codex fallback	2026-03-19 09:00:41 -05:00
fuyongde	f6794685b6	feat: add iFlow CLI support (#1875) ... Add `iflow` as a supported AI agent (the key users pass to --ai) across all relevant configuration files, release scripts, agent context scripts, and README. Includes consistency tests following the same pattern as kimi/tabnine additions. - README: describe `check` generically (git + all AGENT_CONFIG CLI agents) - README: describe `--ai` with reference to AGENT_CONFIG for full list Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>	2026-03-19 06:44:22 -05:00
Greazly	1a21bdef01	Feature/spec kit add pi coding agent pullrequest (#1853) ... * feat(ai): add native support for Pi coding agent by pi+gpt 5.4 * docs(pi): document MCP limitations for Pi agent * fix: unitended kimi agent mention added to update-agent-context.ps1 * fix: address reviewer feedback * Apply suggestions from code review Changes in AGENTS.md weren't part of my PR, but the Copilot feedback seems to be correct is correct. I've doublechecked it with contents of test_agent_config_consistency.py and create-release-packages scripts Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>	2026-03-17 14:50:18 -05:00
黄黄汪	a177a1a6d1	feat: add Trae IDE support as a new agent (#1817) ... * feat: add Trae IDE support as a new agent Add Trae (https://www.trae.ai/) as a supported AI agent in spec-kit. Trae is an IDE-based agent that uses .trae/rules/ directory for project-level rules in Markdown format. Changes across 9 files: - src/specify_cli/__init__.py: Add trae to AGENT_CONFIG (IDE-based, .trae/ folder, rules subdir, no CLI required) - src/specify_cli/extensions.py: Add trae to CommandRegistrar.AGENT_CONFIGS (.trae/rules, markdown format, .md extension) - README.md: Add Trae to supported agents table, CLI examples, and --ai option description - .github/workflows/scripts/create-release-packages.sh: Add trae to ALL_AGENTS array and build case statement - .github/workflows/scripts/create-release-packages.ps1: Add trae to AllAgents array and switch statement - .github/workflows/scripts/create-github-release.sh: Add trae template zip files to release assets - scripts/bash/update-agent-context.sh: Add TRAE_FILE, trae case in update function, and auto-detect block - scripts/powershell/update-agent-context.ps1: Add TRAE_FILE, ValidateSet entry, switch case, and auto-detect block - tests/test_agent_config_consistency.py: Add 8 consistency tests for trae following established kimi/tabnine patterns * fix: correct Generate-Commands parameter names for trae in PowerShell release script Fix incorrect parameter names in the trae case of Build-Variant: - -Format -> -Extension - -ArgsToken -> -ArgFormat - -OutDir -> -OutputDir These now match the Generate-Commands function signature and all other agent entries in the script. Co-authored-by: Copilot <copilot@github.com> * Update release packaging scripts and agent docs * Update Agent.md * Restore format * Adjust order * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Unused * fix: add TRAE_FILE to update_all_existing_agents() for auto-detect support Add missing update_if_new call for TRAE_FILE in the bash update-agent-context.sh script's update_all_existing_agents() function, matching the PowerShell implementation. This ensures running the script without arguments will correctly auto-detect and update existing Trae agent files. * Add configuration for 'trae' in agents.py * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Refactor trae configuration test for clarity * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Update update-agent-context.sh * Fix formatting in update-agent-context.sh --------- Co-authored-by: root <root@g340-cd52-700-60f9-5561-211-6c32.byted.org> Co-authored-by: root <root@g340-cd52-700-c3d1-c735-796-4b9e.byted.org> Co-authored-by: Copilot <copilot@github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>	2026-03-17 10:03:54 -05:00
Copilot	69ee7a836e	feat(presets): Pluggable preset system with catalog, resolver, and skills propagation (#1787) ... * Initial plan * feat(templates): add pluggable template system with packs, catalog, resolver, and CLI commands Co-authored-by: mnriem <15701806+mnriem@users.noreply.github.com> * test(templates): add comprehensive unit tests for template pack system Co-authored-by: mnriem <15701806+mnriem@users.noreply.github.com> * feat(presets): pluggable preset system with template/command overrides, catalog, and resolver - Rename 'template packs' to 'presets' to avoid naming collision with core templates - PresetManifest, PresetRegistry, PresetManager, PresetCatalog, PresetResolver in presets.py - Extract CommandRegistrar to agents.py as shared infrastructure - CLI: specify preset list/add/remove/search/resolve/info - CLI: specify preset catalog list/add/remove - --preset option on specify init - Priority-based preset stacking (--priority, lower = higher precedence) - Command overrides registered into all detected agent directories (17+ agents) - Extension command safety: skip registration if target extension not installed - Multi-catalog support: env var, project config, user config, built-in defaults - resolve_template() / Resolve-Template in bash/PowerShell scripts - Self-test preset: overrides all 6 core templates + 1 command - Scaffold with 4 examples: core/extension template and command overrides - Preset catalog (catalog.json, catalog.community.json) - Documentation: README.md, ARCHITECTURE.md, PUBLISHING.md - 110 preset tests, 253 total tests passing * feat(presets): propagate command overrides to skills via init-options - Add save_init_options() / load_init_options() helpers that persist CLI flags from 'specify init' to .specify/init-options.json - PresetManager._register_skills() overwrites SKILL.md files when --ai-skills was used during init and corresponding skill dirs exist - PresetManager._unregister_skills() restores core template content on preset removal - registered_skills stored in preset registry metadata - 8 new tests covering skill override, skip conditions, and restore * fix: address PR check failures (ruff F541, CodeQL URL substring) - Remove extraneous f-prefix from two f-strings without placeholders - Replace substring URL check in test with startswith/endswith assertions to satisfy CodeQL incomplete URL substring sanitization rule * fix: address Copilot PR review comments - Move save_init_options() before preset install so skills propagation works during 'specify init --preset --ai-skills' - Clean up downloaded ZIP after successful preset install during init - Validate --from URL scheme (require HTTPS, HTTP only for localhost) - Expose unregister_commands() on extensions.py CommandRegistrar wrapper instead of reaching into private _registrar field - Use _get_merged_packs() for search() and get_pack_info() so all active catalogs are searched, not just the highest-priority one - Fix fetch_catalog() cache to verify cached URL matches current URL - Fix PresetResolver: script resolution uses .sh extension, consistent file extensions throughout resolve(), and resolve_with_source() delegates to resolve() to honor template_type parameter - Fix bash common.sh: fall through to directory scan when python3 returns empty preset list - Fix PowerShell Resolve-Template: filter out dot-folders and sort extensions deterministically * fix: narrow empty except blocks and add explanatory comments * fix: address Copilot PR review comments (round 2) - Fix init --preset error masking: distinguish "not found" from real errors - Fix bash resolve_template: skip hidden dirs in extensions (match Python/PS) - Fix temp dir leaks in tests: use temp_dir fixture instead of mkdtemp - Fix self-test catalog entry: add note that it's local-only (no download_url) - Fix Windows path issue in resolve_with_source: use Path.relative_to() - Fix skill restore path: use project's .specify/templates/commands/ not source tree - Add encoding="utf-8" to all file read/write in agents.py - Update test to set up core command templates for skill restoration * fix: remove self-test from catalog.json (local-only preset) * fix: address Copilot PR review comments (round 3) - Fix PS Resolve-Template fallback to skip dot-prefixed dirs (.cache) - Rename _catalog to _catalog_name for consistency with extension system - Enforce install_allowed policy in CLI preset add and download_pack() - Fix shell injection: pass registry path via env var instead of string interpolation * fix: correct PresetError docstring from template to preset * Removed CHANGELOG requirement * Applying review recommendations * Applying review recommendations * Applying review recommendations * Applying review recommendations --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: mnriem <15701806+mnriem@users.noreply.github.com>	2026-03-13 15:09:14 -05:00