github/github-spec-kit
1
0
Fork 0
mirror of https://github.com/github/spec-kit.git synced 2026-07-03 12:28:06 +08:00
Code Issues Packages Projects Releases Wiki Activity
1,091 Commits 18 Branches 182 Tags
copilot/fix-code-for-review-comments
Commit Graph

6 Commits

Author SHA1 Message Date
Seiya Kojima
4ec4635dd1 feat(extensions): per-event hook lists with priority ordering (#2798) 
* feat(extensions): per-event hook lists with priority ordering

The manifest validator restricted each hook event to a single mapping,
even though HookExecutor stores entries as a list per event. This blocked
an extension from running multiple commands on one event (e.g. a
verification step plus a doc-generation step after speckit.plan), and
get_hooks_for_event returned entries in raw insertion order with no way
to influence execution order across or within extensions.

This change:

1. Validator: accept hooks.<event> as either a single mapping or a list
   of mappings. Each entry is validated individually and may carry an
   optional integer `priority` (>= 1, default 10; bool rejected).
2. Command-ref normalization: apply rename / alias->canonical rewriting
   to every entry in the list, not just the head.
3. register_hooks: expand list entries, persist `priority`, and
   purge-and-replace all entries owned by the extension on each event so a
   reinstall whose shape changed (single<->list, or a shorter list) leaves
   no orphaned entries behind.
4. get_hooks_for_event: sort enabled entries by `priority` ascending with
   a stable sort (ties keep insertion order). The existing
   normalize_priority helper is reused as the sort key so corrupted
   on-disk values fall back to the default instead of raising.

Backward compatible: existing single-mapping manifests parse and register
unchanged with priority defaulting to 10. The extension-level `priority`
used by preset/template resolution is independent of the new hook-entry
`priority`.

Implements #2378

* fix(extensions): harden register_hooks per PR review

- Skip non-dict hook entries before .get() so a manifest that bypasses
  validation can't crash register_hooks with AttributeError.
- Normalize `priority` on save via normalize_priority so the on-disk
  config stays clean, mirroring the read-side defense in
  get_hooks_for_event.
- Tests: cover the non-dict-entry skip and add encoding="utf-8" to the
  new tests' manifest writes.

* fix(extensions): purge dropped-event hook orphans on reinstall

register_hooks only purged events the new manifest still declared, so an
extension that dropped an event on reinstall left stale entries for it in
the project config. Purge this extension's entries from undeclared events
(and prune emptied events) before registering; scoped to this extension,
and a no-op for the install/update flow where unregister_hooks runs first.

* fix(extensions): reject boolean priority and complete orphan purge

- normalize_priority falls back to default for bool values
- dedup deletes duplicate commands before re-insert for last-wins ties
- register_hooks purges orphans even when all hooks are dropped

* docs(extensions): document per-event hook lists and priority

- EXTENSION-API-REFERENCE: hook event accepts a mapping or list; add
  priority field reference and last-wins dedup note
- EXTENSION-DEVELOPMENT-GUIDE: add list-form example with priority

* docs(extensions): show both single and list hook forms in schema snippet

* docs(extensions): reference DEFAULT_HOOK_PRIORITY in normalize_priority

normalize_priority hard-coded the default as the literal 10 in both its
signature and docstring, duplicating DEFAULT_HOOK_PRIORITY. Reference the
constant in the signature and drop the literal from the docstring so the
default has a single source of truth.
 2026-06-08 08:03:46 -05:00
Copilot
1a9e4d1d8d feat: Git extension stage 1 — bundled extensions/git with hooks on all core commands (#1941) 
* feat: add git extension with hooks on all core commands

- Create extensions/git/ with 5 commands: initialize, feature,
  validate, remote, commit
- 18 hooks covering before/after for all 9 core commands
- Scripts: create-new-feature, initialize-repo, auto-commit,
  git-common (bash + powershell)
- Configurable: branch_numbering, init_commit_message,
  per-command auto-commit with custom messages
- Add hooks to analyze, checklist, clarify, constitution,
  taskstoissues command templates
- Allow hooks-only extensions (no commands required)
- Bundle extension in wheel via pyproject.toml force-include
- Resolve bundled extensions locally before catalog lookup
- Remove planned-but-unimplemented before/after_commit hook refs
- Update extension docs (API ref, dev guide, user guide)
- 37 new tests covering manifest, install, all scripts (bash+pwsh),
  config reading, graceful degradation

Stage 1: opt-in via 'specify extension add git'. No auto-install,
no changes to specify.md or core git init code.

Refs: #841, #1382, #1066, #1791, #1191

* fix: set git identity env vars in extension tests for CI runners

* fix: address PR review comments

- Fix commands property KeyError for hooks-only extensions
- Fix has_git() operator precedence in git-common.sh
- Align default commit message to '[Spec Kit] Initial commit' across
  config-template, extension.yml defaults, and both init scripts
- Update README to reflect all 5 commands and 18 hooks

* fix: address second round of PR review comments

- Add type validation for provides.commands (must be list) and hooks
  (must be dict) in manifest _validate()
- Tighten malformed timestamp detection in git-common.sh to catch
  7-digit dates without trailing slug (e.g. 2026031-143022)
- Pass REPO_ROOT to has_git/Test-HasGit in create-new-feature scripts
- Fix initialize command docs: surface errors on git failures, only
  skip when git is not installed
- Fix commit command docs: 'skips with a warning' not 'silently'
- Add tests for commands:null and hooks:list rejection

* fix: address third round of PR review comments

- Remove scripts frontmatter from command files (CommandRegistrar
  rewrites ../../scripts/ to .specify/scripts/ which points at core
  scripts, not extension scripts)
- Update speckit.git.commit command to derive event name from hook
  context rather than using a static example
- Clarify that hook argument passthrough works via AI agent context
  (the agent carries conversation state including user's original
  feature description)

* fix: address fourth round of PR review comments

- Validate extension_id against ^[a-z0-9-]+$ in _locate_bundled_extension
  to prevent path traversal (security fix)
- Move defaults under config.defaults in extension.yml to match
  ConfigManager._get_extension_defaults() schema
- Ship git-config.yml in extension directory so it's copied during
  install (provides.config template isn't materialized by ExtensionManager)
- Condition handling in hook templates: intentionally matches existing
  pattern from specify/plan/tasks/implement templates (not a new issue)

* fix: add --allow-empty to git commit in initialize-repo scripts

Ensures git init succeeds even on empty repos where nothing has been
staged yet.

* fix: resolve display names to bundled extensions before catalog download

When 'specify extension add "Git Branching Workflow"' is used with a
display name instead of the ID, the catalog resolver now runs first to
map the name to an ID, then checks bundled extensions again with the
resolved ID before falling back to network download.

Also noted: EXECUTE_COMMAND_INVOCATION and condition handling match the
existing pattern in specify/plan/tasks/implement templates (pre-existing,
not introduced by this PR).

* fix: handle before_/after_ prefixes in auto-commit message derivation

- Strip both before_ and after_ prefixes when deriving command name
  (fixes misleading 'Auto-commit after before_plan' messages)
- Include phase (before/after) in default commit messages
- Clarify README config example is an override, not default behavior

* fix: use portable grep -qw for word boundary in create-new-feature.sh

BSD grep (macOS) doesn't support \b as a word boundary. Replace with
grep -qw which is POSIX-portable.

* fix: validate hook values, numeric --number, and PS warning routing

- Validate each hook value is a dict with a 'command' field during
  manifest _validate() (prevents crash at install time)
- Validate --number is a non-negative integer in bash create-new-feature
  (clear error instead of cryptic shell arithmetic failure)
- Route PowerShell no-git warning to stderr in JSON mode so stdout
  stays valid JSON

---------

Co-authored-by: Manfred Riem <15701806+mnriem@users.noreply.github.com>
 2026-04-07 08:39:35 -05:00
Andrii Furmanets
796b4f47c4 fix: prevent extension command shadowing (#1994) 
* fix: prevent extension command shadowing

* Validate extension command namespaces

* Reuse extension command name pattern
 2026-03-27 10:55:26 -05:00
Matt Van Horn
333a76535b feat(commands): wire before/after hook events into specify and plan templates (#1886) 
* feat(commands): wire before/after hook events into specify and plan templates

Replicates the hook evaluation pattern from tasks.md and implement.md
(introduced in PR #1702) into the specify and plan command templates.
This completes the hook lifecycle across all SDD phases.

Changes:
- specify.md: Add before_specify/after_specify hook blocks
- plan.md: Add before_plan/after_plan hook blocks
- EXTENSION-API-REFERENCE.md: Document new hook events
- EXTENSION-USER-GUIDE.md: List all available hook events

Fixes #1788

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

* Mark before_commit/after_commit as planned in extension docs

These hook events are defined in the API reference but not yet wired
into any core command template. Marking them as planned rather than
removing them, since the infrastructure supports them.

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

* Fix hook enablement to default true when field is absent

Matches HookExecutor.get_hooks_for_event() semantics where
hooks without an explicit enabled field are treated as enabled.

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

* fix(docs): mark commit hooks as planned in user guide config example

The yaml config comment listed before_commit/after_commit as
"Available events" but they are not yet wired into core templates.
Moved them to a separate "Planned" line, consistent with the
API reference.

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

* fix(commands): align enabled-filtering semantics across all hook templates

tasks.md and implement.md previously said "Filter to only hooks where
enabled: true", which would skip hooks that omit the enabled field.
Updated to match specify.md/plan.md and HookExecutor's h.get('enabled', True)
behavior: filter out only hooks where enabled is explicitly false.

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

---------

Co-authored-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
 2026-03-19 06:37:03 -05:00
Copilot
ee922cbde9 feat(extensions): support multiple active catalogs simultaneously (#1720) 
* Initial plan

* feat(extensions): implement multi-catalog stack support

- Add CatalogEntry dataclass to represent catalog entries
- Add get_active_catalogs() reading SPECKIT_CATALOG_URL, project config,
  user config, or built-in default stack (org-approved + community)
- Add _load_catalog_config() to parse .specify/extension-catalogs.yml
- Add _validate_catalog_url() HTTPS validation helper
- Add _fetch_single_catalog() with per-URL caching, backward-compat for DEFAULT_CATALOG_URL
- Add _get_merged_extensions() that merges all catalogs (priority wins on conflict)
- Update search() and get_extension_info() to use merged results
  annotated with _catalog_name and _install_allowed
- Update clear_cache() to also remove per-URL hash cache files
- Add extension_catalogs CLI command to list active catalogs
- Add catalog add/remove sub-commands for .specify/extension-catalogs.yml
- Update extension_add to enforce install_allowed=false policy
- Update extension_search to show source catalog per result
- Update extension_info to show source catalog with install_allowed status
- Add 13 new tests covering catalog stack, merge conflict resolution,
  install_allowed enforcement, and catalog metadata

Co-authored-by: mnriem <15701806+mnriem@users.noreply.github.com>

* docs: update RFC, user guide, and API reference for multi-catalog support

- RFC: replace FUTURE FEATURE section with full implementation docs,
  add catalog stack resolution order, config file examples, merge
  conflict resolution, and install_allowed behavior
- EXTENSION-USER-GUIDE.md: add multi-catalog section with CLI examples
  for catalogs/catalog-add/catalog-remove, update catalog config docs
- EXTENSION-API-REFERENCE.md: add CatalogEntry class docs, update
  ExtensionCatalog docs with new methods and result annotations,
  add catalog CLI commands (catalogs, catalog add, catalog remove)

Also fix extension_catalogs command to correctly show "Using built-in
default catalog stack" when config file exists but has empty catalogs

Co-authored-by: mnriem <15701806+mnriem@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>

* fix: remove extraneous f-string prefixes (ruff F541)

Remove f-prefix from strings with no placeholders in catalog_remove
and extension_search commands.

Co-authored-by: mnriem <15701806+mnriem@users.noreply.github.com>

* fix: address PR review feedback for multi-catalog support

- Rename 'org-approved' catalog to 'default'
- Move 'catalogs' command to 'catalog list' for consistency
- Add 'description' field to CatalogEntry dataclass
- Add --description option to 'catalog add' CLI command
- Align install_allowed default to False in _load_catalog_config
- Add user-level config detection in catalog list footer
- Fix _load_catalog_config docstring (document ValidationError)
- Fix test isolation for test_search_by_tag, test_search_by_query,
  test_search_verified_only, test_get_extension_info
- Update version to 0.1.14 and CHANGELOG
- Update all docs (RFC, User Guide, API Reference)

* fix: wrap _load_catalog_config() calls in catalog_list with try/except

- Check SPECKIT_CATALOG_URL first (matching get_active_catalogs() resolution order)
- Wrap both _load_catalog_config() calls in try/except ValidationError so a
  malformed config file cannot crash `specify extension catalog list` after
  the active catalogs have already been printed successfully

Co-authored-by: mnriem <15701806+mnriem@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: mnriem <15701806+mnriem@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
 2026-03-09 14:30:27 -05:00
Michal Bachorik
f14a47ea7d Add modular extension system (#1551) 
* Add modular extension system for Spec Kit

Implement a complete extension system that allows third-party developers
to extend Spec Kit functionality through plugins.

## Core Features
- Extension discovery and loading from local and global directories
- YAML-based extension manifest (extension.yml) with metadata and capabilities
- Command extensions: custom slash commands with markdown templates
- Hook system: pre/post hooks for generate, task, and sync operations
- Extension catalog for discovering and installing community extensions
- SPECKIT_CATALOG_URL environment variable for catalog URL override

## Installation Methods
- Catalog install: `specify extension add <name>`
- URL install: `specify extension add <name> --from <url>`
- Dev install: `specify extension add --dev <path>`

## Implementation
- ExtensionManager class for lifecycle management (load, enable, disable)
- Support for extension dependencies and version constraints
- Configuration layering (global → project → extension)
- Hook conditions for conditional execution

## Documentation
- RFC with design rationale and architecture decisions
- API reference for extension developers
- Development guide with examples
- User guide for installing and managing extensions
- Publishing guide for the extension catalog

## Included
- Extension template for bootstrapping new extensions
- Comprehensive test suite
- Example catalog.json structure

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

* Update Jira extension to v2.1.0 in catalog

Adds 2-level mode support (Epic → Stories only).

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

* Address PR review feedback

- Fix Zip Slip vulnerability in ZIP extraction with path validation
- Fix keep_config option to actually preserve config files on removal
- Add URL validation for SPECKIT_CATALOG_URL (HTTPS required, localhost exception)
- Add security warning when installing from custom URLs (--from flag)
- Empty catalog.json so organizations can ship their own catalogs
- Fix markdown linter errors (MD040: add language to code blocks)
- Remove redundant import and fix unused variables in tests
- Add comment explaining empty except clause for backwards compatibility

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

* Add comprehensive organization catalog customization docs

- Explain why default catalog is empty (org control)
- Document how to create and host custom catalogs
- Add catalog JSON schema reference
- Include use cases: private extensions, curated catalogs, air-gapped environments
- Add examples for combining catalog with direct installation

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

* Fix test assertions for extension system data structures

- Update test_config_backup_on_remove to use new subdirectory structure
  (.backup/test-ext/file.yml instead of .backup/test-ext-file.yml)
- Update test_full_install_and_remove_workflow to handle registered_commands
  being a dict keyed by agent name instead of a flat list

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

* Address Copilot review feedback

- Fix localhost URL check to use parsed.hostname instead of netloc.startswith()
  This correctly handles URLs with ports like localhost:8080
- Fix YAML indentation error in config-template.yml (line 57)
- Fix double space typo in example.md (line 172)

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

* Add catalog.example.json as reference for organizations

The main catalog.json is intentionally empty so organizations can ship
their own curated catalogs. This example file shows the expected schema
and structure for creating organization-specific catalogs.

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

* Address remaining Copilot security and logic review feedback

- Fix Zip Slip vulnerability by using relative_to() for safe path validation
- Add HTTPS validation for extension download URLs
- Backup both *-config.yml and *-config.local.yml files on remove
- Normalize boolean values to lowercase for hook condition comparisons
- Show non-default catalog warning only once per instance

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

* Ignoring linter for extensions directory

---------

Co-authored-by: iamaeroplane <michal.bachorik@gmail.com>
Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
Co-authored-by: Manfred Riem <manfred.riem@microsoft.com>
 2026-02-10 14:27:20 -06:00