github-actions[bot]GitHubgithub-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>CopilotManfred RiemCopilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
* Fix upgrade-overwrites-copilot-skills: pass force=True to extension skill re-registration after upgrade
Apply the remediation from the bug assessment on issue #3849.
_register_extension_skills() had a skip guard that refused to overwrite
existing SKILL.md files (protecting user customizations). In the upgrade
path, setup() regenerates all core-template SKILL.md files first, then
calls register_enabled_extensions_for_agent(). The guard then sees those
freshly-written core files as 'existing' and skips every extension, leaving
only core template content on disk.
Fix: add force: bool = False to _register_extension_skills() and thread it
through register_enabled_extensions_for_agent() and
_register_extensions_for_agent(). In integration_upgrade(), pass force=True
so extension content layers on top of the just-regenerated core files.
The force flag is off-by-default so plain extension add still protects
user-modified skill files.
Refs #3849
Assisted-by: GitHub Copilot (model: claude-sonnet-4.6, autonomous)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Potential fix for pull request finding 'Unused local variable'
Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
* test: add end-to-end regression guard for upgrade-overwrites-copilot-skills (#3849)
The existing regression tests in TestRegisterExtensionSkillsForceFlag exercise
the new force parameter at the helper level, so without the fix they fail only
with a TypeError (unknown kwarg) rather than on the user-facing behaviour.
Add a command-level test that runs 'specify integration upgrade copilot --skills
--force' end-to-end and asserts the installed git extension's SKILL.md is
restored (with its extension content, not a bare core-template stub) when the
skill directory already exists — the exact skill_dir_preexists path the bug
depends on. The test fails on pre-fix source (the skill is never recreated) and
passes with the fix, so it is a genuine behavioural regression guard rather than
an API-surface check.
Refs #3849
Assisted-by: GitHub Copilot (model: claude-opus-4.8, autonomous)
---------
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Copilot <223556219+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>
* feat: first-class agent-native runtime hooks for integrations
* refactor: rework integration events per maintainer review
- Rename hooks terminology to 'events' (events:, --events flag, events.py).
- Use snake_case names for canonical events consistent with spec-kit vocabulary.
- Fold event config adapters into integration classes via class attributes (CANONICAL_TO_NATIVE, events_config_file, events_format).
- Lift event command-script resolution to core 'specify event run' command.
- Split events sourcing from integration config writing.
- Support first-class Copilot CLI events JSON generation under '.github/hooks/speckit.json'.
- Rewrite and expand full test suite under 'tests/integrations/test_events.py'.
Assisted-by: opencode (model: litellm/gemini-3.5-flash, autonomous)
* fix(events): resolve ruff lint errors blocking CI
Address Copilot review finding #18 (src/specify_cli/__init__.py event-command
import missing # noqa: E402), #19 (unused console import in commands/event.py),
and #20 (unused patch/yaml/Path/integration imports in test_events.py). Also
fix two stray F541 f-string prefixes in _build_opencode_plugin that ruff
flagged in the same job.
Bump dev version 0.14.2.dev0 -> 0.14.2.dev1 and add a CHANGELOG entry per the
AGENTS.md convention for Specify CLI __init__.py changes.
Refs: PR #3704 Copilot inline review (findings #18, #19, #20)
Assisted-by: opencode (model: glm-5.2, autonomous)
* fix(events): make generated native hooks actually execute
Address Copilot review findings that left generated event hooks inert or
schema-invalid after the rework:
- #2: the resolved events map now carries an ordered list of handlers per
event (dict[str, list[dict]]) so two extensions declaring the same event
both run instead of the last one silently winning. collect_extension_events
accumulates; every adapter emits one native entry per handler.
- #6: Claude/Gemini/Qwen/Devin/Tabnine native schema accepts a single
'command' string, not command+args. Each adapter now renders one complete
shell invocation of the dispatcher via _dispatcher_command().
- #7: Gemini measures hook timeouts in milliseconds; add events_timeout_unit
attr and _native_timeout() so the 60s default becomes 60000ms instead of
terminating the dispatcher after 60ms.
- #4: _resolve_event_command_argv() replaces _extract_script_path() —
scripts: values are command strings (e.g. 'scripts/bash/setup-plan.sh --json'),
not bare paths. Resolves the project's sh/ps/py variant, splits safely into
argv, and prepends the interpreter for .py.
- #5: bundled-template fallback now uses _locate_core_pack()/_repo_root()
(core_pack/commands, not the non-existent core_pack/templates/commands).
- #16: all formatters use IntegrationBase.resolve_python_interpreter() so
generated commands honor the project venv and never hard-code python3
(absent on Windows). The opencode TS plugin bakes in the same resolved
interpreter.
- #13: opencode TS plugin runEvent() now throws on failure instead of
process.exit(2), which killed the OpenCode host process; only the failing
hook is rejected.
- #21: user YAML override is validated (event names, non-empty command
strings) before returning; a malformed override is warned about and
ignored rather than crashing installation on cfg.get().
Bump dev version 0.14.2.dev1 -> 0.14.2.dev2 (gemini/__init__.py change) and
add a CHANGELOG entry.
Refs: PR #3704 Copilot inline review (findings #2, #4, #5, #6, #7, #13, #16, #21)
Assisted-by: opencode (model: glm-5.2, autonomous)
* fix(events): merge/teardown idempotency and data safety
Address Copilot review findings on native-config merge and teardown:
- #9: _has_marker now recurses into nested 'hooks' arrays so a matcher-group
containing Specify-owned inner hooks is recognized and replaced on upgrade
instead of accumulating duplicates.
- #11: _merge_json_fragment strips ALL Specify-marked entries from every event
before adding the new set, so an override that drops an event (pre_tool_use
-> stop) removes the stale marked entry instead of leaving it active.
- #3: an empty resolved map (--events false / disabled override) now runs the
native-config removal path instead of early-returning, so prior Specify
hooks are stripped. The shared dispatcher is left untouched (#10).
- #14: teardown deletes a Spec-Kit-created config that is now empty of user
content (rather than leaving '{}' that confused manifest.uninstall()),
while preserving pre-existing configs with user hooks/settings.
- #10: the shared .specify/events.py dispatcher is deleted only when no other
installed event-capable integration's manifest still references it, so
uninstalling one multi-install integration doesn't break the others.
- #8: Copilot's .github/hooks/speckit.json now merges owned entries (with
markers) into a pre-existing file instead of overwriting, and teardown
removes only owned entries (deleting the file when no user hooks remain).
- #22/#23: JSON/JSONC parse failures in native configs (Claude/Cursor/etc.
and opencode.json) abort the merge with a warning instead of resetting user
content to '{}'.
- #12: write destinations are validated (symlinked-ancestor rejection +
containment) before any bytes are written, so a symlinked .specify or
native config directory can't redirect writes outside the repository.
Refs: PR #3704 Copilot inline review (findings #3, #8, #9, #10, #11, #12, #14, #22, #23)
Assisted-by: opencode (model: glm-5.2, autonomous)
* fix(events): honor enabled flag, refresh on extension lifecycle, strict command validation
Address Copilot review findings on sourcing, validation, and lifecycle:
- #1: collect_extension_events now honors the extension registry's 'enabled'
flag — a disabled extension's events are skipped so disabling an extension
actually deactivates its runtime hooks. Adds refresh_integration_events(),
wired into extension add/remove/enable/disable, so installing, removing,
enabling, or disabling an extension regenerates each installed event-capable
integration's native event config (the documented install-after-init flow is
no longer inert, and disabled/removed extension events are stripped).
- #17: validate_events now requires 'command' to be a non-empty string, not
merely truthy, so a value like 'command: [foo]' is rejected at manifest
load instead of rendering into invalid native configuration.
- #15: updated PR #3704 description to the implemented events terminology
(.specify/events.py, events:, --events, integration-events.yml) replacing
the stale bridge.py / runtime_hooks: / --hooks false / integration-hooks.yml
references that no longer match the shipped API.
(#21 — user YAML override validation — was addressed in the prior tier.)
Refs: PR #3704 Copilot inline review (findings #1, #15, #17)
Assisted-by: opencode (model: glm-5.2, autonomous)
* revert: drop CHANGELOG.md/pyproject.toml version bumps from events fixes
Per maintainer request, the events PR no longer carries CHANGELOG entries or
pyproject version revs. This restores both files to their pre-PR (da6c20d9)
state: pyproject.toml back to 0.14.2.dev0 and the [Unreleased] block removed
from CHANGELOG.md. The AGENTS.md version-rev convention for __init__.py
changes is intentionally waived for this PR by maintainer decision.
This also clears the pending merge conflicts with upstream/main on these two
files (upstream's 0.14.2 release commit c0fe0e43): our side now makes no
net change to them relative to the merge-base, so a future upstream merge
takes theirs on both without conflict.
Assisted-by: opencode (model: glm-5.2, autonomous)
* fix(events): compose --events into Copilot/Devin options() (#8, #9)
Copilot and Devin are event-capable, but their options() overrides returned
only --skills without calling super(), so the base class never declared
--events. The documented --integration-options "--events false" opt-out was
therefore rejected as unknown for both adapters.
Both now compose with super().options() (mirroring Codex and Cursor) so
--events is declared alongside --skills. Added a TestEventCapableOptionsCompo
sition test class asserting --events appears in Copilot, Devin, Cursor, and
Codex options() output.
Refs: PR #3704 Copilot review 4790195897 (findings #8, #9)
Assisted-by: opencode (model: glm-5.2, autonomous)
* fix(events): Cursor version field, matcher grouping, Copilot cross-OS
Address three Copilot review findings on native-config generation:
- #7: Cursor's .cursor/hooks.json schema requires top-level "version": 1,
but json-flat used _merge_json_fragment() which only writes hooks, so a
freshly generated file was missing the required schema version. Added a
version kwarg to _merge_json_fragment (preserving a user's value if
present) and the Cursor json-flat branch now passes version=1.
- S3: json-nested placed all handlers under the first handler's matcher, so
two extensions registering the same event with different matchers both ran
for the first matcher and neither for the later. Handlers are now grouped
by distinct matcher, emitting one matcher-group per matcher (handlers
sharing a matcher stay in one group).
- S4: Copilot's bash and powershell fields both received the same
host-resolved command, so a config generated on Linux wrote a POSIX venv
path into the PowerShell hook (and vice-versa). _dispatcher_command gains
a target_os kwarg; Copilot now emits an independent POSIX interpreter
(python3) for bash and a Windows interpreter (python) for powershell, so
the checked-in config works on either OS.
Tests: added TestCursorJsonWriting (version present + preserved) and
matcher-grouping regressions (per-distinct-matcher, shared-matcher); updated
the Copilot generation test to assert bash != powershell with OS-appropriate
interpreters.
Refs: PR #3704 Copilot review 4790195897 (findings #7, S3, S4)
Assisted-by: opencode (model: glm-5.2, autonomous)
* fix(events): anchor py scripts and prefix ps launcher in command runner
Address two Copilot review findings on the core command runner:
- S2: the py variant called build_python_invocation() on the raw scripts:
command string, which left 'scripts/...' anchored at the project root
instead of under .specify/ (or .specify/extensions/<id>/). Every event
command in a project configured with --script py launched a nonexistent
project-root path. The py branch now shares the same base-anchoring as
sh/ps and prepends the resolved interpreter as argv (no shell quoting
needed for subprocess.run(shell=False)).
- S6: the ps variant returned the .ps1 path as the executable, but Windows
subprocess.run(shell=False) cannot execute a PowerShell script directly,
so event dispatch failed on the default Windows script type. The ps branch
now prefixes argv with 'pwsh -File' (PowerShell 7+), falling back to
'powershell -File' (Windows PowerShell) when pwsh is absent.
Tests: added test_py_variant_anchored_under_specify and
test_ps_variant_prefixed_with_powershell_launcher covering the new argv
shapes (interpreter + .specify-anchored path; launcher -File + path).
Refs: PR #3704 Copilot review 4790195897 (findings S2, S6)
Assisted-by: opencode (model: glm-5.2, autonomous)
* fix(events): skip-tracking on parse fail, drop dispatcher claim on retain, honor --events false in refresh, preserve layers on invalid override
Address four Copilot review findings on merge/teardown/refresh safety:
- S5: _merge_json_fragment/_merge_opencode_plugin_ref/_merge_copilot_json now
return bool (wrote). Install branches skip manifest.record_existing() and
created.append() when a merge was skipped on parse failure, so a user's
JSONC/malformed native config is not tracked and manifest.uninstall() can't
later delete the untouched file.
- S1: remove_integration_events now drops this integration's manifest claim on
the shared dispatcher (manifest.remove) even when the file is retained
because another integration references it. Previously the retained file
stayed tracked, so the subsequent manifest.uninstall() in teardown() saw
the matching hash and deleted the file another integration still depended
on. The unit test now exercises full teardown() (not just
remove_integration_events) to cover the gap.
- S7: refresh_integration_events reads each integration's stored
parsed_options via _resolve_integration_options and passes them to
resolve_events, so a persisted --events false is honored across extension
add/enable/disable instead of being discarded (which re-enabled events the
user had disabled).
- #10: an invalid override entry now abandons the entire override and keeps
the accumulated built-in + extension layers, instead of resetting
resolved_override to {} and assigning that empty map to events (which
silently disabled all hooks on a single typo). Only a fully-valid override
(including an explicit events: {}) replaces the prior layers.
Tests: added TestOverridePreserveLayers (invalid entry keeps layers; explicit
empty disables), TestSkippedMergeNotTracked (JSONC not recorded), and
TestDispatcherManifestClaimDroppedOnRetain (full teardown keeps dispatcher
when another integration references it). Added S7 refresh-honors-events-false
regression.
Refs: PR #3704 Copilot review 4790195897 (findings S5, S1, S7, #10)
Assisted-by: opencode (model: glm-5.2, autonomous)
* test(extensions): update stale validation-message assertion
The 'no commands/hooks/events' validation message changed to
'Extension must provide at least one command, hook, or event' when the
events feature added a third provider kind, but test_no_commands_no_hooks
still matched the old 'must provide at least one command or hook' text and
failed on every CI job. Update the regex to the current message.
Refs: PR #3704 CI failure (test_extensions.py:579)
Assisted-by: opencode (model: glm-5.2, autonomous)
* fix(events): forced-teardown data safety, manifest-driven command resolution, toml teardown safe-dest
Address three findings from Copilot review 4791088500:
- S9: _remove_native_event_hooks now unconditionally drops this integration's
manifest claim on the native config, not only when the file was deleted.
Previously a config whose owned entries were cleaned but user content
retained stayed tracked, so teardown(force=True) -> manifest.uninstall(
force=True) deleted the entire user-owned settings file. This is the
config-file mirror of the earlier shared-dispatcher fix.
- S8: _find_command_template resolved extension event commands via a broken
registry lookup (the registry stores per-agent registered_commands
name-lists, not a {name, file} map) and a file-stem scan that only matched
when the .md stem equaled the command name. A manifest mapping
speckit.selftest.extension -> commands/selftest.md resolved as missing. It
now enumerates installed extensions via ExtensionManager.get_extension()
and matches provides.commands[].name -> file, with the directory scan and
core-template lookups kept as fallbacks.
- R3: _remove_toml_entries now validates the destination with
_ensure_safe_destination before read/write, matching the merge path, so a
symlink swap of .codex/config.toml after install can't make teardown
overwrite a file outside the project.
Tests: forced full teardown preserves a user settings file; an extension
command whose file stem differs from its name resolves via the manifest;
TOML teardown rejects a symlinked config destination.
Refs: PR #3704 Copilot review 4791088500 (findings S8, S9, R3)
Assisted-by: opencode (model: glm-5.2, autonomous)
* fix(events): subprocess cwd, shell quoting, TOML matcher escaping, Tabnine ms
Address four findings from Copilot review 4791088500:
- R1: the generated dispatcher and resolve_and_run_event_command now run
their subprocesses with cwd set to the dispatcher-derived project root.
Previously 'specify event run' (and the resolved script) inherited the
agent's working directory, but event_run resolves the project via
Path.cwd(), so a hook fired from a subdirectory targeted the wrong project
and reported the command missing.
- R2: _dispatcher_command now shell-quotes each component (interpreter,
command, event) for the target shell (POSIX via shlex.quote; PowerShell via
single-quoted literals with doubled quotes). An interpreter path containing
spaces or an extension/override command containing shell metacharacters is
passed as a single argument instead of being reinterpreted by the native
hook shell. Claude's prefix is left unquoted so the
shell still expands it (prefix + relative path are fixed, safe strings).
- R4: the Codex TOML matcher is now rendered through the shared TOML escaper
like command, so a matcher containing a quote/backslash/newline/control
character no longer produces malformed config.toml.
- R5: Tabnine declares events_timeout_unit='ms' (its hook schema mirrors
Gemini's BeforeTool/AfterTool), so the 60s default becomes 60000ms instead
of timeout: 60 (60 ms), which would terminate the dispatcher immediately.
Tests: cwd-forced execution from a subdirectory; POSIX/PowerShell quoting of
metacharacter and space-bearing components; TOML matcher with a quote parses
cleanly; Tabnine timeout converts to 60000. Updated the Copilot generation
test for the new quoted args.
Refs: PR #3704 Copilot review 4791088500 (findings R1, R2, R4, R5)
Assisted-by: opencode (model: glm-5.2, autonomous)
* fix(events): POSIX dispatcher path constant + platform-agnostic tests
Three Windows test failures, one a real cross-OS bug:
- W1 (bug): EVENTS_DISPATCHER_REL was str(Path('.specify')/'events.py'),
which yields '.specify\events.py' on Windows. Manifest keys are stored in
POSIX form (.as_posix()), so 'dispatcher_rel in manifest.files' was always
False on Windows: the shared-dispatcher manifest-claim drop was skipped and
manifest.uninstall(force=True) deleted the dispatcher another integration
still depended on. Make it a POSIX constant (.as_posix()) so it matches
manifest keys on every platform.
- W2/W3 (tests): the py/ps argv assertions used endswith() and an exact
launcher-name set that broke on Windows backslash paths and the
pwsh.EXE/full-path launcher returned by shutil.which. Compare in POSIX form
and match the launcher by case-insensitive stem.
Refs: PR #3704 Windows CI failures
Assisted-by: opencode (model: glm-5.2, autonomous)
* fix(events): override layer preservation, matcher validation, event command-ref canonicalization
Address four Copilot review findings:
- C4: a malformed override handler (e.g. "stop: []" or "stop: bad-value")
normalizes to no handlers. Previously the entry was skipped and the override
still adopted, so an override whose only entry was malformed silently
disabled every built-in and extension hook. The empty-handler case now
abandons the whole override (keeps prior layers); an explicit "events: {}"
(no entries) remains a valid disable.
- C6: a non-mapping integration entry (e.g. "claude: bad") was coerced to
"events: {}" and treated as a valid explicit disable. It now warns and
abandons the override, keeping the accumulated layers. Only an explicitly
present, mapping-valued "events" field replaces the prior layers.
- C10: matcher is now validated as a string (or absent) in both
validate_events (manifest) and _validate_resolved_event (override). A
non-string matcher such as "matcher: []" previously passed validation but
crashed by_matcher.setdefault(matcher, ...) with TypeError: unhashable
type, aborting init or refresh.
- C11: ExtensionManifest._validate now applies the same rename + alias-lift
canonicalization to event command references that it already applies to
hook references. An event referencing an auto-corrected command (e.g.
my-ext.boot -> speckit.my-ext.boot) previously kept the obsolete name,
so dispatch reported no command and the event silently no-oped.
Tests: empty-handler/non-mapping override preserves layers; non-string
matcher rejected in manifest and abandoned in override; event command ref
lifted to canonical form with a warning.
Refs: PR #3704 Copilot review (findings C4, C6, C10, C11)
Assisted-by: opencode (model: glm-5.2, autonomous)
* fix(events): protect shared dispatcher from stale cleanup, delete Cursor version stub, non-destructive refresh
Address three Copilot review findings:
- C3: the shared .specify/events.py dispatcher is now in
events_stale_exclusions(). It is written into every event-capable
integration's manifest but reference-counted across them; an upgrade with
--events false omits events.py from the new manifest, so the generic stale
pass would delete it without the refcount check, breaking any other
installed event-capable integration. Its deletion is left to
remove_integration_events(), which checks the refcount.
- C5: _remove_json_entries now deletes a Spec-Kit-created Cursor file that
retains only {"version": 1} after all owned hooks are removed (we added the
version field), mirroring _remove_copilot_entries. Previously the generic
remover only deleted a literally-empty object, so clean teardown left a
generated stub behind.
- C12: refresh_integration_events now resolves first and calls
install_integration_events once, instead of running the destructive
_remove_native_event_hooks pre-step before resolution. A later failure
(invalid destination, write error, formatter error) no longer destroys the
working native config before the new one is written.
install_integration_events already removes stale Specify-marked entries and
handles an empty map (stripping prior hooks), so the pre-step was both
unsafe and redundant.
Tests: dispatcher in stale exclusions; Cursor version-only stub deleted on
teardown; refresh failure preserves the pre-existing config (no pre-strip).
Refs: PR #3704 Copilot review (findings C3, C5, C12)
Assisted-by: opencode (model: glm-5.2, autonomous)
* fix(events): host target uses POSIX quoting, Claude dispatcher double-quoted, & for windows
Address two Copilot review findings on the shell-quoting added in the prior
round (R2):
- C1: _shell_quote("host") now always uses POSIX shlex.quote, not PowerShell
single-quoting on Windows. The single-command-string formats
(Claude/Gemini/Qwen/Devin/Tabnine) are run via the agent's POSIX-ish shell
(Git Bash on Windows), and a single-quoted 'python' is not invoked as a
command by PowerShell without the call operator — so generated hooks failed
to launch the dispatcher on Windows. Safe tokens pass through bare
(python3, speckit.ext.cmd) on every platform. PowerShell single-quoting is
now used only for the explicit target_os="windows" (Copilot's powershell
field), where the quoted interpreter is prefixed with "& " so it is
actually invoked.
- C2: Claude's ${CLAUDE_PROJECT_DIR} dispatcher path is now double-quoted
("${CLAUDE_PROJECT_DIR}/.specify/events.py") so the variable still expands
(double quotes allow expansion in POSIX shells) but a project path
containing spaces no longer word-splits and breaks dispatcher launch.
Tests: host target never emits PowerShell quotes; windows target carries the
& call operator; Claude dispatcher is double-quoted; updated Copilot
generation assertions for the &-prefixed powershell command.
Refs: PR #3704 Copilot review (findings C1, C2)
Assisted-by: opencode (model: glm-5.2, autonomous)
* fix(events): opencode TS plugin resolves dispatcher from directory, execFileSync argv, forwards input+output
Address three Copilot review findings on the opencode TS plugin:
- C8: the dispatcher and interpreter are now resolved per-project at plugin
load from the `directory` OpenCode passes to the plugin factory, not
process.cwd(). OpenCode may be launched from a parent directory or host
another workspace, in which case process.cwd() pointed at the wrong project
and every event failed. The resolver prefers a project-local venv
interpreter, then falls back to python3.
- C9: the dispatcher is launched with execFileSync and an argv array
[interpreter, dispatcher, command, event] instead of a shell command string
built by interpolating the interpreter/command/event into a template
literal. Command/event strings are only validated as non-empty, so quotes or
backticks could previously break the generated TypeScript and shell
metacharacters could execute outside the dispatcher; an interpreter path
with spaces also failed. No shell is involved now.
- C7: tool callbacks now forward both `input` and `output` to runEvent
(combined into one JSON payload), so pre_tool_use can inspect the tool
arguments and post_tool_use can inspect the result — the primary payload for
those events. Previously only `input` was forwarded.
Tests: plugin resolves dispatcher/interpreter from `directory` (no
process.cwd() path.join), uses execFileSync (no shell string), and forwards
output to runEvent for both pre/post_tool_use.
Refs: PR #3704 Copilot review (findings C7, C8, C9)
Assisted-by: opencode (model: glm-5.2, autonomous)
* fix(events): Qwen ms timeout, Devin root-nested format, Copilot agentStop
Address three Copilot review findings on adapter mappings (verified against
each agent's published hook documentation):
- U1: Qwen Code command hooks measure timeout in milliseconds (default
60000), per the Qwen Code hooks docs. The adapter previously inherited the
seconds default, so every generated handler got timeout: 60 (60 ms) and was
killed before the dispatcher could start. Declare events_timeout_unit="ms".
- U2: Devin's .devin/hooks.v1.json is a root event map ({"PreToolUse": [...]})
with no top-level "hooks" wrapper (the docs state "the hooks object is the
entire file"). The adapter reused json-nested, which writes events under a
"hooks" key Devin never reads. Add a json-root-nested format with a matching
writer (_merge_json_root) and remover (_remove_json_root_entries) that
operate on the root event keys, sharing the matcher-grouping, marker, and
JSONC-abort behavior of the nested variants.
- U3: Copilot CLI supports the canonical per-turn stop lifecycle as native
agentStop; add "stop": "agentStop" to the mapping so an extension's stop
handler fires for Copilot.
Tests: Qwen timeout converts to 60000; Devin events written at the root (no
"hooks" wrapper) and teardown preserves user root entries; Copilot stop maps
to agentStop.
Refs: PR #3704 Copilot review (findings U1, U2, U3)
Assisted-by: opencode (model: glm-5.2, autonomous)
* fix(events): collect events via validated manifest, surface refresh failures
Address two Copilot review findings:
- R1: collect_extension_events now reads events from a validated
ExtensionManifest (whose command refs were canonicalized at install
validation, C11) instead of the raw extension.yml YAML. Previously an
event command ref like my-ext.boot was normalized to speckit.my-ext.boot
during install validation, but the on-disk YAML kept the obsolete name;
refresh then emitted it and _find_command_template could not match it,
leaving the hook silently inert. Registry-tracked extensions use the
validated manifest; on-disk extensions not yet in the registry fall back
to the raw YAML (preserving the partial-staged-install scan behavior).
- R3: refresh_integration_events now accumulates per-integration failures
and raises EventRefreshError at the end (after refreshing the others) so
the extension lifecycle commands (add/remove/enable/disable) can't claim
an extension was fully deactivated while a stale native hook may still be
active. A new _refresh_events_and_warn helper surfaces the aggregated
failures as a warning at each call site without aborting the overall
command (the extension was already added/removed/enabled/disabled).
Tests: event command ref canonicalized via the validated manifest;
refresh failure raises EventRefreshError (aggregated) while still preserving
the pre-existing config.
Refs: PR #3704 Copilot review (findings R1, R3)
Assisted-by: opencode (model: glm-5.2, autonomous)
* fix(events): probe venv for specify_cli before selecting it; python on Windows
Address two Copilot review findings on interpreter resolution:
- R2: the dispatcher's _find_specify and the opencode TS resolver both
selected a project-local venv python and ran `-m specify_cli` without
checking that specify_cli is importable there. In a typical project where
Spec Kit is installed globally (or via uv tool) but the project has its own
unrelated virtualenv, every event invoked that interpreter and failed
instead of reaching the PATH `specify` fallback. Both now probe the
candidate interpreter (subprocess `import specify_cli` / execFileSync probe)
before selecting it, falling through to the fallback when the venv lacks
Spec Kit.
- S2: the opencode TS PATH fallback was always `python3`, which is commonly
unavailable on Windows. It is now `python` on Windows
(process.platform === 'win32') and `python3` on POSIX.
Tests: the generated dispatcher contains the _has_specify_cli probe and the
PATH fallback; the opencode TS plugin probes for specify_cli and uses a
platform-appropriate PATH interpreter.
Refs: PR #3704 Copilot review (findings R2, S2)
Assisted-by: opencode (model: glm-5.2, autonomous)
* fix(events): serialize opencode TS plugin string literals as JSON
Address Copilot review finding S1: command and matcher values come from
user/extension YAML but were interpolated into single-quoted TypeScript
literals without escaping. A quote, backslash, or backtick in a command or
matcher produced invalid generated TypeScript and could inject code into the
plugin. _build_opencode_plugin now serializes every interpolated value
(command, event name, native hook key, matcher tool names) as a JSON string
literal via json.dumps, which produces a valid double-quoted, fully-escaped
TS/JS string.
Tests: a command and matcher containing quotes/backticks render inside JSON
double-quoted literals; the dangerous single-quoted form is absent. Updated
the forwards-output test for the new double-quoted literals.
Refs: PR #3704 Copilot review (finding S1)
Assisted-by: opencode (model: glm-5.2, autonomous)
* fix(events): thread per-handler timeout through dispatcher, bash launcher for sh on Windows
Address two Copilot review findings:
- S4: the dispatcher and inner runner both hardcoded timeout=120, so a valid
handler configured with a timeout above 120 seconds could never run for its
full duration. The resolved per-handler timeout now flows through the chain:
_dispatcher_command appends it (in the integration's native unit, plus a
small buffer) as a 4th argument; the generated dispatcher reads sys.argv[3]
and uses it for its inner subprocess and the `event run` invocation;
`event run` accepts a timeout argument and passes it to
resolve_and_run_event_command, which uses it for the script subprocess.
Defaults to 120s when absent (backward compat with already-deployed
dispatchers that don't pass the arg).
- S5: for a project configured with the sh script type on Windows,
subprocess.run(shell=False) cannot execute a .sh file directly (chmod
doesn't change that). The sh variant now prefixes a bash/sh launcher
(resolved via shutil.which) on Windows, mirroring the ps branch's
pwsh -File handling.
Tests: dispatcher reads the timeout arg and uses it; the native command
appends the resolved timeout; the sh variant uses a launcher on Windows.
Refs: PR #3704 Copilot review (findings S4, S5)
Assisted-by: opencode (model: glm-5.2, autonomous)
* fix(events): delete shared dispatcher when last event integration disables events
Address Copilot review finding S3: the empty-resolved-map install path
(--events false upgrade, or override disabling events) stripped prior native
hooks but left the shared dispatcher behind. Because the new manifest no
longer claims it and stale cleanup excludes it (C3), .specify/events.py
became permanently orphaned when this was the last event-capable
integration — uninstall could not remove it.
Extracted the dispatcher refcount cleanup into _cleanup_shared_dispatcher
(shared by remove_integration_events and the empty-map install path) and
called it from the empty-map path so the dispatcher is deleted when no other
installed event-capable integration's manifest references it, while still
being retained when another integration does.
Tests: an --events false upgrade of the last event integration deletes the
dispatcher; with another integration still referencing it, the dispatcher is
retained.
Refs: PR #3704 Copilot review (finding S3)
Assisted-by: opencode (model: glm-5.2, autonomous)
* fix(events): map user_prompt_submit/stop for Gemini and Tabnine
Address two Copilot review findings on adapter mappings:
- S6: Gemini exposes BeforeAgent for the per-turn prompt-submit lifecycle
point (verified against Gemini CLI's hooks docs — BeforeAgent fires after
the user submits a prompt, before planning). The mapping omitted
user_prompt_submit, so valid extension handlers were skipped. Added
user_prompt_submit -> BeforeAgent.
- S7: Tabnine's Gemini-compatible schema also provides BeforeAgent and
AfterAgent, but the mapping omitted user_prompt_submit and stop. Added
user_prompt_submit -> BeforeAgent and stop -> AfterAgent so those
extension events fire instead of being warned about and skipped.
Tests: Gemini and Tabnine mappings include BeforeAgent/AfterAgent.
Refs: PR #3704 Copilot review (findings S6, S7)
Assisted-by: opencode (model: glm-5.2, autonomous)
* fix(events): correct timeout unit threading through dispatcher and opencode TS
Address two Copilot review findings on the per-handler timeout threading
added in the prior round (S4):
- R2: _dispatcher_command passed _native_timeout(timeout_seconds) as the
dispatcher's 4th argument, but the dispatcher interprets that argument as
seconds. For Gemini/Qwen/Tabnine (ms adapters), 60 seconds became 60000
seconds (~16h). It now passes the raw seconds (no unit conversion). The
+5s buffer moves to the native hook timeout field
(_native_timeout(seconds + EVENT_TIMEOUT_BUFFER)) so the agent's outer cap
fires after the dispatcher's inner subprocess timeout — letting the inner
kill its child cleanly instead of being killed mid-flight (which orphaned
the grandchild script process).
- S3: the opencode TS runEvent hardcoded timeout: 60000 (60s) and invoked the
dispatcher without its timeout argument, so handlers configured above 60s
were killed early while the inner runner defaulted to 120s. runEvent now
accepts a timeoutSec parameter (seconds); execFileSync uses
(timeoutSec + buffer) * 1000 ms and appends String(timeoutSec) to the
dispatcher argv, so both layers honor the per-handler timeout.
Tests: the dispatcher arg is raw seconds for ms adapters (60, not 60000); the
native timeout field carries the buffer (65 for a 60s Claude handler); opencode
runEvent threads the per-handler timeout as the 5th argument.
Refs: PR #3704 Copilot review (findings R2, S3)
Assisted-by: opencode (model: glm-5.2, autonomous)
* fix(events): skip disabled extensions in _find_command_template and disk fallback
Address Copilot review finding S1: _find_command_template resolved event
commands without filtering enabled: false — the registry loop used
registry.keys() and the raw directory fallback could also rediscover
disabled extensions. If native cleanup is skipped (e.g. a JSONC config
cannot be parsed), a stale hook would therefore continue executing a
disabled extension.
Extracted the disabled-ID logic into _disabled_extension_ids (shared with
collect_extension_events) and applied it to both the manifest-resolution
loop and the on-disk fallback scan in _find_command_template, so a disabled
extension's command is never resolved for dispatch.
Tests: a disabled extension's command resolves to None via both the manifest
loop and the disk-fallback path.
Refs: PR #3704 Copilot review (finding S1)
Assisted-by: opencode (model: glm-5.2, autonomous)
* fix(events): delete shared dispatcher regardless of fresh manifest claim
Address Copilot review finding S2: _cleanup_shared_dispatcher gated the
no-other-references deletion on `dispatcher_rel in manifest.files`. An
`integration upgrade --integration-options "--events false"` passes a fresh
manifest (created in _migrate_commands) that never recorded the dispatcher,
so the condition was false even though the old on-disk manifest owned the
file — and stale cleanup explicitly excludes it (C3), leaving
.specify/events.py orphaned after the last integration disabled events.
The refcount deletion now runs independently of whether the new manifest
contains the key; manifest.remove() stays conditional (a no-op when the key
is absent).
Tests: an upgrade passing a fresh manifest (no dispatcher claim) still
deletes the shared dispatcher when no other integration references it.
Refs: PR #3704 Copilot review (finding S2)
Assisted-by: opencode (model: glm-5.2, autonomous)
* fix(events): refresh native event config after extension update
Address Copilot review finding S4: the _refresh_events_and_warn helper was
wired to extension add/remove/enable/disable, but not to extension_update,
which replaces the installed extension.yml (remove + install_from_zip).
If an update adds, removes, or changes event declarations, native configs
remained stale until a manual integration upgrade.
extension_update now refreshes once after the update loop finalizes its
successful updates (skipped on rollback/failure), mirroring the other
lifecycle commands.
Refs: PR #3704 Copilot review (finding S4)
Assisted-by: opencode (model: glm-5.2, autonomous)
* fix(events): make the dispatcher self-contained for one-time/temporary installs
Address Copilot review finding R1: the dispatcher required a persistent
`specify` executable at runtime. The supported one-time flow runs
`specify init` through a temporary `uvx` environment that is discarded, so
generated hooks later reached the PATH fallback with no `specify` on PATH
and every event failed.
The generated .specify/events.py is now self-contained:
- Preferred path: it imports specify_cli.events.resolve_and_run_event_command
when the package is importable (durable pip/pipx/uv-tool install), which
handles extension manifests whose file stem differs from the command name
and the project's custom script selection, staying in sync with the CLI.
- Fallback path: an inline stdlib-only resolver finds the command template,
parses its scripts: frontmatter, resolves the project's script variant
(reading .specify/init-options.json directly), and runs the script with
the correct launcher (pwsh/bash/interpreter), so one-time and temporary
installs work without a persistent `specify` executable on PATH.
The `event run` CLI command remains available for manual use; the dispatcher
no longer depends on it.
Tests: the dispatcher delegates to specify_cli when importable and falls back
to the inline resolver when it is not; the inline fallback finds the command
template and runs its script end-to-end (shadowing specify_cli with an empty
package to force the fallback); the preferred path also runs end-to-end.
Refs: PR #3704 Copilot review (finding R1)
Assisted-by: opencode (model: glm-5.2, autonomous)
* fix(events): validate safe destination on all removers and teardown unlinks
Address Copilot review findings (inline #1, suppressed #2, #3):
- Guard all removers (_remove_json_entries, _remove_copilot_entries,
_remove_json_root_entries, _remove_opencode_entries, _remove_native_event_hooks),
_cleanup_shared_dispatcher, and remove_integration_events with
_ensure_safe_destination(dst) before reading, rewriting, or unlinking.
- Prevents teardown or removal operations from overwriting or unlinking external
files if a config file, plugin path, or .specify directory is replaced with
a symlink post-installation.
Tests: added unit tests in TestSafeWriteDestination covering JSON config,
OpenCode plugin, and TOML teardown symlink rejection.
Refs: PR #3704 Copilot review (findings inline #1, suppressed #2, #3)
Assisted-by: opencode (model: glm-5.2, autonomous)
* fix(events): manifest-driven resolution and disabled-extension filter in dispatcher template
Address Copilot review finding (suppressed #1):
- In _EVENTS_DISPATCHER_TEMPLATE's _find_command_template, read
.specify/extensions/.registry to identify disabled extensions (enabled == false).
- Parse provides.commands in each enabled extension's extension.yml to match
command_name to its declared file, so commands whose file stem differs
from the command name (e.g. speckit.selftest.extension -> commands/selftest.md)
resolve correctly when specify_cli is unavailable (one-time uvx installs).
- Skip disabled extensions in both manifest-driven and on-disk fallback scans.
Refs: PR #3704 Copilot review (finding suppressed #1)
Assisted-by: opencode (model: glm-5.2, autonomous)
* fix(events): positive integer timeout validation and OpenCode multi-handler error aggregation
Address Copilot review findings (suppressed #4, #6):
- In validate_events and _validate_resolved_event, validate that timeout (when
present) is a positive integer (isinstance(t, int) and not isinstance(t, bool)
and t > 0). Rejects string, boolean, zero, or negative timeouts at manifest
and override validation time instead of crashing during setup/refresh.
- In _build_opencode_plugin, wrap each runEvent invocation inside _ev() in a
try/catch block, collect error messages, and throw an aggregate error at the
end if any handler failed. Guarantees that all handlers for an event execute
to completion even if an earlier handler throws.
Tests: added TestTimeoutValidation testing string, boolean, and zero timeout
rejections; updated OpenCode plugin merging tests for try/catch error collection.
Refs: PR #3704 Copilot review (findings suppressed #4, #6)
Assisted-by: opencode (model: glm-5.2, autonomous)
* fix(integrations): use native dollar skill invocations
Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 02f9e138-da58-4a60-93b9-eae659d2aa19
* fix(integrations): preserve skill post-process idempotence
Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 02f9e138-da58-4a60-93b9-eae659d2aa19
* fix(integrations): preserve literal skill invocations
Resolve generated command references with the active agent prefix instead of rewriting all slash-form text during post-processing.
Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 02f9e138-da58-4a60-93b9-eae659d2aa19
* fix(integrations): preserve shared invocation prefix
Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 02f9e138-da58-4a60-93b9-eae659d2aa19
* fix(integrations): preserve install invocation prefix
Pass dollar-style skill prefixes through bare-project integration installation and cover the shared template output.
Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 02f9e138-da58-4a60-93b9-eae659d2aa19
* fix(integrations): preserve dollar refs everywhere
Use agent-native invocation prefixes in extension command registration and dynamic shared-script command hints.
Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 02f9e138-da58-4a60-93b9-eae659d2aa19
* fix(shared-infra): preserve dollar command hints
Escape dollar-prefixed commands embedded in Bash strings and propagate the native prefix into installed Python command helpers.
Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 02f9e138-da58-4a60-93b9-eae659d2aa19
* fix(shared-infra): render native helper prefixes
Rewrite installed Bash and PowerShell formatter return expressions so direct callers receive the selected integration's native prefix.
Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 02f9e138-da58-4a60-93b9-eae659d2aa19
* fix(skills): use invocation-neutral hook guidance
Describe hook-derived references as command invocations so dollar-prefixed skills do not receive contradictory slash-command terminology.
Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 02f9e138-da58-4a60-93b9-eae659d2aa19
* test(integrations): expect native fallback invocation
Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 02f9e138-da58-4a60-93b9-eae659d2aa19
* refactor(integrations): centralize invocation prefix selection
Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 02f9e138-da58-4a60-93b9-eae659d2aa19
* fix(integrations): add Kimi /skill: prefix and fix docstrings
- Add SKILL_COLON_AGENTS frozenset and get_invocation_prefix() to
_invocation_style.py so Kimi resolves to '/skill:' in skills mode
- Switch invoke_prefix_for_integration() to use get_invocation_prefix()
instead of the binary dollar/slash check
- Update post_process_skill_content docstring (base.py) to cover both
slash and dollar native invocation forms
- Update _resolve_command_refs_in_skill docstring (presets/__init__.py)
to document the dollar-prefixed result alongside slash forms
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 65ef91d9-4c31-4f31-a009-ed2093fe7f28
Assisted-by: GitHub Copilot (model: claude-sonnet-4.6, autonomous)
* fix(agents): use get_invocation_prefix for Kimi in register_commands
Replace the binary is_dollar_skills_agent ternary with get_invocation_prefix
so that __SPECKIT_COMMAND_*__ tokens in Kimi skill files resolve to
/skill:speckit-<name> rather than /speckit-<name>.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 65ef91d9-4c31-4f31-a009-ed2093fe7f28
Assisted-by: GitHub Copilot (model: claude-sonnet-4.6, autonomous)
* fix(agents): remove unused is_dollar_skills_agent import
Leftover from replacing the inline ternary with get_invocation_prefix.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 65ef91d9-4c31-4f31-a009-ed2093fe7f28
Assisted-by: GitHub Copilot (model: claude-sonnet-4.6, autonomous)
* fix(integrations): use get_invocation_prefix in post_process_skill_content
Replaces the binary is_dollar_skills_agent ternary with get_invocation_prefix
so that Kimi's hook-command note is injected as /skill:speckit-git-commit from
the start. This keeps _inject_hook_command_note idempotent for Kimi: the
previous note with its native prefix now matches on repeated passes, preventing
duplicate note injection.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 65ef91d9-4c31-4f31-a009-ed2093fe7f28
Assisted-by: GitHub Copilot (model: claude-sonnet-4.6, autonomous)
* fix(presets): use get_invocation_prefix in _resolve_skill_command_refs
Replace the binary is_dollar_skills_agent ternary with get_invocation_prefix
so Kimi tokens resolve to /skill:speckit-* directly rather than /speckit-*
(which previously relied on the broad post-process body replacement).
Also fix test_restore_skill_preserves_dollar_command_refs to write raw_core
with the unresolved __SPECKIT_COMMAND_PLAN__ token, exercising the resolver
rather than bypassing it with a pre-resolved string.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 65ef91d9-4c31-4f31-a009-ed2093fe7f28
Assisted-by: GitHub Copilot (model: claude-sonnet-4.6, autonomous)
* docs(presets): document /skill: form in _resolve_skill_command_refs
Add /skill:speckit-<cmd> to the docstring so the contract covers all
three native prefix forms returned by get_invocation_prefix.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 65ef91d9-4c31-4f31-a009-ed2093fe7f28
Assisted-by: GitHub Copilot (model: claude-sonnet-4.6, autonomous)
* test(integrations): add Kimi /skill: prefix coverage
- test_skill_colon_prefix_core_command: resolve_command_refs with /skill: prefix
- test_get_invocation_prefix_skill_colon: get_invocation_prefix returns /skill:
for kimi (skills), / for kimi (non-skills), $ for codex, / for claude
- test_kimi_skill_post_processing_is_idempotent: verifies Kimi's hook-command
note is injected with /skill: prefix and does not duplicate on re-runs
- test_installed_bash_formatter_uses_skill_colon_prefix: shared-infra bash
formatter outputs /skill:speckit-plan when installed with /skill: prefix
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 65ef91d9-4c31-4f31-a009-ed2093fe7f28
Assisted-by: GitHub Copilot (model: claude-sonnet-4.6, autonomous)
* fix(kimi): use get_invocation_prefix in process_template, remove broad replacement
process_template() was still using a binary is_dollar_skills_agent ternary
to select between dollar and slash prefix, so Kimi tokens were emitted as
/speckit-* and then corrected by a broad .replace('/speckit-', '/skill:speckit-')
in KimiIntegration.post_process_skill_content(). That broad replacement would
also rewrite any literal /speckit-* text in generated skill content, contrary
to the PR's token-only behavior.
- Use get_invocation_prefix(agent_name, invoke_separator == '-') in
process_template() so Kimi tokens are emitted as /skill:speckit-* directly.
- Remove the broad .replace() from KimiIntegration.post_process_skill_content();
it is now a no-op (tokens are already correctly prefixed at source).
- Add test_process_template_kimi_uses_skill_colon_prefix to guard the fix.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 65ef91d9-4c31-4f31-a009-ed2093fe7f28
Assisted-by: GitHub Copilot (model: claude-sonnet-4.6, autonomous)
---------
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 02f9e138-da58-4a60-93b9-eae659d2aa19
Copilot-Session: 65ef91d9-4c31-4f31-a009-ed2093fe7f28
* 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>
* 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>
* 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>
* fix: register enabled extensions for agent on integration install/upgrade
install and upgrade only set up the integration's own core commands; only
switch re-registered the enabled extensions' commands for the target agent.
A second integration added via install (or refreshed via upgrade) was
therefore silently missing the extension commands the existing agents
already had (e.g. the bundled agent-context extension).
Extract switch's registration into a shared _register_extensions_for_agent
helper and call it from install and upgrade too, so every installed agent
ends up with every enabled extension's commands — full parity with switch.
Closes#2886
* test: pin skills-mode secondary-agent registration; document #2948 limitation
Extension skill rendering is scoped to the active agent (init-options track a
single ai / ai_skills pair), so a skills-mode agent registered while not active
(e.g. Copilot --skills installed as a secondary integration) gets command files
rather than skills. install/upgrade match extension add here; only switch
renders skills, because it activates the target first.
Add a regression test pinning this behavior and document the limitation on the
shared helper. Per-agent skills parity is tracked separately in #2948.
* fix: don't re-render the active agent's skills when registering a non-active agent
register_enabled_extensions_for_agent runs an active-agent-scoped skills pass
(_register_extension_skills resolves the skills dir from init-options["ai"],
ignoring the passed agent). Routing install/upgrade of a secondary integration
through it re-rendered the *active* skills-mode agent's extension skills as a
side effect — resurrecting skill files the user had deliberately deleted. Gate
the skills pass on the target being the active agent; switch is unaffected
because it activates the target first.
Also harden the skills-mode install test (assert a core skill so --skills is
load-bearing, drop a vacuous registered_skills assertion) and add a regression
test. Surfaced by review of the PR; skills parity for non-active agents stays
tracked in #2948.
* refactor: share the extension-op scaffold and run (un)registration post-commit
Review cleanups, no behavior change on the success path:
- Extract the best-effort ExtensionManager scaffold (lazy import, instantiate,
except -> _print_cli_warning) into _best_effort_extension_op. Both
_register_extensions_for_agent and a new _unregister_extensions_for_agent
delegate to it, removing the duplicate block left inline in switch.
- Invoke the best-effort extension registration AFTER the install/switch/upgrade
try/except has committed, so a failure in it can never trigger the rollback
(install and switch teardown on except).
* docs: clarify extension registration parity scope
* fix(integrations): defer extension registration until use
* fix(tests): remove redundant shutil import
* fix(integrations): backfill extensions for installed switch targets
* fix: preserve .vscode/settings.json and script +x bit on integration upgrade
During 'specify integration upgrade', Phase 2 stale-cleanup removes files
present in the old manifest but absent from the new one. Copilot's setup()
merges into an existing .vscode/settings.json and stops tracking it, so the
file was being deleted on upgrade (destroying user settings). Add a
stale_cleanup_exclusions() hook that integrations use to protect such
conditionally-tracked merge targets. Also restore the executable bit on
shared .sh scripts after the managed-refresh step on POSIX.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* fix: address review on stale-cleanup fix
- Normalize stale_cleanup_exclusions() to POSIX before subtracting from
manifest keys, so exclusions built with os.path.join / backslashes still
match on Windows.
- Strengthen test_upgrade_preserves_existing_vscode_settings to add a
user-defined key and assert it survives the upgrade (via --force, exercising
the merge + stale-cleanup path) instead of the brittle after == before check.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
---------
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* refactor(integrations): co-locate integration commands in integrations/ domain dir
- Remove commands/ stubs (handlers will live in domain dirs)
- Move all integration CLI handlers out of __init__.py into integrations/
- Split into focused modules under integrations/:
_helpers.py (340 lines) — domain helpers
_install_commands.py (306 lines) — install / uninstall
_migrate_commands.py (487 lines) — switch / upgrade
_query_commands.py (442 lines) — list / use / search / info / catalog
_commands.py (34 lines) — app objects + register()
- __init__.py reduced by ~1400 lines; integration block replaced with register() call
- Fix patch paths in tests to new module locations
* fix(integrations): restore original integration list output in refactor
Preserve the CLI Required column, post-table default/installed summary,
and no-installed guidance that were dropped during the no-behavior-change
refactor of integration list into _query_commands.py.
* Potential fix for pull request finding
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* fix(integrations): restore _clear/_update_init_options public imports
The refactor that split integration commands moved
_clear_init_options_for_integration and _update_init_options_for_integration
into integrations/_helpers.py, but tests still import them from the top-level
specify_cli package, causing ImportError. Re-export them with explicit aliases
at the end of __init__.py to preserve the public import surface.
---------
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
