Commit Graph
28 Commits
Author SHA1 Message Date
e9ffc9d8e7 feat: allow overriding default init integration via SPECKIT_INTEGRATION_DEFAULT (#3952)
* feat: allow overriding default init integration via SPECKIT_INTEGRATION_DEFAULT

Resolve the non-interactive/init default integration from the
SPECKIT_INTEGRATION_DEFAULT environment variable, fitting the existing
SPECKIT_INTEGRATION_* namespace. Falls back to the hardcoded "copilot"
default when unset, and warns to stderr (rather than silently falling
back) when the value is not a registered integration key.

Wires the resolver into specify init (interactive prompt default and
non-interactive fallback), the init workflow step, and the bundle init
default. Adds unit and CLI tests and documents the variable.

Closes #3939

Assisted-by: GitHub Copilot (model: claude-opus-4.8, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: eecda55f-fa13-42f7-99bf-bfb0bb8565a0

* test: cover env-var default wiring for picker, workflow step, and bundle

Address PR review: add regression tests so each SPECKIT_INTEGRATION_DEFAULT
wiring site cannot silently revert to the hardcoded constant.

- init.py: interactive picker receives the resolved key as default_key.
- workflow init step: no step/workflow default + env var drives output
  integration and argv.
- bundle _resolve_init_integration: env-var default applies when unspecified,
  while explicit override and manifest-declared integration still win.

Assisted-by: GitHub Copilot (model: claude-opus-4.8, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: eecda55f-fa13-42f7-99bf-bfb0bb8565a0

---------

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: eecda55f-fa13-42f7-99bf-bfb0bb8565a0
2026-08-03 12:20:49 -05:00
ba7ae79c66 Add --extension flag to specify init for opting into extensions at init time (#3914)
* Add --extension flag to specify init for installing extensions at init time

Adds a repeatable --extension flag to `specify init` so users can opt into
extensions (bundled name, local path, or HTTPS URL) during initialization,
without a separate `specify extension add` step.

- New `_install_extension_during_init` helper in commands/init.py that
  auto-detects source type (URL / local path / bundled name / catalog) and
  installs via ExtensionManager. Failures are non-fatal and recorded in the
  tracker without aborting init.
- Extension tracker steps are pre-registered before the Live context and run
  after preset install, before finalize.
- Five new tests in TestExtensionFlag covering bundled name, multiple
  extensions, local absolute path, unknown extension (graceful error), and
  combination with --preset.

Rebased onto upstream/main and adapted to the refactored init command
(moved to src/specify_cli/commands/init.py) from stale PR #2396.

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

* Address review: reuse hardened downloader, refresh events, escape labels, fix bundler call

Responds to review feedback on #3914 and fixes CI (pytest bundler failure).

- Extract shared `install_extension_from_url` helper in extensions/_commands.py
  that reuses the authenticated, redirect-guarded, bounded (50 MiB) download
  and TOCTOU-safe transient archive used by `extension add --from`. Both
  `extension add --from` and `specify init --extension <url>` now go through
  this single downloader instead of a second raw urlopen path.
- Refresh native event configuration once after successful extension installs
  during init (mirrors `_refresh_events_and_warn` in the add path) so an
  extension declaring `events:` has its hooks activated.
- Escape user-controlled extension specs and error text before interpolating
  them into StepTracker labels (Rich markup injection).
- Pass `extensions=None` from bundler's `_run_init` so the init callback no
  longer receives the typer OptionInfo sentinel ('OptionInfo' object is not
  iterable), which broke `test_install_initializes_uninitialized_project`.
- Add init URL coverage in TestExtensionFlag: non-HTTPS rejection and a
  successful HTTPS ZIP install with download-cache cleanup assertion.

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

* Add default-deny trust confirmation for URL extension installs at init

URL-based --extension installs now require explicit trust, matching the
`extension add --from` posture. Interactive sessions show an "Untrusted
Source" panel and prompt (default no); non-interactive sessions deny by
default unless --trust-extension-urls is passed. Trust is resolved before
the Live display since the prompt can't be answered under the spinner.

- Add --trust-extension-urls option and _ext_spec_is_url /
  _confirm_extension_url_trust helpers
- Skip (not abort) unconfirmed URL extensions, consistent with other
  non-fatal extension failures
- Pass trust_extension_urls=False from the bundler init callback
- Add tests for deny-by-default, interactive confirm, and trusted install

Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 8bc6802d-81b8-48f4-8f60-cba3aebc3bb3

---------

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 8bc6802d-81b8-48f4-8f60-cba3aebc3bb3
2026-07-31 11:15:25 -05:00
kanfilandGitHub 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
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
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
Pascal THUETandGitHub 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
5674fd03a9 fix(init): show hyphenated /speckit-<name> in Next Steps for Forge projects (#3642)
The post-init "Next Steps" panel renders recommended slash commands via
the nested `_display_cmd()`. It special-cased dollar-skills agents, kimi,
slash-skills agents, and cline, but not Forge. For a Forge project
`_display_cmd` fell through to `return f"/speckit.{name}"`, printing
`/speckit.constitution`, `/speckit.specify`, etc.

Forge only registers the hyphenated form (`/speckit-<name>`, per
`format_forge_command_name` / `ForgeIntegration.build_command_invocation`,
and the generated command-file tests already assert this), so the panel
told Forge users to run commands that don't exist under the dotted name.

Add `forge_skill_mode` alongside `cline_skill_mode` and include it in the
hyphenated-slash condition, mirroring how cline (also a non-skills
markdown agent with hyphenated commands) is handled. Other agents
unaffected.

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

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-22 07:46:45 -05:00
2a0ada9a6a feat(scripts): port create-new-feature, setup-plan and setup-tasks to Python (#3386)
* feat(scripts): port create-new-feature, setup-plan and setup-tasks to Python

Ports the three core workflow scripts to Python as part of #3280,
following the check-prerequisites PoC pattern from #3302. Adds
resolve_template() to the shared common.py module and parity tests
that run bash and Python side by side.

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

* fix(tests): treat only None env as unset in parity run helper

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

* fix(scripts): fall back to directory scan on any registry error, skip hidden preset dirs

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

* feat(templates): add py: lines for setup_plan and setup_tasks

Ships with the scripts they reference; the remaining templates got
their py: lines in #3403.

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

* fix: support py variant in skills placeholder resolver

resolve_skill_placeholders only accepted sh/ps, so a py init option
fell into the fallback path and {SCRIPT} rendered without an
interpreter prefix. Accept py and prefix the resolved interpreter,
matching process_template. Also guard ps_cmd against a missing
PowerShell with a clear assert.

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

* test: pin clean-error behavior for invalid --number

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

* docs(scripts): reword unused-arg comment to match implementation

The loop accepts and silently ignores extra positional args (it doesn't
build a collected list); match the wording to what the code and
setup-plan.sh actually do.

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

* fix: fall back when configured script variant is missing from frontmatter

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

* fix(scripts): reject signed/whitespace --number values to match bash 10# parity

The bash twin uses $((10#$BRANCH_NUMBER)), which rejects signed and
whitespace-padded values. Python's int() accepted them (e.g. -1),
producing a malformed -01-... prefix that sequential scans ignore.
Restrict --number to unsigned decimal digits before conversion, and
pin the parity with a bash-comparison test.

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

* fix(scripts): complete Python port installation

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

* fix(integrations): fall back for missing script variants

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

* test: make Python script checks platform-aware

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

* fix Windows Python command invocation parity

Use PowerShell's call operator for spaced Python interpreter paths and align setup-tasks missing-template errors across script variants.

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

* fix(scripts): preserve cross-platform Python parity

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

* fix: reject signed PowerShell feature numbers

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

* fix(scripts): align feature number range

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

* fix(scripts): reject exhausted feature numbers

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

* fix(scripts): complete create feature parity

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

* fix(scripts): align create feature outputs

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

* fix(scripts): harden cross-platform parity

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

* fix(scripts): keep truncation JSON clean

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

* fix(scripts): align setup failure parity

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

* fix(scripts): close parity edge cases

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

* fix(scripts): propagate PowerShell setup errors

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

* fix(scripts): harden fallback resolution

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

* fix(scripts): stabilize PowerShell fallbacks

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

* fix(scripts): complete setup-plan parity

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

* fix(cli): require runnable script fallbacks

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

* fix(cli): preserve shell fallback without preference

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

* fix(scripts): restore help and symlink parity

- setup-tasks.ps1: check -Help before unknown-argument validation so
  '-Help --bogus' exits 0 like the Bash/Python variants
- common.py: strip the repo root prefix lexically in persist_feature_json
  instead of resolve(), so a symlinked specs/ still persists the relative
  'specs/NNN-name' path the Bash/PowerShell helpers store

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

* fix(scripts): align persist-hint quoting with shlex.quote

- create-new-feature.sh: replace printf %q with a shell_quote helper that
  emits shlex.quote-identical output, so the persistence hints stay
  byte-identical between the Bash and Python variants (printf %q output
  also varies between bash versions)
- promote the negative --number test to an all-variants parity test now
  that Bash and PowerShell reject signed values consistently
- add a spaced-repo-path parity test for the persistence hints

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

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-07-21 11:40:15 -05:00
74662cffad feat: update Bob integration to skills-based layout for Bob 2.0 (#3415)
* feat: update Bob integration to skills-based layout for Bob 2.0

Bob 2.0 replaces the command-based workflow (.bob/commands/*.md) with
a skills-based layout (.bob/skills/speckit-<name>/SKILL.md), matching
the pattern used by Claude Code, Codex, and other skills-first agents.

- Switch BobIntegration from MarkdownIntegration to SkillsIntegration
- Update folder/dir from .bob/commands to .bob/skills
- Change extension from .md to /SKILL.md (skills layout)
- Add --skills option (default: True) consistent with Codex pattern
- Update tests to inherit from SkillsIntegrationTests (28 tests pass)
- Bump catalog entry to version 2.0.0 with updated description

Assisted-by: IBM Bob (model: claude-sonnet-4-5, autonomous)

* PR comments fix: keep old Bob 1 commands till next release

* Copilot suggested change

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

* feat(bob): address copilot comments, make skills layout default, demote legacy commands to opt-in

* fix(bob): honor legacy_commands in ai_skills persistence and add bob to ALWAYS_SLASH_AGENTS

- init.py: suppress ai_skills=True when --legacy-commands is passed so
  extensions and presets target .bob/commands, not .bob/skills
- _invocation_style.py: add 'bob' to ALWAYS_SLASH_AGENTS so init next-steps
  and hook invocations always show /speckit-<name> (skills is the default
  layout; no ai_skills flag required)

* Copilot suggestion

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

* fix(bob): extend IntegrationBase directly to avoid false isinstance(SkillsIntegration)

- bob/__init__.py: switch BobIntegration base from SkillsIntegration to
  IntegrationBase; add _BobSkillsHelper for skills-mode delegation; set
  invoke_separator='-' explicitly; set _skills_mode flag in setup() so
  consumers can derive the effective mode without isinstance checks
- _helpers.py: replace isinstance(integration, SkillsIntegration) guard
  with getattr(_skills_mode) so legacy-commands mode does not persist
  ai_skills=True
- _invocation_style.py: remove 'bob' from ALWAYS_SLASH_AGENTS — Bob 2.0
  skills are invoked via natural language, not /skill-name slash commands
- integrations/catalog.json: advance updated_at to 2026-07-15

* fix(lint): remove unused SkillsIntegration import from _helpers.py

* Copilot suggested change

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

* feat(bob): add bob skills integration with registrar-based mode detection

* address 3 comments from copilot

* feat(bob): update registrar config to use legacy commands layout

* fix lint

* Suggested fix from Copilot

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

* fix pr comment

* fix pr comment

* fix pr comment

* refactor(bob): resolve skills mode via base-class hooks + fix command-ref separators

Rework the dual-mode handling introduced for Bob 2.0 so an integration's
internal representation never leaks into shared init/install/upgrade code,
and fix the legacy command-reference separator surfaced in review.

Base-class contract:
- Add IntegrationBase.is_skills_mode(parsed_options) — the single hook the
  shared machinery consults to decide whether to persist ai_skills and render
  skill invocations. SkillsIntegration returns True; Copilot honors --skills /
  self._skills_mode; Bob returns `not legacy_commands`.
- Add IntegrationBase.invoke_separator_for_mode(skills_enabled) — resolves the
  command-ref separator from a project's persisted mode for registration paths
  that only have the ai_skills flag (no CLI parsed_options). Default is
  behavior-preserving; Bob maps skills->"-", legacy->".".
- BobIntegration stays on IntegrationBase (mirroring Copilot, the other
  dual-mode agent) and delegates setup() to internal _BobSkillsHelper /
  _BobMarkdownHelper. Removes the _skills_mode method and all
  isinstance(SkillsIntegration) / callable(_skills_mode) probing from
  _helpers.py and init.py.

Fix legacy separator (review feedback): CommandRegistrar.register_commands and
PresetManager._resolve_skill_command_refs previously read the single static
AGENT_CONFIGS[key]["invoke_separator"], so legacy .bob/commands/ extension and
preset command refs rendered /speckit-<cmd> instead of Bob 1.x /speckit.<cmd>.
Both now resolve the separator per project mode via invoke_separator_for_mode.

Tests: add regression coverage for the is_skills_mode / invoke_separator_for_mode
hooks and legacy extension command-ref separators; normalize a width-sensitive
workflow assertion to match its siblings. Full suite green.

Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 63f93544-a77f-4f01-bf04-c88806a97dbf

* fix(bob,copilot): address review — preserve legacy layout, dual-mode separators, extension-skill token resolution

Addresses PR review 4716036212 (3 comments):

1. Bob legacy-install regression: `use`/`switch`/`upgrade` on an existing
   Bob 1.x project (only `.bob/commands/` on disk, no stored
   `legacy_commands`) called `is_skills_mode(None)` -> True and rewrote
   `ai_skills=True`, silently switching extension/command-reference handling
   to the skills layout. `is_skills_mode` now takes an optional `project_root`;
   Bob preserves an already-installed legacy layout until an explicit upgrade
   creates `.bob/skills/`. A fresh project still defaults to skills.

2. Copilot dual-mode separator: `invoke_separator_for_mode` was inherited
   from the base (mode-independent) and returned Copilot's static `.`, so
   preset/extension command refs in a Copilot skills project rendered
   `/speckit.<name>` instead of `/speckit-<name>`. Override it on Copilot to
   track the persisted `ai_skills` state, consistent with
   `build_command_invocation` and `effective_invoke_separator`.

3. Bob extension-skill command-ref tokens: verified that merging main's
   generic `_resolve_command_ref_tokens` (#3544) resolves Bob's tokens via
   the `CONDITIONAL_SLASH_AGENTS` path (`/speckit-<name>`); added Bob to the
   command-ref regression parametrize plus dedicated Bob use-path tests.

All tests pass (full suite green; merged with current main incl. #3544).

Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 63f93544-a77f-4f01-bf04-c88806a97dbf

* fix(bob): resolve command-ref separator with project-aware mode before shared-infra refresh (review #3415)

The `use`/`switch` paths refresh shared infrastructure via
`_with_integration_setting()` / `_invoke_separator_for_integration()`,
which previously resolved the invoke separator through
`effective_invoke_separator` / `is_skills_mode` WITHOUT a project_root.
For a pre-PR Bob 1.x project (.bob/commands/ on disk, no stored options),
this defaulted to the skills "-" separator and rewrote rendered
shared-template command refs to /speckit-*, even though ai_skills stayed
false. Thread project_root through effective_invoke_separator, the two
runtime helpers, and every call site so Bob's on-disk legacy detection
governs the separator before shared infra is refreshed.

Add a rendered-shared-template regression test covering `use --force`.

Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 63f93544-a77f-4f01-bf04-c88806a97dbf

* fix(bob): scope persisted ai_skills flag to active agent when resolving command-ref separator (review #3415)

`register_commands` runs once per detected agent, but the persisted
`ai_skills` flag describes only the active integration (`opts["ai"]`).
When another agent (e.g. Copilot) is active in skills mode while a
legacy `.bob/commands` layout is also present, the previous code passed
that global `True` to Bob's `invoke_separator_for_mode`, rewriting Bob
1.x command refs to `/speckit-*` instead of `/speckit.*`.

Only consult the persisted flag for the agent it describes
(`opts["ai"] == agent_name`); otherwise resolve the separator from the
agent's own project-aware `effective_invoke_separator(None, project_root)`.

Add regression tests covering the mismatched-active-agent case and a
control for Bob-active skills mode.

Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 63f93544-a77f-4f01-bf04-c88806a97dbf

* fix(bob): detect Spec Kit layout from managed artifacts, not any skills dir (review #3415)

Two related mis-detections from review 4723246468:

1. `BobIntegration.is_skills_mode` treated the mere presence of a
   `.bob/skills/` directory as proof the project is skills-based. A legacy
   Spec Kit install (managed `.bob/commands/speckit.*.md`) that also carried
   unrelated Bob 2 skills would be misclassified as skills, so
   `integration use bob` persisted `ai_skills` and rewrote shared refs.
   Now the layout is inferred from managed Spec Kit artifacts: legacy/command
   mode only when managed `speckit.*.md` command files exist and no managed
   `speckit-*` skill dirs do.

2. The `register_commands` separator for an inactive agent used a disk-based
   `effective_invoke_separator(None, project_root)` fallback that could pick
   the skills separator even though the registrar writes the static command
   layout (`.bob/commands/*.md`). Inactive agents now resolve the separator
   from the registrar's actual output layout (`extension == "/SKILL.md"`),
   so command-layout files keep `/speckit.*` refs regardless of sibling dirs.

Update the affected hook/E2E tests to use managed artifacts and add
regression tests for the mixed-layout and inactive-registrar scenarios.

Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 63f93544-a77f-4f01-bf04-c88806a97dbf

* fix(bob): apply managed-artifact detection on upgrade + consistent skill post-processing (review #3415)

Two issues from review 4723782860:

1. `BobIntegration.setup()` resolved the layout via `is_skills_mode(parsed_options)`
   WITHOUT `project_root`, so `integration upgrade bob` on a Bob 1.x install
   (managed `.bob/commands/speckit.*.md`, no stored options) ignored the
   existing command files, generated skills, and stale-deleted the legacy
   commands — silently migrating the project. Pass `project_root` so the same
   managed-artifact detection used by `use` also governs upgrades.

2. Only `_BobSkillsHelper` overrode `post_process_skill_content` to suppress
   the shared slash-command hook note. Preset/extension skill generators call
   that hook on the registered `BobIntegration`, which inherited
   `IntegrationBase`'s note-injecting default. Repeat the no-op (delegating to
   the skills helper) on the registered class so every Bob skill-generation
   path is consistent with intent-activated core Bob skills.

Add regression tests for the upgrade-preservation and post-processing paths.

Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 63f93544-a77f-4f01-bf04-c88806a97dbf

* feat(bob): add --skills migration opt-in; fix separator + manifest loss (review #3415)

Address review #3415 (4724160183):

- Comment 1: Add an explicit `--skills` opt-in to BobIntegration. It forces
  the skills layout over on-disk auto-detection, giving legacy Bob 1.x
  installs a supported migration path
  (`integration upgrade bob --integration-options="--skills"`). `--skills`
  and `--legacy-commands` are mutually exclusive (clean exit-1 error).

- Comment 2: In CommandRegistrar.register_commands, derive the command-ref
  separator from the output layout (agent_config["extension"]) for the
  active agent too, not the persisted ai_skills flag. A command-layout file
  (.bob/commands/*.md, .github/agents/*.agent.md) always renders /speckit.*;
  only a /SKILL.md scaffold uses /speckit-*. Dual-layout agents (Bob,
  Copilot) write skills via their own setup()/skills path, so
  register_commands only ever emits their command-layout files.

- Comment 3: Update docs/reference/integrations.md Bob entry to document the
  skills-based default (.bob/skills/), the deprecated --legacy-commands
  opt-out, and the --skills migration path.

Also fix a latent manifest-loss bug surfaced by the migration path: the
upgrade Phase 2 stale-file cleanup built a throwaway manifest sharing the
integration key and called uninstall(), which always deleted
{key}.manifest.json. Any layout-shrinking upgrade (e.g. legacy->skills)
thus wiped the freshly-saved manifest, leaving the project untracked and
un-upgradeable. uninstall() now takes remove_manifest (default True); the
stale-cleanup pass passes False.

Adds regression tests for the --skills opt-in, mutual exclusion, corrected
active-agent separator, remove_manifest=False, and an end-to-end
legacy->skills migration that verifies the manifest survives and the
project remains upgradeable. Full suite: 4555 passed, 5 skipped.

Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 63f93544-a77f-4f01-bf04-c88806a97dbf

* docs(agents): align token-resolution comment with output-layout separator rule (review #3415)

Address review #3415 (4725516805). The comment above resolve_command_refs
still described the removed state-based behavior ("resolve it from the
integration using the project's persisted skills state"). Update it to
describe the output-layout rule that register_commands now uses: _sep is
derived from the layout this registrar writes (a /SKILL.md scaffold uses the
skills separator; a command-layout file uses the command separator), not the
persisted ai_skills state. Comment-only change; no behavior change.

Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 63f93544-a77f-4f01-bf04-c88806a97dbf

* fix(bob): reconcile extension artifacts on layout change (review #3415)

When a dual-mode agent (Bob) flips between the legacy commands layout and
the skills layout during `integration upgrade` (via `--skills` /
`--legacy-commands`), the old layout's extension command/skill files were
left orphaned: Phase 2 stale cleanup only removes files tracked by the
*integration* manifest, while extension artifacts are tracked in the
extension registry. Detect the layout flip by comparing whether the old vs
new manifest tracks a `/SKILL.md` scaffold, and when it changed, unregister
the agent's extension artifacts before the existing re-registration so they
are recreated in the new layout (and the per-agent registry is updated).

Preset artifacts are documented as a known, pre-existing cross-cutting gap:
no agent-scoped preset re-registration exists in use/switch/upgrade for any
agent, so reconciling them is out of scope for this Bob migration.

Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 63f93544-a77f-4f01-bf04-c88806a97dbf

* fix(bob): reject layout migration when preset overrides are installed (review #3415)

A command↔skills layout change during `integration upgrade` cannot reconcile
preset artifacts: presets track their command/skill files in per-preset
`registered_commands`/`registered_skills` metadata, and there is no
agent-scoped preset re-registration anywhere in the CLI. Migrating would
delete a preset's old-layout files without recreating them in the new layout
and leave the preset registry claiming artifacts that no longer exist.

Detect the intended layout via `is_skills_mode` (so a plain same-layout
upgrade is unaffected) and, when it flips while preset overrides are
installed for the agent, reject the upgrade *before any mutation* with an
actionable error pointing at the remove → upgrade → reinstall workaround.
Extension artifacts are still reconciled for the safe (no-preset) case.

Adds a regression test and documents the migration caveat in the Bob
integration reference entry.

Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 63f93544-a77f-4f01-bf04-c88806a97dbf

* fix(bob): restrict layout reconciliation to the active integration (review #3415)

`integration_upgrade` supports upgrading a secondary (non-active) integration,
but the layout-change extension reconciliation was unsafe there.
`ExtensionManager.unregister_agent_artifacts()` treats the unscoped
per-extension `registered_skills` list as belonging to the passed agent and,
when that agent's skills directory is absent, falls back to scanning every
agent's skills directory — so reconciling a secondary Bob layout flip could
delete or untrack the *active* agent's extension skills. The subsequent
re-registration cannot repair that because extension skill rendering is
intentionally scoped to the active agent (#2948).

Gate the unregister-before-register reconciliation on `installed_key == key`
so it only runs for the active integration. Secondary agents only ever have
extension command files (skills are active-agent-only), which the existing
re-registration rewrites in place, so skipping the unregister orphans nothing
new. Adds a regression test asserting a secondary Bob layout change leaves the
active agent's extension skill intact on disk and in the registry.

Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 63f93544-a77f-4f01-bf04-c88806a97dbf

* fix(bob): fail closed when preset registry is unreadable (review #3415)

Address review 4744636079:

- _migrate_commands: the preset guard previously failed *open* — a
  registry read/parse error returned an empty "no presets" list, so a
  --force layout-changing upgrade could delete preset-overridden command
  files while their registry state was unknown. Read the registry file
  directly and raise _PresetRegistryUnreadableError on any read/parse
  failure or malformed structure, rejecting the migration before any
  mutation. A genuinely absent registry still returns [] (safe).

- bob: correct the is_skills_mode docstring — upgrade *does* run setup();
  disk detection is needed because legacy Bob 1.x installs never persisted
  a legacy_commands option, so the stored mode is unavailable.

- tests: add fail-closed E2E (corrupted registry rejected, valid-empty
  allowed) plus a unit test for _installed_presets_affecting_agent covering
  absent / corrupted / malformed / valid / affecting-agent cases.

Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 63f93544-a77f-4f01-bf04-c88806a97dbf

* fix(bob): fail closed on malformed preset entries too (review #3415)

Address review 4745191015: the preset guard read a parseable registry but
silently skipped malformed per-preset metadata and treated a malformed
registered_commands value as "no matching artifacts". A registry such as
{"presets":{"p1":[]}} therefore allowed a layout migration even though p1's
ownership is unknown, risking deletion of preset-managed files. Now raise
_PresetRegistryUnreadableError for a non-dict preset entry, a non-dict
registered_commands, or a non-list registered_skills. Extend the unit test
to cover these malformed shapes.

Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 63f93544-a77f-4f01-bf04-c88806a97dbf

---------

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Manfred Riem <15701806+mnriem@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
2026-07-21 09:20:20 -05:00
0d780162f9 fix(bundle): surface a clean BundlerError on a malformed bundle download URL (#3586)
`_download_manifest` and its `_require_https` helper parsed the catalog
entry's `download_url` with an unguarded `urlparse(url)`. A malformed
authority — e.g. an unclosed IPv6 bracket like `https://[::1` — makes
`urlparse` (or `.hostname` on older Pythons) raise a raw `ValueError`. The
three `bundle` CLI commands (`info`, `install`, `update`) only catch
`BundlerError`, so that `ValueError` escaped as an uncaught traceback.

Wrap both parse sites in the same `try/except ValueError -> BundlerError`
guard already used by the sibling `_validate_remote_url` (and established by
the merged catalog-URL fix #3576), so a bad `download_url` reports a clean,
actionable error in every mode.

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-17 09:52:35 -05:00
fd101d531e feat(integrations): add Grok Build skills-based integration (#3535)
* feat(integrations): add Grok Build skills-based integration

Add first-class support for xAI Grok Build via SkillsIntegration, installing
speckit skills under .grok/skills and wiring init/invocation/catalog surfaces.

Assisted-by: Grok Build (model: grok-4, supervised)

* test+docs: address Copilot review on Grok multi-install and next steps

Assert init next-steps guidance for Grok (.grok/skills, /speckit-*) and
clarify that multi-install safety is path/manifest isolation, not
agent-context defaults such as shared AGENTS.md.

* fix(integrations): Grok headless --always-approve and isolation paths

Document Grok multi-install isolation as .grok/skills and .grok/rules.
Override build_exec_args to pass --always-approve so non-interactive
dispatch is not blocked at tool permission gates.

* docs(integrations): list only managed .grok/skills for Grok isolation

Multi-install isolation documents Spec Kit-managed paths; Grok only
writes .grok/skills, so drop the read-only .grok/rules entry.

* fix(integrations): always-slash Grok hooks and refresh catalog date

Move grok to ALWAYS_SLASH_AGENTS so hooks never emit /speckit.plan when
ai_skills is missing/false. Update slash-format tests, persist ai_skills
on init, and bump catalog updated_at for the Grok entry.

---------

Co-authored-by: Nate Chadwick <1232206+natechadwick@users.noreply.github.com>
Co-authored-by: test <test@example.com>
2026-07-15 14:55:25 -05:00
353851e966 fix(presets): seed constitution from preset constitution-template (#3272) (#3276)
* fix(presets): seed constitution from preset constitution-template (#3272)

The constitution is the only template materialized to a live file
(.specify/memory/constitution.md) rather than resolved on demand, yet
ensure_constitution_from_template hardcoded a copy from the core template
and ignored PresetResolver. Combined with init seeding the constitution
before preset installation, a preset's constitution-template (e.g.
strategy: replace with a ratified constitution) could never go live.

Changes:
- ensure_constitution_from_template now resolves constitution-template
  through PresetResolver, so a preset/override/extension wins and core is
  the fallback.
- init seeds the constitution after preset installation so init --preset
  uses the resolved stack.
- install_from_directory re-seeds memory/constitution.md from the resolved
  preset template, guarded to only act when the memory file is missing or
  still contains generic placeholder tokens — authored constitutions are
  never overwritten. Covers preset add and install_from_zip.
- Tests for preset seeding, placeholder re-seed, authored-constitution
  preservation, override resolution, and resolver-aware init seeding.

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

* fix(presets): compose constitution-template when seeding memory

Take on review feedback from Copilot and gglachant:
- constitution seeding previously copied the top layer file path verbatim
  even when the winning layer used a composing strategy
  (prepend/append/wrap), which could leave {CORE_TEMPLATE} unresolved.
- both seeding paths now inspect resolver layers and only copy verbatim for
  replace; non-replace strategies materialize composed content via
  PresetResolver.resolve_content().
- add regression tests for wrap strategy composition in both
  PresetManager seeding and ensure_constitution_from_template.
- add a drift-guard test pinning _CONSTITUTION_PLACEHOLDER_TOKENS to the
  placeholders in templates/constitution-template.md.

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

* refactor(presets): unify constitution template materialization

Address latest Copilot feedback on the constitution seeding path:
- moved resolver/layer I/O behind the existing-memory fast path in init
- corrected tracker output for composed materialization
- deduplicated materialization logic shared by init and preset install seeding
  into presets._materialize_constitution_template()

Behavior is unchanged for replace strategies (copy verbatim) and remains
composed for prepend/append/wrap via resolve_content().

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

* fix(init): restore shutil import

The constitution materialization refactor removed the module import, but init
still uses shutil.rmtree when cleaning up a failed new-project initialization.
Restore the import so the required ruff check passes.

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

* fix(presets): harden constitution materialization

Address the outstanding review batch for preset constitution seeding:
- use checked atomic writes and reject symlinked memory paths
- replace placeholder heuristics with hash/source provenance
- rematerialize unchanged generated constitutions by resolver priority
- preserve authored or edited constitutions, including placeholder mentions
- warn non-fatally when post-install materialization cannot complete
- retain exact core-template comparison for legacy projects without provenance

Add focused provenance, priority, symlink, and failure-path coverage, and
update integration inventories for the generated provenance sidecar.

Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 1b2c095d-b45c-4d52-8d56-bd6121d96ab6

* fix(presets): reconcile constitution after removal

When the removed preset supplied constitution-template, rematerialize the
winning remaining resolver layer only if provenance proves the live file is
still generated and unchanged. Preserve edited constitutions and report
post-removal reconciliation failures as non-fatal warnings.

Add coverage for restoring the core layer, falling back from a removed
higher-priority preset, and preserving edited generated content.

Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 1b2c095d-b45c-4d52-8d56-bd6121d96ab6

* fix(presets): tighten legacy constitution provenance

Trust only the immutable bundled/source constitution template when migrating
legacy projects without provenance. Do not infer core provenance from mutable
project templates or preset source labels, including IDs beginning with core.

Also detect convention-based constitution-template files before preset removal
so unchanged generated constitutions reconcile to the next resolver layer.

Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 1b2c095d-b45c-4d52-8d56-bd6121d96ab6

* fix(presets): preserve files with invalid provenance

Use immutable-core legacy migration only when the provenance sidecar is absent.
If a sidecar is malformed or its hash does not match the live constitution,
treat the file as edited and preserve it.

Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 1b2c095d-b45c-4d52-8d56-bd6121d96ab6

* fix(presets): reconcile constitution on stack changes

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

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

Copilot-Session: 1b2c095d-b45c-4d52-8d56-bd6121d96ab6

* fix(presets): guard constitution reconciliation edges

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

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

Copilot-Session: 1b2c095d-b45c-4d52-8d56-bd6121d96ab6

---------

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
2026-07-15 11:42:50 +01:00
993083405e fix(init): don't block on confirmation for 'init --here' without a TTY (#3236)
* fix(init): don't block on confirmation for 'init --here' without a TTY

When 'specify init --here' targets a non-empty directory without --force, it called typer.confirm() unconditionally. In a non-interactive session (no TTY -- CI, piped, agent) there is no input, so the prompt reads EOF and aborts unhelpfully (or blocks), with no actionable message. The named-project path already fails fast and points to --force; --here was the inconsistent outlier.

Guard the confirmation with the existing _stdin_is_interactive() helper: when non-interactive, print a clear 'directory not empty; re-run with --force' error and exit 1 instead of prompting.

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

* fix(init): honor piped confirmation for 'init --here'; only fail-fast on empty stdin

The first version of this fix short-circuited on '_stdin_is_interactive()' (isatty) before typer.confirm, which broke 'init --here' when confirmation is piped (e.g. 'echo y | specify init --here' / CliRunner input='y\n') -- a non-TTY pipe with valid input was wrongly rejected, regressing test_init_here_without_force_preserves_shared_infra. Instead, call typer.confirm normally (piped 'y'/'n' is honored) and catch the Abort/EOFError it raises only when stdin is empty, converting that to the actionable '--force' guidance. This keeps the UX win for the no-input case without rejecting piped input.

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

* fix(init): distinguish interactive cancel from no-input; defer merge warning

Address Copilot review on the --here non-empty path: (1) treat typer.Abort during an interactive confirm (e.g. Ctrl+C) as a normal cancellation (exit 0), and only emit the '--force' guidance + exit 1 when there is no TTY (empty stdin / EOF) -- no longer conflating the two; (2) move the 'will be merged / may overwrite' warning so it only shows when actually proceeding (force) or folded into the confirmation prompt, not on the fail-fast path where nothing is merged.

Piped confirmation (e.g. 'echo y | specify init --here') is still honored, which is why the prompt is attempted rather than refused outright when non-interactive -- the existing test_init_here_without_force_preserves_shared_infra pipes 'y' and must succeed.

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

* fix(init): fail fast on non-interactive --here instead of prompting

Per Copilot review: do not call typer.confirm when stdin is not a TTY -- an open-but-idle non-TTY stdin (CI/agent) could block on the prompt. When the directory is non-empty and --force is not given, fail fast with '--force' guidance unless an interactive terminal is present. Interactive confirm still offers the merge-but-preserve path (distinct from --force, which overwrites); a Ctrl+C there is treated as a normal cancellation (exit 0). The merge/overwrite warning is only printed when actually proceeding, not on the fail-fast path.

Updated the preserve-merge E2E test to simulate an interactive terminal so it exercises the confirm path (non-interactive sessions now require --force).

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

* fix(init): honor piped y/n for 'init --here', error only on no-input

Per maintainer review: restore the second-revision shape. Calling
typer.confirm normally keeps 'echo y | specify init --here' reaching the
non-destructive preserve-merge path (and piped 'n' cancels with exit 0).
Only when no confirmation input is available at all (closed/empty stdin
-> typer.Abort/EOFError) is it converted into the actionable error that
points at --force. This drops the _stdin_is_interactive fail-fast that
broke the common piped-confirm idiom and made preserve-merge
interactive-only. The preserve test no longer needs to monkeypatch
_stdin_is_interactive - it passes on the real contract.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(init): preserve interactive-cancel semantics; fold merge risk into the prompt

Two review-driven refinements to the 'init --here' non-empty confirm, keeping
the maintainer-endorsed control flow (piped y/n honored; non-interactive EOF →
actionable --force error):

1. typer.confirm raises typer.Abort for BOTH an interactive Ctrl+C and an EOF on
   closed/empty stdin. Catching it unconditionally reported 'no confirmation
   input available, use --force' and exited 1 even when the user cancelled at a
   real TTY. Branch on _stdin_is_interactive(): a TTY cancel is a normal exit 0
   ('Operation cancelled'); only non-interactive EOF becomes the --force error.

2. Fold the merge-risk warning into the confirmation question instead of printing
   it unconditionally beforehand, so the EOF/no-input path (which exits without
   changing anything) no longer prints a misleading 'will be merged' line first.

Adds test_init_here_interactive_cancel_exits_zero (fails before: exit 1 with
--force; passes after: exit 0, 'cancelled', pre-existing file untouched). The
non-interactive EOF and piped-y preserve-merge tests are unchanged and still pass.

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-13 15:29:15 -05:00
ba1f13a8b1 fix(bundle): reject file:// / local download_url — catalog URLs are HTTPS-only (#3344)
* fix(bundle): resolve file:// download_url via the file-URL helper

_download_manifest built the local path from raw parsed.path, which
keeps the leading slash of file:///C:/x (yielding a \C:\x path that
never exists on Windows) and skips percent-decoding (my%20bundles stays
encoded on every OS) — so a catalog entry whose download_url is the
canonical URI Python itself produces via Path.as_uri() always fails
with 'Bundle manifest not found'. Route the file scheme through the
existing bundler.services.adapters._file_url_to_path helper, which
already handles drive letters, UNC hosts, and percent-decoding for
catalog file:// URLs (make_catalog_fetcher). The bare-path branch is
unchanged.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(bundle): reject file:// / local download_url; catalog URLs are HTTPS-only

Per maintainer review (route B): file:// in a catalog download_url was
never intended — catalog URLs are HTTPS-only (http for localhost) across
the extensions/presets/workflows catalog systems, and disk installs go
through the positional path (specify bundle install <path>), handled by
_local_manifest_source before catalog resolution. Remove the
file:///bare-path branch from _download_manifest and route everything
through _download_remote_manifest (HTTPS-only via _require_https), with an
actionable error pointing at the positional install. Invert the file://
tests to assert rejection (+ a positional-path resolution test), and
migrate the three bundle-info contract tests off local download_urls onto
an HTTPS-only entry with a mocked manifest fetch.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(bundle): validate HTTPS before the offline gate in _download_manifest

Per review: for a non-local download_url the offline check ran before any
URL validation, so an invalid/non-HTTPS scheme surfaced a misleading
'Network access disabled' error under --offline when the real problem is
the URL would be rejected even online. Call _require_https before the
offline gate so the correct error is reported in every mode.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(bundle): reword non-HTTPS download_url error to not mislabel scheme-less URLs

A scheme-less download_url (urlparse scheme == "") can be a bare
filesystem path OR a missing-scheme value like 'example.com/foo.zip',
not necessarily file://. Reword the reject error to state the real
HTTPS-only constraint and enumerate what is rejected (file://, local
path, scheme-less), instead of labeling every case 'local/file://'.

Behavior unchanged; the 'bundle install' actionable hint is preserved,
so the existing reject-path tests still pass.

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

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
2026-07-13 09:15:55 -05:00
3b30e40aaa fix: resolve GitHub release asset API URL for private repo bundle downloads (#3136)
* fix: resolve GitHub release asset API URL for private repo bundle downloads

For private/SSO-protected GitHub repos, browser release download URLs
(https://github.com/<owner>/<repo>/releases/download/<tag>/<asset>)
redirect to an HTML/SSO page instead of delivering the asset, causing
bundle manifest downloads to fail.

Extends the pattern from #2855 (presets/workflows) to cover the bundle
manifest download path in _download_remote_manifest:

- Resolves browser release URLs to GitHub REST API asset URLs via
  resolve_github_release_asset_api_url before downloading
- Direct REST API asset URLs (api.github.com/repos/.../releases/assets/<id>)
  are passed through directly
- Both cases use Accept: application/octet-stream so the API returns the
  binary payload rather than JSON metadata
- The original catalog URL is used to determine artifact format (.zip vs
  YAML) since the resolved API URL does not carry the file extension

Adds two CLI-level contract tests:
- bundle info resolves browser release URL via GitHub tags API
- bundle info passes direct API asset URL through with octet-stream

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix: detect ZIP payload by magic bytes; add zip and API-asset tests

Address Copilot review feedback on PR #3136:

1. Detect ZIP payloads by magic bytes (PK\x03\x04) in addition to the
   '.zip' URL suffix so that direct GitHub REST asset URLs — which carry
   no file extension — are correctly routed through the ZIP extraction
   path when the asset is a ZIP bundle artifact.

2. Add two new contract tests:
   - test_bundle_info_resolves_github_browser_release_url_zip: exercises
     the '.zip' browser release URL path end-to-end, verifying the tags
     API lookup fires, octet-stream header is used, and bundle.yml is
     successfully extracted from the ZIP payload.
   - test_bundle_info_api_asset_url_zip_detected_by_magic_bytes: verifies
     that a direct REST asset URL returning ZIP bytes is detected by magic
     and parsed correctly without a tags API call.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix: improve error message, broaden ZIP magic, drop unused tmp_path

Address second-round Copilot review feedback on PR #3136:

- Error message: when the download fails, report the original catalog
  download_url so the user knows which entry to fix; include the resolved
  REST API URL when it differs for easier debugging.
- ZIP detection: broaden the magic-bytes check from PK\x03\x04 to raw[:2]
  == b"PK", covering all valid ZIP variants (local-file header PK\x03\x04,
  empty-archive PK\x05\x06, spanned/split PK\x07\x08).
- Tests: remove the unused tmp_path parameter from
  test_bundle_info_resolves_github_browser_release_url_zip.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix: use full 4-byte ZIP signatures instead of 2-byte PK prefix

Address Copilot feedback: raw[:2] == b"PK" is too broad and could
misclassify any payload starting with ASCII "PK" as a ZIP, producing
a confusing "not a valid bundle" error.

Use the three specific 4-byte ZIP magic signatures instead:
  PK\x03\x04 — local file header (standard ZIP)
  PK\x05\x06 — end-of-central-directory (empty archive)
  PK\x07\x08 — data descriptor / spanning marker

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix: harden _download_remote_manifest parsing and tighten tests

- Promote _ZIP_SIGNATURES to module-level constant (was redefined per call)
- Use PurePosixPath for URL path suffix extraction so query strings and
  fragments are ignored and URL paths are treated as POSIX on all OSes
- Move yaml/BundleManifest imports to function top to flatten the
  previously nested try/except into a single handler with explicit
  except _yaml.YAMLError and except Exception clauses
- Re-add None guard on _local_manifest_source return: the function is
  typed Optional[BundleManifest] and without the guard a None return
  propagates silently to callers that degrade gracefully rather than
  raising an actionable error; comment explains it is defensive not dead
- Assert exact resolved asset URL in browser-URL download tests, not
  just the Accept header, so a regression where download uses the
  original URL instead of the resolved one would be caught
- Add resolution-failure test: when tags API finds no matching asset the
  code falls back to the original URL and exits non-zero with Error:

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(bundle): pass github_provider_hosts() for GHES private release downloads

Extends the GHES support pattern from extensions and presets (#2855, #3157)
to the bundle manifest download path: resolve_github_release_asset_api_url
now receives github_hosts=github_provider_hosts() so browser release URLs
from GitHub Enterprise Server instances are resolved via /api/v3 rather
than falling back to the unauthenticated download path.

Also adds a contract test covering the GHES resolution path for
_download_remote_manifest (analogous to the existing github.com tests).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* test(bundle): remove unused ghes_entry variable from GHES contract test

The dict was defined but never consumed — the test drives GHES host
recognition entirely through the github_provider_hosts() patch.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(bundle): include source URL in remote manifest parse errors

Thread the catalog URL (and resolved API URL when it differs) into the
YAML parse, generic parse, and ZIP-extraction error paths of
_download_remote_manifest so failures point at the offending source
instead of an opaque temp path. Addresses PR review feedback.

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

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-authored-by: Manfred Riem <15701806+mnriem@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-07-01 16:30:20 -05:00
c5fb3dc86f fix(bundle): send command errors to stderr so --json stdout stays parseable (#3235)
The bundle command group's _fail() helper is documented as printing 'to stderr', and the module contract is 'human logs go to stderr/console' while --json 'emits machine-readable data on stdout'. But it called console.print(), and the shared console writes to STDOUT, so every bundle error (every command routes through _fail) landed on stdout -- corrupting the JSON stream that --json consumers parse.

Add a stderr-bound err_console to _console.py (its documented role as the single Console source) and use it in _fail. stdout now carries only the JSON payload.

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-29 15:46:56 -05:00
53d9543355 feat: make agent-context extension a full opt-in (#3097)
* docs: add Spec Kit spec for agent-context full opt-in

Use Spec Kit's own specify workflow to author the spec that makes the
agent-context extension a full opt-in, removing all agent-context
configuration/support from the Python codebase and removing the
deprecation message. Force-added despite specs/ being gitignored; the
generated artifact will be purged prior to merge.

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

* docs: add Spec Kit plan artifacts for agent-context full opt-in

Phase 0/1 of the SDD plan workflow: plan.md, research.md, data-model.md,
quickstart.md, and contracts/cli-behavior.md. Constitution Check is a
documented no-op (repo has no ratified constitution). Force-added despite
specs/ being gitignored; generated artifacts will be purged prior to merge.

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

* docs: correct Constitution Check against ratified v1.0.0

Earlier draft wrongly treated the gate as a no-op; the fork's main is 16
commits behind upstream/main, which carries .specify/memory/constitution.md.
Re-evaluate the feature against Principles I-V (all PASS) and note that
Principle I mandates keeping context_file as a declared class attribute,
validating the R1 metadata decision.

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

* docs: refresh plan artifacts against synced upstream/main

After syncing fork main to upstream and rebasing, re-scan the current
agent-context surface. Upstream generalized the single context_file into a
plural context_files concept with new resolver helpers
(_resolve_context_files, _resolve_context_file_values,
_format_context_file_values) and upsert/remove now loop over multiple
files. Update research.md, data-model.md, contracts, quickstart grep
guards, and the plan summary to cover the expanded removal scope.

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

* docs: add Spec Kit tasks for agent-context full opt-in

Phase 2 of SDD: dependency-ordered tasks.md (30 tasks) organized by the
three user stories, with mandatory test tasks (Constitution Principle II)
and a foundational phase decoupling __CONTEXT_FILE__ resolution from the
extension config. Includes the extension self-seeding task (T015) and a
static guard test (T002) enforcing zero agent-context references in the CLI.

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

* feat!: remove agent-context lifecycle from the Specify CLI

Make the agent-context extension a full opt-in. The CLI no longer
installs the extension during init, writes agent-context-config.yml,
or creates/updates/removes the managed Spec Kit section in agent
context files. Context-section upsert/remove, marker resolution,
extension-enabled gating, the config helpers, and the obsolete inline
deprecation warning are all removed. Integration context_file stays as
inert metadata; __CONTEXT_FILE__ now resolves from registry metadata.

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

* feat(agent-context): self-seed context file from the active integration

When agent-context-config.yml has no context_file/context_files, the
bundled bash and PowerShell update scripts now resolve the context file
from the active integration in .specify/init-options.json via the
integration registry, so the extension no longer depends on the CLI
writing its config.

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

* test+docs: update suite and docs for agent-context opt-in

Update integration/extension tests to expect no agent-context install,
config, or context-section writes during init. Add a static guard test
(test_agent_context_cli_free.py) asserting the CLI source is free of
agent-context lifecycle symbols, plus backward-compatibility tests for
legacy projects. Refresh AGENTS.md, the extension README, and add a
CHANGELOG entry describing the opt-in behavior change.

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

* fix(agent-context): warn on self-seed failure, correct docs, speed up guard test

Address PR review feedback:
- Self-seed scripts (bash + PowerShell) now emit an actionable warning when
  an active integration is configured but specify_cli cannot be imported by
  the chosen Python (e.g. pipx installs), or when the integration declares no
  context file, instead of silently falling through to 'nothing to do'.
- Correct the extension README disable note: command rendering never reads the
  extension config; __CONTEXT_FILE__ is always substituted from integration
  metadata, so a stale context_files value cannot affect rendering.
- Cache CLI source reads in the static guard test via a module-scoped fixture
  so the directory walk happens once instead of once per forbidden symbol.

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

* feat(agent-context): ship self-owned per-agent context-file defaults

The extension now bundles agent-context-defaults.json (key→context_file
map) and self-seeds from it, dropping any dependency on the Specify CLI
registry. Both the bash and PowerShell update scripts read the bundled
JSON map keyed by the active integration from init-options.json.

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

* feat!: remove all agent-context state from the Specify CLI

Strip every context_file reference from the CLI: the field on all 35
integration classes, the IntegrationBase plumbing (process_template
param/step, _context_file_display, docstrings), the __CONTEXT_FILE__
resolution in agents.py, the legacy context_file/context_markers
popping in _helpers.py, and the context_file template in
integration_scaffold.py. Also drop the Agent context update step and
__CONTEXT_FILE__ placeholder from templates/commands/plan.md.

The agent-context extension now solely owns all context-file knowledge,
including the per-agent default mapping.

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

* test: drop context_file coverage and guard against CLI reintroduction

Remove CONTEXT_FILE attrs and context_file assertions across the base
mixins, all 35 per-integration test files, shared integration tests, and
conftest stubs. Rewrite the base-mixin context tests to assert no managed
section is written and no __CONTEXT_FILE__ placeholder survives. Extend
the CLI-free static guard to forbid context_file, __CONTEXT_FILE__, and
_context_file_display in src/specify_cli, and have the extension tests
copy the bundled defaults JSON so self-seed runs without the CLI.

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

* docs: reflect full removal of agent-context state from the CLI

Update AGENTS.md (integration examples, required-fields table, context
behavior section, pitfalls), CHANGELOG, and the SDD spec artifacts
(FR-007, SC-002, data-model) to state that the CLI carries no
context_file and the extension fully owns the per-agent default mapping
via agent-context-defaults.json.

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

* docs: align SDD artifacts with full context_file removal

Update research.md (R1, R2, R4, summary table), contracts/cli-behavior.md
(C3, C5), tasks.md (Phase 2, T026, notes), plan.md (Principle I, source
map), and checklists/requirements.md so the spec artifacts reflect the
implemented decision: the CLI carries no context_file attribute or
__CONTEXT_FILE__ resolution, and the per-agent defaults map lives in the
extension. Resolves PR review #4548130110.

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

* docs: scrub stale context-file mentions from CLI docstrings

Update the multi_install_safe docstring (drop the removed "context file"
invariant), the RovoDev setup docstring (no longer upserts a context
section), the Copilot module docstring (drop the context-file line), and
tighten the _update_init_options_for_integration note. Pure docstring
changes — no behavioral impact. Resolves PR review #4548237085.

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

* test+docs: harden agent-context test helper and fix stale docs

- base.py: document multi_install_safe as an optional subclass attribute
  in the IntegrationBase docstring.
- test_cli.py: clarify the init-options assertion is guarding against
  leftover legacy agent-context keys, not relocation.
- test_extension_agent_context.py: _install_agent_context_config now
  asserts the bundled agent-context-defaults.json exists and always
  copies it, so self-seeding tests fail loudly instead of silently
  skipping when the map is missing.
- test_integration_cursor_agent.py: drop Path/IntegrationManifest imports
  left unused after removing the context-section frontmatter tests.

Resolves PR review #4548293116.

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

* chore: remove gitignored SDD artifacts from specs/

The specs/001-agent-context-full-optin/ artifacts were force-added for
dogfooding visibility, but specs/ is gitignored and these were always
intended to be purged before merge. Remove them so merging does not add
an intentionally-untracked directory to repo history.

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

* chore: keep CHANGELOG.md identical to upstream

CHANGELOG.md is auto-generated at release time, so the branch should not
carry a manual entry. Restore it to match upstream/main exactly.

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

* fix: preserve Cursor .mdc frontmatter in agent-context updater scripts

The bundled agent-context updater scripts wrote the managed section as
plain text. For Cursor-style `.mdc` targets this dropped the required
`---\nalwaysApply: true\n---` frontmatter, reintroducing the rule-loading
bug originally fixed in #1699. Port the `_ensure_mdc_frontmatter` logic
into both the bash and PowerShell updaters: prepend frontmatter when
missing, repair `alwaysApply` when set to the wrong value, and leave
non-`.mdc` targets untouched. Add regression tests covering both shells.

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

* test: scope CLI-free guard to agent-context-specific symbols

Drop the bare "context_file" substring from FORBIDDEN_SYMBOLS so the
guard no longer fails on unrelated future CLI fields named context_file.
The list still covers agent-context-specific identifiers (__CONTEXT_FILE__,
_context_file_display, _resolve_context_files, _resolve_context_file_values).

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

* fix: harden agent-context bash self-seed against malformed init JSON

Two robustness fixes in the embedded Python self-seed logic:
- Coerce the integration value from init-options.json to a string only when
  it is actually a string; otherwise treat it as unset so a corrupted
  dict/list value degrades to the existing nothing-to-do behavior instead of
  breaking the agents-map lookup.
- Normalize agent-context-defaults.json: only use 'agents' when both the JSON
  root and the 'agents' value are dicts, so a wrong-shaped (but valid) JSON
  falls back to the warning path instead of raising on .get.

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

* fix: correct PowerShell hyphenated key lookup and regex replace count

- Self-seed now reads the defaults mapping via
  $defaults.agents.PSObject.Properties[$integrationKey].Value instead of
  member access ($defaults.agents.$integrationKey), which parsed hyphenated
  keys like 'cursor-agent'/'kiro-cli' as subtraction and failed to resolve.
- Replace the static [regex]::Replace(..., 1) call, whose trailing 1 was
  interpreted as RegexOptions.IgnoreCase rather than a replacement count, with
  an instance Regex whose Replace(input, replacement, 1) limits to the first
  match as intended.

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

* fix: make bash .mdc frontmatter guard case-insensitive

The bash updater only injected Cursor .mdc frontmatter when ctx_path ended
in lowercase '.mdc', so a mixed/upper-case extension (e.g. specify-rules.MDC)
was skipped and Cursor would not auto-load the rule file. Compare against the
casefolded path. The PowerShell variant already uses -match, which is
case-insensitive by default, so no change is needed there.

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

* docs: document separator-agnostic agent-context update invocation

The README hard-coded the dot-notation slash command
(/speckit.agent-context.update), which hyphen-separator agents like Forge and
Cline do not recognize. Document the canonical command ID plus both slash
invocations so users copy the form their agent accepts.

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

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-06-29 15:27:26 -05:00
6a3ee9b64e feat: add ZCode (Z.AI) integration (#3063)
* feat: add ZCode (Z.AI) integration

Add a skills-based integration for ZCode, Z.AI's Claude-Code-style
agent. ZCode uses the same SKILL.md layout as Claude Code, so spec-kit
installs workflows into .zcode/skills/speckit-<name>/SKILL.md, invoked
in chat as $speckit-<name>.

- ZcodeIntegration(SkillsIntegration) with .zcode/ folder and --skills option
- Register in INTEGRATION_REGISTRY
- Catalog entry (tags: cli, skills, z-ai)
- Tests via SkillsIntegrationTests mixin
- Document in integrations reference and README

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix: render $speckit-* invocations for ZCode skills

ZCode is documented as a skills agent invoked with $speckit-<command>,
but the central invocation rendering only special-cased codex, so
specify init Next Steps and extension hooks rendered the dotted
/speckit.<command> form instead.

Centralize the $speckit-* decision in a DOLLAR_SKILLS_AGENTS set with an
is_dollar_skills_agent() helper, and route both init Next Steps and
HookExecutor._render_hook_invocation through it. Add ZCode invocation
regression tests mirroring the existing Codex/Kimi coverage.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-22 12:14:18 -05:00
487af97864 feat: add specify bundle command (#3070)
* docs: dogfood Spec Kit — bundler SDD artifacts + constitution

Scaffold Spec Kit (--integration copilot) and run the full SDD workflow
against the `specify bundle` subcommand feature:

- spec.md (4 user stories, 31 FRs, 8 success criteria) + clarifications
- plan.md, research.md, data-model.md, contracts/, quickstart.md
- tasks.md (43 dependency-ordered tasks, organized by user story)
- Spec Kit Constitution v1.0.0 (code quality, testing, UX, performance,
  dependency/security principles) derived from deep codebase analysis
- plan Constitution Check + tasks grounded against the ratified principles

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

* feat(bundler): add `specify bundle` subcommand for role-based setups

Implements the Spec Kit Bundler as a `specify bundle ...` subcommand group
that calls existing primitive machinery in-process with zero new dependencies,
per the v1.0.0 constitution (Principles I-V).

Adds the `specify_cli.bundler` package (models, services, lib helpers) and the
`commands/bundle` Typer group wiring search, info, list, install, update,
remove, validate, build, init, and catalog list/add/remove (with --json and
--offline). Includes manifest/catalog schemas, version + integration-clash
gating, discovery-only refusal, idempotent install with atomic rollback,
non-collateral removal, and offline-first catalog resolution.

Ships an 82-test suite (contract/unit/integration), four sample role bundles
(product-manager, business-analyst, security-researcher, developer), README
"Bundles" docs, and an AGENTS.md pitfall on the test-venv gotcha. Marks
tasks T001-T043 complete and records follow-ups T044 (live in-process
primitive dispatch) and T045 (install from a local artifact path).

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

* docs(contributing): document running the full test suite via project .venv

Add a "Running the full test suite" subsection under Automated checks covering
`uv pip install -e ".[test]"` + `.venv/bin/python -m pytest`, with the
shared/global editable-install contamination caveat that mirrors the AGENTS.md
pitfall.

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

* feat(bundler): wire real in-process primitive install + local-artifact install

Closes the two follow-ups left after the initial bundler landing.

T044 — DefaultPrimitiveInstaller now performs real installs through existing
machinery instead of raising "use the primitive command" errors:
- presets/extensions install via their reusable managers
  (install_from_directory / install_from_zip); bundled assets install fully
  offline, catalog assets are fetched only when the network is allowed.
- workflows/steps delegate to the existing `workflow add` / `workflow step add`
  command callables in-process (project root as cwd), avoiding any duplicated
  download/validation logic (Principle I).
- `--offline` is threaded through DefaultPrimitiveInstaller(allow_network=…) so
  network-only kinds refuse with an actionable message rather than silently
  reaching out.

T045 — `specify bundle install` now accepts a local path (a built .zip
artifact, a bundle directory, or a bundle.yml) and installs directly without
consulting the catalog stack; bundle-ids still resolve via the stack.

Adds 13 tests (routing, offline gating, local-source resolution, and an
end-to-end offline build → install → list → remove of the bundled
agent-context extension). Bundler suite: 95 passing; ruff clean. Marks T044
and T045 complete in tasks.md.

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

* docs(bundler): append Phase 8 convergence tasks from converge assessment

Ran the converge command: assessed the codebase against spec.md, plan.md,
tasks.md, and the v1.0.0 constitution. Appended 7 traceable gap-closure tasks
(T046–T052) as a new "Phase 8: Convergence" section. Append-only — no existing
tasks were modified and no application code was changed.

Findings: 1 CRITICAL (Constitution III — bundle group undocumented under
docs/reference/), 3 HIGH (FR-005/SC-007 validate references; FR-009/SC-002 info
expansion; FR-012 install-time init), 3 MEDIUM (FR-013 integration precedence;
FR-020 surface overlaps; FR-028 update refresh).

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

* Implement Phase 8 convergence tasks (T046–T052)

Close the gaps the converge command found between the bundler spec/plan/
constitution and the code:

- T046: add docs/reference/bundles.md documenting the full `specify bundle`
  command group; link it from docs/reference/overview.md (Constitution III).
- T047: wire a reference checker into `bundle validate` (services/references.py);
  online runs fail and name unresolved component references, offline runs warn.
- T048: expand `bundle info` to enumerate the full component set (versions,
  preset priority/strategy) plus the bundle integration — info == install.
- T049/T050: `bundle install`/`bundle init` now scaffold an uninitialized
  project via the existing `specify init` machinery, choosing the integration by
  precedence (override → bundle-declared → Copilot + OS default script type).
- T051: surface foreseeable component overlaps during info and install.
- T052: `bundle update` refreshes already-installed components via a new
  refresh path in install_bundle, preserving primitive-level overrides.

Adds unit/contract/integration coverage (107 tests pass).

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

* converge: append Phase 9 (T053) — surface bundle trust indicator

Re-run of converge after Phase 8. The seven Phase 8 tasks are verified closed.
One residual partial gap remains: the `verified`/trust indicator (FR-010,
FR-027) is exposed only in `bundle info --json`, absent from `bundle search`
(the primary discovery surface) and `bundle info` text. Appended as a single
new task for implement to complete. Append-only; no code changed.

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

* Implement T053 — surface bundle trust indicator in discovery

`bundle search` (text + JSON) and `bundle info` (text + JSON) now expose each
catalog entry's verification/trust level (verified vs community), so users can
judge a bundle's trust before installing, per FR-010 / FR-027. Previously
`verified` was only present in `bundle info --json`.

Adds contract coverage; 108 tests pass.

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

* docs: dogfood Spec Kit — bundler SDD artifacts + constitution

Scaffold Spec Kit (--integration copilot) and run the full SDD workflow
against the `specify bundle` subcommand feature:

- spec.md (4 user stories, 31 FRs, 8 success criteria) + clarifications
- plan.md, research.md, data-model.md, contracts/, quickstart.md
- tasks.md (43 dependency-ordered tasks, organized by user story)
- Spec Kit Constitution v1.0.0 (code quality, testing, UX, performance,
  dependency/security principles) derived from deep codebase analysis
- plan Constitution Check + tasks grounded against the ratified principles

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

* feat(bundler): add `specify bundle` subcommand for role-based setups

Implements the Spec Kit Bundler as a `specify bundle ...` subcommand group
that calls existing primitive machinery in-process with zero new dependencies,
per the v1.0.0 constitution (Principles I-V).

Adds the `specify_cli.bundler` package (models, services, lib helpers) and the
`commands/bundle` Typer group wiring search, info, list, install, update,
remove, validate, build, init, and catalog list/add/remove (with --json and
--offline). Includes manifest/catalog schemas, version + integration-clash
gating, discovery-only refusal, idempotent install with atomic rollback,
non-collateral removal, and offline-first catalog resolution.

Ships an 82-test suite (contract/unit/integration), four sample role bundles
(product-manager, business-analyst, security-researcher, developer), README
"Bundles" docs, and an AGENTS.md pitfall on the test-venv gotcha. Marks
tasks T001-T043 complete and records follow-ups T044 (live in-process
primitive dispatch) and T045 (install from a local artifact path).

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

* docs(contributing): document running the full test suite via project .venv

Add a "Running the full test suite" subsection under Automated checks covering
`uv pip install -e ".[test]"` + `.venv/bin/python -m pytest`, with the
shared/global editable-install contamination caveat that mirrors the AGENTS.md
pitfall.

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

* feat(bundler): wire real in-process primitive install + local-artifact install

Closes the two follow-ups left after the initial bundler landing.

T044 — DefaultPrimitiveInstaller now performs real installs through existing
machinery instead of raising "use the primitive command" errors:
- presets/extensions install via their reusable managers
  (install_from_directory / install_from_zip); bundled assets install fully
  offline, catalog assets are fetched only when the network is allowed.
- workflows/steps delegate to the existing `workflow add` / `workflow step add`
  command callables in-process (project root as cwd), avoiding any duplicated
  download/validation logic (Principle I).
- `--offline` is threaded through DefaultPrimitiveInstaller(allow_network=…) so
  network-only kinds refuse with an actionable message rather than silently
  reaching out.

T045 — `specify bundle install` now accepts a local path (a built .zip
artifact, a bundle directory, or a bundle.yml) and installs directly without
consulting the catalog stack; bundle-ids still resolve via the stack.

Adds 13 tests (routing, offline gating, local-source resolution, and an
end-to-end offline build → install → list → remove of the bundled
agent-context extension). Bundler suite: 95 passing; ruff clean. Marks T044
and T045 complete in tasks.md.

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

* docs(bundler): append Phase 8 convergence tasks from converge assessment

Ran the converge command: assessed the codebase against spec.md, plan.md,
tasks.md, and the v1.0.0 constitution. Appended 7 traceable gap-closure tasks
(T046–T052) as a new "Phase 8: Convergence" section. Append-only — no existing
tasks were modified and no application code was changed.

Findings: 1 CRITICAL (Constitution III — bundle group undocumented under
docs/reference/), 3 HIGH (FR-005/SC-007 validate references; FR-009/SC-002 info
expansion; FR-012 install-time init), 3 MEDIUM (FR-013 integration precedence;
FR-020 surface overlaps; FR-028 update refresh).

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

* Implement Phase 8 convergence tasks (T046–T052)

Close the gaps the converge command found between the bundler spec/plan/
constitution and the code:

- T046: add docs/reference/bundles.md documenting the full `specify bundle`
  command group; link it from docs/reference/overview.md (Constitution III).
- T047: wire a reference checker into `bundle validate` (services/references.py);
  online runs fail and name unresolved component references, offline runs warn.
- T048: expand `bundle info` to enumerate the full component set (versions,
  preset priority/strategy) plus the bundle integration — info == install.
- T049/T050: `bundle install`/`bundle init` now scaffold an uninitialized
  project via the existing `specify init` machinery, choosing the integration by
  precedence (override → bundle-declared → Copilot + OS default script type).
- T051: surface foreseeable component overlaps during info and install.
- T052: `bundle update` refreshes already-installed components via a new
  refresh path in install_bundle, preserving primitive-level overrides.

Adds unit/contract/integration coverage (107 tests pass).

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

* converge: append Phase 9 (T053) — surface bundle trust indicator

Re-run of converge after Phase 8. The seven Phase 8 tasks are verified closed.
One residual partial gap remains: the `verified`/trust indicator (FR-010,
FR-027) is exposed only in `bundle info --json`, absent from `bundle search`
(the primary discovery surface) and `bundle info` text. Appended as a single
new task for implement to complete. Append-only; no code changed.

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

* Implement T053 — surface bundle trust indicator in discovery

`bundle search` (text + JSON) and `bundle info` (text + JSON) now expose each
catalog entry's verification/trust level (verified vs community), so users can
judge a bundle's trust before installing, per FR-010 / FR-027. Previously
`verified` was only present in `bundle info --json`.

Adds contract coverage; 108 tests pass.

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

* fix(bundler): address PR review — annotations, Windows paths, HTTPS, errors, reproducible builds

Resolves automated review feedback on github/spec-kit#3070:

- validator: drop redundant string-quoting on ReferenceChecker's
  `str | None` return so the annotation evaluates as a real union under
  `from __future__ import annotations`.
- adapters: normalize Windows drive-letter paths (e.g. C:\...) to the
  local-file branch so offline file catalogs resolve on Windows.
- adapters: enforce HTTPS (HTTP only for localhost) and require a host on
  remote catalog URLs before any network call, mirroring
  specify_cli.catalogs URL validation (MITM/downgrade protection).
- adapters: pass `origin` to loads_json for local files and HTTP payloads
  so JSON parse errors name the real source instead of <string>.
- manifest: parse component `priority` defensively, raising an actionable
  BundlerError on non-integer values instead of a raw ValueError.
- packager: write zip members with a fixed timestamp + permissions so
  identical inputs yield byte-for-byte identical artifacts (genuinely
  reproducible builds), and strengthen the determinism test accordingly.

Adds regression tests for priority validation, plain-HTTP/host rejection,
and byte-level artifact reproducibility (111 bundler tests pass; ruff clean).

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

* fix(bundler): address PR review round 2 — nested output dir + file:// URLs

- packager: when --output points inside the bundle directory, exclude the
  whole output subtree from collection so previously-built artifacts are
  never re-packaged (prevents broken reproducibility and unbounded growth).
- adapters: resolve file:// catalog URLs via url2pathname and preserve
  netloc, so Windows file URLs (file:///C:/...) and UNC shares
  (file://server/share) resolve correctly instead of dropping the host or
  producing /C:/x.

Adds regression tests for nested-output exclusion and file:// resolution
(113 bundler tests pass; ruff clean).

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

* fix(bundler): address PR review round 3 — discovery UX + hardening

- bundle search/info: fall back to the built-in/user catalog stack instead of
  requiring a Spec Kit project, so discovery works in a fresh directory (and
  the README/quickstart examples now match actual behavior). install still
  auto-initializes a project as before.
- packager: traverse with os.walk(followlinks=False) and prune symlinked
  directories before descending, so a symlink-to-dir can no longer pull in
  out-of-tree files (which previously turned "skip symlinks" into a hard
  ensure_within() failure and did extra filesystem work).
- records: parse contributed-component priority defensively, raising an
  actionable BundlerError on a corrupt records file instead of leaking a raw
  ValueError/traceback.
- installer: give install_bundle's manifest parameter an explicit
  BundleManifest | None type for a clearer, safer service API.

Adds regression tests for project-less search/info, symlinked-dir pruning,
and corrupt-priority records (117 bundler tests pass; ruff clean).

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

* fix(bundler): address PR review round 4 + markdownlint exclusions

Review fixes:
- bundle info: expand the manifest regardless of install policy so
  discovery-only bundles remain inspectable (only install is refused).
- _download_manifest: handle local .zip download_url by extracting bundle.yml
  (via _local_manifest_source), and add a real remote HTTPS fetch path using
  the shared authenticated, redirect-validated open_url client (HTTPS enforced
  on the initial URL and every redirect; offline still refuses).
- _run_init: thread the --offline flag through to the init callback so
  `bundle install/init --offline` never performs network init.
- conflict.ConflictReport: use field(default_factory=list) and drop the
  None + __post_init__ workaround.
- CatalogSource.from_dict: parse priority defensively, raising an actionable
  BundlerError naming the source + offending value instead of a raw ValueError.

markdownlint:
- Exclude .specify/, .github/, and specs/ (and their subdirectories) from
  markdownlint so the in-flight dogfooding scaffolding doesn't trip the linter.

Adds regression tests for discovery-only info, local-zip download_url, and
non-integer catalog priority (120 bundler tests pass; ruff clean; the PR's own
markdown lints clean).

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

* fix(bundler): address PR review round 5 + ignore generated files in whitespace check

Review fixes:
- packager: exclude any prior build artifact for this bundle (matching
  <id>-*.zip), not just the current output path, so older artifacts next to
  bundle.yml are never re-packaged.
- docs(bundles): correct the note — `search` and `info` work without a project
  (they fall back to the built-in/user catalog stack); only list/update/remove/
  catalog require an initialized project.

CI / generated files:
- .gitattributes: mark the generated dogfooding scaffolding (.specify/**, the
  speckit .github agent/prompt files, copilot-instructions.md, specs/**) with
  -whitespace so `git diff --check` (the Lint workflow's whitespace gate) stops
  flagging emitted trailing whitespace. These files are produced by
  `specify init` and are scrubbed before merge.

Adds a regression test for prior-artifact exclusion (121 bundler tests pass;
ruff clean).

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

* fix(bundler): collision-resistant catalog ids, canonical local paths, explicit uninstalled result

Addresses review round 6 (PR #3070):
- catalog_config._derive_id now combines host label with the URL path stem so
  multiple catalogs from the same host get distinct, stable default ids.
- add_source canonicalizes local file paths to absolute before persisting, so
  project config no longer depends on the caller's cwd.
- InstallResult gains a dedicated `uninstalled` list; remove_bundle no longer
  overloads `installed` for removals, and the CLI prints from `uninstalled`.

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

* fix(bundler): confine config writes, guard indeterminate integration, fix validate docs

Addresses review round 7 (PR #3070):
- save_records and catalog_config._write now pass within=project_root to
  dump_json/dump_yaml, refusing symlinked .specify paths that escape the
  project (defense-in-depth, matching the rest of the codebase).
- resolve_install_plan now fails when a bundle pins an integration but the
  project's active integration cannot be determined and no explicit
  --integration override was given, instead of silently adopting the bundle's
  required integration (FR-019 guard). CLI passes integration_explicit.
- docs/reference/bundles.md: corrected the validate semantics to describe the
  actual best-effort online behavior (unreachable catalogs warn, not fail).

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

* fix(bundler): Windows path handling + review round 8 hardening

Fix Windows CI failures:
- is_safe_relpath now rejects POSIX-absolute (/abs) and Windows drive-absolute
  (C:\x, UNC) paths on every OS, instead of passing them through on Windows
  where os.path.isabs('/abs') is False and Path('/abs').parts yields '\\'.
- _download_manifest treats a Windows drive-letter download_url (C:\bundle.yml,
  which urlparse reads as scheme 'c') as a local file, fixing the empty
  component set in `bundle info` on Windows.

Address review round 8 (PR #3070):
- Bundled workflows now install under --offline (locate via
  _locate_bundled_workflow) instead of being refused unconditionally.
- bundle update preserves the original installed_at timestamp on refresh
  (import find_record; reuse the existing record's timestamp).
- _derive_id lowercases the host label so 'Example.com' and 'example.com'
  produce the same deterministic id.
- CatalogEntry.from_dict validates 'tags' is a list and 'verified' is a real
  boolean, raising BundlerError on invalid untrusted shapes.

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

* fix(bundler): normalize SemVer prerelease spellings before version parsing

Addresses review round 9 (PR #3070): parse_version and is_semver now apply the
same prerelease normalization (mirroring specify_cli._version._normalize_tag)
so SemVer spellings like 1.2.3-rc1 / 1.2.3-alpha1 validate and compare
consistently across is_semver, parse_version, and satisfies. Leading 'v' is
also stripped. Keeps the manifest validator and constraint checks in agreement.

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

* fix(bundler): no collateral removal + enforce manifest-pinned versions

Addresses review round 10 (PR #3070):
- install_bundle records only the components this bundle actually contributed:
  freshly-installed components, plus pre-existing ones already owned by this
  bundle (refresh) or a sibling bundle (shared/refcounted). A component that is
  installed on disk but tracked by no bundle was installed independently and is
  no longer attributed, so `bundle remove` won't uninstall it (FR-022).
- preset/extension/workflow install paths now verify the active catalog's
  advertised version matches the manifest-pinned component.version before
  downloading/installing, raising BundlerError on mismatch so bundles stay
  reproducible. When a catalog advertises no version the pin can't be enforced
  and installation proceeds.

Added regression tests: independent pre-existing component survives removal;
version-mismatch refusal (helper + workflow path).

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

* feat(scripts): add SPECIFY_INIT_DIR to target a member project from the repo root (#2892)

* feat(scripts): add SPECIFY_INIT_DIR to target a member project from the repo root

Resolve an explicit SPECIFY_INIT_DIR project override once in the core
get_repo_root / Get-RepoRoot, so a non-interactive / CI caller can target a
member project (the directory containing .specify/) from a monorepo root
without cd. Strict by design: the path must exist and contain .specify/,
otherwise it hard-errors with no silent fallback.

- Single resolver in core; the git feature-branch script inherits it by
  sourcing core, with no per-extension copies.
- PS resolver verifies the resolved path is a directory (Resolve-Path also
  succeeds for files) so a file value errors as "not an existing directory".
- get_feature_paths splits decl/assignment so a SPECIFY_INIT_DIR failure
  propagates instead of being masked by `local`.
- create-new-feature-branch: when core is absent (only git-common loaded) and
  SPECIFY_INIT_DIR is set, hard-error rather than silently using the git root.
- Document SPECIFY_INIT_DIR and SPECIFY_FEATURE_DIRECTORY in the core reference.
- Tests for valid/relative/trailing-slash/file/missing/no-.specify targets,
  feature-axis composition, the no-core guard, and a PowerShell mirror.

* fix: guard SPECIFY_INIT_DIR with stale core scripts

* docs: clarify SPECIFY_FEATURE_DIRECTORY precedence wording

* fix: normalize trailing slash in PowerShell SPECIFY_INIT_DIR resolver

Resolve-Path preserves a trailing separator from its input, so a
SPECIFY_INIT_DIR ending in a slash returned a root that didn't match the
bash resolver (whose `cd && pwd` strips it). That broke
test_ps_trailing_slash_tolerated on the CI runners, which do have pwsh.
Trim it with TrimEndingDirectorySeparator (no-op on a bare root or a path
with no trailing separator).

Also fix the misleading test comment: the PowerShell mirror runs on the
CI ubuntu/windows runners (they ship pwsh), it is not skipped there.

* test: normalize bash path expectations on Windows

* docs: clarify SPECIFY_INIT_DIR root helpers

* chore: sync dogfooded .specify core scripts with SPECIFY_INIT_DIR

Mirror the SPECIFY_INIT_DIR resolver (resolve_specify_init_dir in
common.sh) into the committed dogfooding .specify/scripts/bash copies so
the git extension's create-new-feature-branch.sh finds an up-to-date
common.sh instead of failing with "requires updated Spec Kit core
scripts". Fixes the test_init_dir.py CI failures.

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

* fix(bundler): harden remote catalog fetch and config parsing

- adapters: route catalog HTTP fetches through the shared authenticated
  client (authentication.http.open_url) so auth.json tokens apply and the
  Authorization header is stripped on cross-host/downgrade redirects.
  Reject any redirect that leaves HTTPS via a redirect_validator and
  re-validate the final URL after redirects, closing the urlopen
  auto-redirect MITM/downgrade gap.
- catalog_config._read: raise an actionable BundlerError when the config
  top level is not a mapping, 'catalogs' is not a list, or an entry is
  not a mapping, instead of letting list(<str>) produce a downstream
  AttributeError.

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

* fix(bundler): tighten record read confinement, policy gate, and precedence

Addresses review 4534504799:

- records.load_records: confine the read via ensure_within(project_root,
  ...) so a symlinked/traversal-escaping .specify cannot read arbitrary
  files outside the project (matches the write path's within= guard).
- catalog_config._slug: lowercase so derived catalog ids are
  deterministic across platforms and case-variant duplicates can't slip
  past the case-sensitive dup check.
- installer.install_bundle: reword the docstring's misleading "atomic on
  failure" claim to describe the real scoped guarantee (record written
  only on full success; rollback limited to newly-installed components).
- bundle update: enforce the source install_policy like install, refusing
  to update from a discovery-only source (FR-025).
- catalog source precedence: the CLI now passes ~/.specify as the user
  config dir so project > user > built-in precedence is actually
  reachable (previously the user scope was silently ignored).
- .gitattributes: scope the specs whitespace exemption to the generated
  dogfooding feature dir (specs/001-spec-kit-bundler/**) instead of all
  of specs/**.

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

* fix(bundler): no collateral refresh, catalog id integrity, loud info

Addresses review 4534571362:

- installer: in refresh mode (bundle update) only re-apply already-
  installed components that this bundle (or a sibling) owns. Components
  installed independently and tracked by no bundle are now skipped, never
  refreshed, so update cannot make collateral changes (FR-022).
- catalog.load_catalog_payload: validate each entry's own id is present
  and matches its enclosing bundles key, rejecting catalogs that would
  otherwise list a spoofed or unresolvable id.
- bundle info: stop swallowing manifest download failures. If the
  manifest can't be resolved (e.g. --offline against an https download_url
  or a download failure), surface the error and exit non-zero instead of
  silently degrading to catalog `provides` counts, preserving the "info
  == what install applies" guarantee.

Added regressions: refresh leaves independently-installed components
untouched, catalog id key/field mismatch + missing id rejection, and
info exits non-zero when the manifest is unresolvable offline.

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

* fix(bundler): confine catalog-config and integration-marker reads

Addresses review 4534716790: two more state reads bypassed the
symlink/path-escape confinement that records and the write paths already
enforce.

- catalog_config._read: validate the config path with
  ensure_within(project_root, ...) before exists()/read, so a symlinked
  .specify resolving outside project_root is rejected instead of read.
- lib.project.active_integration: confine the .specify/integration.json
  read the same way; an out-of-tree escape is treated as "not
  determinable" (returns None) rather than followed.

Added regressions covering both via a symlinked .specify pointing
outside the project root.

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

* fix(bundler): validate manifest tags, disambiguate derived ids by full host

Addresses review 4534768419:

- manifest.from_dict: reject a non-list `tags` (e.g. a bare string) instead
  of splitting it character-by-character, matching the catalog parser and
  the schema contract (tags = list of strings).
- catalog_config._derive_id: derive ids from the full host (TLD included)
  so example.com and example.net no longer collide on the same id. Updated
  the affected id assertions.
- CHANGELOG: call out the new `specify bundle` command group in the
  unreleased section (the PR's headline user-facing feature).
- .gitattributes: clarify the specs whitespace exemption — the dogfooding
  feature dir is scrubbed before merge (not retained), so it doesn't weaken
  checks for kept docs.

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

* chore(gitattributes): retain whitespace exemption for constitution.md

The project constitution (.specify/memory/constitution.md) is the one
dogfooding artifact carried forward past the pre-merge scrub. Give it its
own standalone whitespace exemption so it survives removal of the broader
.specify/** generated-scaffolding exemption.

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

* fix(bundler): accurate uninstall count, confine catalog read, safe bundle id

Addresses review 4534812056:

- installer.remove_bundle: only count a component as uninstalled when
  installer.remove() actually ran; components already absent on disk are
  reported as skipped, keeping the uninstalled count accurate.
- catalog.load_source_stack: confine the project-scoped .specify config read
  with ensure_within, so a symlinked .specify/ resolving outside the project
  root is refused (consistent with the bundler's other guarded reads).
- manifest: enforce a filesystem-safe slug for bundle.id in structural
  validation; packager.build_bundle adds an ensure_within defense-in-depth
  check so a crafted id can never push the artifact outside the output dir.

Also reverts the CHANGELOG entry (the changelog is updated separately).

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

* fix(bundler): validate requires/provides shapes in manifest and catalog

Addresses review 4534855443:

- manifest: validate requires.tools and requires.mcp as list-of-strings via
  a shared _parse_str_list helper (also reused for tags), so a bare string
  like `tools: docker` is rejected with an actionable BundlerError instead of
  being split character-by-character.
- catalog.CatalogEntry.from_dict: validate that `requires` and `provides` are
  mappings before accessing them, so an untrusted catalog payload with
  `requires: "..."` raises a named BundlerError rather than escaping as a raw
  AttributeError traceback.

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

* fix(bundler): require README.md when building a bundle artifact

Addresses review 4534938014: build_bundle now fails early with an
actionable error when README.md is missing, matching the documented
artifact contract (manifest + README) instead of silently producing a
bundle with no human-facing description.

Also reverts CHANGELOG.md to the upstream/main copy.

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

* fix(bundler): validate record shapes; drop stale install --refresh claim

Addresses review 4534969692:

- records.InstalledBundleRecord.from_dict: hard-error when
  contributed_components is not a list, instead of iterating a corrupt
  bare string character-by-character.
- records.load_records: validate the top-level 'bundles' field is a list and
  fail with a clear BundlerError when a corrupt file makes it a mapping/string.
- PR description: remove the inaccurate "supports --refresh" note from
  `bundle install` (refresh is the `bundle update` path); docs already omit it.

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

* fix(bundler): refuse symlinked .specify, reject bad url schemes, IPv6 ids

Addresses review 4534997724:

- lib.project.find_project_root: a symlinked .specify is no longer accepted
  as a project root (is_dir() follows symlinks), matching the confinement the
  rest of the CLI applies and avoiding confusing downstream failures.
- catalog_config.add_source: reject unsupported url schemes (ssh://, ftp://,
  ...) up front instead of silently treating them as local paths; local paths
  containing ':' but not '://' are still allowed.
- catalog_config._derive_id: derive the host via urlparse().hostname so IPv6
  literals, credentials, and ports no longer corrupt the derived id.

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

* fix(bundler): strict semver, narrow artifact skip, preserve priority 0

Addresses review 4535084048:

- versioning.is_semver: enforce a full MAJOR.MINOR.PATCH SemVer (with optional
  pre-release/build) via a dedicated regex, instead of accepting any
  packaging.version.Version-parseable string (e.g. "1", "1.0"). This makes
  BundleManifest.structural_errors() reject non-semver versions.
- packager: narrow the prior-artifact skip pattern to semver-named zips
  (<id>-<x.y.z>.zip) so legitimate assets like <id>-assets.zip are still
  packaged.
- primitives (preset + extension install): use an explicit `is None` check so
  an intentional priority of 0 is preserved instead of being replaced by the
  default.

Adds regressions: non-semver rejection ("1"/"1.0"/"1.2.3.4"), asset-not-
excluded vs semver-artifact-excluded, and priority-0 pass-through.

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

* fix(bundler): artifact regex for prerelease+build; clarify integration/priority docs

Addresses review 4535132279:

- packager: the prior-artifact skip regex now matches semver names carrying
  both a prerelease and build-metadata segment (e.g. 1.0.0-rc1+build5), so such
  an existing artifact is excluded rather than re-packaged — keeping builds
  bounded/deterministic, consistent with is_semver().
- docs/reference/bundles.md: correct the install integration wording.
  --integration selects the integration when initializing a new project and
  confirms the target when a pinned bundle's active integration can't be
  determined; it does NOT override a bundle that targets a specific integration
  (a mismatch aborts with no changes).
- examples/security-researcher README: reword the preset priority note in terms
  of the numeric comparison (ascending priority order) to avoid inverting the
  meaning.

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

* fix(bundler): --integration can't bypass clash guard; honest rollback docs

Addresses review 4535159341:

- bundle install: for an already-initialized project, the project's recorded
  active integration is now authoritative. --integration no longer overrides it
  (which let a copilot project install a claude-pinned bundle via
  `--integration claude`, bypassing the FR-019 clash guard). The override still
  selects the integration at init time and confirms the target only when the
  active integration cannot be determined.
- docs/reference/bundles.md: reword the install guarantee to match the
  implementation — no provenance record is written unless the install fully
  succeeds, and rollback of this run's components is best-effort (removal errors
  are swallowed, so partial on-disk state may remain). Dropped the inaccurate
  "atomic / rolls back everything" claim.

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

* fix(bundler): validate component kind/id when loading records

Addresses review 4535194606: _component_from_dict now rejects a contributed
component whose 'kind' is not a supported component kind or whose 'id' is
empty, raising a BundlerError that explicitly flags the records file as
corrupt. Previously such a record loaded successfully and only failed later
(e.g. in primitive_manager() during bundle remove/update) with a less
actionable error.

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

* fix(bundler): address review 4535234003 (7 findings)

- versioning: tolerate an uppercase `V` prefix in `_normalize_semver` and
  `is_semver`, mirroring specify_cli._version tag normalization (V -> v) so
  `V1.2.3` parses and validates consistently.
- validator: import BundlerError and narrow the speckit_version constraint
  except clause to `BundlerError` only, so programming errors are no longer
  masked behind an "invalid constraint" message.
- bundle update: accept `--integration` and thread it through
  resolve_install_plan the same way `bundle install` does (override used only
  when the active integration can't be auto-detected), so integration-pinned
  bundles can be updated where `.specify/integration.json` is missing/unreadable.
- bundle validate: fold reference warnings into `report.warnings` so the
  ValidationReport is the single warning channel at the CLI layer.

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

* test(bundler): make update --integration help assertion ANSI-safe

Rich can split the "--integration" option label with ANSI escape codes
between the two leading dashes, so the literal substring check failed under
CI's terminal settings. Match the un-split option word instead, mirroring how
test_bundle_help_lists_all_commands checks bare command names.

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

* fix(bundler): preserve exec bits in artifacts; document install-time pins

Addresses review 4535280786:

- packager.build_bundle: no longer forces every ZIP member to 0644, which
  stripped the executable bit from bundled scripts (e.g. extension hook
  scripts) and could break them after extraction. Permissions are now
  normalized reproducibly to 0755 when the source file has any execute bit
  set, otherwise 0644 — identical inputs still yield byte-for-byte identical
  artifacts.
- installer.install_bundle + docs/reference/bundles.md: document that version
  pins are enforced install-time only. Because primitive is_installed checks
  are id-based (not version-aware), an already-present component is skipped
  during install without comparing its on-disk version to the manifest pin;
  pins are guaranteed applied only on a real install or `bundle update` refresh.

Added a regression asserting executable sources map to 0755 and plain files to
0644 in the built artifact.

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

* test(bundler): skip exec-bit packager test on Windows

Windows filesystems do not carry Unix execute bits, so chmod(0o755) is a no-op
and the source file reports no execute bit — the packager then correctly stores
the member as 0644. The assertion that an executable source maps to 0755 is only
meaningful on POSIX, so skip it on nt rather than asserting platform-specific
behavior.

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

* fix(bundler): normalize prerelease spellings inside version constraints

Addresses review 4535327154: parse_version() normalized SemVer prerelease
spellings (e.g. 1.2.3-rc1 -> 1.2.3rc1) but parse_constraint() passed the
constraint to packaging.SpecifierSet unmodified, so ">=1.2.3-rc1" raised
InvalidSpecifier even though the same spelling is accepted for installed
versions. parse_constraint() now normalizes the version portion of each
comma-separated clause via the shared _normalize_semver helper, so prerelease
handling is consistent across versions and constraints.

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

* fix(bundler): validate schema versions and required record identity fields

Addresses review 4535351596:

- records.load_records: validate the on-disk 'schema_version' (required;
  forward-compatible across same-major minor bumps) and fail fast with an
  actionable error on a missing/unknown version, rather than silently parsing a
  possibly-incompatible format and risking incorrect bundle attribution/removal.
- records.InstalledBundleRecord.from_dict: treat missing 'bundle_id' or
  'version' as corruption and raise BundlerError, instead of coercing them to
  empty strings that let later list/remove/update operations behave
  unpredictably.
- catalog_config._read: validate 'schema_version' when present (same-major
  compatibility) and fail fast on an unsupported version so an incompatible
  future config shape can't be mis-parsed into a wrong effective catalog stack.

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

* chore(bundler): scrub generated dogfooding scaffold before merge

The bundler feature was developed by dogfooding Spec Kit on itself. Now that
the work is complete, remove all generated scaffolding so it does not land in
the repository on merge:

- specs/001-spec-kit-bundler/** (spec, plan, research, data-model, contracts,
  quickstart, tasks, checklists)
- .specify/** (extensions, integrations, scripts, templates, workflows,
  feature/init/integration metadata)
- .github/agents/speckit.*.agent.md, .github/prompts/speckit.*.prompt.md, and
  .github/copilot-instructions.md (Copilot integration scaffold)

Retained: .specify/memory/constitution.md — the single dogfooding artifact
carried forward — with its whitespace exemption in .gitattributes.

.gitattributes and .markdownlint-cli2.jsonc are reverted to the upstream
baseline (plus the constitution whitespace exemption), dropping the now-moot
exemptions for the removed scaffold.

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

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Pascal THUET <pascal.thuet@arte.tv>
2026-06-19 17:07:20 -05:00
0c29d890ab feat: add /speckit.converge command (#3001)
* Add /speckit.converge SDD artifacts and project scaffolding

Dogfood the converge feature through Spec Kit's own workflow:

- spec.md, plan.md, tasks.md, research, data-model, contracts, quickstart
- requirements checklist for the feature
- ratified constitution v1.0.0 (.specify/memory)
- Specify project scaffolding (.specify/, .github agent + prompt files)

Defines a built-in /speckit.converge command that assesses spec/plan/tasks
against the codebase and appends remaining work as new tasks (no git, no
change tracking, append-only). Implementation not yet started.

Excludes unrelated working-tree changes to agents.py, extensions.py,
test_extensions.py, catalog.community.json, and README.md.

* Implement /speckit.converge command

Add the built-in converge command that assesses the codebase against a
feature's spec.md, plan.md, and tasks.md and appends remaining unbuilt work
as new traceable tasks to tasks.md (append-only; no git, no change tracking).

- templates/commands/converge.md: full command body (load artifacts, assess
  code, classify findings missing/partial/contradicts/unrequested, append
  '## Phase N — Convergence' tasks with source-ref + gap-type, read-only
  guardrails, converged branch, handoff, before/after_converge hooks)
- Register converge as a core command across all enumeration sites
  (SKILL_DESCRIPTIONS, _FALLBACK_CORE_COMMAND_NAMES, ARGUMENT_HINTS, and the
  integration test command lists incl. copilot/generic file inventories)
- init.py Next Steps panel + README Core Commands table
- tasks.md: T001-T024 complete (T025 manual quickstart pending)

Full suite green: 2343 passed.

* Record quickstart validation results for /speckit.converge (T025)

All six quickstart scenarios validated (GitHub Copilot agent, macOS/zsh):
S1 gap->appended traceable task, S2 implement+re-converge, S3 converged leaves
tasks.md unchanged, S4 read-only boundaries, S5 missing-prereq stop, S6 cross-
integration install (copilot + windsurf). Automated suite: 2343 passed.

* Record 2026-06-16 re-verification results for /speckit.converge (T025)

* 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>

* Fix integration upgrade deleting settings.json and dropping script +x

Two upgrade-path bugs surfaced during converge E2E validation:

- copilot upgrade stale-deleted .vscode/settings.json because setup() only tracks the file when it creates it; on upgrade the pre-existing file is merged and left untracked, so Phase 2 stale cleanup removed it. Add an integration-level stale_cleanup_exclusions() hook (CopilotIntegration returns {.vscode/settings.json}) and subtract it from stale_keys.

- shared .specify/scripts/*.sh lost their execute bit because the managed refresh rewrites them with the bundled source mode (often 0o644) and nothing restored perms. Call ensure_executable_scripts() after the managed-refresh block (POSIX only).

Add regression tests in TestIntegrationUpgrade covering both fixes (validated to fail without the fixes).

* fix: resolve markdownlint errors in PR files

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

* chore: clean up runtime state files from PR

Remove .specify state files that are per-project runtime artifacts:
- feature.json, init-options.json, integration.json
- manifest files, extension registry, bug artifacts

These are generated by 'specify init' and should not be committed.

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

* feat: fold converge artifacts from #3003 and #3005

- Add speckit.converge Copilot agent and prompt files (#3003)
- Add regression test for Claude argument hints (#3005)
- Remove invalid converge entry from Claude argument hints
- Fix documentation removing branch-prefix fallback claims

Supersedes: #3003, #3005

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

* chore: remove non-converge specify scaffolding from PR

Remove .specify/ artifacts, non-converge .github/agents and prompts,
and copilot-instructions.md that were generated by 'specify init'
and are not part of the converge command feature.

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

* chore: remove SDD spec artifacts from PR

Remove specs/001-converge-command/ — the spec/plan/tasks/research SDD
artifacts produced while building this feature. spec-kit does not track
a specs/ directory on main (those are outputs of running the workflow on
the repo, not part of the shipped tool).

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

* chore: remove generated Copilot converge command files

Remove .github/agents/speckit.converge.agent.md and
.github/prompts/speckit.converge.prompt.md — these are generated by
'specify init --integration copilot' from templates/commands/converge.md
(all __SPECKIT_COMMAND_*__/{SCRIPT} tokens are resolved). main tracks no
.github/agents or .github/prompts files; the template is the source of truth.

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

* chore: split out unrelated integration-upgrade fix

Move the stale_cleanup_exclusions / executable-bit upgrade fix
(base.py, copilot, _migrate_commands.py, test_integration_subcommand.py)
out of this PR into its own change. This PR is now scoped purely to the
/speckit.converge command.

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

* fix: add converge to core command template ordering

converge is a core command in SKILL_DESCRIPTIONS but was missing from
_CORE_COMMAND_TEMPLATE_ORDER, so it sorted with the fallback rank. Add it
after 'implement' to keep core-command ordering consistent across
integrations.

Addresses review feedback on #3001.

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

* docs: make converge findings example neutral

Replace the self-referential sample evidence text in the Convergence
Findings table with a neutral placeholder so agents are less likely to copy
nonsensical template-specific findings into real output.

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>

* docs: clarify converge scope and hook outcome wording

- Remove FR-specific parenthetical from code-scope rule so it doesn't imply
  a hard FR-001 reference exists in every feature
- Replace unsupported 'pass outcome to hook context' instruction with explicit
  in-session outcome reporting before hook listing

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

* docs: align converge task example with tasks format

Use  (no colon) in the convergence task example so it
matches tasks-template formatting and downstream expectations.

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

* Clarification of usage

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>

* docs: align converge phase/task-id format with tasks template

- Use  (colon) for consistency with tasks template
- Clarify appended task IDs must be zero-padded ( style)
- Update checklist example to a concrete zero-padded ID ()

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

* docs: standardize converge phase heading format

Use  consistently in converge.md (including the
append-only contract section) to match Step 7 and tasks template style.

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

---------

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-06-17 14:47:00 -05:00
75aee19c6e fix: disable Rich Live transient mode on Windows to prevent PS 5.1 hang (#2938)
* fix: disable Rich Live transient mode on Windows to prevent PS 5.1 hang

PowerShell 5.1's legacy console host does not reliably support VT escape
sequences. Rich's Live(transient=True) attempts cursor restoration on
context exit, which hangs indefinitely on that console.

Set transient=False when sys.platform == 'win32' in both init.py (progress
tracker) and _console.py (select_with_arrows). The only cosmetic effect is
that progress output remains visible after completion on Windows.

Fixes #2927

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

* test: address review feedback on test quality

- Use captured['transient'] instead of .get() for clearer KeyError on failure
- Source guards now assert both the platform check AND transient=_transient usage
- Remove unused imports (MagicMock retained as it's used, removed pytest)

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

* test: use regex in source guards for resilience to formatting changes

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

* test: use single DOTALL regex to verify assignment flows into Live()

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

* fix: skip duplicate tracker print on Windows when transient=False

When transient is False, Rich leaves the Live output on screen. The
subsequent console.print(tracker.render()) would duplicate it. Gate
it behind _transient so Windows users see the tracker exactly once.

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

---------

Co-authored-by: Manfred Riem <mnriem@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-06-17 06:48:42 -05:00
1150d32aee Add Zed integration (#2780)
* feat: add Zed integration

* fix: update integrations stats grid to 31 for consistency

* fix: address Copilot review feedback

- Remove non-actionable --skills flag from ZedIntegration (Zed is always
  skills-based, like Agy)
- Align zed_skill_mode predicate with ai_skills for consistency across
  init output and hook rendering
- Consolidate claude/cursor/zed slash-skill return blocks in
  _render_hook_invocation to reduce duplication
- Override test_options_include_skills_flag for Zed (no --skills flag)

* Potential fix for pull request finding

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

* fix: address Copilot review round 2

- Make zed_skill_mode unconditional in hook rendering (Zed is always
  skills-based, no --skills option)
- Add test_init_persists_ai_skills_for_zed that exercises the actual
  CLI init path and verifies HookExecutor renders /speckit-plan
  without manual init-options manipulation

* Potential fix for pull request finding

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

* fix: address copilot review feedback for zed integration

- Update integration count from 31 to 33 in docs/index.md (32 integrations + Generic)
- Make zed_skill_mode unconditional to match extensions.py behavior
- Consolidate slash-skill integrations into a set for consistency
- Move os import to module level in test_integration_zed.py

* fix: refine slash-skill logic and ai-skills validation

- Fix slash-skill integrations: Claude/Cursor require ai_skills=true; Zed/Agy/Devin are always skills
- Allow --ai-skills with --integration (not just --ai) to fix validation error

* fix: remove unused variables and update ai-skills help text

- Add agy_skill_mode and devin_skill_mode variables to fix F841 lint error
- Use all skill mode variables in the slash-skill conditional check
- Update --ai-skills help text to reflect it works with --integration too

* fix: add trae_skill_mode to hook invocation for consistency

Trae is a SkillsIntegration like Zed/Agy/Devin, so it should also be treated
as always-skills-based in hook invocation rendering.

* fix: make Agy always skills-based for consistency

AgyIntegration is a SkillsIntegration subclass with no --skills option,
so it should be treated as always skills-based (like Zed, Devin, Trae).
This aligns init.py skill mode detection with extensions.py hook rendering.

* fix: gate agy_skill_mode and refactor _render_hook_invocation to use sets

Addressed Copilot review comments:

- Restored _is_skills_integration guard on agy_skill_mode in init.py
  to be defensive about runtime integration type.
- Refactored _render_hook_invocation() in extensions.py to use
  always_slash/conditional_slash frozensets instead of individual
  per-agent booleans, eliminating unused variables (F841) and making
  it harder for conditions to drift between integrations.
- Centralized slash-skill determination so adding a new unconditional
  slash-skill integration is a one-key addition.

* fix: address latest Copilot review comments

- Added copilot to CONDITIONAL_SLASH_AGENTS for consistent
  hook invocation rendering with init.py
- Moved always_slash/conditional_slash frozensets to module
  scope to avoid per-call reallocation
- Replaced manual os.chdir() with monkeypatch.chdir() in test
- Overrode test_options_include_skills_flag for Zed (no --skills)

* fix: address latest Copilot review comments

- Removed redundant local import yaml in _register_extension_skills
  (yaml is already imported at module scope)
- Split --ai-skills usage hint into two separate print statements
  for better readability
- Changed integrations count from '33' to '30+' to avoid future drift

* fix: re-add _is_skills_integration definition lost in merge

The _is_skills_integration variable was accidentally dropped during the
web UI merge resolution of upstream/main's removal of legacy --ai flags.
Re-added the definition via isinstance(resolved_integration, SkillsIntegration)
check so that skill-mode booleans work correctly.

* fix: gate zed_skill_mode on _is_skills_integration for consistency

Aligns zed_skill_mode with the other skills-based agents (codex, claude,
cursor-agent, copilot) which all use _is_skills_integration gating.
Since ZedIntegration extends SkillsIntegration, behavior is unchanged.

* fix: remove unused claude_skill_mode and cursor_skill_mode locals in _render_hook_invocation

These variables became unused after the refactor to ALWAYS_SLASH_AGENTS /
CONDITIONAL_SLASH_AGENTS sets. Claude and Cursor-Agent are now handled by the
CONDITIONAL_SLASH_AGENTS path, so the separate boolean locals are dead code.

Fixes ruff F841 and addresses Copilot review feedback that was repeated across
multiple review rounds.

* fix: align agy/trae invocation format in init next-steps with hook rendering and build_command_invocation

- Moved agy and trae from '-<name>' (dollar/Codex format) to
  '/speckit-<name>' (slash format) in _display_cmd() to match:
  - HookExecutor._render_hook_invocation() (ALWAYS_SLASH_AGENTS for trae,
    CONDITIONAL_SLASH_AGENTS for agy)
  - SkillsIntegration.build_command_invocation() (default: /speckit-<name>)
- The '$' prefix is specific to Codex; all other skills agents use '/'.

* fix: address Copilot review comments on hook invocation consistency

- Add is_slash_skills_agent() helper to extensions.py to centralize the
  agent-to-invocation-format mapping, reducing drift risk between
  HookExecutor._render_hook_invocation() and init.py _display_cmd()
- Use the shared helper in both locations; init.py now imports and
  delegates to is_slash_skills_agent() instead of maintaining its own
  per-agent boolean matrix
- Fix test_hooks_render_skill_invocation to use ai_skills=False,
  proving Zed renders /speckit-<name> unconditionally
- Add parameterized TestSlashSkillsSets covering all agents in
  ALWAYS_SLASH_AGENTS and CONDITIONAL_SLASH_AGENTS with ai_skills
  both true and false

* fix: address Copilot review comments on type safety and test API

- Make is_slash_skills_agent() accept str | None to match its call sites
  (init_options.get("ai") can return None)
- Refactor TestSlashSkillsSets to use public execute_hook() API instead of
  private _render_hook_invocation() method

* fix: address Copilot review comments on typing and naming clarity

- Add from __future__ import annotations to extensions.py so PEP 604
  unions (str | None) are safe regardless of Python version
- Add clarifying _ai_skills_enabled local variable in init.py's
  _display_cmd() to make the semantic meaning explicit when passing it
  to is_slash_skills_agent()

* fix: move invocation-style logic into shared _invocation_style module

- Extract ALWAYS_SLASH_AGENTS, CONDITIONAL_SLASH_AGENTS, and
  is_slash_skills_agent() from extensions.py into new _invocation_style.py
  module, eliminating the awkward init.py -> extensions.py import
  dependency for invocation-style decision logic
- Both HookExecutor._render_hook_invocation() and init.py _display_cmd()
  now import from the shared module instead of one subsystem importing
  from the other
- Revert /SKILL.md change: the leading slash is semantically significant
  (path component vs filename suffix)

* fix: add None guard before i.options() in test_options_include_skills_flag

get_integration() returns IntegrationBase | None, so i.options()
is a type error without a None check.

* fix: override test_options_include_skills_flag for Zed (always skills, no --skills flag)

Zed is always skills-based and doesn't expose a --skills option.
Override the inherited base test to assert --skills is absent.

* fix: rename test and skip inherited test_options_include_skills_flag for Zed

- Skip inherited test_options_include_skills_flag (not applicable — Zed
  is always skills-based with no --skills flag)
- Add test_options_do_not_include_skills_flag with correct name matching
  the assertion (--skills is absent)

* fix: add defensive non-string check in is_slash_skills_agent

Reject non-string values for selected_ai to prevent TypeError from
set membership checks when persisted init-options contain corrupted
data (e.g. list or dict instead of string).

---------

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
2026-06-16 17:29:08 -05:00
927f54feea feat: make git extension opt-in and remove --no-git at v0.10.0 (#2873)
* feat(init)!: make git extension opt-in and remove --no-git at v0.10.0

- Remove --no-git parameter from specify init command
- Remove git extension auto-installation from init flow
- Git repository initialization (git init) still runs when git is available
- Remove --no-git from all test invocations across the test suite
- Update docs to reflect opt-in git extension behavior
- Replace TestGitExtensionAutoInstall with TestGitExtensionOptIn tests

BREAKING CHANGE: specify init no longer auto-installs the git extension.
Use `specify extension add git` to install it explicitly.
The --no-git flag has been removed.

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

* refactor(scripts): remove git operations from core scripts

Git functionality is now entirely managed by the git extension.
Core scripts only handle directory-based feature creation and numbering.

- Remove has_git(), check_feature_branch(), git branch creation from core
- Simplify number detection to use only spec directory scanning
- Remove HAS_GIT output from get_feature_paths()
- Remove git remote fetching and branch querying
- Keep BRANCH_NAME output key for backward compatibility

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

* refactor: remove all git operations from core

- Remove is_git_repo() and init_git_repo() dead code from _utils.py
- Remove --branch-numbering from init command
- Remove git from 'specify check' (now extension-only)
- Update docs: git is optional prerequisite, check command description
- Fix tests to reflect no-git-in-core reality (fallback to main)

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

* refactor(scripts): remove directory scanning and branch fallback from core

Core scripts now resolve feature context exclusively from:
1. SPECIFY_FEATURE env var (set by git extension)
2. .specify/feature.json (persisted by specify command)

Removed find_feature_dir_by_prefix() and directory scanning heuristics —
these are the git extension's responsibility. Scripts error clearly when
no feature context is available.

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

* feat: introduce feature_numbering, deprecate branch_numbering in init-options

- specify command template now reads feature_numbering (preferred) with
  fallback to branch_numbering (deprecated) from init-options.json
- Git extension reads git-config.yml > feature_numbering > branch_numbering
- init now writes feature_numbering: sequential to init-options.json
- Deprecation warning emitted when branch_numbering is used as fallback

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

* fix: remove trailing whitespace in common.ps1

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

* feat(scripts): persist SPECIFY_FEATURE_DIRECTORY env var to feature.json

When SPECIFY_FEATURE_DIRECTORY is set, get_feature_paths() now writes the
value to .specify/feature.json so future sessions without the env var can
still resolve the feature directory. The write is idempotent — it skips
when the file already contains the same value.

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

* fix: address review feedback — error messages and docs

- Update error messages in common.sh and common.ps1 to reference
  SPECIFY_FEATURE_DIRECTORY instead of SPECIFY_FEATURE (which no longer
  resolves feature directories)
- Fix get_current_branch comment (returns empty string, not error)
- Update upgrade.md to reference SPECIFY_FEATURE_DIRECTORY with correct
  example paths
- Update local-development.md troubleshooting: replace stale 'Git step
  skipped' row with actionable git extension guidance

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

* fix(scripts): harden feature.json persistence

- Use json_escape in printf fallback when jq is unavailable (common.sh)
- Replace utf8NoBOM encoding with UTF8Encoding($false) for PowerShell
  5.1 compatibility (common.ps1)

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

* refactor(scripts): remove dead feature_json_matches_feature_dir functions

These guards are no longer needed since the branch-name validation they
protected against has been removed from check-prerequisites.

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

* refactor(git-ext): rename create-new-feature to create-new-feature-branch

The git extension's script only creates the git branch — rename it to
reflect that responsibility. The core create-new-feature.sh/.ps1 handles
feature directory creation and feature.json persistence.

Also includes fixes from review feedback:
- common.sh: _persist_feature_json uses json_escape fallback
- common.ps1: Save-FeatureJson uses UTF8Encoding for PS 5.1 compat
- common.ps1: case-sensitive path stripping on non-Windows
- create-new-feature.sh/ps1: output both SPECIFY_FEATURE and
  SPECIFY_FEATURE_DIRECTORY
- setup-tasks.sh: fix stale 'Validate branch' comment

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

* fix(tests): update references to renamed git extension scripts

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

* fix(tests): remove duplicate EXT_CREATE_FEATURE assignments

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

---------

Co-authored-by: Manfred Riem <mnriem@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-06-09 06:13:07 -05:00
CopilotGitHubCopilotcopilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>Manfred Riem
7106858c4e feat!: remove legacy --ai, --ai-commands-dir, and --ai-skills flags (0.10.0) (#2872)
* Initial plan

* feat!: remove legacy --ai, --ai-commands-dir, and --ai-skills flags at 0.10.0

* refactor(tests): rename stale test_ai_help_* methods to test_agent_config_*

* fix: address review — derive agent folder for generic integration and remove redundant test

- Security notice now falls back to integration_parsed_options['commands_dir']
  when AGENT_CONFIG folder is None (generic integration).
- Remove test_agent_config_includes_kiro_cli which duplicates the assertion
  in test_runtime_config_uses_kiro_cli_and_removes_q.

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

* docs: scrub all remaining --ai flag references from source and tests

- Remove dead AI_ASSISTANT_ALIASES, AI_ASSISTANT_HELP, and
  _build_ai_assistant_help() from _agent_config.py
- Update comments/docstrings in extensions.py, presets.py, and
  integration subpackages to reference 'skills mode' or
  '--integration' instead of the removed flags
- Fix catalog.json generic integration description
- Update test docstrings/comments in test_extension_skills.py,
  test_extensions.py, and test_presets.py

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

* test: remove legacy --ai flag rejection tests

The flags are fully removed from the CLI; typer handles unknown options
generically. No custom rejection logic exists to test.

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

* revert: remove manual CHANGELOG.md entry

CHANGELOG is generated automatically; manual edits should not be made.

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

* fix: make generic catalog description self-explanatory

Include the required --commands-dir sub-option in the description so
readers don't need to look up integration docs.

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

* fix(tests): rename duplicate test classes to avoid shadowing

The rename from Test*AutoPromote to Test*Integration collided with the
existing Test*Integration(SkillsIntegrationTests) base classes, causing
the shared test suites to be silently overwritten. Rename the CLI init
flow classes to Test*InitFlow instead.

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

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: Manfred Riem <mnriem@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-06-05 14:56:28 -05:00
ee17b04784 refactor(integrations): co-locate integration commands in integrations/ domain dir (PR-5/8) (#2720)
* refactor(integrations): co-locate integration commands in integrations/ domain dir

- Remove commands/ stubs (handlers will live in domain dirs)
- Move all integration CLI handlers out of __init__.py into integrations/
- Split into focused modules under integrations/:
    _helpers.py           (340 lines) — domain helpers
    _install_commands.py  (306 lines) — install / uninstall
    _migrate_commands.py  (487 lines) — switch / upgrade
    _query_commands.py    (442 lines) — list / use / search / info / catalog
    _commands.py           (34 lines) — app objects + register()
- __init__.py reduced by ~1400 lines; integration block replaced with register() call
- Fix patch paths in tests to new module locations

* fix(integrations): restore original integration list output in refactor

Preserve the CLI Required column, post-table default/installed summary,
and no-installed guidance that were dropped during the no-behavior-change
refactor of integration list into _query_commands.py.

* Potential fix for pull request finding

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

* fix(integrations): restore _clear/_update_init_options public imports

The refactor that split integration commands moved
_clear_init_options_for_integration and _update_init_options_for_integration
into integrations/_helpers.py, but tests still import them from the top-level
specify_cli package, causing ImportError. Re-export them with explicit aliases
at the end of __init__.py to preserve the public import surface.

---------

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
2026-06-02 12:21:19 -05:00
Pedro BarbosaandGitHub 44aac9f6e4 feat: add native Cline integration (#2508)
* test: strip ansi to make asserts work

* feat: add native Cline integration
2026-06-01 11:20:48 -05:00
CopilotGitHubcopilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>Manfred RiemCopilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
50da3a0f77 Extract agent context updates into bundled agent-context extension (#2546)
* Initial plan

* Extract agent context updates into bundled agent-context extension

* Potential fix for pull request finding 'Unused import'

Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>

* Potential fix for pull request finding 'Unused import'

Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>

* fix: address review comments on agent-context extension

- bash: parse init-options.json with a single python3 invocation instead
  of three separate read_json_field calls, for parity with the PowerShell
  ConvertFrom-Json approach and to avoid divergent error semantics
- bash: use parameter expansion to strip PROJECT_ROOT prefix from plan
  path instead of sed interpolation, avoiding special-character fragility
- powershell: limit Get-ChildItem to -Depth 1 so plan.md discovery matches
  the bash glob specs/*/plan.md (one level deep) — fixes cross-platform
  inconsistency with nested plan.md files
- powershell: replace Substring+Length relative-path with
  [System.IO.Path]::GetRelativePath for robustness across case/PSDrive
  differences
- __init__.py: move agent-context extension install to after
  save_init_options so init-options.json is present when hooks run
- __init__.py: seed context_markers in init-options only when
  context_file is truthy; avoids noise for integrations without a context
  file
- integrations/base.py: narrow blanket except Exception in
  _resolve_context_markers to ImportError / (OSError, ValueError) so
  unexpected bugs surface instead of being silently swallowed

* fix: gate context_markers in _update_init_options_for_integration on context_file

Apply the same gating logic used during `specify init`: only write
context_markers to init-options.json when the integration actually has a
context_file set.  When switching to an integration without a context file
the stale markers are removed, keeping the two init paths consistent.

* fix: move context_file/context_markers from init-options.json to agent-context extension config

* Potential fix for pull request finding 'Unused global variable'

Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>

* fix: clarify local import comment in agents.py

* Fix remaining agent-context review findings

* Fix follow-up agent-context review issues

* Address review feedback: narrow except, improve PyYAML messaging, surface config-written note

* Fix double-space in PyYAML install hint message

* Potential fix for pull request finding 'Empty except'

Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>

* Potential fix for pull request finding 'Empty except'

Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>

* Address latest agent-context review feedback

* Harden bash config parse output handling

* Clarify ImportError-only fallback comment

* Apply review feedback: drop dead try/except, guard ext-config creation, explicit ConvertFrom-Yaml check

* Remove redundant $Options = $null in PS1 catch block

* Add constitution directives, deprecation warning, agent-context auto-install, and init flow fix

- Add constitution-loading directive to specify, clarify, tasks, checklist, taskstoissues commands
- Add deprecation warning (v0.12.0) in upsert_context_section()
- Auto-install agent-context extension during specify init
- Move context_file from init-options.json to agent-context extension config
- Add tests: deprecation warning, corrupt config, constitution directives
- Update file inventories across all integration tests

* Address review: fix init ordering, test coverage, and hermes inventory

- Move agent-context extension install after init-options.json is saved
  so skill registration can read ai_skills + integration key
- Write extension config after install (avoids template overwriting context_file)
- Fix test_defaults_when_markers_field_missing to truly test missing markers key
- Update hermes tests to allow extension-installed agent-context skill

* Address review: chmod ordering, preserve markers, PS1 Python check, YAML key order

- Move ensure_executable_scripts after agent-context extension install
  so extension scripts get execute bits set
- Use preserve_markers=True on reinit to keep user-customized markers
- Add Python 3 version check in PowerShell fallback (matching bash behavior)
- Add sort_keys=False to yaml.safe_dump for stable config output

* Address review: path traversal guards and docstring fix

- Reject absolute paths and '..' segments in context_file in both bash and
  PowerShell scripts to prevent writes outside the project root
- Fix docstring in _update_init_options_for_integration to accurately
  describe marker preservation behavior

* Address review: strict enabled check, docstring, segment-level path traversal

- Use 'is not False' for enabled check so only literal False disables
- Update upsert_context_section docstring to mention disabled-extension return
- Fix path traversal guards to check actual path segments, not substrings
  (allows filenames like 'notes..md' while rejecting '../' traversal)

* Address review: UnicodeError handling, missing extension warning

- Add UnicodeError to exception tuples in _load_agent_context_config and
  _resolve_context_markers so garbled UTF-8 config files fall back to defaults
- Emit error (with reinstall command) instead of silent skip when bundled
  agent-context extension is not found during init

* Address review: bash backslash traversal guard, wheel packaging

- Reject backslash separators and Windows drive-letter paths in bash
  context_file validation (prevents traversal on Git-Bash/Windows)
- Add extensions/agent-context to pyproject.toml force-include so the
  bundled extension is included in wheel builds

* Address review: write extension config before init-options.json

- Reorder writes in _update_init_options_for_integration so the
  agent-context extension config is updated first; if it fails,
  init-options.json remains consistent with the previous state

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: Manfred Riem <15701806+mnriem@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
2026-05-30 06:37:18 -05:00
7f33dca87c refactor: create commands/ package and move init handler (PR-4/8) (#2615)
* refactor: create commands/ package and move init handler (PR-4/8)

- Extract agent configuration constants (AGENT_CONFIG, AI_ASSISTANT_HELP,
  SCRIPT_TYPE_CHOICES, etc.) to _agent_config.py to avoid circular imports
- Create commands/ package skeleton with stub modules for each command group
- Move init command handler (~670 lines) from __init__.py to commands/init.py
  using the register(app) pattern; lazy imports inside the handler body
  prevent circular dependencies with __init__.py
- Re-export AGENT_CONFIG, AI_ASSISTANT_HELP, SCRIPT_TYPE_CHOICES from
  __init__.py for backward compatibility
- Add tests/test_commands_package.py to verify package structure

* fix(tests): update patch targets after moving init handler to commands/init.py

_stdin_is_interactive and select_with_arrows are now bound in
specify_cli.commands.init, not specify_cli directly.

* fix(lint): remove unused imports and mark re-exports in __init__.py

- Remove shutil, shlex top-level imports (used lazily inside functions)
- Remove rich.live.Live import (moved to commands/init.py)
- Mark select_with_arrows and _locate_bundled_workflow as explicit
  re-exports to satisfy ruff F401

* chore: add from __future__ import annotations to new modules

Aligns with the project convention established in _console.py, _assets.py,
_utils.py, and other modules.

* docs(cli): align init help with bundled scaffolding

Potential fix for pull request finding

Update command package documentation and init help text to reflect the current implementation: init uses bundled assets and integration setup, while placeholder command modules are import anchors until extracted. Remove the unused tracker-active flag assignment that had no reader in the codebase.

Constraint: --offline is hidden/no-op and init no longer downloads templates from GitHub releases

Rejected: Add no-op register functions to placeholder modules | would imply extracted command groups are implemented there

Confidence: high

Scope-risk: narrow

Directive: Keep CLI help text aligned with the actual init scaffolding path

Tested: uv run specify init --help; uv run pytest tests/test_commands_package.py tests/test_agent_config_consistency.py -q; uv run pytest tests/test_commands_package.py tests/test_console_imports.py tests/integrations/test_cli.py -q

Not-tested: full test suite

* fix(init): align preset failure reporting with _print_cli_warning helper

Use the _print_cli_warning helper (introduced in main) for preset install
failures so that output matches the expected format:
  "Failed to install preset '<name>': ..."
  "Continuing without the optional preset."

* fix(init): remove unused lazy imports

The init command imported CLI error formatting helpers through its circular-dependency-safe lazy import block, but the module does not use them. Remove those imports so ruff does not report F401.

Constraint: uvx ruff check src/ must pass.

Rejected: Wire the helpers into init error handling | Existing preset warnings already use _print_cli_warning, and changing behavior is unnecessary for this lint fix.

Confidence: high

Scope-risk: narrow

Directive: Keep lazy import blocks limited to names consumed in the importing module.

Tested: uvx ruff check src/

Not-tested: Full pytest suite

Co-authored-by: OmX <omx@oh-my-codex.dev>

---------

Co-authored-by: OmX <omx@oh-my-codex.dev>
2026-05-26 08:06:35 -05:00