You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Resolve extension hooks with native pre/post scripts - #4883
Replace repeated in-prompt parsing and ordering of extension pre/post hooks with native, self-contained Bash, PowerShell, and Python dispatch scripts. Agent command templates consume the ordered JSON result and invoke the selected extension commands; the scripts do not attempt to execute agent commands. Render the script path across command, skill, and preset installations, and serialize the supported canonical configuration for the native resolvers. Invalid or unsupported configuration fails explicitly rather than silently dropping hooks.
The ten core hook-bearing command templates shrink from 18,736 to 14,680 words (4,056 fewer; 21.6%). Tests cover priority/order, disabled/conditional/optional hooks, invalid input, cross-runtime parity, integration rendering, and package contents; the GitHub extension command stays in parity with its core counterpart.
Testing
Tested locally with uv run specify --help
Ran existing tests with uv sync && uv run pytest
Tested with a sample project (if applicable)
Equivalent local checks: .venv/bin/specify --help — passed; LC_ALL=en_US.UTF-8 .venv/bin/python -m pytest tests -q --tb=short — 9,839 passed, 19 skipped, 62 warnings; git diff --check — passed. The exact checklist commands and an agent-driven sample-project slash-command run were not performed, so their boxes remain unchecked.
Manual test results
Agent: Not run | OS/Shell: macOS / Bash, PowerShell and Python resolver behavior covered by automated tests
Command tested
Notes
Agent-driven slash commands
Not run; automated command-rendering and hook-dispatch tests passed.
AI Disclosure
I did not use AI assistance for this contribution
I did use AI assistance (fill in the disclosure below)
AI disclosure: Implemented with GitHub Copilot using GPT-6 Sol in autonomous agent mode (reasoning effort not explicitly configured; default settings). AI authored the implementation, tests, and documentation; no human line-by-line review is claimed.
Validate condition types to keep hook behavior consistent
scripts/python/pre_hooks.py:49
condition is never type-validated. For example, YAML condition: false is loaded here as a falsey boolean and the hook executes, while the Bash/PowerShell parsers retain false as non-empty text and skip the same hook. Since the documented field is a string or null, reject non-string/non-null values before filtering so the selected script variant cannot change whether a mandatory hook runs.
Existing configs are incompatible after parser migration
src/specify_cli/extensions/__init__.py:5729
This only canonicalizes .specify/extensions.yml on its next write. Projects upgraded from an earlier release can still have valid PyYAML output wrapped at the previous default width; both new native parsers reject those continuation lines, so upgrading the integration can make every hook check fail until some unrelated extension command rewrites the file. Migrate/rewrite existing configuration during the shared-infrastructure upgrade, or keep the native parsers compatible with the previously emitted layout.
🧠 Review effort: Balanced
Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.
Use Bash 3-compatible scalar checks, validate condition types across runtimes, and canonicalize existing extension configuration when shared infrastructure is installed or upgraded. Guard invalid and symlinked configurations and cover old wrapped YAML with executable resolver regressions.
Assisted-by: GitHub Copilot (model: GPT-6 Sol, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Addressed this review in 2865187. The Bash resolver no longer uses Bash 4-only lowercase expansion; a regression invokes both hook phases through macOS /bin/bash 3.2. All three resolvers now reject non-string/non-null condition values consistently. Shared-infrastructure installation/upgrade canonicalizes existing valid extension YAML, including legacy wrapped prompts, before native dispatch; invalid and symlinked config is not overwritten. Validation: LC_ALL=en_US.UTF-8 .venv/bin/python -m pytest tests -q --tb=short — 9,928 passed, 19 skipped, 62 warnings before the final symlink guard; after that guard, the affected hook, upgrade, and template suites passed (300 passed). /bin/bash -n scripts/bash/pre-hooks.sh scripts/bash/post-hooks.sh and git diff --check passed. Posted on behalf of @mnriem by GitHub Copilot (GPT-6 Sol, autonomous agent mode, default reasoning settings); AI authored this fix, regression tests, documentation update, and review-round summary, with no human line-by-line review claimed.
Validate YAML fields in every event, not only the target
scripts/bash/pre-hooks.sh:232
Fields are parsed only while $target is true, so malformed YAML scalars in any other event are silently accepted. For example, resolving before_plan succeeds when before_tasks contains extension: 'unterminated, whereas the Python resolver rejects the file during safe_load. Validate every event's fields while collecting only the requested event so invalid configuration cannot produce variant-dependent success.
Validate non-target event entries for cross-runtime parity
scripts/powershell/pre-hooks.ps1:137
Non-target event entries bypass Add-HookField, so malformed quoted scalars or unsupported field values elsewhere in the same YAML file are never validated. The same file therefore fails under Python but succeeds under PowerShell depending on which event is requested. Parse/validate every event and only gate adding hooks to the result on $target.
Standardize priority range normalization across runtimes
scripts/python/pre_hooks.py:63
Priority normalization has no shared numeric range across runtimes. For example, 2147483648 is retained here and by Bash but PowerShell falls back to 10; a 16-digit value is retained here while both native variants fall back. This changes hook order based on the installed script variant. Define one supported range/coercion rule in all three resolvers and cover its boundaries in the parity test.
Addressed this review in 219c013. Migration now rejects a symlinked .specify parent as well as a symlinked extensions.yml, without modifying the external config. Bash and PowerShell validate fields in non-target events; Python now validates every configured event too, while all three return hooks only for the requested event. Priorities share the same coercion and 1..2147483647 range, with out-of-range values falling back to 10; cross-runtime tests cover the boundaries and ordering. Validation on this exact change: LC_ALL=en_US.UTF-8 .venv/bin/python -m pytest tests -q --tb=short — 9,966 passed, 19 skipped, 62 warnings; /bin/bash -n scripts/bash/pre-hooks.sh scripts/bash/post-hooks.sh and git diff --check passed. Posted on behalf of @mnriem by GitHub Copilot (GPT-6 Sol, autonomous agent mode, default reasoning settings); AI authored the fix, tests, documentation, and this review-round summary, with no human line-by-line review claimed.
Reject empty quoted identifiers in Bash for every event, update the upstream deprecation contract to assert scripted dispatch, and bump the GitHub extension and catalog to 1.0.3 so installed copies receive the command update.
Assisted-by: GitHub Copilot (model: GPT-6 Sol, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
After emitting the block above you MUST actually invoke the hook and wait for it to finish before continuing. Run it the same way you would run the command yourself in this agent/session (the invocation may differ from the literal `{command}` id shown above, e.g. a skills-mode agent runs it as `/skill:speckit-...` or `$speckit-...`). Emitting the block alone does not run the hook.
- If no hooks are registered or `.specify/extensions.yml` does not exist, skip silently
Run `{PRE_HOOK_SCRIPT} taskstoissues` from the project root and read its JSON result. If it fails or returns `error`, tell the user why no hooks were checked (including mandatory hooks), then continue the core command. For each returned hook in order: invoke mandatory commands in this agent and wait for completion before proceeding; surface optional commands with their prompt and description without executing them automatically. Use the invocation syntax for the installed agent/skills mode. If `hooks` is empty, continue silently.
Invoke Bash explicitly when Git Bash is usable and skip Bash-only cases otherwise; retain native PowerShell and Python coverage on Windows while preserving Bash coverage on macOS and Linux.
Assisted-by: GitHub Copilot (model: GPT-6 Sol, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This replacement drops the contract that skills explain how dispatcher IDs map to installed command names. The resolver still returns dotted IDs such as speckit.git.commit, while skills are registered under hyphenated names and agent-specific prefixes. SkillsIntegration._inject_hook_command_note() (and the Cline/Junie overrides) only matches the removed - For each executable hook... wording, so the generated skills now contain no conversion guidance; these assertions mask that regression. Update the post-processing trigger for the new dispatch wording and retain coverage for slash, dollar, and /skill: invocation forms.
Fixed the Windows CI assertion in commit 3436206d: the replacement-preset test now checks the platform-native default rather than always expecting Bash, and also covers explicit Bash, PowerShell, and Python selections. No runtime behavior changed. The two remaining open findings already have fixes and passing regressions on this branch: extension updates restore missing shared dispatchers, and projection readers tolerate Git CRLF conversion while rejecting corrupted content. Local validation: LC_ALL=en_US.UTF-8 .venv/bin/python -m pytest -q tests/test_hook_dispatch_scripts.py tests/contract/test_bundled_extension_update_delivery.py --disable-warnings --tb=short — 549 passed, 4 skipped (Windows PowerShell 5.1 cases unavailable on macOS); LC_ALL=en_US.UTF-8 .venv/bin/python -m pytest -q tests/specify_cli/presets/test_manager_commands.py --disable-warnings --tb=short — 76 passed. Windows CI for this commit has not yet been verified. Posted on behalf of @mnriem with GitHub Copilot (GPT-6.1 Sol, autonomous mode); AI-authored test changes and review-round summary, without line-by-line human review.
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
Human assessment is needed to evaluate an unpublished moderate-severity concern about the legacy Bash resolver accepting malformed YAML as a mandatory command.
Reject unescaped internal double quotes before the Bash resolver can expose malformed YAML as a mandatory hook. Preserve escaped quotes and backslashes, with pre/post regression coverage across all three runtimes.
Assisted-by: GitHub Copilot (model: GPT-6.1 Sol, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Addressed the malformed-YAML concern from review 5705146982 in 7c92ebc7. The review did not publish an inline reproduction; I reproduced a concrete case where command: "speckit.git.commit" "extra" with optional: false was returned successfully by Bash as a mandatory command, while Python and PowerShell rejected the YAML. Bash now rejects unescaped internal double quotes. Added pre/post coverage for malformed extension, command, and prompt values, plus positive coverage preserving escaped quotes and backslashes across all three runtimes. Before the fix, 12 new Bash cases failed and the other 30 new cases passed; afterward, LC_ALL=en_US.UTF-8 .venv/bin/python -m pytest -q tests/test_hook_dispatch_scripts.py --disable-warnings --tb=short passed with 580 passed and 4 skipped (Windows PowerShell 5.1 cases unavailable on macOS). bash -n scripts/bash/pre-hooks.sh scripts/bash/post-hooks.sh and git diff --check also passed. The two older unresolved findings retain their existing fixes and regression coverage. Posted on behalf of @mnriem with GitHub Copilot (GPT-6.1 Sol, autonomous mode); AI-authored code, regression tests, and review-round summary, without line-by-line human review.
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
Human assessment is needed because legacy Bash priority parsing and interacting preset replacement, alias, and re-registration paths can prevent hooks from executing.
Parse zero-padded YAML exponents as decimal in Bash. Provision dispatchers for surviving preset aliases and re-registration, while preserving commands when dispatcher paths are unsafe. Align the bundled GitHub extension compatibility floor with the merged CLI release baseline.
Assisted-by: GitHub Copilot (model: GPT-6.1 Sol, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
The reason will be displayed to describe this comment to others. Learn more.
🟡 Changes recommended
A configuration-change race can silently skip mandatory hooks, while runtime re-registration and legacy YAML parsing defects can leave dispatchers missing or resolve hooks incorrectly.
Addressed review 5712136720 in 804d7575, following the upstream merge in f29ced5a. Bash now parses zero-padded YAML exponents as decimal instead of crashing or changing hook order. Preset registration provisions dispatchers for surviving aliases and fallback variants during re-registration, while preserving existing commands when dispatcher paths are unsafe; fully shadowed aliases do not require unused dispatchers. The new regressions produced 15 failures before the fixes. The upstream merge resolves the conflicts that prevented fresh CI runs, preserves both sides’ tests, and advances the bundled GitHub extension to 1.0.5 with a compatibility floor of Spec Kit 1.2.6. Local validation: LC_ALL=en_US.UTF-8 .venv/bin/python -m pytest -n 2 -q --disable-warnings --tb=short — 14,520 passed, 136 skipped; the subsequent compatibility-floor check, LC_ALL=en_US.UTF-8 .venv/bin/python -m pytest -q tests/extensions/github/test_github_extension.py tests/contract/test_bundled_extension_update_delivery.py tests/specify_cli/extensions/test_command_add.py --disable-warnings --tb=short — 277 passed, 1 skipped; uvx ruff@0.15.0 check src tests and bash -n scripts/bash/pre-hooks.sh scripts/bash/post-hooks.sh — passed. Fresh CI on this head is green across all six platform/Python test jobs, lint, version guard, audit, and CodeQL. Windows/Python 3.13 initially hit an access-denied checkpoint error in an unrelated workflow test and passed the approved single-job rerun without further code changes. Posted on behalf of @mnriem with GitHub Copilot (GPT-6.1 Sol, autonomous mode); AI-authored merge resolution, code, tests, documentation, and review-round summary, without line-by-line human review.
Publish and recheck unique projection generations across all runtimes, including byte-identical configuration reversions. Preserve legacy YAML null-comment semantics and protect PowerShell resolver metadata from extra fields. Reuse dispatcher provisioning before extension command and skill re-registration, with cross-runtime and Windows PowerShell 5.1 regression cases.
Assisted-by: GitHub Copilot (model: GPT-6.1 Sol, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
Human assessment is needed because omitted findings identify parser edge cases that accept invalid YAML in both shells and reject valid timestamp-shaped strings in PowerShell.
Addressed the four new findings from review 5713876183 in 3a3b6bf7, with a Windows fixture correction in 621b10a7. Projection writers invalidate the old generation before writing and publish a fresh UUID token last; every runtime captures and rechecks it, rejecting configuration changes that revert to identical YAML bytes. Legacy Bash and PowerShell treat comment-only conditions as null while preserving quoted hash strings. PowerShell assigns only supported hook fields, preventing extra YAML from overwriting resolver metadata. Installation and command/skill re-registration share fallback-dispatcher provisioning, preserving prior artifacts when a dispatcher path is unsafe. The initial regression set produced 33 failures before the fixes. Local validation: LC_ALL=en_US.UTF-8 .venv/bin/python -m pytest -n 2 -q --disable-warnings --tb=short — 14,603 passed, 162 skipped; uvx ruff@0.15.0 check src tests and bash -n scripts/bash/pre-hooks.sh scripts/bash/post-hooks.sh — passed. Windows CI confirmed the new PowerShell 5.1 generation and legacy-parser cases. Its reported fallback-test failures came from Windows setup already having PowerShell dispatchers; the corrected fixture explicitly removes them, and all four cases pass locally. Fresh CI for 621b10a7 is still running; GitHub API 502/504 errors interrupted continuous monitoring, so final CI success is not yet verified. Posted on behalf of @mnriem with GitHub Copilot (GPT-6.1 Sol, autonomous mode); AI-authored code, tests, documentation, and review-round summary, without line-by-line human review.
This branch has not been deployed
No deployments
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
triage-nice-to-haveVerdict: evidence-backed fix or greenlit feature — land after review
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Replace repeated in-prompt parsing and ordering of extension pre/post hooks with native, self-contained Bash, PowerShell, and Python dispatch scripts. Agent command templates consume the ordered JSON result and invoke the selected extension commands; the scripts do not attempt to execute agent commands. Render the script path across command, skill, and preset installations, and serialize the supported canonical configuration for the native resolvers. Invalid or unsupported configuration fails explicitly rather than silently dropping hooks.
The ten core hook-bearing command templates shrink from 18,736 to 14,680 words (4,056 fewer; 21.6%). Tests cover priority/order, disabled/conditional/optional hooks, invalid input, cross-runtime parity, integration rendering, and package contents; the GitHub extension command stays in parity with its core counterpart.
Testing
uv run specify --helpuv sync && uv run pytestEquivalent local checks:
.venv/bin/specify --help— passed;LC_ALL=en_US.UTF-8 .venv/bin/python -m pytest tests -q --tb=short— 9,839 passed, 19 skipped, 62 warnings;git diff --check— passed. The exact checklist commands and an agent-driven sample-project slash-command run were not performed, so their boxes remain unchecked.Manual test results
Agent: Not run | OS/Shell: macOS / Bash, PowerShell and Python resolver behavior covered by automated tests
AI Disclosure
AI disclosure: Implemented with GitHub Copilot using GPT-6 Sol in autonomous agent mode (reasoning effort not explicitly configured; default settings). AI authored the implementation, tests, and documentation; no human line-by-line review is claimed.