fix(templates): report an unreadable extensions.yml instead of skipping hooks silently - #4456
Open
avp9-nexus wants to merge 2 commits into
Open
fix(templates): report an unreadable extensions.yml instead of skipping hooks silently#4456avp9-nexus wants to merge 2 commits into
avp9-nexus wants to merge 2 commits into
Conversation
…ng hooks silently All ten core command templates told the agent: "If the YAML cannot be parsed or is invalid, skip hook checking silently and continue normally". A `.specify/extensions.yml` with one malformed line was therefore treated exactly like a manifest with no hooks, and a mandatory hook (`optional: false`, as registered by the bundled git extension) was disabled without anything saying so. The agent now tells the user that the manifest could not be read (with the parser error) and that no hooks were checked, then continues as before. Control flow is unchanged; only the silence is removed. Adds tests/test_command_template_hooks.py, which pins the wording at both hook sites of every template that reads extensions.yml.
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The regression tests must cover the parser error, mandatory-hook warning, and continuation behavior.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Updates core command templates to report unreadable .specify/extensions.yml files rather than silently skipping hooks.
Changes:
- Adds explicit parse-error and skipped-hook warnings across ten templates.
- Adds prompt-text regression tests for both hook sites.
File summaries
| File | Description |
|---|---|
tests/test_command_template_hooks.py |
Adds regression checks, but does not cover all required warning and continuation clauses. |
templates/commands/taskstoissues.md |
Reports unreadable hook manifests. |
templates/commands/tasks.md |
Reports unreadable hook manifests. |
templates/commands/specify.md |
Reports unreadable hook manifests. |
templates/commands/plan.md |
Reports unreadable hook manifests. |
templates/commands/implement.md |
Reports unreadable hook manifests. |
templates/commands/converge.md |
Reports unreadable hook manifests. |
templates/commands/constitution.md |
Reports unreadable hook manifests. |
templates/commands/clarify.md |
Reports unreadable hook manifests. |
templates/commands/checklist.md |
Reports unreadable hook manifests. |
templates/commands/analyze.md |
Reports unreadable hook manifests. |
Review details
- Files reviewed: 11/11 changed files
- Comments generated: 1
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
The regression test only pinned two of the five clauses the new instruction carries. Dropping the parser error, the mandatory-hook warning or the continuation clause from every template would have left it green. It now pins all five: the manifest could not be read, the parser error is included, no hooks were checked, mandatory (optional: false) hooks are named, and the command then continues. Checked by mutation: removing any one clause from all ten templates fails the ten parametrized cases.
mnriem
added
the
triage-nice-to-have
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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
All ten core command templates (
templates/commands/*.md) check.specify/extensions.ymlforhooks.before_*/hooks.after_*entries, and all of them contain this instruction at both hook sites:So a manifest with a single malformed line is treated exactly like a manifest with no hooks. A mandatory hook (
optional: false) is skipped, and nothing tells the user. The bundledgitextension registers two of those (before_constitution,before_specify), so this is reachable from a stock install: corrupt one line ofextensions.ymland/speckit.specifyruns without creating the feature branch, silently."No hooks registered" and "manifest unreadable" should not produce the same silence. #2901 made sure the agent actually runs a mandatory hook instead of only emitting the directive; this PR closes the other way the same hook can vanish.
What changes
.specify/extensions.ymlcould not be read (include the parser error) and that no hooks were checked, including any mandatory (optional: false) hooks registered there, then continue normally (or then continue to the Completion Report where the original line said so).tests/test_command_template_hooks.py: for every template that readsextensions.yml, asserts that the old wording is gone and that each parse-failure line tells the user both facts (manifest could not be read, no hooks were checked). It reads the templates as text because the behaviour lives in the prompt.Testing
uv sync && uv run pytest(equivalent environment: Python 3.14.4,pip install -e .[test], offline sandbox). Full suite: 7737 tests, 196 skipped, 4 failures. The same 4 fail onmain(4a7341a) in the same sandbox and are environmental:test_bash_command_hint_falls_back_to_awk_when_jq_and_python3_broken,test_ps_variant_prefixed_with_powershell_launcher, and the twotest_bundler_referencescases that need the network. None of them reads a template.templates/commands/plan.mdtoHEAD, the twoplan.mdcases fail with the expected message, restored the change, 21 pass again.uv run specify --help(no CLI code touched)AI Disclosure
The finding, the wording of the replacement lines, the test file and this description were drafted with an AI coding assistant (Claude Code) during a read-only review of the repository, and reviewed line by line by the maintainer of this fork before opening the PR. Replies on this PR will be written the same way and disclosed as such.