From fd7368413d316beafa62401c990890106365c125 Mon Sep 17 00:00:00 2001 From: Artap Date: Sun, 6 Sep 2026 20:10:40 +0200 Subject: [PATCH 1/2] fix(templates): report an unreadable extensions.yml instead of skipping 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. --- templates/commands/analyze.md | 4 +- templates/commands/checklist.md | 4 +- templates/commands/clarify.md | 4 +- templates/commands/constitution.md | 4 +- templates/commands/converge.md | 4 +- templates/commands/implement.md | 4 +- templates/commands/plan.md | 4 +- templates/commands/specify.md | 4 +- templates/commands/tasks.md | 4 +- templates/commands/taskstoissues.md | 4 +- tests/test_command_template_hooks.py | 70 ++++++++++++++++++++++++++++ 11 files changed, 90 insertions(+), 20 deletions(-) create mode 100644 tests/test_command_template_hooks.py diff --git a/templates/commands/analyze.md b/templates/commands/analyze.md index 2e13af58ae..7de2d28f0d 100644 --- a/templates/commands/analyze.md +++ b/templates/commands/analyze.md @@ -19,7 +19,7 @@ You **MUST** consider the user input before proceeding (if not empty). **Check for extension hooks (before analysis)**: - Check if `.specify/extensions.yml` exists in the project root. - If it exists, read it and look for entries under the `hooks.before_analyze` key -- If the YAML cannot be parsed or is invalid, skip hook checking silently and continue normally +- If the YAML cannot be parsed or is invalid, do not skip silently: tell the user that `.specify/extensions.yml` could not be read (include the parser error) and that no hooks were checked, including any mandatory (`optional: false`) hooks registered there, then continue normally - Filter out hooks where `enabled` is explicitly `false`. Treat hooks without an `enabled` field as enabled by default. - For each remaining hook, do **not** attempt to interpret or evaluate hook `condition` expressions: - If the hook has no `condition` field, or it is null/empty, treat the hook as executable @@ -205,7 +205,7 @@ Ask the user: "Would you like me to suggest concrete remediation edits for the t After reporting, check if `.specify/extensions.yml` exists in the project root. - If it exists, read it and look for entries under the `hooks.after_analyze` key -- If the YAML cannot be parsed or is invalid, skip hook checking silently and continue normally +- If the YAML cannot be parsed or is invalid, do not skip silently: tell the user that `.specify/extensions.yml` could not be read (include the parser error) and that no hooks were checked, including any mandatory (`optional: false`) hooks registered there, then continue normally - Filter out hooks where `enabled` is explicitly `false`. Treat hooks without an `enabled` field as enabled by default. - For each remaining hook, do **not** attempt to interpret or evaluate hook `condition` expressions: - If the hook has no `condition` field, or it is null/empty, treat the hook as executable diff --git a/templates/commands/checklist.md b/templates/commands/checklist.md index 5ee239d309..6e33f5d422 100644 --- a/templates/commands/checklist.md +++ b/templates/commands/checklist.md @@ -49,7 +49,7 @@ You **MUST** consider the user input before proceeding (if not empty). **Check for extension hooks (before checklist generation)**: - Check if `.specify/extensions.yml` exists in the project root. - If it exists, read it and look for entries under the `hooks.before_checklist` key -- If the YAML cannot be parsed or is invalid, skip hook checking silently and continue normally +- If the YAML cannot be parsed or is invalid, do not skip silently: tell the user that `.specify/extensions.yml` could not be read (include the parser error) and that no hooks were checked, including any mandatory (`optional: false`) hooks registered there, then continue normally - Filter out hooks where `enabled` is explicitly `false`. Treat hooks without an `enabled` field as enabled by default. - For each remaining hook, do **not** attempt to interpret or evaluate hook `condition` expressions: - If the hook has no `condition` field, or it is null/empty, treat the hook as executable @@ -350,7 +350,7 @@ Sample items: **Check for extension hooks (after checklist generation)**: Check if `.specify/extensions.yml` exists in the project root. - If it exists, read it and look for entries under the `hooks.after_checklist` key -- If the YAML cannot be parsed or is invalid, skip hook checking silently and continue normally +- If the YAML cannot be parsed or is invalid, do not skip silently: tell the user that `.specify/extensions.yml` could not be read (include the parser error) and that no hooks were checked, including any mandatory (`optional: false`) hooks registered there, then continue normally - Filter out hooks where `enabled` is explicitly `false`. Treat hooks without an `enabled` field as enabled by default. - For each remaining hook, do **not** attempt to interpret or evaluate hook `condition` expressions: - If the hook has no `condition` field, or it is null/empty, treat the hook as executable diff --git a/templates/commands/clarify.md b/templates/commands/clarify.md index ea2f20d519..8663b6908b 100644 --- a/templates/commands/clarify.md +++ b/templates/commands/clarify.md @@ -23,7 +23,7 @@ You **MUST** consider the user input before proceeding (if not empty). **Check for extension hooks (before clarification)**: - Check if `.specify/extensions.yml` exists in the project root. - If it exists, read it and look for entries under the `hooks.before_clarify` key -- If the YAML cannot be parsed or is invalid, skip hook checking silently and continue normally +- If the YAML cannot be parsed or is invalid, do not skip silently: tell the user that `.specify/extensions.yml` could not be read (include the parser error) and that no hooks were checked, including any mandatory (`optional: false`) hooks registered there, then continue normally - Filter out hooks where `enabled` is explicitly `false`. Treat hooks without an `enabled` field as enabled by default. - For each remaining hook, do **not** attempt to interpret or evaluate hook `condition` expressions: - If the hook has no `condition` field, or it is null/empty, treat the hook as executable @@ -245,7 +245,7 @@ Context for prioritization: {ARGS} Check if `.specify/extensions.yml` exists in the project root. - If it does not exist, or no hooks are registered under `hooks.after_clarify`, skip to the Completion Report. - If it exists, read it and look for entries under the `hooks.after_clarify` key. -- If the YAML cannot be parsed or is invalid, skip hook checking silently and continue to the Completion Report. +- If the YAML cannot be parsed or is invalid, do not skip silently: tell the user that `.specify/extensions.yml` could not be read (include the parser error) and that no hooks were checked, including any mandatory (`optional: false`) hooks registered there, then continue to the Completion Report. - Filter out hooks where `enabled` is explicitly `false`. Treat hooks without an `enabled` field as enabled by default. - For each remaining hook, do **not** attempt to interpret or evaluate hook `condition` expressions: - If the hook has no `condition` field, or it is null/empty, treat the hook as executable diff --git a/templates/commands/constitution.md b/templates/commands/constitution.md index 7b2f3684fb..4965426215 100644 --- a/templates/commands/constitution.md +++ b/templates/commands/constitution.md @@ -42,7 +42,7 @@ and commands read the constitution at runtime and are not modified here. **Check for extension hooks (before constitution update)**: - Check if `.specify/extensions.yml` exists in the project root. - If it exists, read it and look for entries under the `hooks.before_constitution` key -- If the YAML cannot be parsed or is invalid, skip hook checking silently and continue normally +- If the YAML cannot be parsed or is invalid, do not skip silently: tell the user that `.specify/extensions.yml` could not be read (include the parser error) and that no hooks were checked, including any mandatory (`optional: false`) hooks registered there, then continue normally - Filter out hooks where `enabled` is explicitly `false`. Treat hooks without an `enabled` field as enabled by default. - For each remaining hook, do **not** attempt to interpret or evaluate hook `condition` expressions: - If the hook has no `condition` field, or it is null/empty, treat the hook as executable @@ -148,7 +148,7 @@ Write only `.specify/memory/constitution.md`; do not create or modify template s **Check for extension hooks (after constitution update)**: Check if `.specify/extensions.yml` exists in the project root. - If it exists, read it and look for entries under the `hooks.after_constitution` key -- If the YAML cannot be parsed or is invalid, skip hook checking silently and continue normally +- If the YAML cannot be parsed or is invalid, do not skip silently: tell the user that `.specify/extensions.yml` could not be read (include the parser error) and that no hooks were checked, including any mandatory (`optional: false`) hooks registered there, then continue normally - Filter out hooks where `enabled` is explicitly `false`. Treat hooks without an `enabled` field as enabled by default. - For each remaining hook, do **not** attempt to interpret or evaluate hook `condition` expressions: - If the hook has no `condition` field, or it is null/empty, treat the hook as executable diff --git a/templates/commands/converge.md b/templates/commands/converge.md index a177c31371..5d29b74db6 100644 --- a/templates/commands/converge.md +++ b/templates/commands/converge.md @@ -20,7 +20,7 @@ You **MUST** consider the user input before proceeding (if not empty). - Check if `.specify/extensions.yml` exists in the project root. - If it exists, read it and look for entries under the `hooks.before_converge` key -- If the YAML cannot be parsed or is invalid, skip hook checking silently and continue normally +- If the YAML cannot be parsed or is invalid, do not skip silently: tell the user that `.specify/extensions.yml` could not be read (include the parser error) and that no hooks were checked, including any mandatory (`optional: false`) hooks registered there, then continue normally - Filter out hooks where `enabled` is explicitly `false`. Treat hooks without an `enabled` field as enabled by default. - For each remaining hook, do **not** attempt to interpret or evaluate hook `condition` expressions: - If the hook has no `condition` field, or it is null/empty, treat the hook as executable @@ -238,7 +238,7 @@ Append to the **end** of `tasks.md`, per the append contract: After producing the result, check if `.specify/extensions.yml` exists in the project root. - If it exists, read it and look for entries under the `hooks.after_converge` key -- If the YAML cannot be parsed or is invalid, skip hook checking silently and continue normally +- If the YAML cannot be parsed or is invalid, do not skip silently: tell the user that `.specify/extensions.yml` could not be read (include the parser error) and that no hooks were checked, including any mandatory (`optional: false`) hooks registered there, then continue normally - Filter out hooks where `enabled` is explicitly `false`. Treat hooks without an `enabled` field as enabled by default. - For each remaining hook, do **not** attempt to interpret or evaluate hook `condition` expressions: - If the hook has no `condition` field, or it is null/empty, treat the hook as executable diff --git a/templates/commands/implement.md b/templates/commands/implement.md index 742c45e185..f98ba525de 100644 --- a/templates/commands/implement.md +++ b/templates/commands/implement.md @@ -19,7 +19,7 @@ You **MUST** consider the user input before proceeding (if not empty). **Check for extension hooks (before implementation)**: - Check if `.specify/extensions.yml` exists in the project root. - If it exists, read it and look for entries under the `hooks.before_implement` key -- If the YAML cannot be parsed or is invalid, skip hook checking silently and continue normally +- If the YAML cannot be parsed or is invalid, do not skip silently: tell the user that `.specify/extensions.yml` could not be read (include the parser error) and that no hooks were checked, including any mandatory (`optional: false`) hooks registered there, then continue normally - Filter out hooks where `enabled` is explicitly `false`. Treat hooks without an `enabled` field as enabled by default. - For each remaining hook, do **not** attempt to interpret or evaluate hook `condition` expressions: - If the hook has no `condition` field, or it is null/empty, treat the hook as executable @@ -183,7 +183,7 @@ Note: This command assumes a complete task breakdown exists in tasks.md. If task Check if `.specify/extensions.yml` exists in the project root. - If it does not exist, or no hooks are registered under `hooks.after_implement`, skip to the Completion Report. - If it exists, read it and look for entries under the `hooks.after_implement` key. -- If the YAML cannot be parsed or is invalid, skip hook checking silently and continue to the Completion Report. +- If the YAML cannot be parsed or is invalid, do not skip silently: tell the user that `.specify/extensions.yml` could not be read (include the parser error) and that no hooks were checked, including any mandatory (`optional: false`) hooks registered there, then continue to the Completion Report. - Filter out hooks where `enabled` is explicitly `false`. Treat hooks without an `enabled` field as enabled by default. - For each remaining hook, do **not** attempt to interpret or evaluate hook `condition` expressions: - If the hook has no `condition` field, or it is null/empty, treat the hook as executable diff --git a/templates/commands/plan.md b/templates/commands/plan.md index 836e25070c..d559e517ae 100644 --- a/templates/commands/plan.md +++ b/templates/commands/plan.md @@ -27,7 +27,7 @@ You **MUST** consider the user input before proceeding (if not empty). **Check for extension hooks (before planning)**: - Check if `.specify/extensions.yml` exists in the project root. - If it exists, read it and look for entries under the `hooks.before_plan` key -- If the YAML cannot be parsed or is invalid, skip hook checking silently and continue normally +- If the YAML cannot be parsed or is invalid, do not skip silently: tell the user that `.specify/extensions.yml` could not be read (include the parser error) and that no hooks were checked, including any mandatory (`optional: false`) hooks registered there, then continue normally - Filter out hooks where `enabled` is explicitly `false`. Treat hooks without an `enabled` field as enabled by default. - For each remaining hook, do **not** attempt to interpret or evaluate hook `condition` expressions: - If the hook has no `condition` field, or it is null/empty, treat the hook as executable @@ -78,7 +78,7 @@ You **MUST** consider the user input before proceeding (if not empty). Check if `.specify/extensions.yml` exists in the project root. - If it does not exist, or no hooks are registered under `hooks.after_plan`, skip to the Completion Report. - If it exists, read it and look for entries under the `hooks.after_plan` key. -- If the YAML cannot be parsed or is invalid, skip hook checking silently and continue to the Completion Report. +- If the YAML cannot be parsed or is invalid, do not skip silently: tell the user that `.specify/extensions.yml` could not be read (include the parser error) and that no hooks were checked, including any mandatory (`optional: false`) hooks registered there, then continue to the Completion Report. - Filter out hooks where `enabled` is explicitly `false`. Treat hooks without an `enabled` field as enabled by default. - For each remaining hook, do **not** attempt to interpret or evaluate hook `condition` expressions: - If the hook has no `condition` field, or it is null/empty, treat the hook as executable diff --git a/templates/commands/specify.md b/templates/commands/specify.md index 54151e8b42..52b2b24565 100644 --- a/templates/commands/specify.md +++ b/templates/commands/specify.md @@ -23,7 +23,7 @@ You **MUST** consider the user input before proceeding (if not empty). **Check for extension hooks (before specification)**: - Check if `.specify/extensions.yml` exists in the project root. - If it exists, read it and look for entries under the `hooks.before_specify` key -- If the YAML cannot be parsed or is invalid, skip hook checking silently and continue normally +- If the YAML cannot be parsed or is invalid, do not skip silently: tell the user that `.specify/extensions.yml` could not be read (include the parser error) and that no hooks were checked, including any mandatory (`optional: false`) hooks registered there, then continue normally - Filter out hooks where `enabled` is explicitly `false`. Treat hooks without an `enabled` field as enabled by default. - For each remaining hook, do **not** attempt to interpret or evaluate hook `condition` expressions: - If the hook has no `condition` field, or it is null/empty, treat the hook as executable @@ -240,7 +240,7 @@ Given that feature description, do this: Check if `.specify/extensions.yml` exists in the project root. - If it does not exist, or no hooks are registered under `hooks.after_specify`, skip to the Completion Report. - If it exists, read it and look for entries under the `hooks.after_specify` key. -- If the YAML cannot be parsed or is invalid, skip hook checking silently and continue to the Completion Report. +- If the YAML cannot be parsed or is invalid, do not skip silently: tell the user that `.specify/extensions.yml` could not be read (include the parser error) and that no hooks were checked, including any mandatory (`optional: false`) hooks registered there, then continue to the Completion Report. - Filter out hooks where `enabled` is explicitly `false`. Treat hooks without an `enabled` field as enabled by default. - For each remaining hook, do **not** attempt to interpret or evaluate hook `condition` expressions: - If the hook has no `condition` field, or it is null/empty, treat the hook as executable diff --git a/templates/commands/tasks.md b/templates/commands/tasks.md index 64146a35aa..dedd87dd9d 100644 --- a/templates/commands/tasks.md +++ b/templates/commands/tasks.md @@ -28,7 +28,7 @@ You **MUST** consider the user input before proceeding (if not empty). **Check for extension hooks (before tasks generation)**: - Check if `.specify/extensions.yml` exists in the project root. - If it exists, read it and look for entries under the `hooks.before_tasks` key -- If the YAML cannot be parsed or is invalid, skip hook checking silently and continue normally +- If the YAML cannot be parsed or is invalid, do not skip silently: tell the user that `.specify/extensions.yml` could not be read (include the parser error) and that no hooks were checked, including any mandatory (`optional: false`) hooks registered there, then continue normally - Filter out hooks where `enabled` is explicitly `false`. Treat hooks without an `enabled` field as enabled by default. - For each remaining hook, do **not** attempt to interpret or evaluate hook `condition` expressions: - If the hook has no `condition` field, or it is null/empty, treat the hook as executable @@ -99,7 +99,7 @@ You **MUST** consider the user input before proceeding (if not empty). Check if `.specify/extensions.yml` exists in the project root. - If it does not exist, or no hooks are registered under `hooks.after_tasks`, skip to the Completion Report. - If it exists, read it and look for entries under the `hooks.after_tasks` key. -- If the YAML cannot be parsed or is invalid, skip hook checking silently and continue to the Completion Report. +- If the YAML cannot be parsed or is invalid, do not skip silently: tell the user that `.specify/extensions.yml` could not be read (include the parser error) and that no hooks were checked, including any mandatory (`optional: false`) hooks registered there, then continue to the Completion Report. - Filter out hooks where `enabled` is explicitly `false`. Treat hooks without an `enabled` field as enabled by default. - For each remaining hook, do **not** attempt to interpret or evaluate hook `condition` expressions: - If the hook has no `condition` field, or it is null/empty, treat the hook as executable diff --git a/templates/commands/taskstoissues.md b/templates/commands/taskstoissues.md index 36c12316e5..f982448906 100644 --- a/templates/commands/taskstoissues.md +++ b/templates/commands/taskstoissues.md @@ -20,7 +20,7 @@ You **MUST** consider the user input before proceeding (if not empty). **Check for extension hooks (before tasks-to-issues conversion)**: - Check if `.specify/extensions.yml` exists in the project root. - If it exists, read it and look for entries under the `hooks.before_taskstoissues` key -- If the YAML cannot be parsed or is invalid, skip hook checking silently and continue normally +- If the YAML cannot be parsed or is invalid, do not skip silently: tell the user that `.specify/extensions.yml` could not be read (include the parser error) and that no hooks were checked, including any mandatory (`optional: false`) hooks registered there, then continue normally - Filter out hooks where `enabled` is explicitly `false`. Treat hooks without an `enabled` field as enabled by default. - For each remaining hook, do **not** attempt to interpret or evaluate hook `condition` expressions: - If the hook has no `condition` field, or it is null/empty, treat the hook as executable @@ -77,7 +77,7 @@ git config --get remote.origin.url **Check for extension hooks (after tasks-to-issues conversion)**: Check if `.specify/extensions.yml` exists in the project root. - If it exists, read it and look for entries under the `hooks.after_taskstoissues` key -- If the YAML cannot be parsed or is invalid, skip hook checking silently and continue normally +- If the YAML cannot be parsed or is invalid, do not skip silently: tell the user that `.specify/extensions.yml` could not be read (include the parser error) and that no hooks were checked, including any mandatory (`optional: false`) hooks registered there, then continue normally - Filter out hooks where `enabled` is explicitly `false`. Treat hooks without an `enabled` field as enabled by default. - For each remaining hook, do **not** attempt to interpret or evaluate hook `condition` expressions: - If the hook has no `condition` field, or it is null/empty, treat the hook as executable diff --git a/tests/test_command_template_hooks.py b/tests/test_command_template_hooks.py new file mode 100644 index 0000000000..3359ccf2ff --- /dev/null +++ b/tests/test_command_template_hooks.py @@ -0,0 +1,70 @@ +"""Command templates must not tell the agent to skip hook checking silently. + +Every core command template reads ``.specify/extensions.yml`` before and after +its main work, looking for ``hooks.before_*`` / ``hooks.after_*`` entries. A +manifest that could not be parsed used to be treated exactly like a manifest +with no hooks: the agent was told to "skip hook checking silently and continue +normally". A mandatory hook (``optional: false``, the kind the bundled ``git`` +extension registers) could therefore be disabled by a single malformed line, +and nothing would say so. + +These tests pin the replacement wording: an unreadable manifest is reported to +the user (the parser error, and the fact that no hooks were checked) before +the command continues. They read the templates as text on purpose: the +behaviour lives in the prompt, so the prompt is what must be checked. +""" + +import re +from pathlib import Path + +import pytest + +REPO_ROOT = Path(__file__).parent.parent +TEMPLATES_DIR = REPO_ROOT / "templates" / "commands" + +_HOOK_KEY = re.compile(r"`hooks\.(before|after)_[a-z_]+`") +_PARSE_FAILURE_LINE = re.compile( + r"^.*If the YAML cannot be parsed or is invalid.*$", re.MULTILINE +) +_SILENT = "skip hook checking silently" +_REPORTED = ("could not be read", "no hooks were checked") + +HOOK_TEMPLATES = sorted( + p.name + for p in TEMPLATES_DIR.glob("*.md") + if _HOOK_KEY.search(p.read_text(encoding="utf-8")) +) + + +def test_hook_templates_discovered(): + # Guard: the glob must find the templates that read extensions.yml, + # otherwise the parametrized tests below would pass by vacuity. + assert {"specify.md", "plan.md", "tasks.md", "implement.md"} <= set( + HOOK_TEMPLATES + ) + + +@pytest.mark.parametrize("name", HOOK_TEMPLATES) +def test_unreadable_manifest_is_never_skipped_silently(name: str): + text = (TEMPLATES_DIR / name).read_text(encoding="utf-8") + assert _SILENT not in text, ( + f"{name}: an unreadable .specify/extensions.yml may still be skipped " + "silently, which disables mandatory hooks without saying so" + ) + + +@pytest.mark.parametrize("name", HOOK_TEMPLATES) +def test_every_parse_failure_line_reports_before_continuing(name: str): + text = (TEMPLATES_DIR / name).read_text(encoding="utf-8") + lines = _PARSE_FAILURE_LINE.findall(text) + # One line for the before-hook check, one for the after-hook check. + assert len(lines) >= 2, ( + f"{name}: expected a parse-failure instruction at both hook sites, " + f"found {len(lines)}" + ) + for line in lines: + for phrase in _REPORTED: + assert phrase in line, ( + f"{name}: parse-failure instruction does not tell the user " + f"{phrase!r}: {line.strip()}" + ) From 108b9e8a3bd8d52fdb5850bc09b8a061377e9cd0 Mon Sep 17 00:00:00 2001 From: Artap Date: Sun, 6 Sep 2026 20:53:16 +0200 Subject: [PATCH 2/2] test(templates): pin every clause of the parse-failure instruction 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. --- tests/test_command_template_hooks.py | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/tests/test_command_template_hooks.py b/tests/test_command_template_hooks.py index 3359ccf2ff..d1c7677bae 100644 --- a/tests/test_command_template_hooks.py +++ b/tests/test_command_template_hooks.py @@ -27,7 +27,17 @@ r"^.*If the YAML cannot be parsed or is invalid.*$", re.MULTILINE ) _SILENT = "skip hook checking silently" -_REPORTED = ("could not be read", "no hooks were checked") +# Every clause of the replacement instruction, so that dropping any one of +# them from the templates fails the test: the manifest could not be read, the +# parser error is shown, no hooks were checked, mandatory hooks are named, and +# the command still continues afterwards. +_REPORTED = ( + "could not be read", + "include the parser error", + "no hooks were checked", + "including any mandatory (`optional: false`) hooks", + "then continue", +) HOOK_TEMPLATES = sorted( p.name