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..d1c7677bae --- /dev/null +++ b/tests/test_command_template_hooks.py @@ -0,0 +1,80 @@ +"""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" +# 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 + 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()}" + )