Commit Graph

1633 Commits

Author SHA1 Message Date
kanfil
f8e474d6fd feat: first-class agent-native runtime hooks for integrations (#3704)
* 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)
2026-07-29 08:00:26 -05:00
Ali jawwad
1fff7a196d fix(extensions): guard the required manifest sections so one bad extension cannot break extension list (#3797)
ExtensionManifest.REQUIRED_FIELDS only checks key PRESENCE, so a section that is
written but left empty (`provides:` -> None) or given the wrong shape
(`provides: []`) passes it and then fails on first use:

    extension: null  -> TypeError: argument of type 'NoneType' is not iterable
    requires:  null  -> TypeError: argument of type 'NoneType' is not iterable
    provides:  null  -> AttributeError: 'NoneType' object has no attribute 'get'
    provides:  []    -> AttributeError: 'list' object has no attribute 'get'

Neither is a ValidationError, so both escape the callers that already handle
malformed manifests. list_installed() catches ValidationError only and has a
deliberate "Corrupted extension" fallback, so a single bad extension took down
the whole command -- reproduced end-to-end:

    before: specify extension list -> exit 1, raw AttributeError, no output
    after:  specify extension list -> exit 0, the good extension listed, the
            bad one shown as "Corrupted extension"

Add an isinstance guard for each required section, mirroring the nested guards
already in this function ("Invalid provides.commands: expected a list", "Invalid
hooks: expected a mapping") and _load_yaml's document-root check. Only the three
REQUIRED sections lacked one.

`provides: {}` is unaffected: it is a well-shaped mapping, so an extension that
provides only hooks still validates, and with no hooks it keeps the pre-existing
"must provide at least one command or hook" message. Both are locked by tests.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-29 07:54:25 -05:00
Noor ul ain
b048e339a5 fix(presets): escape installed preset metadata in Rich output (#3826)
* fix(presets): escape installed preset metadata in Rich output

`preset.yml` is user-editable, but the installed-preset display paths
interpolated its fields straight into `console.print`, where Rich parses
`[...]` as a style tag. PR #3773 escaped the *catalog* branch of these
commands; the local branch was left behind, so the same field rendered
correctly from a catalog and incorrectly once installed.

Two failure modes:

- Silent data loss: a description `Does [stuff] nicely` renders as
  `Does  nicely`.
- Hard crash: an unbalanced tag such as `Broken [/red] tag` raises
  `rich.errors.MarkupError`, aborting `preset list`/`preset info` with a
  traceback and exit code 1 — the preset cannot be inspected at all.

Escaped the installed branch of `preset list` (name/id/version/
description) and `preset info` (name/id/version/description/author/tags/
repository/license plus the per-template description), and the catalog
branch's tags join that the earlier sweep missed.

`preset resolve` was unescaped throughout: it echoes its own
`template_name` argument, so `preset resolve 'no[/red]such'` crashed on
user input alone. Also escaped the resolved paths, layer sources, and
composition-error message.

Separately, the composition chain's `[{strategy_label}]` was consumed as
a style tag, so every chain line printed a blank label instead of
`[base]`/`[append]`. Escaped the literal bracket as `\[`, matching the
step-graph line in `workflow info`.

Regression tests in `TestInstalledPresetRichMarkup` cover all five
behaviours; each fails before this change.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* test(presets): cover catalog tags and resolve escapes

Addresses Copilot review feedback on #3826: two escapes added by the
previous commit had no regression assertion, so they could be reverted
with the suite still green.

- `test_info_escapes_catalog_markup` asserted every catalog field except
  `tags`; the new tag assertion only exercised an installed preset. Assert
  the rendered tags join in the catalog branch too.
- The escapes on `preset resolve`'s resolved path, layer source, and
  composition-error message were untested. Add three cases patching
  `PresetResolver` to feed markup through the top-layer line, the no-layer
  `resolve_with_source` fallback, and a markup-bearing `resolve_content`
  exception.

Test-the-test: with `_commands.py` reverted to the pre-fix revision, 9 of
the 10 markup tests fail (was 5); with the fix applied all 10 pass.

A closing tag cannot be embedded in the mocked path — `Path` treats the
`/` as a separator — so the path assertion uses an opening tag for the
swallowing case and the unbalanced tag rides on the adjacent `source`
field on the same line.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Assisted-by: Claude Code (model: claude-opus-5, under direct human supervision)

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-29 07:48:28 -05:00
Ali jawwad
d99170fb5e fix(workflows): dispatch prompt steps via the resolved executable (#3793)
PromptStep._try_dispatch runs `subprocess.run(exec_args, ...)` with an
UNRESOLVED argv[0] -- a bare name like `claude`. On Windows subprocess.run calls
CreateProcess, which does not consult PATHEXT, so an agent CLI installed as a
`.cmd`/`.bat` shim (the usual npm layout) raises FileNotFoundError [WinError 2].
That OSError is swallowed by the method's `except OSError: return None`, and
execute() then reports "CLI not found or not installed" -- even though the
step's own preflight `shutil.which(...)` two lines earlier just found it.

The sibling path does not have this bug: IntegrationBase.dispatch_command (used
by the `command` step) resolves argv[0] through shutil.which first, added in
8e5643d for exactly this reason. Same machine, same integration, CLI present as
a .cmd shim:

    type: prompt  -> failed    "integration 'claude' CLI not found or not installed."
    type: command -> completed

Primitive confirmation: bare `subprocess.run(["fakeagent"])` raises
[WinError 2] while `subprocess.run([shutil.which("fakeagent")])` runs fine.

Reuse the path the preflight already resolved (`fallback_cli_path`) instead of
calling which() again, so the shim is executed. On POSIX it is the same
executable, so behaviour is unchanged there.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-29 07:44:04 -05:00
Manfred Riem
f04a36a629 chore: release 0.14.4, begin 0.14.5.dev0 development (#3850)
* chore: bump version to 0.14.4

* chore: begin 0.14.5.dev0 development

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
2026-07-29 07:09:24 -05:00
Ali jawwad
be33d2a5f6 fix(bundler): degrade non-UTF-8 config reads into BundlerError (#3784)
yamlio.py is the single chokepoint for every bundler read, and its module
docstring states the contract: "All reads/writes go through these functions so
that IO failures degrade into actionable BundlerError rather than raw
tracebacks."

Both readers catch only OSError, but `Path.read_text(encoding="utf-8")` and
`json.load()` raise UnicodeDecodeError on a non-UTF-8 file --
`issubclass(UnicodeDecodeError, OSError)` is False (its MRO is UnicodeError ->
ValueError). So the decode error escaped uncaught:

    load_yaml: LEAKED UnicodeDecodeError -> 'utf-8' codec can't decode byte 0xff
    load_json: LEAKED UnicodeDecodeError -> 'utf-8' codec can't decode byte 0xff

In load_json, json.JSONDecodeError does not help: it is a *sibling* of
UnicodeDecodeError, not a parent.

This is realistic rather than theoretical -- on Windows, PowerShell 5.1's
`Out-File` and `>` default to UTF-16, so a hand-edited
`.specify/bundle-catalogs.yml` or records file hits it.

Widen both read clauses to `(OSError, UnicodeError)`, matching the sibling
catalog readers (catalogs.py:101, workflows/catalog.py:336). JSONDecodeError
deliberately stays FIRST so malformed-but-decodable JSON keeps its more
specific "Invalid JSON" message; a regression test locks that ordering.

Write paths are unaffected -- verified that dump_yaml/dump_json do not leak
UnicodeEncodeError (both escape unencodable input), so this stays scoped to the
two read paths.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-28 17:29:27 -05:00
Ali jawwad
751eae727e fix(workflows): escape the step-progress line so step ids render (and / stops failing the run) (#3783)
`workflow run` and `workflow resume` both print the step-progress line as
`f"  ▸ [{sid}] {label} …"`. Rich parses the bracketed step id as a style tag,
which produces three failures on main:

1. The id is SILENTLY SWALLOWED on every run -- the only identifying content on
   the line. `id: greet` prints "  ▸  shell …"; "[greet]" is absent.
2. An id that forms a closing tag FAILS THE WHOLE RUN. `validate_workflow`
   places no charset restriction on step ids, so `id: "/"` is a valid workflow;
   the callback then raises MarkupError, which propagates into execute()'s
   handler -> run persisted as `failed` with empty `step_results`, the step
   never executed, exit 1 with a Rich internals error.
3. An id that is a real style (`bold`, `red`) is applied as FORMATTING to the
   rest of the line.

The unescaped `label` (from `step_config["command"]`) compounds it.

Escape the literal bracket with `\[` and escape both interpolated values, at
both sites. This mirrors the `\[<type>]` step-graph precedent already in this
file (workflow_info). Escaping only the values is NOT sufficient -- the
f-string's own brackets are what Rich consumes.

Verified through the real CLI: ids `greet`/`bold`/`a]b` now render verbatim, and
`id: "/"` goes from a failed run to `Status: completed`.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-28 17:28:22 -05:00
github-actions[bot]
4ad7ef2b42 Update Agent Parity Governance preset to v0.4.1 (#3830)
Update agent-parity-governance preset submitted by @hindermath to:
- presets/catalog.community.json (version, download_url, documentation, description, updated_at)
- docs/community/presets.md community presets table

Closes #3829

Assisted-by: GitHub Copilot (model: claude-sonnet-4.6, 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>
2026-07-28 17:25:54 -05:00
Ali jawwad
6ef96373e2 fix(integrations): reject empty --commands-dir in generic raw_options (#3714)
* fix(integrations): reject empty --commands-dir in generic raw_options

GenericIntegration._resolve_commands_dir has a parity gap: the parsed-options
branch guards emptiness (`if commands_dir:`), but the raw_options fallback
returned the value verbatim with no check. So `--integration-options=
"--commands-dir="` (or `--commands-dir ""`) resolves to `""`, which makes
setup() compute `dest = project_root / "" == project_root` and write every
speckit command file (specify.md, plan.md, ...) directly into the PROJECT
ROOT — silently bypassing the documented "--commands-dir is required"
contract and polluting the repo root.

Apply the same non-empty guard to the raw_options branch so an empty value
falls through to the existing "required" ValueError on every input form.
Non-empty values resolve exactly as before.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(integrations): reject a BLANK --commands-dir, not just an empty one

Self-review follow-up: bare truthiness only closes the empty-string subset. A
whitespace-only value passed both branches (verified: raw "--commands-dir ' '"
returned ' ', parsed {"commands_dir": "  "} returned '  '), so command files
still landed in a directory literally named " " instead of failing with the
documented "required" error.

Require a non-BLANK value and normalize the padding, in the parsed branch as
well as raw_options so the two cannot drift apart -- a padded but real value
(" .myagent/cmds ") now resolves to ".myagent/cmds" rather than being rejected,
matching how other padded config references are normalized.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(integrations): use strip() only to test blankness, return the value verbatim

Address review feedback: normalizing with strip() changed EXISTING valid values,
contrary to this PR's "no behaviour change for valid usage" claim -- a quoted
`--commands-dir ' commands '` previously targeted the literal ` commands `
directory and would have started writing to `commands` instead.

The blankness test still uses strip(), but the accepted value is now returned
unchanged, so the fix stays limited to empty/blank input. Test updated
accordingly: a padded non-blank value must round-trip verbatim (quoted in
raw_options, since shlex.split() consumes unquoted padding before this code
sees it).

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-28 17:25:04 -05:00
Ali jawwad
4bc79fe243 fix(presets): guard non-list/non-mapping provides.templates in PresetManifest (#3712)
* fix(presets): guard non-list/non-mapping provides.templates in PresetManifest

PresetManifest._validate iterated provides["templates"] with no shape guards,
unlike the sibling ExtensionManifest. A malformed third-party preset.yml
crashed with a raw TypeError that escapes the install handler's
PresetValidationError/PresetError catch and dumps an unhandled traceback:

  templates: 5       -> "'int' object is not iterable"
  templates: [null]  -> "argument of type 'NoneType' is not iterable"
  templates: [5]     -> "argument of type 'int' is not iterable"

(and a string/list entry raised the misleading "Template missing 'type',
'name', or 'file'"). Add a container list-guard and a per-entry mapping-guard
that raise a clean PresetValidationError, mirroring ExtensionManifest's
provides.commands guards. Valid manifests (list of mappings) are unaffected.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(presets): check provides.templates type before emptiness

Address review feedback: the new shape guard sat behind the existing truthiness
check, so a FALSY non-list (templates: 0/false/null/''/{}) still reported the
misleading "Preset must provide at least one template" instead of the type
error. Only truthy non-lists (5, "oops", {"a": 1}) reached the guard, which is
why the original test (templates: 5) passed.

Split the checks: presence -> container type -> emptiness. A falsy non-list now
reports "expected a list"; an EMPTY LIST keeps the "at least one template"
message, since that genuinely is a well-typed container with no templates.
Parametrize the non-list test over truthy AND falsy values, and add a
regression guard for the empty-list message.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* test(presets): drop the redundant empty-list templates test

Address review feedback: the added test duplicated the pre-existing
test_no_templates_provided -- both set provides.templates to [] and assert the
same "must provide at least one template" error. That test already guards the
empty-list result of the type-before-emptiness ordering, so keeping mine only
added maintenance. Left a pointer comment where it was.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-28 17:24:01 -05:00
Ali jawwad
596a31ee0f fix(auth): resolve az via shutil.which so azure-cli token works on Windows (#3709)
* fix(auth): resolve az via shutil.which so azure-cli token works on Windows

AzureDevOpsAuth._acquire_via_az_cli runs subprocess.run with a bare "az".
On Windows the Azure CLI is installed as az.cmd, and subprocess.run calls
CreateProcess, which does not consult PATHEXT -- so a bare "az" fails with
WinError 2 even after `az login`, and azure-cli token acquisition silently
returns None (the OSError is swallowed).

Resolve the executable with shutil.which("az") (which honors PATHEXT) before
the call, mirroring the maintainer's own fix in integrations/base.py for the
same CreateProcess/.cmd issue. `or "az"` preserves prior behavior (and the
existing not-installed OSError path) when az is absent. POSIX is unaffected.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(auth): require an absolute az path so the CWD cannot hijack the lookup

Self-review catch on my own change: resolving with a bare
`shutil.which("az") or "az"` widened an execution surface. On Windows
shutil.which prepends the CURRENT DIRECTORY to the search path (unless
NoDefaultCurrentDirectoryInExePath is set) AND honors PATHEXT, so a stray
.\az.cmd / .\az.bat in the working directory resolves ahead of the real Azure
CLI -- for a credential operation. Verified: with the real az scrubbed from
PATH, shutil.which("az") returns '.\az.CMD'.

Accept the resolution only when it is absolute; otherwise fall back to the bare
"az" (which also preserves the existing not-installed OSError path). A
legitimate install always resolves absolutely, so the Windows .cmd fix this PR
exists for is unaffected. The not-installed and PATHEXT tests are extended with
relative-result cases, all of which fail before this commit.

Note: integrations/base.py resolves executables the same way; hardening that
shared path is a separate concern and is left untouched here.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* test(auth): build the mocked az path with the host's path rules

Fixes the macOS CI failure. The test hardcoded a Windows absolute path, but the
production code calls os.path.isabs() -- on POSIX runners "C:\Program
Files\..." reads as RELATIVE, so the fallback branch ran and argv[0] was "az"
instead of the resolved path.

Construct the path with os.path.join(os.path.abspath(os.sep), ...) so it is
absolute under the host's rules, and assert against that value. The fallback
test's inputs (".\az.CMD", "az.cmd", "./az") are relative under both ntpath
and posixpath, so they were already portable.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-28 17:21:42 -05:00
Ali jawwad
52a6514e38 fix(workflows): reject falsy non-mapping workflow-catalogs.yml top level (#3707)
* fix(workflows): reject falsy non-mapping workflow-catalogs.yml top level

WorkflowCatalog._load_catalog_config parsed the config with
`yaml.safe_load(...) or {}`, then checked `isinstance(data, dict)`. The
`or {}` coerces a FALSY non-mapping top level (`[]`, `false`, `0`, `''`) to
`{}` *before* the guard runs, so those are silently swallowed as "empty
config" and fall back to the built-in defaults -- while a TRUTHY non-mapping
(`5`, a bare list) correctly raises. Same silent-swallow inconsistency the
bundler catalog reader fixed for its own config.

Drop the `or {}` and branch on `None` (empty document / explicit `null`)
explicitly: `None` stays a valid no-op, every non-mapping (falsy or truthy)
now raises the same actionable error. Correct configs are unaffected.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* docs(workflows): describe the catalog-config fallthrough accurately

The comment said a None return means "no project catalogs, fall back to the
built-in defaults". Both halves were imprecise: _load_catalog_config serves the
project AND user configs, and get_active_catalogs falls through env -> project
-> user -> built-in, so a None from the project layer moves on to the USER
config; the built-in defaults apply only once every layer returned None.

Reword the loader comment and the mirror test docstring. Comments only -- no
behaviour change.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(workflows): close the same falsy-mask gap in 'catalogs' and StepCatalog

Self-review follow-up: the top-level fix left the identical asymmetry live five
lines below, and again in this file's twin loader.

1. WorkflowCatalog._load_catalog_config: the ``catalogs`` shape check sat behind
   an emptiness check, so a FALSY non-list (``catalogs: {}``/``''``/``0``/
   ``false``) was silently swallowed as "no catalogs" while ``catalogs: 5``
   raised. Verified before this commit: ``catalogs: {}`` -> None (no error).
   Shape now checked first; absent/explicit-null and empty-list stay no-ops
   (matching the bundler's reader).

2. StepCatalog._load_catalog_config -- the step-catalog twin, read the same way
   -- still had ``yaml.safe_load(...) or {}``, so falsy non-mappings bypassed its
   isinstance guard (``[]`` -> None while ``5`` raised). Same two guards applied,
   keeping the two loaders in lockstep.

Eight new parametrized cases, all failing before this commit.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* test(workflows): move StepCatalog guards into TestStepCatalog and add the nested case

Address three review points:

1. The StepCatalog regression tests sat inside TestWorkflowCatalog, so a
   targeted `pytest ...::TestStepCatalog` run skipped them entirely. Moved into
   that class, where the duplicated twin loader belongs.
2. StepCatalog had no nested-value coverage (only top-level). Added the
   parametrized falsy ``catalogs:`` case, plus the absent/null/empty no-op
   cases. Verified against upstream/main's catalog.py: 8 fail there, pass here.
3. Dropped the inaccurate parity parenthetical. src/specify_cli/catalogs.py
   RAISES for missing/empty ``catalogs`` and coerces a null document to {}, so
   it is not the behavior this loader matches -- the comment now just states
   what changed (only the misreported shapes) without claiming parity.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-28 17:20:21 -05:00
Ali jawwad
3dad624e5d fix(integrations): render hyphenated /speckit-<name> for Droid (always-slash agent) (#3688)
* fix(integrations): render hyphenated /speckit-<name> for Droid (always-slash agent)

DroidIntegration is an always-skills agent: it installs commands as
.factory/skills/speckit-<name>/SKILL.md and its build_command_invocation
returns the hyphenated /speckit-<name>. But "droid" was missing from every
_invocation_style set, so is_slash_skills_agent("droid", True) returned False
and both HookExecutor._render_hook_invocation and `specify init` next-steps
fell through to the dotted /speckit.<name> form — a command Droid never
registers.

Add "droid" to ALWAYS_SLASH_AGENTS, matching its always-skills siblings
grok/trae/zed/devin (each added there by their own integration PR; droid's
#3587 omitted it).

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* test(integrations): assert Droid is ALWAYS-slash (disabled case too)

Address review: the test only covered ai_skills=True, which would also pass
if Droid were miscategorized as CONDITIONAL_SLASH. Add the ai_skills=False
assertion — True there is what distinguishes an ALWAYS_SLASH agent from a
conditional one.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-28 17:14:31 -05:00
github-actions[bot]
0231ee056b [preset] Update A11Y Governance preset to v0.4.2 (#3828)
* Update A11Y Governance preset to v0.4.2

Update a11y-governance preset submitted by @hindermath to:
- presets/catalog.community.json (version, download_url, documentation, description, updated_at)
- docs/community/presets.md community presets table

Closes #3827

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

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

---------

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 <175728472+Copilot@users.noreply.github.com>
2026-07-28 16:55:37 -05:00
github-actions[bot]
ca2b494335 [preset] Update Parallel Autonomous Run Governance to v0.2.4 (#3825)
* Update Parallel Autonomous Run Governance preset to v0.2.4

Update parallel-autonomous-run-governance preset submitted by @hindermath:
- presets/catalog.community.json (version, download_url, documentation, description, requires.extensions, tags, updated_at)
- docs/community/presets.md community presets table

Closes #3824

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

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

---------

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 <175728472+Copilot@users.noreply.github.com>
2026-07-28 16:23:34 -05:00
Quratulain-bilal
86c4610b7d fix: correct Optional type annotation for _resolved_dir parameter (#3801) 2026-07-28 16:07:27 -05:00
Quratulain-bilal
a2b0d0d3c1 fix: add timeout to prompt step subprocess execution (#3768)
The prompt step subprocess.run() had no timeout, allowing a hung
LLM invocation to block the entire workflow engine indefinitely.
The shell step already defaults to 300s timeout.

Add timeout parameter (defaulting to 300s, matching shell step)
and handle subprocess.TimeoutExpired gracefully.
2026-07-28 15:49:45 -05:00
Quratulain-bilal
56b1839fba fix: handle tags containing / in GitHub release asset URL resolution (#3767)
The tag extraction in resolve_github_release_asset_api_url split the
URL path on / and assumed the tag was a single segment at index 4.
Tags containing literal / (e.g. feature/v1) would be split across
multiple segments, causing the tag to be truncated to only the first
part and the asset name to include leftover tag segments.

Fix by reconstructing the tag as all segments between 'download' and
the final asset segment: tag = '/'.join(parts[4:-1]), asset = parts[-1].
2026-07-28 15:45:38 -05:00
Marsel Safin
98b2551ade fix(presets): escape catalog metadata in discovery output (#3773)
Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-07-28 15:36:57 -05:00
github-actions[bot]
9bb7206d3b Update Autonomous Run Governance preset to v0.3.3 (#3823)
Update autonomous-run-governance preset submitted by @hindermath to:
- presets/catalog.community.json (version, download_url, documentation, description, tags, updated_at)
- docs/community/presets.md community presets table

Closes #3821

Assisted-by: GitHub Copilot (model: claude-sonnet-4.6, 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>
2026-07-28 15:18:57 -05:00
Quratulain-bilal
89e204ca3c fix: use bounded read for integration catalog HTTP responses (#3763)
* fix(skills): match closing frontmatter delimiter on its own line

SkillsIntegration.setup parsed each command template's frontmatter with
raw.split("---", 2). A bare substring split stops at the first `---`
*anywhere*, so a template whose description embeds `---` (e.g.
"Separate sections with --- markers") truncated the parsed frontmatter:
later keys were dropped, the description fell back to the generic default,
and the leftover frontmatter spilled into the skill body.

Scan for the closing `---` on its own line instead, for both the
description parse and the body strip. The frontmatter block is parsed
unstripped so trailing newlines in literal (|) block scalars still survive,
and the body slice keeps the newline after the marker so output stays
byte-for-byte identical to the old split for well-formed templates.

Adds regression tests covering the dashed-description truncation and the
frontmatter-spilled-into-body cases.

* fix: use bounded read for integration catalog HTTP responses

The integration catalog fetch used unbounded resp.read() to read
HTTP responses into memory. A malicious or misconfigured catalog
server could return an arbitrarily large response causing OOM.

Replace with read_response_limited() capped at MAX_JSON_METADATA_BYTES
(1 MiB), consistent with how other JSON fetch paths in the codebase
(_version.py, _github_http.py, authentication/azure_devops.py) already
enforce bounded reads.

Pass error_type=IntegrationCatalogError so oversized catalogs are
caught by the existing per-entry recovery path in
_get_merged_integrations() rather than aborting the entire merge.

Add regression test verifying oversized responses are rejected as
IntegrationCatalogError and that healthy catalogs remain usable.
2026-07-28 14:32:31 -05:00
21Silva
88e997306f docs: add Simplified Chinese translation of README (#3740)
Add README.zh-CN.md with a hand-crafted (non-machine) Chinese
translation of the project README, and add a language switcher
link at the top of both README files.

Code blocks, command names, badges, and links are kept identical to
the English source; only prose is translated.

Co-authored-by: yifosheng001 <yifosheng001@ke.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-28 13:59:39 -05:00
github-actions[bot]
186ca25c99 Update Intake Sequencing Governance preset to v0.2.2 (#3809)
Update intake-sequencing-governance preset submitted by @hindermath to:
- presets/catalog.community.json (version, download_url, documentation, description, provides)
- docs/community/presets.md community presets table

Closes #3807

Assisted-by: GitHub Copilot (model: claude-sonnet-4.6, 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>
2026-07-28 13:18:10 -05:00
Ali jawwad
a9c9905400 fix(workflows): reject non-string/non-boolean 'condition' in if/while/do-while steps (#3706)
* fix(workflows): reject non-string 'condition' in if/while/do-while steps

`if_then`, `while_loop`, and `do_while` validate() confirm `condition` is
present but never that it is a string. execute() feeds it to
`evaluate_condition()`, which returns a non-string as-is and takes `bool()`
of it -- so `condition: [1, 2]` (a list authoring mistake) silently resolves
to `True`, branching wrongly / spinning the loop to `max_iterations`, with no
error reported.

Reject a present-but-non-string `condition` at validation, mirroring the
existing prompt/shell/command 'must be a string' guards. `"true"`/`"false"`
and expressions like `"{{ ... }}"` are strings, so they stay valid.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* docs(workflows): describe the evaluate_expression/evaluate_condition split accurately

Address review feedback: the guard comments attributed the non-string
pass-through to evaluate_condition(), which always returns a bool. It is
evaluate_expression() (called by evaluate_condition) that returns a non-string
unchanged; evaluate_condition then applies bool() to that value.

Reword all four sites (if/while/do-while guards + the mirror test comment) to
name the two stages correctly. Comments only -- no behaviour change.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(workflows): keep a literal bool 'condition' valid

Self-review catch: the guard rejected EVERY non-string, which broke an input
that previously worked. An unquoted ``condition: false`` is idiomatic YAML and
resolves exactly today -- evaluate_expression passes the bool through and
evaluate_condition's bool() is a no-op (verified: evaluate_condition(False) is
False, (True) is True). The if/while steps even default ``condition`` to the
bool ``False`` themselves, so bool is the field's natural type, not an
authoring mistake.

Accept (str, bool) and reject only the genuinely silent-coercion types
(list/dict/int/float, e.g. condition: [1, 2] is always True). Message updated
to "must be a string or boolean"; the bad-value tests drop True and gain 1.5,
and each step gains a positive bool case.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-28 11:40:36 -05:00
Marsel Safin
054fb7723d fix(bundle): escape catalog metadata in discovery output (#3774)
* fix(bundle): escape catalog metadata in discovery output

Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix(bundle): escape provides fallback values

Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-07-28 11:35:49 -05:00
Noor ul ain
8bfe6e14d1 fix(workflows,extensions): tolerate non-list catalog tags in search/info display (#3770)
`workflow search`, `workflow info`, `extension search` and `extension info`
crashed with `TypeError: 'int' object is not iterable` when a catalog entry
carried a scalar `tags:` value (e.g. `tags: 5`). Catalog payloads are
user-editable YAML/JSON, so this shape reaches the display unvalidated.

Both backends already guard their tag *filter* with
`isinstance(raw_tags, list)` — `WorkflowCatalog.search` and
`ExtensionCatalog.search` skip a non-list `tags` cleanly. Only the display
paths were unguarded: they tested truthiness (`if info.get("tags"):`) and
then iterated. A scalar is truthy but not iterable, so `--tag` filtering
survived while plain `search`/`info` rendering blew up.

Note this is distinct from the non-string *element* handling added in
#3746/#3747: coercing elements with `str(t) for t in ...` does not help when
`tags` is not a sequence at all. The fix is the guard the sibling
integration commands already use — `integrations/_query_commands.py:332,402`
gate on `isinstance(tags, list) and tags`. This aligns workflows and
extensions with that reference pattern, leaving all four tag-join display
sites in these modules consistent.

Regression tests drive the full CLI via CliRunner and cover search + info in
one case per module; both fail before the fix with the exact TypeError.


Assisted-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-28 11:24:19 -05:00
Marsel Safin
a482fb2fce fix: correct nullable resolved directory annotation (#3771)
Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-07-28 11:23:02 -05:00
Noor ul ain
05cb3cba34 fix(presets): tolerate non-string and non-list catalog fields in preset search/info (#3769)
* fix(presets): tolerate non-string and non-list catalog fields in preset search/info

`preset search` and `preset info` crashed with a raw traceback on catalog
payloads that are valid YAML/JSON but not string-typed. Catalog files are
user-editable, so these shapes reach the code unvalidated.

`PresetCatalog.search` had three unguarded assumptions:

- `--author` called `.lower()` on the raw value → `AttributeError: 'int'
  object has no attribute 'lower'` for `author: 789`.
- the query searchable-text join passed raw `name`/`description` through →
  `TypeError: sequence item 0: expected str instance, int found`.
- the `--tag` filter iterated `tags` without a list check, so a scalar
  `tags: 5` (truthy, not iterable) raised `TypeError: 'int' object is not
  iterable`.

PR #3743 fixed only the non-string *elements* of `tags` here; a non-list
`tags` and the `author`/`name`/`description` fields were still unguarded.
The sibling catalogs already handle all of these — `extensions/__init__.py`
and `integrations/catalog.py` coerce with `str(...)` and gate on
`isinstance(raw_tags, list)`. This aligns presets with them.

The same scalar-`tags` crash reached the four display sites in
`presets/_commands.py`, so those now gate on `isinstance(tags, list)`,
matching `integrations/_query_commands.py`. Note `PresetManifest.tags`
returns `self.data.get("tags", [])` and manifest validation does not
enforce list-ness, so a local `preset.yaml` with `tags: 5` validates
successfully and then crashed `preset info` — hence the guard on the
local-manifest branch too.

While here, `preset search` printed tags unescaped, so a tag containing
`[bold]` was silently swallowed as a Rich style tag; it now routes through
`_escape_markup` like the `preset list` line directly above it.

Regression tests in `TestPresetTagsNonString` drive the full CLI path via
CliRunner. All five fail before the fix, each with the exact exception it
targets.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Assisted-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Potential fix for pull request finding

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

* chore: regenerate security audit requirements (annotated-doc 0.0.5)

The Security Audit workflow's "Check committed audit requirements are
current" step regenerates requirements with `uv pip compile --upgrade`,
which now resolves annotated-doc==0.0.5. Re-sync the committed snapshot
so the check passes. No pyproject dependency changes; upgrade drift only.

Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: b3d0333b-4a36-4f69-9273-3ac0c3f46481

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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>
Copilot-Session: b3d0333b-4a36-4f69-9273-3ac0c3f46481
2026-07-28 11:07:51 -05:00
Marsel Safin
2e44ed60e8 fix(integrations): escape catalog metadata in discovery output (#3772)
* fix(integrations): escape catalog metadata in discovery output

Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix(integrations): escape unknown query IDs

Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-07-28 10:20:31 -05:00
github-actions[bot]
999f8e6497 Update Verify Review Ship extension to v0.4.2 (#3792)
Update verify-review-ship extension submitted by @cadugevaerd to:
- extensions/catalog.community.json (version, download_url, sha256, updated_at)
- docs/community/extensions.md community extensions table

Closes #3791

Assisted-by: GitHub Copilot (model: claude-sonnet-4.6, 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>
2026-07-28 10:20:07 -05:00
Ben Buttigieg
655a3cb8ca fix(integrations): preserve native skill invocation prefixes (#3663)
* 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
2026-07-28 15:48:40 +01:00
github-actions[bot]
809b4c5e26 Update Intake Review Governance preset to v0.2.0 (#3796)
Update intake-review-governance preset submitted by @hindermath:
- presets/catalog.community.json (version, download_url, documentation, description, scripts, tags)
- docs/community/presets.md community presets table

Closes #3794

Assisted-by: GitHub Copilot (model: claude-sonnet-4.6, 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>
2026-07-28 09:37:09 -05:00
github-actions[bot]
1354eade99 fix(constitution): stop propagating guidance into templates (#3737) (#3790)
Issue #3737 asked the /constitution command to synchronize constitutional
guidance into every effective task/plan/spec template, including active
preset-provided replacements. This changes the fix's direction: rather than
teach the command to discover and edit more template layers, it removes the
template-propagation behavior entirely.

Why this is the correct fix:

- The governed templates do not embed constitutional content. plan-template
  carries a runtime placeholder ("[Gates determined based on constitution
  file]") and spec-template/tasks-template reference no principles at all.
- The consuming commands read .specify/memory/constitution.md at runtime and
  derive their Constitution Check gates live (plan, tasks), and analyze is the
  dedicated drift checker that validates spec/plan/tasks against the
  constitution. Enforcement is therefore already automatic and always current.
- Statically editing template files fights the preset/override composition
  system: a replace preset shadows an edited core template entirely, and a
  hand-edited versioned preset file is clobbered on its next update. Presets and
  extensions are formalized, versioned artifacts the command must not mutate.

So the original bug (constitution edits missing active preset templates) is
resolved by not propagating at all: the runtime read is the single source of
truth. The /constitution command is scoped to its own artifact — it drafts and
writes the constitution and reports a Sync Impact Report changelog, and no
longer reads, edits, or reports on plan/spec/tasks/preset/extension templates.

Refs #3737

Assisted-by: GitHub Copilot (model: Claude Opus 4.8, supervised)

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 5b80589c-74e8-42e5-b2cb-7a7e0d69a964
2026-07-28 09:17:32 -05:00
Manfred Riem
2a29b534ae chore: release 0.14.3, begin 0.14.4.dev0 development (#3795)
* chore: bump version to 0.14.3

* chore: begin 0.14.4.dev0 development

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
2026-07-28 09:15:50 -05:00
github-actions[bot]
d8d62756fe Update Intake Authoring Governance preset to v0.3.0 (#3788)
Update intake-authoring-governance preset submitted by @hindermath to:
- presets/catalog.community.json (version, download_url, documentation, description, provides, tags, updated_at)
- docs/community/presets.md community presets table

Closes #3780

Assisted-by: GitHub Copilot (model: claude-sonnet-4.6, 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>
2026-07-28 08:34:17 -05:00
Faqeha Noor
9602ad2edf fix(copilot): honor preset command template overrides (#3592)
* fix(copilot): honor preset command template overrides

* fix(copilot): resolve canonical preset command names

---------

Co-authored-by: Faqeha Noor <faqehanoor022@gmail.com>
2026-07-28 08:26:12 -05:00
orize
39f2ac3c63 clarify: require real interrogatives, ban topic-label questions (#3745)
* clarify: require real interrogatives, ban topic-label questions

Agents often present topic labels or bare requirement ids as "questions",
which are not answerable on their own. Require a full interrogative under
**Question:**, a plain-language stake sentence, then Recommended/options.

Co-authored-by: Cursor <cursoragent@cursor.com>

* Update templates/commands/clarify.md

* clarify: allow requirement ids only after the ?

Resolves Copilot feedback: an interrogative ending in ? cannot also have
a parenthesized id "at the end of the question." Exact format is now
`**Question:** <interrogative>?` or `**Question:** <interrogative>? (FR-023)`.

Co-authored-by: Cursor <cursoragent@cursor.com>

---------

Co-authored-by: Lorin O'Brien <lorin@pronto.net>
Co-authored-by: Cursor <cursoragent@cursor.com>
2026-07-28 07:58:12 -05:00
Eric X Engstfeld
7fc5b236c8 feat: Add Alquimia AI integration (#2734)
* Add alquimia-ai as new integration: https://alquimia.ai

* Fix test cases for alquimia-ai integration. Add alquimia-ai to workflow.yml

* Add install url to alquimia-ai integration

* Renamed alquimia-ai to alquimia (cli native denomination)

* Fix unit tests for alquimia integration

* Minor fix in alquimia integration

* Fix typos and copilot findings

* Potential fix for pull request finding

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

* Potential fix for pull request finding

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

* Update tests cases and lint formatting

* Final fixes

* Rename alquimia_ai to alquimia module integration

* Make cli optional for alquimiia integration

* resolve review comments

* Fix copilot review

* Minor fixes: naming, remove unused code

* Update tests cases. Fix issues

* Fix unit tests

* Add alquimia context to default agent-context extension. Update cli requirment to support workflows

* Fix hints (suggestion)

* Add Alquimia AI as agent in github issue template. Fix unit tests

* Address review comments. Update docs

* Update test cases

---------

Co-authored-by: Eric Engstfeld <ericengstfeld@Erics-MacBook-Pro.local>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
2026-07-28 07:54:13 -05:00
Pascal THUET
118062eac4 harden: secure extension and preset archive downloads (#3141)
* harden: secure extension and preset archive downloads

Adopt the shared download-security primitives from #3140 across extension and
preset catalog, package, direct-URL, and ZIP-install flows:

- bound catalog, package, and inline manifest reads;
- verify catalog SHA-256 values when present;
- replace path-only extraction with bounded traversal/symlink-safe extraction;
- validate malformed hosts and ports before opening download URLs;
- handle normalized trailing-backslash directory entries consistently.

Redirect enforcement and checksum verification remain owned by the shared
helpers already on main; this commit wires them into extension and preset
behavior.

Assisted-by: OpenAI Codex (model: GPT-5, autonomous)

* harden: close archive and catalog download edge cases

Preflight ZIP central directories before ZipFile allocates them, bound both
declared and actual payload sizes, and reject ambiguous or non-portable archive
paths before extraction.

Keep extension update manifest selection consistent with extraction, reject
unsafe catalog-derived output filenames and malformed URL types, and escape
untrusted values in download errors.

Add regression coverage for parser differentials, collisions, platform-specific
filenames, bounded call sites, and failure ordering.

Assisted-by: OpenAI Codex (model: GPT-5, autonomous)

* harden: address download security review feedback

Assisted-by: OpenAI Codex (model: GPT-5, autonomous)

* harden: close ZIP preflight review gaps

Assisted-by: OpenAI Codex (model: GPT-5, autonomous)

* fix: harden extension update preflight and rollback

Assisted-by: OpenAI Codex (model: GPT-5, autonomous)

* fix: harden extension update rollback

Assisted-by: OpenAI Codex (model: GPT-5, autonomous)
2026-07-28 07:52:07 -05:00
Quratulain-bilal
0117a7b977 fix: correct Optional type annotation for context_note parameter (#3765)
The context_note parameter in CommandRegistrar methods was annotated as
\str = None\ which is a type lie — the default is None but the type
hint says str. Static type checkers (mypy/pyright) would flag this as
an error. Changed to \Optional[str] = None\ for correctness, consistent
with how extension_id (same class) is already typed.
2026-07-27 16:12:20 -05:00
Noor ul ain
2355fcb350 Update AGENTS.md (#2626)
* docs: layer contributor-onboarding sections onto AGENTS.md

Rebased onto current main and reworked so the additions match the
current architecture rather than the stale base this branch was written
against. The original revision documented the retired Windsurf
integration and a CLI-managed `context_file` field that no longer
exists (context files are now owned by the opt-in agent-context
extension), and described the manifest at the wrong path with a
non-existent API.

This version keeps all current AGENTS.md content unchanged and adds four
onboarding-focused sections, verified against the code:

- Quickstart — Add a New Integration in 5 Steps (links into the existing
  step-by-step section; notes context files are extension-owned)
- IntegrationManifest — File Tracking (correct path
  .specify/integrations/<key>.manifest.json and real API:
  record_file / record_existing / hash-guarded uninstall)
- Error Handling and Debugging (symptom/cause/fix table + debug tips)
- Contribution Checklist

Purely additive (+88 lines, no deletions); all internal anchors resolve.

Assisted-by: Claude Opus 4.8 (model: claude-opus-4-8, autonomous)
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* Potential fix for pull request finding

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

* Potential fix for pull request finding

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
2026-07-27 15:17:38 -05:00
Noor ul ain
98c9e67ce2 fix(extensions): tolerate non-string catalog name in display-name lookup (#3747)
* fix(extensions): tolerate non-string catalog name in display-name lookup

_resolve_catalog_extension() filters catalog search results by display
name with `ext["name"].lower() == argument.lower()`. Extension catalog
JSON is user-editable, so a hand-authored non-string name (e.g.
`name: 123`) crashes the filter with `AttributeError: 'int' object has
no attribute 'lower'`, taking down `extension info <name>` and
`extension add <name>`. A missing `name` key would likewise KeyError.

Coerce defensively with `str(ext.get("name", "")).lower()`, matching the
ambiguous-match display block just below (which already str()-coerces
name for the same reason). A bad-named entry simply doesn't match,
yielding a clean not-found error instead of a traceback.

Adds a regression test invoking `extension info <name>` against a
mocked catalog whose search result has `name: 123`; it fails pre-fix
with AttributeError.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* Potential fix for pull request finding

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

* Potential fix for pull request finding

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
2026-07-27 15:00:26 -05:00
Noor ul ain
6136706ef3 fix(presets): coerce non-string catalog tags before joining (#3743)
* fix(presets): coerce non-string catalog tags before joining

Preset catalog payloads are user-editable YAML/JSON, so a `tags:` list
can legitimately contain non-strings (e.g. numeric tags). The preset
list/search/info display paths and the catalog search backend joined
tags with a raw `", ".join(...)` / used `t.lower()`, which raised
`TypeError: sequence item N: expected str instance, int found` (or
`AttributeError` on `.lower()`) and crashed the command.

Sibling command surfaces already guard this — extensions, integrations,
and workflows coerce with `str(t) for t in ...`. This aligns presets:

- `_commands.py`: `preset list`, `preset search`, and both `preset info`
  branches now join `str(t) for t in ...`.
- `__init__.py` `PresetCatalog.search`: tag filter uses `str(t).lower()`
  and the searchable-text join coerces tags to `str`.

Adds regression tests driving `preset search` and `preset info` through
CliRunner with numeric tags; both fail before the fix with the TypeError.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* Potential fix for pull request finding

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
2026-07-27 14:14:59 -05:00
Marsel Safin
683bfd00c9 fix: register extensions for the active integration only (#3459)
* fix: register extensions for the active integration only

extension add registered commands for every detected agent, and
integration upgrade back-filled enabled extensions for non-active
integrations. Maintainer direction on #2948: treat the project as
single-active. Only the active integration gets extension artifacts;
use/switch rescaffold the target when the user selects it.

- extension add now routes through the all-agents pass restricted to
  the active integration (only_agent), keeping detection and
  missing-skills-dir recovery safeguards. Projects without recorded
  init-options fall back to detection-based registration.
- integration upgrade re-registers extensions only when upgrading the
  active integration, reversing the #2886 back-fill for non-active
  targets at maintainer request.

Fixes #2948

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix: address review feedback on active-only extension registration

- Restrict the extension-add active-integration fallback to projects
  with no recorded active key at all. A recorded but unsupported key
  (e.g. "generic", deliberately excluded from AGENT_CONFIGS) no longer
  falls back to registering every detected agent.
- Apply the same single-active rule to preset command overrides:
  PresetManager._register_commands now scopes registration to the
  active integration via only_agent.
- Add PresetManager.register_enabled_presets_for_agent, mirroring
  ExtensionManager.register_enabled_extensions_for_agent, and call it
  from integration use/switch/upgrade (active only) alongside the
  existing extension re-registration so presets are rescaffolded on
  activation instead of being written for inactive integrations.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix: address second round of review feedback (priority order, fail-closed, docs)

- register_enabled_presets_for_agent now processes presets in reverse
  priority order (lowest-precedence first) so the highest-precedence
  preset is written last and actually wins after `integration use`
  rescaffolds two overlapping preset command overrides. Verified this
  reproduces the previously reported reversed-priority bug and that the
  fix resolves it.
- _register_commands_for_active_agent now checks for the "ai" key's
  presence separately from its value: a missing key still falls back to
  detection-based registration for all agents, but a recorded, malformed
  value (non-string or empty, e.g. [] or null) now fails closed
  (registers nothing) instead of being treated as "no active
  integration" or reaching AGENT_CONFIGS.get() with an unhashable key
  and raising TypeError.
- Updated docs/reference/presets.md and docs/reference/integrations.md
  to describe active-only preset/extension registration and clarify
  that `integration use`/`switch` is the activation point for
  installed extensions and presets, and that `upgrade` only
  re-registers them for the active integration.

Adds regression tests: two enabled presets overriding the same command
with different priorities (priority winner must survive `use`
rescaffolding), and a malformed recorded `ai` value ([]) for
`extension add`.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix: address third round of review feedback (multi-integration semantics)

Fixes five deeper active-only registration bugs surfaced by Copilot review
after 2486c08, all in the presets/extensions single-active integration
rule (#2948):

1. presets: _reconcile_composed_commands (run after install/remove)
   bypassed the active-only filter entirely, writing composition-winner
   command files for every detected non-skill agent via
   register_commands_for_non_skill_agents. Added an only_agent param to
   that registrar method (mirroring register_commands_for_all_agents)
   and threaded it through all 5 reconciliation call sites.

2. presets: `integration use copilot` with --skills (ai_skills: true)
   wrote both the static .agent.md command file AND the SKILL.md
   mirror for the same override. Mirrored the extension path's
   ai_skills guard in both _register_commands and the reconciliation
   pass: a command-backed active agent running in skills mode is
   excluded from non-skill command registration.

3. presets: registered_skills was a flat list, so switching between
   two skill-mode agents (e.g. Claude -> Codex) and then removing the
   preset only restored the currently active agent's directory,
   permanently orphaning the other. _unregister_skills now restores
   every existing skill-mode agent directory instead of only the
   active one.

4. extensions: load_init_options() collapses "no file" and "corrupted
   file" into the same {}, so the round-2 fail-closed fix didn't
   actually distinguish them. Added a shared
   resolve_active_agent_for_registration() helper in _init_options.py
   that checks file existence separately from parse success, returning
   a distinct sentinel for "file absent" vs None for "corrupted or
   invalid". extensions/__init__.py now uses this helper.

5. presets: same corruption-collapsing bug in _register_commands's
   active_agent resolution. Now uses the same shared helper as (4).

Adds regression tests for all five: reconciliation active-only
filtering, copilot --skills dual-write prevention, multi-skill-agent
switch+remove, and corrupted init-options fail-closed behavior for
both extension add and preset add. Each test was verified to fail
against the pre-fix code and pass with the fix.

Targeted (883) and full (3923 passed, 109 skipped) suites pass; ruff
check clean.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix: address fourth round of review feedback (skill registration provenance)

Replace the "enumerate every skill-mode directory and restore all of them"
approach from the previous round with precise per-agent provenance
tracking, per reviewer feedback that the enumerate-and-restore-everything
design was unsound:

- registered_skills changes from a flat List[str] to Dict[str, List[str]]
  (agent name -> skill names actually written), mirroring the shape
  registered_commands already uses. _register_skills now returns this
  per-agent mapping instead of a bare list, and every call site
  (register_enabled_presets_for_agent, install_from_directory, the
  _reconcile_skills "was this skill previously managed" check) is updated
  to read/merge the new shape. Legacy flat-list registry entries from
  before this change are still readable: writes self-migrate the format,
  and _normalize_registered_skills() handles the transitional read paths.

- _unregister_skills now restores exactly the agent directories recorded
  for a preset instead of guessing at every skill-mode integration that
  happens to exist on disk. This fixes two problems with the old
  enumerate-everything design: (1) it could silently overwrite or delete
  another preset's (or a user's) override in an agent directory the
  current preset never actually touched, and (2) it depended on
  transient per-process integration state (_skills_mode), which is unset
  in a fresh CLI invocation for mode-selectable integrations like Copilot
  --skills, permanently orphaning their overrides after a process
  restart. Registries written before this change (flat list, no agent
  provenance) fall back to best-effort restoration under only the
  currently active agent, matching the pre-existing guarantee level.

- Every directory resolved from persisted provenance is now validated
  through the project's shared symlink/containment guard
  (_ensure_safe_shared_directory) before any file in it is read, written,
  or removed, since restoration may target an agent that isn't currently
  active and its directory can't be assumed safe just because a name was
  recorded for it.

- _tracked_skill_agent_dirs() (the enumeration helper introduced last
  round) is removed; it's superseded by the provenance-based design.

Adds regression tests: a symlinked skills directory is rejected during
removal; removing one preset does not disturb a different preset's
override in another agent's directory; and a Copilot --skills
registration installed, then removed after switching agents in a fresh
PresetManager instance (simulating a new process), is still correctly
restored. Updates existing skill-registration assertions across
test_presets.py and test_integration_claude.py for the new per-agent
registry shape.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix: address fifth round of review feedback (symlink presence, rescaffold reconciliation, shared skills dir)

- _init_options.py: resolve_active_agent_for_registration() now treats a
  dangling init-options.json symlink as present (path.is_symlink() check
  alongside path.exists()), since Path.exists() follows symlinks and
  returns False for a broken one. Previously a broken symlink fell back
  to the legacy "no file" path and registered every detected agent
  instead of failing closed.
- presets/__init__.py (register_enabled_presets_for_agent): the
  integration use/switch rescaffold path now collects affected command
  names across all presets processed and runs
  _reconcile_composed_commands/_reconcile_skills once after the loop,
  matching install/remove. Previously rescaffolding wrote each preset's
  raw content directly with no follow-up reconciliation, so a
  project-level override (the highest-priority layer) could be clobbered
  by a lower-precedence preset after switching agents.
- presets/__init__.py (_unregister_skills): multiple integrations can
  share one physical skills directory (agy/codex/zed all resolve to
  .agents/skills). Provenance restoration now groups recorded agent
  entries by resolved directory and restores each physical directory
  exactly once, preferring the currently active agent's renderer when it
  owns that directory (otherwise any recorded owner, chosen
  deterministically). Previously each recorded agent key triggered its
  own restore pass against the same directory, with whichever agent was
  iterated last silently winning regardless of which agent was active.

Adds regression tests for each: a dangling init-options.json symlink
failing closed for both preset resolution and extension add; integration
use rescaffold preserving a project override over a lower-priority
preset; and a codex/agy shared-directory removal restoring the directory
exactly once in the active agent's format.

Targeted (tests/integrations/test_integration_subcommand.py,
tests/test_presets.py, tests/test_extensions.py,
tests/test_extension_skills.py,
tests/integrations/test_integration_opencode.py,
tests/integrations/test_integration_claude.py): 930 passed.
Full suite: 3930 passed, 109 skipped.
ruff check: clean on files touched by this change.

Refs #2948

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix: guard skill subdirectories and active-agent scoping in preset reconciliation

Fix 4 issues from round-6 review of the active-only integration
registration work (#2948):

- remove(): removed_cmd_names only collected primary command names from
  registered_commands + manifest aliases, missing commands that were
  only ever registered via skills mode (ai_skills guard returns no
  command names for command-backed integrations in skills mode). This
  skipped reconciliation entirely when removing a higher-priority
  skills-mode preset, causing _unregister_skills() to fall back to
  core/extension content instead of the surviving lower-priority
  preset's override. Now every command template's primary name is
  added to removed_cmd_names unconditionally.

- _reconcile_composed_commands(): the "composed is None" branch (fires
  when no replace-strategy layer remains for a command, e.g. after
  removing a wrap/append preset's base) called unregister_commands()
  across every configured non-skill agent, ignoring only_agent. This
  deleted historical artifacts from integrations that were never active
  for the preset. Now filtered by only_agent like the rest of the file.

- Added _validate_skill_subdir() helper (reusing
  _ensure_safe_shared_directory/_validate_safe_shared_directory from
  shared_infra.py) and applied it at every site that reads or writes an
  individual skill subdirectory (_register_skills,
  _unregister_skills_in_dir, _reconcile_skills' override_skills
  restoration loop). _safe_skills_dir_for_agent only validated the
  parent skills directory; a symlinked leaf subdirectory (e.g.
  .claude/skills/speckit-specify) would slip past that check since
  is_dir()/exists() follow symlinks, letting write_text/rmtree operate
  through it to an arbitrary location outside the project.

Added regression tests: removing a higher-priority skills-only preset
restores the surviving lower-priority preset's content; composed-is-None
unregistration only touches the active agent; symlinked skill subdirectory
rejected on restore; symlinked skill subdirectory rejected on write.

Targeted (934) and full (3934 passed, 109 skipped) test suites and ruff
check pass clean.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix: persist command registration before fallible skills phase on rescaffold

Fix remaining round-6 review findings on the active-only integration
registration work (#2948):

- register_enabled_presets_for_agent(): registered_commands and
  registered_skills were merged and persisted together in a single
  registry.update() call after both the commands and skills phases ran.
  If _register_skills() raised, the per-preset try/except swallowed it
  before that update() call was reached, even though _register_commands()
  had already written a real command file to disk. That file became
  untracked, so preset removal could no longer clean it up.
  install_from_directory() already persists registered_commands
  immediately after the commands phase, before starting the independently
  fallible skills phase; rescaffold now does the same.

- test_presets.py: renamed a misleading claude_dir variable (pointing at
  Gemini's command directory) in
  test_composed_none_unregister_respects_active_agent to reuse the
  existing gemini_commands_dir variable already defined earlier in the
  same test.

Added regression test
test_rescaffold_persists_commands_before_fallible_skills_phase:
simulates a skills-phase failure during rescaffold and asserts the
command file already written to disk is still tracked in
registered_commands.

Verified all other round-6 findings (preset active-integration scoping,
preset reconciliation/remove paths, skills-mode switching, override
precedence during rescaffold, skill-subdirectory symlink safety) are
already addressed by prior commits in this branch; re-checked each
against current code before concluding no further change was needed.

Targeted (tests/test_presets.py, tests/test_extensions.py: 689 passed)
and full (3935 passed, 109 skipped) suites and ruff check on changed
files pass clean.

Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix: unregister stale opposite-mode preset artifact on same-agent skills toggle

Fix an Important gap in register_enabled_presets_for_agent() surfaced by
quality review (#2948): toggling ai_skills for the *same already-active*
command-backed agent (e.g. `integration upgrade copilot` after flipping
ai_skills, with copilot staying active throughout) left a stale artifact
from the previous mode behind, violating the command/skill mutual-
exclusion invariant this PR otherwise enforces.

- command -> skills: _register_commands()'s ai_skills guard makes the
  commands phase a no-op, but the previously-written command file (e.g.
  .agent.md) and its registered_commands[agent] entry were never cleaned
  up, so it lingered alongside the newly written SKILL.md.
- skills -> command: _get_skills_dir() stops resolving a skills directory
  once ai_skills is off, making the skills phase a no-op, but the
  previously-written SKILL.md and its registered_skills[agent] entry were
  never cleaned up, so it lingered alongside the newly (re)written command
  file.

register_enabled_presets_for_agent() now resolves once per call whether
agent_name is a command-backed integration (extension != "/SKILL.md") and
the current ai_skills state, then narrowly unregisters the stale opposite-
mode entry for that agent via the existing _unregister_commands /
_unregister_skills helpers before persisting updated tracking — mirroring
the same per-agent, per-preset isolation already used elsewhere in this
method. Native skill-only agents (claude, codex, ...) are unaffected:
they have no command/skill toggle, so registered_commands and
registered_skills legitimately co-exist for them by design. The trailing
reconciliation pass, project-override precedence, and per-preset
partial-failure isolation are all unchanged.

Added red-first regression tests exercising the real install +
register_enabled_presets_for_agent rescaffold path in both toggle
directions:
- test_rescaffold_toggle_command_to_skills_removes_stale_command_file
- test_rescaffold_toggle_skills_to_command_removes_stale_skill_file

Both failed against the prior code (stale artifact persisted / registry
still tracked it) and pass after the fix.

Targeted (tests/test_presets.py, tests/test_extensions.py: 691 passed)
and full (3937 passed, 109 skipped) suites and ruff check on changed
files pass clean.

Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix: migrate legacy flat-list registered_skills on rescaffold even when unchanged

Fix a valid finding from GitHub Copilot's review of HEAD b9d9053 (#2948):
register_enabled_presets_for_agent() normalizes a legacy flat-list
registered_skills value (predating per-agent provenance) to the
{agent_name: [...]} dict shape in memory via _normalize_registered_skills,
but the persistence check only compared the two *normalized* forms. When
the freshly rescaffolded skill names are identical to what the legacy
list already held — the common case, since nothing about the preset or
skill actually changed — that comparison is a no-op and registry.update()
is skipped, leaving the *raw* on-disk value as the un-migrated flat list.

A later switch to a different skill-mode agent and removal then follows
_unregister_skills's legacy best-effort path (restore only the currently
active agent's directory) instead of the per-agent provenance path,
permanently orphaning the first agent's override.

Fix: track the raw (pre-normalization) existing value and force
persistence whenever it's a non-empty list, independent of whether the
normalized content changed. Traced registered_commands for the same
class of bug: its registry value has always been Dict[str, List[str]]
(no legacy flat-list format ever existed for it — the existing
`if not isinstance(existing_commands, dict): existing_commands = {}`
guard is not a lossy migration path), so this fix stays scoped to
registered_skills only.

Added red-first regression test
test_rescaffold_migrates_legacy_flat_list_registered_skills: installs a
preset, overwrites its registry entry with a raw legacy flat list,
rescaffolds the *same* active agent with unchanged skill names, and
asserts the raw registry is migrated to per-agent dict form. Extends the
scenario with a switch to a second skill-mode agent and preset removal
to prove both agents' directories restore cleanly instead of orphaning
the first. Failed against the prior code (raw value stayed a list) and
passes after the fix.

Targeted (tests/test_presets.py, tests/test_extensions.py: 692 passed)
and full (3938 passed, 109 skipped) suites and ruff check on changed
files pass clean.

Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix: reconcile before fallible skills phase, infer legacy skill provenance, and unregister stale extension artifacts on toggle

Three findings from the Copilot review on HEAD b9d9053/3a1e749:

1. `register_enabled_presets_for_agent()` only recorded a preset's command
   names into `affected_cmd_names` (the set later passed to
   `_reconcile_composed_commands`/`_reconcile_skills`) in the loop that ran
   *after* `_register_skills()`, inside the same per-preset `try` block. If
   `_register_skills` raised, the `except` caught it and `continue`d before
   that loop ever ran — so a preset whose commands phase already wrote real
   content to disk never got reconciled against the full priority stack,
   leaving its raw content in place instead of a project override or
   higher-precedence preset's content. Fix: record the manifest's command
   names immediately after the commands phase succeeds and persists, before
   calling the independently fallible `_register_skills()`.

2. The legacy flat-list `registered_skills` migration (added for the
   previous review round) attributed every name in the list to whichever
   agent was currently being (re)activated. If the first operation after
   upgrading from a pre-#2948 registry was a direct switch to a *different*
   skill-mode agent (e.g. a legacy Claude override, then `integration use
   codex` with no intervening Claude rescaffold), the migrated dict only
   recorded `{"codex": [...]}`, permanently losing Claude's actual
   provenance and orphaning its override on later removal. Fix: added
   `_infer_legacy_skill_provenance()`, which probes every configured
   skill-mode agent's directory (via the same safe, symlink-validated
   helpers already used for restore/removal) for a `SKILL.md` whose
   frontmatter records this exact preset as the owner
   (`metadata.source == "preset:<pack_id>"`). A name found under more than
   one directory is attributed to every matching agent (the preset may have
   been active while the user switched between several skill-mode agents
   before provenance tracking existed); names that can't be matched to any
   directory still fall back to the previously-active best-effort
   behaviour. Directory grouping for shared-path aliases (e.g.
   agy/codex/zed all resolving to `.agents/skills`) intentionally does not
   call `.resolve()` on the path, since doing so diverges from
   `project_root`'s own resolution state on platforms where a path
   component is itself a symlink (e.g. macOS's `/var` -> `/private/var`)
   and made every subsequent containment check spuriously fail.

3. `register_enabled_extensions_for_agent()` has the same command/skill
   mutual-exclusion gap the preset path had (fixed in a previous round):
   toggling `ai_skills` for the *same active* agent left the opposite
   mode's artifact behind. Command -> skills left the extension's
   `.agent.md` file and its `registered_commands[agent]` entry in place
   once `skills_mode_active` made the commands phase a no-op. Skills ->
   command left the extension's `SKILL.md` file in place, since an empty
   `_register_extension_skills()` result (because this agent's skills
   directory no longer resolves once `ai_skills` is off) was treated as
   "nothing to register" rather than "this was rendered here before and is
   now stale". This diverges from the preset path in one respect:
   `registered_skills` for extensions has always been a flat list with no
   per-agent provenance (extension skills are only ever rendered for the
   active agent, never per-preset-per-agent tracked), so the fix resolves
   ownership by checking which of the extension's tracked skill names
   still exist as directories under this specific agent's directory before
   removing them — mirroring the same technique `unregister_agent_artifacts`
   already uses for full agent deactivation, but scoped narrowly to firing
   only when a toggle is actually detected (`skills_mode_active` /
   `command_mode_active`), so a same-mode re-run never disturbs
   already-correct artifacts or a user's manual customizations.

Regression tests (all confirmed red before their respective fix, green
after):
- tests/test_presets.py::TestPresetSkills::test_rescaffold_reconciles_override_even_when_skills_phase_fails
- tests/test_presets.py::TestPresetSkills::test_rescaffold_legacy_flat_list_direct_switch_preserves_original_agent
- tests/test_extension_skills.py::TestExtensionSkillRegistration::test_rescaffold_toggle_command_to_skills_removes_stale_extension_command_file
- tests/test_extension_skills.py::TestExtensionSkillRegistration::test_rescaffold_toggle_skills_to_command_removes_stale_extension_skill_file

Verification: tests/test_presets.py + tests/test_extensions.py +
tests/test_extension_skills.py (753 passed), tests/integrations/ (1768
passed, 1 skipped), full suite `pytest tests -q` (3942 passed, 109
skipped), `ruff check` on changed files clean.

Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix: broaden legacy skill provenance inference to command-backed agents

_infer_legacy_skill_provenance() only probed agents whose registrar
config statically declares extension == "/SKILL.md", excluding
command-backed agents (e.g. Copilot) that can also render preset
overrides as SKILL.md files when ai_skills is enabled. A real
preset-owned .github/skills/.../SKILL.md written while Copilot was the
active skills-mode agent was therefore never probed and got
misattributed entirely to whichever agent activated first after the
upgrade, permanently orphaning Copilot's override on later removal.

Broaden the candidate set to every configured integration
(CommandRegistrar.AGENT_CONFIGS), reusing the existing safe-path
helper (_safe_skills_dir_for_agent, itself built on the shared
_get_skills_dir resolver) rather than inventing new path-construction
logic. The existing preset-marker match (metadata.source ==
"preset:<pack_id>") continues to gate every attribution, so
command-mode agents that never rendered this preset's skill are not
falsely attributed.

Add red-first regression tests: a legacy flat-list entry owned by
Copilot in skills mode, switched directly to Claude with no
intervening Copilot rescaffold, now migrates to a per-agent dict
covering both agents, and removal restores both agents' files instead
of orphaning Copilot's override; plus a negative-case test confirming
a command-mode Copilot with no preset-owned skill marker is not
falsely attributed during the same migration.

Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix: preserve extension skill tracking for mirrors in other agent dirs

The skills -> command toggle cleanup in
register_enabled_extensions_for_agent() recomputed the remaining
tracked registered_skills names by checking only the toggling agent's
own skills directory. Since registered_skills is a single flat list
shared across every agent an extension has ever been activated under
(skills are only ever rendered for the active agent, so there is no
per-agent registry key), a name whose mirror still existed under a
*different*, previously-active agent's directory was incorrectly
dropped from tracking as soon as the current agent's own copy was
removed. A later full removal only iterates registered_skills, so the
orphaned mirror under the other agent's directory was never found or
cleaned up.

Add _extension_owned_skill_names(), which re-verifies ownership across
every configured agent's skills directory (deduped by shared path) the
same way the existing _unregister_extension_skills() fallback scan
already does, keeping a name only when a SKILL.md with a matching
metadata.source == "extension:<id>" marker is found somewhere -
read-only, no directory creation, no symlink escape. Use it instead of
re-checking only the toggling agent's own directory when recomputing
what remains tracked after narrow stale-mirror cleanup.

Add a red-first regression test: Auggie is activated in skills mode
first (writing a mirror), then Copilot is activated in skills mode
(writing its own mirror for the same names), then Copilot toggles to
command mode. Before the fix, registered_skills lost both names
entirely even though Auggie's mirrors were untouched on disk; after
the fix tracking is preserved and a subsequent full removal correctly
cleans up Auggie's remaining mirrors too.

Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix: reject symlinked skills-directory escape in extension skill scans

_extension_owned_skill_names() and the fast/fallback paths of its
sibling _unregister_extension_skills() called skills_candidate.resolve()
and then checked children relative to that already-resolved candidate.
If the candidate directory itself (e.g. .gemini/skills) was a symlink
pointing outside the project root, both the resolve() call and the
subsequent containment check silently passed through the symlink
instead of rejecting it:

- _extension_owned_skill_names() would falsely attribute ownership to
  a marker-matching SKILL.md living outside the project.
- _unregister_extension_skills()'s fast path (an explicit skills_dir,
  as passed by the toggle-cleanup call site) and its fallback scan
  (used during full extension removal) would both shutil.rmtree() the
  external directory, deleting unrelated content outside the project.

Fix by validating the candidate directory itself with the existing
_validate_safe_shared_directory() shared-infra helper before any probe
or delete: it rejects a symlink at any path component (walking down
from the project root, including the final component) without ever
resolving through it, and is already used elsewhere in the codebase for
the same class of shared-directory containment check. Unsafe
candidates are skipped/refused rather than followed.

Add red-first security regression tests reproducing each of the three
call sites with a `.gemini/skills` symlink pointing at an external
directory containing a marker-matching SKILL.md and an unrelated
precious_file.txt: provenance inference must not attribute the name,
and both the explicit-skills_dir fast path and the None-skills_dir
fallback scan must leave the external directory and file untouched.
Existing valid shared/deduped directory tests (e.g. agy/amp/codex/zed
sharing .agents/skills) continue to pass, confirming legitimate shared
directories still clean up correctly.

Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Fix unscoped extension-skill removal and legacy preset provenance on direct remove

- _unregister_extension_skills(): omitting skills_dir now always triggers
  the full multi-directory fallback scan instead of narrowing to the
  currently active agent's directory. Previously, remove() (the only
  caller that omits skills_dir) would resolve the active agent's dir and
  take the scoped fast path, orphaning a previously-active second agent's
  extension skill mirror during full removal.

- PresetManager.remove(): infer legacy flat-list registered_skills
  provenance (reusing _infer_legacy_skill_provenance from the prior
  rescaffold fix) before invoking _unregister_skills, so a direct
  `preset remove` with no intervening rescaffold/switch also restores
  every previously-active agent's directory instead of only the
  currently active one.

Added regression tests:
- test_remove_while_second_agent_still_in_skills_mode_cleans_up_first_agent_mirror
- test_remove_infers_legacy_flat_list_provenance_without_prior_rescaffold

Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Keep unregister_agent_artifacts scoped to its agent when directory is absent

ExtensionManager.unregister_agent_artifacts() converted its resolved
agent_skills_dir to None whenever that directory didn't exist, before
calling _unregister_extension_skills(). After 1d8f9e3, omitting
skills_dir means "genuinely unscoped removal": scan every configured
agent's directory, reserved for ExtensionManager.remove()'s full
project cleanup. Since unregister_agent_artifacts is agent-scoped (used
by switch to clean up the previous integration's artifacts), this
caused it to delete every other agent's live extension skill mirrors
whenever the target agent's own directory happened to be absent, e.g.
unregistering an agent that was never activated.

Fix: always pass the explicit, agent-scoped skills_dir, even when it
doesn't exist on disk, so the fast path is a safe no-op for an absent
directory instead of falling back to the all-agents scan. Registry
reconciliation (dropping removed names from the flat registered_skills
list) now only runs when the agent's directory actually exists, so an
absent directory can't be misread as "these names were removed
everywhere" and wipe tracking for mirrors that still legitimately live
under other agents' directories.

Added regression test:
- test_unregister_agent_artifacts_stays_scoped_when_agent_dir_absent

Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Preserve global skill tracking across agents in unregister_agent_artifacts

The present-directory branch of ExtensionManager.unregister_agent_artifacts()
recomputed "remaining" registered_skills only by checking whether each name
still existed under the just-cleaned agent's own directory. registered_skills
is a single flat list shared across every agent an extension was ever
activated under (skills are only ever rendered for the currently active
agent, so there's no per-agent registry key). Repro: auggie and copilot both
have mirrors for the same extension; unregister_agent_artifacts("auggie")
correctly removes auggie's own mirror, sees the names absent from auggie's
(now empty) directory, and stores an empty registered_skills list - even
though copilot's mirror is still live on disk and now untracked. A later full
remove() then reads an empty registry and leaves copilot's mirror orphaned.

Fix: after the agent-scoped cleanup, recompute remaining names with
_extension_owned_skill_names(), which scans every safe, configured agent
skills directory (not just the one just cleaned) and keeps a name only if a
marker-verified SKILL.md for this extension still exists somewhere. This is
the same helper already used for the analogous same-agent toggle-cleanup
case, so no new abstraction was introduced. Explicit per-agent cleanup,
marker ownership verification, and symlink/containment safety are unchanged.

Added regression test:
- test_unregister_agent_artifacts_preserves_tracking_for_other_agent_mirror

Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Reconcile every historical agent on preset removal; validate child skill dirs

Fixes 3 findings from the Copilot review on HEAD 31c9b97 (#2948):

1. presets/__init__.py: remove()'s command reconciliation only recreated
   the surviving preset's content for the currently active agent, even
   though the removed preset's registered_commands could span multiple
   historical (now-inactive) agents recorded via prior rescaffolds. Now
   remove() captures every historical agent registered_commands actually
   targeted (before mutation) and passes it as extra_agents through
   _reconcile_composed_commands -> _register_for_non_skill_agents /
   _register_command_from_path -> registrar.register_commands_for_non_
   skill_agents, so the active-only restriction for install/use is
   preserved while post-removal reconciliation restores every touched
   directory.

2. presets/__init__.py: the analogous gap existed for skills. _unregister_
   skills() now returns {skills_dir: renderer_agent} for every directory it
   actually restored, and _reconcile_skills() accepts extra_skills_dirs to
   reconcile each of those directories (via a new apply_to_dir() helper),
   not only the currently active skills directory. _register_skills() gained
   optional target_dir/target_agent overrides (forcing
   create_missing_skills off for non-active directories) so a historical
   directory is only ever restored, never seeded with brand-new skills.

3. extensions/__init__.py: _extension_owned_skill_names() and both the
   fast and fallback paths of _unregister_extension_skills() validated only
   the parent skills_dir for symlink escape, then resolved
   skills_dir / skill_name and checked containment relative to that
   already-resolved parent. A per-skill child that is itself a symlink to
   a different, legitimate skill directory within the same (safe) root
   passed that containment check, so deleting/attributing through the
   symlink name could destroy or misattribute an unrelated skill reached
   only via the alias. All three call sites now run the shared
   _validate_safe_shared_directory() component-wise check against the full
   skills_dir / skill_name path (not just the parent) before any read or
   delete, rejecting a symlinked child outright rather than following it,
   even when its resolved target remains in-bounds.

Regression tests added (all confirmed red against pre-fix code, green
after):
- test_remove_reconciles_command_for_every_historical_agent
- test_remove_reconciles_skill_for_every_historical_agent
- test_extension_owned_skill_names_rejects_symlinked_child_skill_dir
- test_unregister_extension_skills_explicit_dir_rejects_symlinked_child
- test_unregister_extension_skills_fallback_rejects_symlinked_child

Tests: tests/test_presets.py (361), tests/test_extension_skills.py (69),
tests/test_extensions.py (338) all pass; tests/integrations (1768 passed,
1 skipped) pass; full suite 3902 passed / 74 skipped (90 pre-existing,
environment-only git-signing tests deselected — confirmed failing
identically on the pre-change baseline due to local 1Password SSH-agent
signing, unrelated to this change). ruff check clean on all changed files.

Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Persist historical reconciliation ownership; defer destructive toggle cleanup; validate registry-provided skill names

Round 11 review findings (5 comments on HEAD ab6c28c), three root causes:

A) Historical-agent reconciliation wrote surviving content to disk but
   discarded the returned per-agent write map, so the preset's own
   registered_commands/registered_skills never learned about directories
   reconciliation restored on its behalf. A later removal of that same
   preset then orphaned those directories. Added
   _merge_pack_registered_commands/_merge_pack_registered_skills and wired
   them into _reconcile_composed_commands and _reconcile_skills's
   apply_to_dir so every actual write is merged back into the winning
   preset's registry metadata.

B) Command<->skills toggle on an already-active agent deleted the old
   artifact before the replacement registration ran, in both
   presets/__init__.py's register_enabled_presets_for_agent and
   extensions/__init__.py's register_enabled_extensions_for_agent. If the
   replacement step raised, both artifacts were lost. Deferred the
   destructive cleanup until after the replacement phase completes
   without raising (register-new-then-remove-old ordering); the mirror
   skills->command direction was already safe since the new command file
   is always registered unconditionally before any cleanup runs.

C) _unregister_skills_in_dir and _infer_legacy_skill_provenance joined a
   registry-provided (untrusted) skill name directly onto a directory
   before any name-shape validation. An absolute in-project name discards
   the intended parent directory entirely (Path's "/" operator drops the
   left side for an absolute right side), letting a corrupted registry
   entry escape the intended skills subtree while still resolving inside
   the project root - passing the existing containment/symlink check.
   Added a centralized _is_safe_registry_skill_name guard (rejecting
   non-strings, empty strings, absolute paths, multi-component paths, and
   "."/".." ) and applied it before every path join derived from
   registry-provided skill names in both functions. Also fixed
   _infer_legacy_skill_provenance's unmatched-name fallback, which
   previously still attributed rejected names to fallback_agent even
   after the loop skipped them.

Added red-first regressions for all three root causes, covering: a
two-preset historical-command-agent survivor scenario, an analogous
skill-agent survivor scenario, injected skills-phase failure during a
preset command->skills toggle and the extension equivalent, a direct
unit test of the new name-safety guard, an absolute-path escape attempt
against _unregister_skills_in_dir, and a false-attribution attempt
against _infer_legacy_skill_provenance.

Tests: tests/test_presets.py (367 passed), tests/test_extension_skills.py
+ tests/test_extensions.py (408 passed), tests/integrations (1768
passed, 1 skipped), full suite tests -q deselecting the pre-existing
1Password-signing-affected tests/extensions/git/test_git_extension.py
(3909 passed, 74 skipped, 90 deselected). ruff check clean on all
changed files.

Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Verify replacement actually landed before retiring stale toggle artifacts

The command<->skills toggle cleanup added for #2948 deferred destructive
removal of the old-mode artifact until after the replacement registration
call completed without raising. That was necessary but not sufficient:
none of _register_skills(), _register_commands(),
register_commands_for_agent(), or _register_extension_skills() raise on
a missing source template, a safety-validation skip, or a corrupted
manifest entry — they simply return an empty or partial result. Treating
"did not raise" as "fully replaced" meant a stale artifact could still be
deleted (or its tracking dropped) even though its specific replacement
never actually landed, leaving neither artifact in place for that logical
command/skill.

Fix all four affected toggle directions by checking the replacement
call's actual return value before allowing any destructive step:

- presets command->skills (register_enabled_presets_for_agent): only
  unregister a stale command name once its corresponding skill name
  (via the existing _skill_names_for_command() helper) is confirmed
  present in the skills call's returned names for that agent; the
  remainder stays tracked and on disk.
- presets skills->command (register_enabled_presets_for_agent): only
  unregister a stale skill name once its corresponding command name is
  confirmed present in the commands call's returned names for that
  agent, using the same helper.
- extensions skills->command (register_enabled_extensions_for_agent):
  only remove a skill mirror once the matching command (mapped via the
  existing HookExecutor._skill_name_from_command() helper) is confirmed
  present in register_commands_for_agent's returned names.
- extensions command->skills (register_enabled_extensions_for_agent):
  only remove a deferred stale command once its matching skill name is
  confirmed present in _register_extension_skills()'s returned names.

All four reuse the existing command<->skill name-derivation helpers
rather than inventing new mapping logic. Registry tracking is updated to
retain exactly the unreplaced subset rather than being popped wholesale,
so partially-successful toggles leave correct, minimal tracking behind.

Added 8 new regression tests (4 presets, 4 extensions) covering both the
fully-empty and genuinely-partial result cases for each of the four
toggle directions, using real missing-source-file scenarios (not mocked
return values) to exercise the actual code paths. Confirmed red before
the fix and green after for all 8.

Focused (test_presets.py, test_extension_skills.py, test_extensions.py,
tests/integrations): 2551 passed, 1 skipped.
Full suite (tests, excluding the pre-existing environment-local
1Password-signing git-extension failures): 3917 passed, 74 skipped, 90
deselected.
ruff check: clean.

Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Retire alias command groups on toggle; scope preset cleanup to switched-away agent (#2948)

Fixes three current Copilot review findings on HEAD d0d152e:

1. Command->skills toggle cleanup only matched a stale command's own
   name against the returned replacement skill name. Aliases
   (CommandRegistrar tracks and returns primary + alias names flattened
   into one list) never have their own skill rendered -- only the
   primary command's skill is rendered -- so an alias's name could never
   match, leaving its command artifact and tracking behind forever even
   after the primary's replacement landed. Fixed identically in both
   presets (register_enabled_presets_for_agent) and extensions
   (register_enabled_extensions_for_agent): build a primary->alias
   mapping from the manifest, group stale names by primary, and
   retire/keep the whole group together based solely on whether the
   primary's skill replacement actually landed.

2. `integration switch` to a not-yet-installed target unregistered the
   old agent's extension artifacts but had no preset equivalent, so a
   preset's command overrides (including custom preset commands) and
   skill mirrors for the deactivated agent lingered as orphans. Added
   `PresetManager.unregister_agent_artifacts()`, mirroring
   `ExtensionManager.unregister_agent_artifacts()`: scoped strictly to
   the given agent, migrates a legacy flat-list `registered_skills`
   entry via existing on-disk provenance inference before removing
   anything (so other agents' real ownership is preserved rather than
   guessed or dropped), and guards against double-processing an
   artifact through both the commands and skills paths for native
   SKILL.md agents. Wired via a new `_unregister_presets_for_agent()`
   helper into the integration switch command's existing old-agent
   cleanup phase.

Added red-first regression tests:
- tests/test_presets.py: alias-group retire/keep/partial-multi-group
  tests for the command->skills toggle; unregister_agent_artifacts
  scoping tests for commands and legacy-list skill provenance.
- tests/test_extension_skills.py: alias-group retire/keep tests for the
  extension command->skills toggle.
- tests/integrations/test_integration_subcommand.py: end-to-end switch
  test proving a preset's custom command override is cleaned up when
  switching to a not-yet-installed integration, with tracking updated
  correctly and the new agent's registration unaffected.

All new tests confirmed red (AttributeError / orphaned file assertions)
before the fix and green after. Full suite: 3980 passed, 109 skipped.
ruff check clean on all changed files.

Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix: track reconciled extension artifacts

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Fix native skill preset reconciliation

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Fix shared native skill cleanup

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Fix partial preset rescaffold tracking

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Fix preset agent skill lifecycle

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Clarify preset removal reconciliation

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix(integrations): address upgrade review feedback

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix(presets): reconcile partial command writes

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix: address active artifact cleanup review

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix: defer preset skill cleanup to winning command

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix: track reconciled and partial preset skills

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix: reconcile project overrides to legacy skills

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix: harden preset skill writes and rollback

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix(presets): harden legacy skill restoration

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix(presets): preserve non-owned legacy skills

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix: validate reconciled skill paths

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix(presets): preserve reconciled skill ownership

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix(presets): clean reconciled agent skills

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix: keep legacy cleanup project-local

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix(presets): keep active agent's artifacts in its current mode on remove

A partially failed command<->skills toggle leaves stale tracking
(registered_commands or registered_skills) for the active agent, and
remove() replayed that history regardless of the agent's current mode:

- extra_agents re-admitted the active skills-mode agent into command
  reconciliation, recreating its command file from a surviving lower
  preset even though only_agent excluded it.
- _unregister_skills restored (and _reconcile_skills reapplied) a skill
  artifact for the active command-mode agent instead of deleting the
  preset-owned leftover.

The active agent's participation is now decided exclusively by its
current mode: reconciliation strips it from extra_agents, and removal
routes its stale skills through _delete_agent_preset_skills. Historical
replay still applies to inactive agents only (#2948).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix: filter uninstalled-extension commands in reconciliation; allow active-agent layout change with presets

Two follow-ups to the upstream-main merge:

- Preset reconciliation (_reconcile_composed_commands) now skips
  extension-scoped commands (speckit.<ext>.<cmd>) whose extension is not
  installed, at the single chokepoint every install/remove/rescaffold
  pass funnels through. Registration already refused them, so
  reconciliation could materialize files no registry entry tracks. The
  duplicated per-call-site filters collapse into one
  _extension_installed_for_command helper.

- The #3415 layout-change guard predates this PR's agent-scoped preset
  rescaffold: for the active integration, _register_presets_for_agent
  now re-registers enabled presets in the new layout and retires the
  old layout's stale files, so an active-agent command<->skills toggle
  proceeds and reconciles instead of being rejected. The guard still
  rejects non-active agents (no rescaffold runs for them) and still
  fails closed on an unreadable registry.
  _installed_presets_affecting_agent also understands the per-agent
  dict shape of registered_skills this PR writes, instead of raising
  'malformed'.

Regression tests: rescaffold with an uninstalled extension's command,
CLI-level legacy<->skills toggle with an installed preset (both
directions), secondary-agent rejection, and dict-shaped
registered_skills in the guard helper.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix: reject active layout change while a disabled preset owns artifacts

The post-upgrade preset rescaffold iterates enabled presets only, and a
disabled preset's artifacts are deliberately frozen until removal, so an
active-agent command<->skills layout change cannot reconcile them.
_installed_presets_affecting_agent now reports each preset's enabled
state and the guard rejects the migration while any affected preset is
disabled, with re-enable/remove guidance. Enabled presets and non-active
rejection behave as before.

Regression test: disabled preset blocks the toggle untouched; re-enabling
unblocks it and reconciles.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* docs: replace placeholder prefix in two safety comments

Comment-only: spell out why skill deletion is restricted to
project-local directories (flat/legacy provenance cannot prove
home-directory ownership) instead of an undefined placeholder word.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* docs: correct guard-helper docstring to active-only registration model

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix: fail closed on non-list values in per-agent preset provenance

A dict-shaped registered_skills/registered_commands entry with a
non-list value (e.g. null) left ownership undecidable but read as "no
artifacts", letting a layout-changing upgrade proceed on a malformed
registry. Validate values are lists and raise
_PresetRegistryUnreadableError otherwise, matching the guard's
fail-closed contract. Unit test covers both fields.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix: drop eager extension unregister on layout-changing upgrade

Unregistering the agent's extension artifacts before re-registration
deleted files and registry tracking up front, so a failed or partial
re-registration left the extension with no artifacts at all. Retirement
of each opposite-mode artifact already belongs to
register_enabled_extensions_for_agent's deferred toggle cleanup, which
removes an old artifact only after its replacement is confirmed. Also
keeps disabled extensions consistent with disabled presets: artifacts
stay frozen in place with intact tracking.

Regression test corrupts the installed extension manifest so
re-registration fails, then asserts the old-layout artifacts and their
registry tracking survive the upgrade.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix: rescaffold fallback integration after failed switch rollback

When Phase 2 of a switch fails, rollback restores another installed
integration as the default via _set_default_integration but never
re-registered extensions or presets for it. Under active-only
registration the fallback may never have received any artifacts (it
was installed while another integration was active), and Phase 1
already unregistered the outgoing agent's artifacts — leaving the
restored default unusable. Rescaffold both extensions and presets
(best-effort) after the fallback default is successfully restored.

Regression test: secondary codex install with the git extension, a
failing switch to generic, then asserts codex ends up with registered
extension artifacts after rollback.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* docs: explain load-bearing pre-create loop in _reconcile_skills

The per-skill _validate_skill_subdir(create=True) loop looks like dead
code (its result is unused), but it re-creates the tracked skill
subdirectories that _unregister_skills just deleted so
_register_skills's only-overwrite-existing gate passes during a
historical-directory restore. Removing it fails
test_skill_reconciliation_preserves_per_directory_names.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix: preserve dashed-description skill tracking

Use the shared frontmatter parser when verifying surviving extension skill mirrors so delimiter substrings cannot hide provenance metadata.

Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix: skip absent extension skills during reconciliation

Filter extension-scoped commands before skill reconciliation so historical preset tracking and project overrides cannot recreate artifacts for uninstalled extensions.

Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix: preserve partial native skill cleanup

Coordinate native-skill command cleanup with registered skill coverage per agent and command so partial rescaffolds cannot orphan preset artifacts.

Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-07-27 14:09:40 -05:00
Noor ul ain
42c7230aa9 fix(extensions): tolerate non-string tags in catalog search (#3746)
* fix(extensions): tolerate non-string tags in catalog search

ExtensionCatalog.search() assumed catalog `tags` were always strings:
the tag filter called `t.lower()` and the query path did
`" ".join([...] + tags)`. Extension catalog JSON is user-editable, so a
hand-authored `tags: [1, 2]` crashed search with AttributeError (tag
filter) or TypeError (query join).

Coerce defensively by filtering to `isinstance(t, str)` and guarding the
tags value as a list, matching the reference-correct sibling in
integrations/catalog.py. Non-string tags are now skipped rather than
raising.

Adds a regression test driving search(tag=...) and search(query=...)
against a catalog with mixed string/int tags; both fail pre-fix.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(extensions): also coerce non-string author/name in catalog search

The same ExtensionCatalog.search() method had two more string
assumptions on user-editable catalog fields: the author filter called
`ext_data.get("author", "").lower()` (AttributeError on a numeric
author) and the query searchable-text joined `name`/`description`
uncoerced (TypeError on a numeric name). Coerce both defensively,
matching the reference-correct integrations/catalog.py::search.

Extends the regression test with non-string author/name coverage;
fails pre-fix with AttributeError at the author filter.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-27 11:58:23 -05:00
Quratulain-bilal
e6a3ccfb27 fix(extensions): hyphenate command names in 'extension info' listing (#3744)
The 'Commands:' section of 'specify extension info' for a locally installed
extension printed each command in its manifest dotted form
(e.g. speckit.jira.sync). Cline and Forge register hyphenated command names
(/speckit-jira-sync), so on those projects the displayed names did not match
what the user actually invokes.

Format each name through the active agent's command-name formatter, mirroring
the parity 'extension add' already applies to its 'Provided commands' listing
(#3669) and completing the Forge/Cline command-name parity from #3641/#3642.

Adds a regression test asserting the hyphenated form appears (and the dotted
form does not) for a Forge project.
2026-07-27 11:56:21 -05:00
Noor ul ain
962f9f0765 fix(workflows): escape remaining untrusted fields in workflow info (#3731)
* fix(workflows): escape remaining untrusted fields in `workflow info`

Follow-up to #3690, which escaped only the step-graph brackets. Every
other metadata field `workflow info` prints is untrusted content
(workflow.yml or catalog JSON), and console.print has Rich markup
enabled, so an unescaped `[...]` in any of them is parsed as a style tag
and silently swallowed:

- definition path: name, version, author, description, integration, and
  each input's name/type
- catalog path: name, version, description, tags, and the "not found"
  workflow id

A description of `Does [stuff] nicely` rendered as `Does  nicely`; an
integration of `claude [code]` rendered as `claude `. Route every field
through _escape_markup, matching the sibling `workflow list` / catalog
`search` commands, so bracketed text renders literally.

Add two regression tests covering the definition and catalog paths; both
fail on the pre-fix source (fields with brackets come back truncated).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* test: cover version + not-found-id escapes in workflow info

Addresses Copilot review feedback on the workflow-info markup-escape
tests:

- The definition-path and catalog-path regression tests left `version`
  bracket-free and never asserted it, so the version escapes could be
  removed without failing. Use bracketed version values and assert they
  survive verbatim.
- The newly escaped not-found identifier is a separate output path that
  no test reached. Add a case where local load raises FileNotFoundError
  and catalog lookup returns None, invoke `workflow info` with a
  bracketed ID, and assert the literal ID is preserved in the error.

Verified each new assertion fails when its source escape is removed
(test-the-test).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-27 11:55:00 -05:00
Ali jawwad
c1028e5506 fix(extensions): guard non-numeric catalog downloads in search/info rendering (#3710)
* fix(extensions): guard non-numeric catalog downloads in search/info rendering

`specify extension search` and `specify extension info <id>` format a catalog
entry's `downloads` field with the `:,` thousands separator, guarded only by
`is not None`. Catalog payloads are only shape-validated -- individual fields
are never type-checked and `_get_merged_extensions` returns raw catalog dicts
-- so an entry with a non-numeric `downloads` (e.g. the JSON string "1500",
realistic from a community / SPECKIT_CATALOG_URL / project catalog) makes the
`:,` format raise `ValueError: Cannot specify ',' with 's'`, aborting the
whole command with an uncaught traceback.

Group-format `downloads` only when it is actually numeric; otherwise render it
as-is. Numeric values (int/float, incl. bool) format identically, so correct
catalogs are byte-for-byte unchanged. Every other field in these two renderers
is already `str()`-wrapped; this closes the one unguarded field.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(extensions): escape the non-numeric downloads fallback for Rich markup

Address review feedback: the fallback interpolated the untrusted catalog value
straight into a Rich-rendered string, so guarding the ``:,`` ValueError just
traded it for a MarkupError -- a catalog entry with downloads "[/red]foo" still
aborted `extension search`/`info`, and balanced tags could restyle the output.

Wrap the fallback in _escape_markup(str(...)) at both sites, matching how every
other catalog field in these renderers is already escaped. Numeric values keep
the identical ``:,`` branch, so correct catalogs are unchanged.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(extensions): escape 'stars' too, in the same stats string

Follow-up to the downloads escaping: `stars` is the other catalog-controlled
value joined into the same Rich-rendered stats line, and it was still raw --
verified that stars "[/red]x" raises the same MarkupError and aborts
`extension info`/`search`. Hardening one of the two adjacent values would have
left the reported defect reachable through the sibling field.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-27 11:25:01 -05:00
Mateus Cardoso
9e150cd3b2 fix(agent-context): apply default markers when config markers are blank (bash) (#3736)
When the extension config omits context_markers (or sets them blank),
relying on the built-in defaults, the Bash port aborted with "malformed
config parser output" and never updated the context file, while the
Python (`or DEFAULT_*`) and PowerShell (default-initialized) ports handled
it correctly.

The config parser prints three lines (context_files JSON, marker_start,
marker_end), captured via `_raw_opts="$(...)"`. Command substitution strips
trailing newlines, so blank marker lines collapse the output to fewer than
three, tripping the `(( ${#_opts_lines[@]} < 3 ))` guard and making the
DEFAULT_START/END substitution unreachable — the exact case it was written
for.

Require only the context_files line and default the marker lines to empty
(`${_opts_lines[1]:-}` / `${_opts_lines[2]:-}`) so the existing
DEFAULT_START/END fallback fills them in. Add a parity regression test with
blank markers (it fails on the old guard and passes with the fix).

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-27 10:33:56 -05:00
Noor ul ain
99dc915ae3 fix: escape Rich markup in catalog list output (#3738)
The `catalog list` subcommands for workflows, workflow steps, presets,
and integrations printed user-editable catalog fields (name/url/
description from the `*-catalogs.yml` files) through `console.print`
with Rich markup enabled. Any bracketed content such as a description
`Does [stuff] nicely` was parsed as a style tag and silently swallowed,
and a malformed tag could raise while rendering.

Route each untrusted field through the module's already-imported
`escape` helper, matching the pattern already used by
`extension catalog list`.

Adds regression tests for all four commands that inject bracketed
name/url/description and assert the brackets survive verbatim in the
output.

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-27 10:15:37 -05:00