From 1838e74deac60866a85c9658f3cebd4e8cc0a446 Mon Sep 17 00:00:00 2001 From: Nguyen Thanh Dat Date: Tue, 25 Aug 2026 19:25:19 +0700 Subject: [PATCH 1/7] fix(implement): count checkbox markers outside code fences only The checklist gate counted every `- [ ]` / `- [x]` line in every checklist file, fenced blocks included. A checklist that documents the checkbox format with an example fence therefore reported unchecked items nobody can ever tick, and /speckit-implement stops on a non-zero unchecked count -- so writing down the format blocked implementation. /speckit-clarify already scopes its scan to markers outside code fences, so this was also the two commands disagreeing about what a checklist item is. They now state the same rule. Closes #4272 --- templates/commands/implement.md | 7 +-- tests/unit/test_checklist_scan_contract.py | 58 ++++++++++++++++++++++ 2 files changed, 62 insertions(+), 3 deletions(-) create mode 100644 tests/unit/test_checklist_scan_contract.py diff --git a/templates/commands/implement.md b/templates/commands/implement.md index f98ba525de..2fd7c4854f 100644 --- a/templates/commands/implement.md +++ b/templates/commands/implement.md @@ -58,10 +58,11 @@ You **MUST** consider the user input before proceeding (if not empty). - `checklists/requirements.md` is the built-in spec-quality checklist maintained by `__SPECKIT_COMMAND_SPECIFY__` and `__SPECKIT_COMMAND_CLARIFY__`; custom checklists generated by `__SPECKIT_COMMAND_CHECKLIST__` are reviewer-owned requirements-quality review artifacts - For custom checklists, `[x]` means the reviewer determined the requirements-quality criterion is satisfied; it does NOT mean implementation work is complete - Scan all checklist files in the checklists/ directory + - Count only checkbox lines **outside of code fences**, the same rule `__SPECKIT_COMMAND_CLARIFY__` applies. A checklist that documents the checkbox format inside a fence is showing an example, not tracking work, and counting those examples blocks implementation on items nobody can ever tick - For each checklist, count: - - Total items: All lines matching `- [ ]` or `- [X]` or `- [x]` - - Checked items: Lines matching `- [X]` or `- [x]` - - Unchecked items: Lines matching `- [ ]` + - Total items: All lines matching `- [ ]` or `- [X]` or `- [x]` outside code fences + - Checked items: Lines matching `- [X]` or `- [x]` outside code fences + - Unchecked items: Lines matching `- [ ]` outside code fences - Create a status table: ```text diff --git a/tests/unit/test_checklist_scan_contract.py b/tests/unit/test_checklist_scan_contract.py new file mode 100644 index 0000000000..3e961162df --- /dev/null +++ b/tests/unit/test_checklist_scan_contract.py @@ -0,0 +1,58 @@ +"""Every command that scans checkbox markers must say it skips code fences. + +A checklist is free to *document* the checkbox format inside a fenced block. Counting +those example markers reports items nobody can tick, and `/speckit-implement` treats a +non-zero unchecked count as a reason to stop — so an example fence blocks implementation +(#4272). `/speckit-clarify` already scoped its scan to markers outside code fences; this +keeps the two commands from drifting apart again, and holds any future command that +starts counting markers to the same rule. +""" + +from __future__ import annotations + +import re +from pathlib import Path + +import pytest + +PROJECT_ROOT = Path(__file__).resolve().parent.parent.parent +COMMAND_DIRS = [ + PROJECT_ROOT / "templates" / "commands", + *sorted((PROJECT_ROOT / "presets").glob("*/commands")), +] + +# The instruction that tells the agent which lines are checkbox markers. Written to catch +# the phrasing both commands use rather than one exact sentence. +SCAN_INSTRUCTION = re.compile(r"lines matching\s+`- \[ \]`", re.IGNORECASE) +FENCE_EXCLUSION = re.compile(r"outside\s+(?:of\s+)?code\s+fences", re.IGNORECASE) + + +def scan_instructions() -> list[tuple[Path, int, str]]: + """Every line in a command template that defines what counts as a checkbox marker.""" + found: list[tuple[Path, int, str]] = [] + for directory in COMMAND_DIRS: + if not directory.is_dir(): + continue + for path in sorted(directory.glob("*.md")): + for number, line in enumerate(path.read_text(encoding="utf-8").splitlines(), start=1): + if SCAN_INSTRUCTION.search(line): + found.append((path, number, line)) + return found + + +def test_the_contract_is_actually_stated_somewhere() -> None: + """Guard against the regex silently matching nothing and the test passing vacuously.""" + assert scan_instructions(), "no command template defines a checkbox-marker scan any more" + + +@pytest.mark.parametrize( + ("path", "number", "line"), + scan_instructions(), + ids=lambda value: value.name if isinstance(value, Path) else str(value), +) +def test_marker_scans_exclude_code_fences(path: Path, number: int, line: str) -> None: + assert FENCE_EXCLUSION.search(line), ( + f"{path.relative_to(PROJECT_ROOT)}:{number} tells the agent to match checkbox " + f"markers without excluding fenced code blocks, so an example fence is counted " + f"as real work:\n {line.strip()}" + ) From 1b3489f522781941d2f8807fd0e07a331784678b Mon Sep 17 00:00:00 2001 From: Nguyen Thanh Dat Date: Fri, 11 Sep 2026 08:34:46 +0700 Subject: [PATCH 2/7] test(implement): guard the checked-items marker definition too The scan-instruction pattern only recognised a definition whose first marker is unchecked, so implement.md's "Checked items: Lines matching `- [X]`" line was never parametrised. Removing its code-fence exclusion left the suite green. Match a checked or unchecked first marker: four definitions are now guarded instead of three, and that removal fails. --- tests/unit/test_checklist_scan_contract.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/unit/test_checklist_scan_contract.py b/tests/unit/test_checklist_scan_contract.py index 3e961162df..1edf5d48cc 100644 --- a/tests/unit/test_checklist_scan_contract.py +++ b/tests/unit/test_checklist_scan_contract.py @@ -22,8 +22,10 @@ ] # The instruction that tells the agent which lines are checkbox markers. Written to catch -# the phrasing both commands use rather than one exact sentence. -SCAN_INSTRUCTION = re.compile(r"lines matching\s+`- \[ \]`", re.IGNORECASE) +# the phrasing both commands use rather than one exact sentence. The first marker may be +# checked or unchecked: `/speckit-implement` defines its checked count on `- [X]` alone, +# and that definition needs the same exclusion as the other two. +SCAN_INSTRUCTION = re.compile(r"lines matching\s+`- \[[ xX]\]`", re.IGNORECASE) FENCE_EXCLUSION = re.compile(r"outside\s+(?:of\s+)?code\s+fences", re.IGNORECASE) From b6d1c3bc7b5850c1d235a3f1da092443d46bb950 Mon Sep 17 00:00:00 2001 From: Nguyen Thanh Dat Date: Tue, 25 Aug 2026 19:39:39 +0700 Subject: [PATCH 3/7] fix(taskstoissues): scope issue dedup to the feature the tasks belong to Task IDs are local to a feature -- every tasks.md restarts at T001 -- but the dedup matched existing issues on the bare ID. So once feature 001-auth had an issue titled T001, running the command for 002-billing saw "T001 exists" and skipped it. The task was never created and nothing said so, which is a silent gap in exactly the multi-feature repos this command targets. The canonical title now carries the feature directory basename, and a task is skipped only when an existing issue matches both that identifier and the ID. The ID keeps its own word boundaries inside the prefixed title, so the \bT\d{3,}\b matching from #2968 is unchanged. Issues filed before the prefix existed carry a bare `T001: ...`; those are still recognised for their own feature, so upgrading does not re-create work that is already tracked. Closes #4271 --- templates/commands/taskstoissues.md | 7 ++- .../unit/test_taskstoissues_feature_scope.py | 62 +++++++++++++++++++ 2 files changed, 66 insertions(+), 3 deletions(-) create mode 100644 tests/unit/test_taskstoissues_feature_scope.py diff --git a/templates/commands/taskstoissues.md b/templates/commands/taskstoissues.md index f982448906..dc279d89dd 100644 --- a/templates/commands/taskstoissues.md +++ b/templates/commands/taskstoissues.md @@ -64,9 +64,10 @@ git config --get remote.origin.url > [!CAUTION] > ONLY PROCEED TO NEXT STEPS IF THE REMOTE IS A GITHUB URL -1. **Fetch existing issues for deduplication**: Before creating anything, build the set of task IDs you are about to process from `tasks.md` (each is a `T` followed by **at least** three digits, e.g. `T001` — `__SPECKIT_COMMAND_CONVERGE__` assigns new IDs with `T{M+1:03d}`, which is a floor rather than a cap, so once a file has more than 999 tasks the IDs are four digits or longer). Then use the GitHub MCP server's `list_issues` tool to look for issues that already cover those IDs. Do not pass a `state` value, since omitting it makes the tool return both open and closed issues. Request `perPage: 100` to keep the number of calls down, and since the tool uses cursor-based pagination, request pages with the `after` parameter (using the `endCursor` from the previous response). For each issue title, match it against the task ID pattern `\bT\d{3,}\b` (the `{3,}` accepts four-digit and longer IDs — with `\d{3}` a title containing `T1000` would not match at all, because the trailing `\b` cannot fall between two digits, so that task would be silently neither deduplicated nor created; word boundaries still stop a token like `ST001` from matching, and force the whole digit run to be consumed so `T100` can never match inside `T1000`; this also recognises titles written as `T001 ...`, `T001: ...` or `[T001] ...`) and, when it matches one of your task IDs, mark that ID as already having an issue. Stop paginating as soon as every task ID has been matched, or when there are no more pages, so you do not keep fetching the whole repository's issue history once all task IDs are accounted for. This bounds the number of calls on repos with large issue histories and still prevents duplicates when the command is re-run after `tasks.md` is regenerated or the skill is re-invoked. -1. For each task in the list, use the GitHub MCP server to create a new issue in the repository that is representative of the Git remote. Task lines in `tasks.md` start with a markdown checkbox, so first strip the leading `- [ ]` (and any `[P]` / `[US#]` markers) to recover the task ID and its description. Create the issue with a single canonical title of the form `T001: `, with the ID written once followed by the task description (for example, the line `- [ ] T001 Create project structure` becomes the title `T001: Create project structure`). - - **Skip** any task whose ID is already present in the set of existing issues from the previous step, and report it (for example, `T001 already has an issue, skipping`). +1. **Fetch existing issues for deduplication**: Before creating anything, build the set of task IDs you are about to process from `tasks.md` (each is a `T` followed by **at least** three digits, e.g. `T001` — `__SPECKIT_COMMAND_CONVERGE__` assigns new IDs with `T{M+1:03d}`, which is a floor rather than a cap, so once a file has more than 999 tasks the IDs are four digits or longer). Then use the GitHub MCP server's `list_issues` tool to look for issues that already cover those IDs. Do not pass a `state` value, since omitting it makes the tool return both open and closed issues. Request `perPage: 100` to keep the number of calls down, and since the tool uses cursor-based pagination, request pages with the `after` parameter (using the `endCursor` from the previous response). For each issue title, match it against the task ID pattern `\bT\d{3,}\b` (the `{3,}` accepts four-digit and longer IDs — with `\d{3}` a title containing `T1000` would not match at all, because the trailing `\b` cannot fall between two digits, so that task would be silently neither deduplicated nor created; word boundaries still stop a token like `ST001` from matching, and force the whole digit run to be consumed so `T100` can never match inside `T1000`; this also recognises titles written as `T001 ...`, `T001: ...` or `[T001] ...`) and, when it matches one of your task IDs, mark that ID as already having an issue **only if the title also carries this feature's identifier** (see below). Task IDs restart at `T001` in every feature's `tasks.md`, so an unscoped match means the first feature to reach the tracker permanently suppresses `T001` for every later feature -- a silent gap in exactly the multi-feature repos this command is for. Stop paginating as soon as every task ID has been matched, or when there are no more pages, so you do not keep fetching the whole repository's issue history once all task IDs are accounted for. This bounds the number of calls on repos with large issue histories and still prevents duplicates when the command is re-run after `tasks.md` is regenerated or the skill is re-invoked. +1. For each task in the list, use the GitHub MCP server to create a new issue in the repository that is representative of the Git remote. Task lines in `tasks.md` start with a markdown checkbox, so first strip the leading `- [ ]` (and any `[P]` / `[US#]` markers) to recover the task ID and its description. Create the issue with a single canonical title of the form `[] T001: `, where `` is the basename of FEATURE_DIR parsed in step 1 (the `NNN-name` spec directory, e.g. `002-billing`), followed by the ID written once and then the task description (for example, the line `- [ ] T001 Create project structure` in feature `002-billing` becomes the title `[002-billing] T001: Create project structure`). The ID keeps its own word boundaries, so the `T\d{3,}` matching above is unchanged by the prefix. + - **Skip** a task only when an existing issue matches **both** this feature's identifier and the task ID, and report it (for example, `[002-billing] T001 already has an issue, skipping`). A `T001` belonging to another feature is a different task and must not suppress this one. + - Issues created before this scoping exists carry a bare `T001: ...` title. Treat those as matching only when no `[]` prefix is present anywhere in the fetched set for that ID, so an upgrade does not re-create issues that are already tracked. - Only create issues for tasks that do not yet have a matching issue. > [!CAUTION] diff --git a/tests/unit/test_taskstoissues_feature_scope.py b/tests/unit/test_taskstoissues_feature_scope.py new file mode 100644 index 0000000000..29c6981fe5 --- /dev/null +++ b/tests/unit/test_taskstoissues_feature_scope.py @@ -0,0 +1,62 @@ +"""Issue dedup in `/speckit-taskstoissues` must be scoped to one feature. + +Task IDs are local to a feature: every `tasks.md` restarts at `T001`. Matching existing +issues by task ID alone means the first feature to reach the tracker permanently +suppresses `T001` for every later feature — the tasks are silently never created, which +is worse than the duplicates the matching was tightened to prevent (#4271). + +These assert the template still carries the scoping, so a later edit to that step cannot +quietly drop it again. +""" + +from __future__ import annotations + +import re +from pathlib import Path + +import pytest + +PROJECT_ROOT = Path(__file__).resolve().parent.parent.parent +TEMPLATE = PROJECT_ROOT / "templates" / "commands" / "taskstoissues.md" + + +@pytest.fixture(scope="module") +def template_text() -> str: + assert TEMPLATE.is_file(), f"missing command template: {TEMPLATE}" + return TEMPLATE.read_text(encoding="utf-8") + + +def _line_containing(text: str, needle: str) -> str: + matches = [line for line in text.splitlines() if needle in line] + assert matches, f"no instruction line contains {needle!r} any more" + return "\n".join(matches) + + +def test_the_canonical_title_carries_the_feature_identifier(template_text: str) -> None: + """Without the feature in the title there is nothing for the dedup to scope on.""" + title_rule = _line_containing(template_text, "canonical title") + assert re.search(r"`\[\]\s+T001:", title_rule), ( + "the canonical issue title no longer names the feature, so two features' T001 " + f"issues are indistinguishable:\n {title_rule.strip()}" + ) + assert "FEATURE_DIR" in title_rule, ( + "the title rule should say where comes from (the FEATURE_DIR parsed in " + f"step 1):\n {title_rule.strip()}" + ) + + +def test_the_skip_rule_requires_both_feature_and_task_id(template_text: str) -> None: + skip_rule = _line_containing(template_text, "**Skip**") + assert "both" in skip_rule.lower(), ( + "the skip rule must require the feature identity as well as the task ID, or a " + f"sibling feature's T001 suppresses this one:\n {skip_rule.strip()}" + ) + assert "feature" in skip_rule.lower(), skip_rule.strip() + + +def test_pre_existing_unscoped_issues_are_still_recognised(template_text: str) -> None: + """Upgrading must not re-create issues that were filed before the prefix existed.""" + assert re.search(r"before this scoping exists|bare `T001", template_text), ( + "the template no longer says what to do with issues created before the feature " + "prefix, so an upgrade would duplicate every already-tracked task" + ) From 9d5c1f1d24fca0ef741958d231d68302e64011a0 Mon Sep 17 00:00:00 2001 From: Nguyen Thanh Dat Date: Wed, 2 Sep 2026 22:05:49 +0700 Subject: [PATCH 4/7] fix(taskstoissues): write \b as two characters, not a backspace byte MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The sentence added in the previous commit — "so the `\bT\d{3,}\b` matching above is unchanged by the prefix" — reached the file with two literal U+0008 BACKSPACE bytes where `\b` was meant. My editing pipeline interpreted the escape rather than passing it through. A control character cannot appear in a YAML block scalar, so the generator fell back to a double-quoted flow scalar for the whole prompt, and `test_yaml_has_prompt` failed on `speckit.taskstoissues.yaml`: AssertionError: speckit.taskstoissues.yaml missing prompt block scalar Reproduced and pinned locally: with the two bytes present the test fails with that exact message, and with them written as `\b` the goose suite is 38/38. The rendered sentence is unchanged — it was always meant to read `\b`. --- templates/commands/taskstoissues.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/templates/commands/taskstoissues.md b/templates/commands/taskstoissues.md index dc279d89dd..d830857f4a 100644 --- a/templates/commands/taskstoissues.md +++ b/templates/commands/taskstoissues.md @@ -65,7 +65,7 @@ git config --get remote.origin.url > ONLY PROCEED TO NEXT STEPS IF THE REMOTE IS A GITHUB URL 1. **Fetch existing issues for deduplication**: Before creating anything, build the set of task IDs you are about to process from `tasks.md` (each is a `T` followed by **at least** three digits, e.g. `T001` — `__SPECKIT_COMMAND_CONVERGE__` assigns new IDs with `T{M+1:03d}`, which is a floor rather than a cap, so once a file has more than 999 tasks the IDs are four digits or longer). Then use the GitHub MCP server's `list_issues` tool to look for issues that already cover those IDs. Do not pass a `state` value, since omitting it makes the tool return both open and closed issues. Request `perPage: 100` to keep the number of calls down, and since the tool uses cursor-based pagination, request pages with the `after` parameter (using the `endCursor` from the previous response). For each issue title, match it against the task ID pattern `\bT\d{3,}\b` (the `{3,}` accepts four-digit and longer IDs — with `\d{3}` a title containing `T1000` would not match at all, because the trailing `\b` cannot fall between two digits, so that task would be silently neither deduplicated nor created; word boundaries still stop a token like `ST001` from matching, and force the whole digit run to be consumed so `T100` can never match inside `T1000`; this also recognises titles written as `T001 ...`, `T001: ...` or `[T001] ...`) and, when it matches one of your task IDs, mark that ID as already having an issue **only if the title also carries this feature's identifier** (see below). Task IDs restart at `T001` in every feature's `tasks.md`, so an unscoped match means the first feature to reach the tracker permanently suppresses `T001` for every later feature -- a silent gap in exactly the multi-feature repos this command is for. Stop paginating as soon as every task ID has been matched, or when there are no more pages, so you do not keep fetching the whole repository's issue history once all task IDs are accounted for. This bounds the number of calls on repos with large issue histories and still prevents duplicates when the command is re-run after `tasks.md` is regenerated or the skill is re-invoked. -1. For each task in the list, use the GitHub MCP server to create a new issue in the repository that is representative of the Git remote. Task lines in `tasks.md` start with a markdown checkbox, so first strip the leading `- [ ]` (and any `[P]` / `[US#]` markers) to recover the task ID and its description. Create the issue with a single canonical title of the form `[] T001: `, where `` is the basename of FEATURE_DIR parsed in step 1 (the `NNN-name` spec directory, e.g. `002-billing`), followed by the ID written once and then the task description (for example, the line `- [ ] T001 Create project structure` in feature `002-billing` becomes the title `[002-billing] T001: Create project structure`). The ID keeps its own word boundaries, so the `T\d{3,}` matching above is unchanged by the prefix. +1. For each task in the list, use the GitHub MCP server to create a new issue in the repository that is representative of the Git remote. Task lines in `tasks.md` start with a markdown checkbox, so first strip the leading `- [ ]` (and any `[P]` / `[US#]` markers) to recover the task ID and its description. Create the issue with a single canonical title of the form `[] T001: `, where `` is the basename of FEATURE_DIR parsed in step 1 (the `NNN-name` spec directory, e.g. `002-billing`), followed by the ID written once and then the task description (for example, the line `- [ ] T001 Create project structure` in feature `002-billing` becomes the title `[002-billing] T001: Create project structure`). The ID keeps its own word boundaries, so the `\bT\d{3,}\b` matching above is unchanged by the prefix. - **Skip** a task only when an existing issue matches **both** this feature's identifier and the task ID, and report it (for example, `[002-billing] T001 already has an issue, skipping`). A `T001` belonging to another feature is a different task and must not suppress this one. - Issues created before this scoping exists carry a bare `T001: ...` title. Treat those as matching only when no `[]` prefix is present anywhere in the fetched set for that ID, so an upgrade does not re-create issues that are already tracked. - Only create issues for tasks that do not yet have a matching issue. From 50fbc4a9ab48e29b9e9d68e2b1692835eaff2a74 Mon Sep 17 00:00:00 2001 From: Nguyen Thanh Dat Date: Fri, 11 Sep 2026 08:36:46 +0700 Subject: [PATCH 5/7] fix(taskstoissues): ask about a bare legacy title instead of guessing The upgrade rule treated a bare `T001: ...` title as this feature's whenever no scoped title existed for that ID. That is exactly the state of every feature on its first run after upgrading, so a bare T001 filed for 001-auth still suppressed 002-billing's T001: the #4271 skip, back. A bare title names no feature, and neither does the absence of a scoped one, so no inference from the tracker can settle it. Skipping drops a task silently; creating duplicates one an existing user already tracks. The command now lists every ID that matched only a bare title, with the issue and this feature's description for the task, and asks before creating anything. Confirmed issues are skipped, and retitled to the scoped form only if the user agrees, so the question does not recur. --- templates/commands/taskstoissues.md | 2 +- .../unit/test_taskstoissues_feature_scope.py | 30 +++++++++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/templates/commands/taskstoissues.md b/templates/commands/taskstoissues.md index d830857f4a..484b3bfe8e 100644 --- a/templates/commands/taskstoissues.md +++ b/templates/commands/taskstoissues.md @@ -67,7 +67,7 @@ git config --get remote.origin.url 1. **Fetch existing issues for deduplication**: Before creating anything, build the set of task IDs you are about to process from `tasks.md` (each is a `T` followed by **at least** three digits, e.g. `T001` — `__SPECKIT_COMMAND_CONVERGE__` assigns new IDs with `T{M+1:03d}`, which is a floor rather than a cap, so once a file has more than 999 tasks the IDs are four digits or longer). Then use the GitHub MCP server's `list_issues` tool to look for issues that already cover those IDs. Do not pass a `state` value, since omitting it makes the tool return both open and closed issues. Request `perPage: 100` to keep the number of calls down, and since the tool uses cursor-based pagination, request pages with the `after` parameter (using the `endCursor` from the previous response). For each issue title, match it against the task ID pattern `\bT\d{3,}\b` (the `{3,}` accepts four-digit and longer IDs — with `\d{3}` a title containing `T1000` would not match at all, because the trailing `\b` cannot fall between two digits, so that task would be silently neither deduplicated nor created; word boundaries still stop a token like `ST001` from matching, and force the whole digit run to be consumed so `T100` can never match inside `T1000`; this also recognises titles written as `T001 ...`, `T001: ...` or `[T001] ...`) and, when it matches one of your task IDs, mark that ID as already having an issue **only if the title also carries this feature's identifier** (see below). Task IDs restart at `T001` in every feature's `tasks.md`, so an unscoped match means the first feature to reach the tracker permanently suppresses `T001` for every later feature -- a silent gap in exactly the multi-feature repos this command is for. Stop paginating as soon as every task ID has been matched, or when there are no more pages, so you do not keep fetching the whole repository's issue history once all task IDs are accounted for. This bounds the number of calls on repos with large issue histories and still prevents duplicates when the command is re-run after `tasks.md` is regenerated or the skill is re-invoked. 1. For each task in the list, use the GitHub MCP server to create a new issue in the repository that is representative of the Git remote. Task lines in `tasks.md` start with a markdown checkbox, so first strip the leading `- [ ]` (and any `[P]` / `[US#]` markers) to recover the task ID and its description. Create the issue with a single canonical title of the form `[] T001: `, where `` is the basename of FEATURE_DIR parsed in step 1 (the `NNN-name` spec directory, e.g. `002-billing`), followed by the ID written once and then the task description (for example, the line `- [ ] T001 Create project structure` in feature `002-billing` becomes the title `[002-billing] T001: Create project structure`). The ID keeps its own word boundaries, so the `\bT\d{3,}\b` matching above is unchanged by the prefix. - **Skip** a task only when an existing issue matches **both** this feature's identifier and the task ID, and report it (for example, `[002-billing] T001 already has an issue, skipping`). A `T001` belonging to another feature is a different task and must not suppress this one. - - Issues created before this scoping exists carry a bare `T001: ...` title. Treat those as matching only when no `[]` prefix is present anywhere in the fetched set for that ID, so an upgrade does not re-create issues that are already tracked. + - Issues created before this scoping exists carry a bare `T001: ...` title, and a bare title carries no feature identity: it cannot show which feature's `T001` it tracks, and the absence of a scoped title for that ID does not show it either. So never decide a bare match on your own, in either direction -- skipping silently drops this feature's task when the issue belongs to another feature, and creating duplicates it when the issue is this feature's. Before creating anything, list every task ID that matched only a bare title, each with the issue number, the issue title and this feature's description for that task, and ask the user which of those issues already track this feature's tasks. Skip the ones the user confirms and create the rest with the scoped title. Then offer to retitle each confirmed issue to the scoped form (`[] T001: ...`) so later runs match it without asking, and retitle only the issues the user agrees to. - Only create issues for tasks that do not yet have a matching issue. > [!CAUTION] diff --git a/tests/unit/test_taskstoissues_feature_scope.py b/tests/unit/test_taskstoissues_feature_scope.py index 29c6981fe5..9120b63210 100644 --- a/tests/unit/test_taskstoissues_feature_scope.py +++ b/tests/unit/test_taskstoissues_feature_scope.py @@ -60,3 +60,33 @@ def test_pre_existing_unscoped_issues_are_still_recognised(template_text: str) - "the template no longer says what to do with issues created before the feature " "prefix, so an upgrade would duplicate every already-tracked task" ) + + +def test_a_bare_title_is_never_decided_on_the_agents_own(template_text: str) -> None: + """A bare `T001: ...` title names no feature, so nothing in it can settle a match. + + Treating it as this feature's whenever no scoped title exists for the ID brings + #4271 straight back on the first run after upgrading: a bare `T001` filed for + `001-auth` suppresses `002-billing`'s `T001` because billing has no scoped issue + yet. Treating it as another feature's duplicates every task an existing user + already tracks. Only the user can say which it is. + """ + legacy_rule = _line_containing(template_text, "before this scoping exists") + assert "ask the user" in legacy_rule, ( + "the rule for pre-prefix issues must hand the ambiguous matches to the user " + f"rather than resolve them:\n {legacy_rule.strip()}" + ) + assert not re.search(r"treat (those|them) as matching", legacy_rule, re.IGNORECASE), ( + "a bare title is being treated as a match on the agent's own inference, which " + f"lets another feature's T001 suppress this one:\n {legacy_rule.strip()}" + ) + assert re.search(r"before creating anything", legacy_rule, re.IGNORECASE), ( + "the question has to come before any issue is created, or the answer can no " + f"longer prevent a duplicate:\n {legacy_rule.strip()}" + ) + + +def test_confirmed_legacy_issues_are_only_retitled_with_consent(template_text: str) -> None: + """Retitling is what stops the question recurring, and it edits the user's issues.""" + legacy_rule = _line_containing(template_text, "before this scoping exists") + assert "retitle" in legacy_rule and "agrees" in legacy_rule, legacy_rule.strip() From e2c0415494c19ca08cce60d13956529b4c8df264 Mon Sep 17 00:00:00 2001 From: Nguyen Thanh Dat Date: Fri, 11 Sep 2026 08:55:36 +0700 Subject: [PATCH 6/7] fix(converge): stop while tasks are unchecked, so a re-run cannot re-append them Converge is meant to assess a finished implementation, and nothing enforced that. Run while tasks.md still had open work, it assessed the code anyway, found that unbuilt work as new gaps and appended it again under fresh IDs, so every re-run duplicated its own remediation tasks (#4269). And a run that found nothing new reported `converged`, whose report says the implementation satisfies the spec while tracked work is still open. Step 1 now checks for unchecked tasks (outside code fences, the rule implement counts by) and stops before any assessment, listing them and pointing to implement, with tasks.md untouched. Once every task is checked, each task joins the intent inventory, so one marked done but not built is a finding traced to its ID. Anything appended makes the next run stop again until it is implemented. The handoff and docs/reference/agentic-sdd.md describe the gate, and the tests pin the lifecycle. Closes #4269 --- docs/reference/agentic-sdd.md | 4 +- templates/commands/converge.md | 25 ++++- tests/unit/test_converge_prerequisite.py | 121 +++++++++++++++++++++++ 3 files changed, 144 insertions(+), 6 deletions(-) create mode 100644 tests/unit/test_converge_prerequisite.py diff --git a/docs/reference/agentic-sdd.md b/docs/reference/agentic-sdd.md index 7c8de58ff0..1c4e1f81f9 100644 --- a/docs/reference/agentic-sdd.md +++ b/docs/reference/agentic-sdd.md @@ -147,7 +147,7 @@ Verify each stage works before moving to the next. ## `/speckit.converge` -Assesses the codebase against the feature's spec, plan, and tasks to confirm nothing was missed. It is **append-only**: it never edits or deletes code, and its only possible write is adding tasks to `tasks.md`. Run it only after `/speckit.implement` has run on the current `tasks.md`. +Assesses the codebase against the feature's spec, plan, and tasks to confirm nothing was missed. It is **append-only**: it never edits or deletes code, and its only possible write is adding tasks to `tasks.md`. Run it only after `/speckit.implement` has completed every task in `tasks.md`: if any task is still unchecked, converge stops before assessing anything, lists the unchecked tasks, and tells you to run `/speckit.implement` first, leaving `tasks.md` unchanged. ```text /speckit.converge @@ -156,7 +156,7 @@ Assesses the codebase against the feature's spec, plan, and tasks to confirm not It first prints a severity-graded findings summary, then resolves to one of two outcomes: - **Converged** — no gaps found. `tasks.md` is left byte-for-byte unchanged and you'll see a clean result like `✅ Converged — the implementation satisfies the spec, plan, and tasks.` You're done; proceed to review or open a PR. -- **Tasks appended** — gaps found. Converge appends them as new tasks under a Convergence section in `tasks.md` and tells you how many. Run `/speckit.implement` again to complete them, then `/speckit.converge` once more. Each pass finds fewer items; repeat until it reports converged. +- **Tasks appended** — gaps found. Converge appends them as new tasks under a Convergence section in `tasks.md` and tells you how many. Run `/speckit.implement` again to complete them, then `/speckit.converge` once more (until they are checked off, converge stops at its prerequisite check rather than appending them a second time). Each pass finds fewer items; repeat until it reports converged. ## `/speckit.taskstoissues` diff --git a/templates/commands/converge.md b/templates/commands/converge.md index f6a44c3e91..f334625625 100644 --- a/templates/commands/converge.md +++ b/templates/commands/converge.md @@ -63,7 +63,7 @@ state of the code, determine which requirements, acceptance criteria, plan decis existing tasks are unmet, incomplete, or only partially satisfied, and **append each piece of remaining work as a new, traceable task** at the bottom of `tasks.md` so that `__SPECKIT_COMMAND_IMPLEMENT__` can complete it. This command MUST run only after -`__SPECKIT_COMMAND_IMPLEMENT__` has run on the current `tasks.md`, and after `__SPECKIT_COMMAND_TASKS__` has produced a complete `tasks.md`. +`__SPECKIT_COMMAND_IMPLEMENT__` has completed every task in the current `tasks.md` (Step 1 checks, and stops if any task is unchecked), and after `__SPECKIT_COMMAND_TASKS__` has produced a complete `tasks.md`. This is **not** a diff tool and does **not** track changes. It assesses the present state of the code relative to the feature's artifacts — no git, no branch comparison, no history. @@ -100,6 +100,17 @@ Run `{SCRIPT}` once from repo root and parse JSON for FEATURE_DIR and AVAILABLE_ If `spec.md`, `plan.md`, or `tasks.md` is missing, STOP with a clear, actionable message naming the prerequisite command to run (`__SPECKIT_COMMAND_SPECIFY__` for a missing spec, `__SPECKIT_COMMAND_PLAN__` for a missing plan, `__SPECKIT_COMMAND_TASKS__` for missing tasks). Do not produce partial output. + +**Enforce the implement prerequisite before assessing anything.** Scan `tasks.md` for +unchecked tasks — lines matching `- [ ]` outside code fences, the rule `__SPECKIT_COMMAND_IMPLEMENT__` counts by — +and if there are any, STOP: report how many are unchecked and list their task IDs, and tell +the user to run `__SPECKIT_COMMAND_IMPLEMENT__` to complete them before converging. Leave +`tasks.md` byte-for-byte unchanged, report no findings, and do not continue to Step 2. This +is neither outcome of Step 7: an unchecked task is work that is already tracked but not yet +built, so assessing the code while one is open would report that same work again as a new +gap, and a run that reached `converged` would claim the implementation is complete while +tracked work remains. Converge assesses a finished implementation; it does not re-plan an +unfinished one. For single quotes in args like "I'm Groot", use escape syntax: e.g 'I'\''m Groot' (or double-quote if possible: "I'm Groot"). ### 2. Load Artifacts (Progressive Disclosure) @@ -137,6 +148,10 @@ Create an internal model (do not echo raw artifacts): - **Requirements inventory**: one stable key per FR-### / SC-### / user-story acceptance scenario (e.g. `US1/AC2`), plus the plan decisions and constitution principles that impose buildable obligations. +- **Task inventory**: every task in `tasks.md` — all of them checked by this point — with + the work it describes and the file paths it names. A task marked done whose work is + absent from the code, or only partly there, is a finding like any other, traced to + that task's ID. - **Code-scope map**: from the file paths named in `plan.md` and `tasks.md`, plus a keyword search for the concepts each requirement describes, derive the set of source files and components in scope for assessment. Bound the assessment to these — do **not** infer @@ -216,7 +231,8 @@ Append to the **end** of `tasks.md`, per the append contract: ``` `` traces the task to its origin: e.g. `FR-003`, `SC-002`, - `US1/AC2`, `plan: storage decision`, `Constitution II`. + `US1/AC2`, `plan: storage decision`, `Constitution II`, or a task ID such as `T017` + when a task marked done is not reflected in the code. `` is one of `missing`, `partial`, `contradicts`, `unrequested`. @@ -234,8 +250,9 @@ Append to the **end** of `tasks.md`, per the append contract: ### 8. Provide Next Actions (Handoff) - On `tasks_appended`: state how many tasks were appended under which phase, and recommend - running `__SPECKIT_COMMAND_IMPLEMENT__` to complete them; note that a follow-up converge - run will find fewer or no remaining items. + running `__SPECKIT_COMMAND_IMPLEMENT__` to complete them; note that converge will stop at + its prerequisite check until those tasks are checked off, and re-assess everything once + they are. - On `converged`: recommend proceeding to review / opening a PR. No further implement pass is needed for this feature's specified scope. diff --git a/tests/unit/test_converge_prerequisite.py b/tests/unit/test_converge_prerequisite.py new file mode 100644 index 0000000000..f2528811ee --- /dev/null +++ b/tests/unit/test_converge_prerequisite.py @@ -0,0 +1,121 @@ +"""`/speckit-converge` assesses a finished implementation, so it enforces that first. + +Run while `tasks.md` still had unchecked tasks, converge assessed the code anyway. The +work those tasks track is not built yet, so it came back as fresh gaps and was appended +again under new IDs — every re-run duplicated its own remediation tasks (#4269). And when +nothing new turned up, the run reported `converged`, telling the user the implementation +was complete while tracked work was still open. + +The fix is the lifecycle, not a dedup rule: converge stops before analysis while any task +is unchecked and sends the user to `/speckit-implement`. Once every task is checked it +assesses the code against every artifact, each task included, and anything it appends +makes the next run stop again until that work is done. These pin that shape. +""" + +from __future__ import annotations + +import re +from pathlib import Path + +import pytest + +PROJECT_ROOT = Path(__file__).resolve().parent.parent.parent +TEMPLATE = PROJECT_ROOT / "templates" / "commands" / "converge.md" +REFERENCE = PROJECT_ROOT / "docs" / "reference" / "agentic-sdd.md" + +GATE_HEADING = "Enforce the implement prerequisite" + + +@pytest.fixture(scope="module") +def template_text() -> str: + assert TEMPLATE.is_file(), f"missing command template: {TEMPLATE}" + return TEMPLATE.read_text(encoding="utf-8") + + +def _section(text: str, heading: str) -> str: + """The body of the `### ` section whose heading contains *heading*.""" + match = re.search( + rf"^###[^\n]*{re.escape(heading)}[^\n]*\n(.*?)(?=^### |\Z)", + text, + re.MULTILINE | re.DOTALL, + ) + assert match, f"no section headed {heading!r} any more" + return match.group(1) + + +def _gate(text: str) -> str: + """The prerequisite paragraph, up to the next blank line.""" + start = text.find(GATE_HEADING) + assert start != -1, "converge no longer checks that implement has finished" + end = text.find("\n\n", start) + return text[start : end if end != -1 else len(text)] + + +def test_the_gate_runs_before_anything_is_assessed(template_text: str) -> None: + """It has to sit in Step 1: a check after the assessment cannot stop it.""" + step_one = _section(template_text, "1. Initialize Convergence Context") + assert GATE_HEADING in step_one, ( + "the prerequisite check is not part of Step 1, so the codebase is assessed " + "before converge knows whether implement has finished" + ) + assert "do not continue to Step 2" in _gate(template_text) + + +def test_an_unchecked_task_stops_the_run_and_names_implement(template_text: str) -> None: + gate = _gate(template_text) + assert "STOP" in gate, gate + assert "__SPECKIT_COMMAND_IMPLEMENT__" in gate, ( + f"the stop must tell the user which command finishes the work:\n{gate}" + ) + assert re.search(r"list their task IDs", gate), ( + f"the stop must say which tasks are open, not only that some are:\n{gate}" + ) + assert "byte-for-byte unchanged" in gate, ( + f"a stopped run must not write to tasks.md:\n{gate}" + ) + + +def test_the_gate_counts_tasks_the_way_implement_does(template_text: str) -> None: + """An example checkbox inside a fence is not an open task (#4272).""" + gate = _gate(template_text) + assert "`- [ ]`" in gate, gate + assert re.search(r"outside\s+code\s+fences", gate), ( + f"the gate would treat a documented example checkbox as unfinished work:\n{gate}" + ) + + +def test_a_stopped_run_is_not_reported_as_converged(template_text: str) -> None: + """`converged` tells the user the implementation is complete; open work says otherwise.""" + gate = _gate(template_text) + assert "neither outcome of Step 7" in gate, gate + append_step = _section(template_text, "7. Append Convergence Tasks") + assert not re.search(r"take the `converged` path", append_step), ( + "already-tracked work is being routed to `converged`, whose report says the " + "implementation satisfies the spec" + ) + + +def test_every_task_is_part_of_what_is_assessed(template_text: str) -> None: + """A task ticked off without its work in the code is a gap too.""" + inventory = _section(template_text, "3. Build the Intent Inventory") + assert "**Task inventory**" in inventory, ( + "the intent inventory no longer includes the tasks themselves, so a task " + "marked done but never built is invisible" + ) + assert re.search(r"marked done whose work is\s+absent", inventory), inventory + + +def test_the_handoff_describes_the_gate(template_text: str) -> None: + handoff = _section(template_text, "8. Provide Next Actions") + assert "prerequisite check" in handoff, ( + "after appending, the handoff should say the next converge stops until the new " + f"tasks are done:\n{handoff}" + ) + + +def test_the_reference_docs_describe_the_gate() -> None: + text = REFERENCE.read_text(encoding="utf-8") + section = text[text.find("## `/speckit.converge`") :] + assert re.search(r"still unchecked, converge stops", section), ( + "docs/reference/agentic-sdd.md no longer says converge stops on unchecked tasks" + ) From 42e8d4ed04f479c3e30a286d6c6203dfc8a23059 Mon Sep 17 00:00:00 2001 From: Nguyen Thanh Dat Date: Wed, 30 Sep 2026 09:22:15 +0700 Subject: [PATCH 7/7] fix(commands): coalesce overlapping findings, and page before calling a match bare-only Two faults the review found in this PR's own additions. converge: the task inventory reaches the same gap the requirements inventory already reaches -- `T017` exists because `FR-003` asked for it -- so absent code produced a finding under each, and Step 7 appended two remediation tasks for one piece of work, each carrying half the trace and both returning on the next run. Step 4 now coalesces items that one change to the same code would close, into a single finding whose `source-refs` keeps every origin, written comma-separated in one checklist item. Items that merely share a file stay separate. taskstoissues: scoping split "matched" into a scoped match and a bare-only match, but the early exit still fired on either. A first page of bare `T001: ...` titles therefore ended pagination before the scoped issues on a later page were seen, and every ID was handed to the user as bare-only -- a prompt for issues already matched, and a duplicate for every wrong answer. The exit now requires a scoped match for every ID, and the bare-only list is assembled only once the pages have run out. Contract tests over both templates. Reverting either template to the reviewed version turns the matching tests red. --- templates/commands/converge.md | 28 ++++- templates/commands/taskstoissues.md | 4 +- .../unit/test_converge_finding_coalescing.py | 103 ++++++++++++++++++ .../unit/test_taskstoissues_feature_scope.py | 33 ++++++ 4 files changed, 160 insertions(+), 8 deletions(-) create mode 100644 tests/unit/test_converge_finding_coalescing.py diff --git a/templates/commands/converge.md b/templates/commands/converge.md index f334625625..7134680078 100644 --- a/templates/commands/converge.md +++ b/templates/commands/converge.md @@ -151,7 +151,9 @@ Create an internal model (do not echo raw artifacts): - **Task inventory**: every task in `tasks.md` — all of them checked by this point — with the work it describes and the file paths it names. A task marked done whose work is absent from the code, or only partly there, is a finding like any other, traced to - that task's ID. + that task's ID. Most such tasks exist because a requirement asked for them, so the same + gap is reachable from both inventories; Step 4 coalesces those into one finding rather + than reporting the work twice. - **Code-scope map**: from the file paths named in `plan.md` and `tasks.md`, plus a keyword search for the concepts each requirement describes, derive the set of source files and components in scope for assessment. Bound the assessment to these — do **not** infer @@ -177,8 +179,19 @@ For each item in the intent inventory, inspect the current code in scope and pro (surfaced for awareness — converge does **not** delete code, it only appends a task to review/justify or remove it). -Each `Finding` records: a stable id, the `source-ref` it traces to, the `gap-type`, a -severity, and a short human-readable description with the evidence (the file/area observed). +Each `Finding` records: a stable id, the `source-refs` it traces to (one or more), the +`gap-type`, a severity, and a short human-readable description with the evidence (the +file/area observed). + +**One finding per piece of work, however many inventory items point at it.** The two +inventories overlap by design: `T017` exists because `FR-003` asked for it, so code that +is absent produces a gap under both, and emitting one finding each would append two tasks +for one gap in Step 7 — each carrying half the trace, and each re-appearing together on +the next run. Coalesce them into a single `Finding` and list every item that led to it in +`source-refs` (`FR-003, T017`). Keep all of them: the requirement says what is owed, the +task ID says where the bookkeeping went wrong, and dropping either loses that half. Two +items are the same work when one change to the same code would close both — findings that +merely sit in the same file are not, and must stay separate. **Edge cases:** @@ -227,12 +240,15 @@ Append to the **end** of `tasks.md`, per the append contract: zero-padded IDs `T{M+1:03d}, T{M+2:03d}, …`: ```markdown - - [ ] T042 per () + - [ ] T042 per () ``` - `` traces the task to its origin: e.g. `FR-003`, `SC-002`, + `` traces the task to its origin: e.g. `FR-003`, `SC-002`, `US1/AC2`, `plan: storage decision`, `Constitution II`, or a task ID such as `T017` - when a task marked done is not reflected in the code. + when a task marked done is not reflected in the code. A coalesced finding names every + origin it was coalesced from, comma-separated (`FR-003, T017`), in **one** checklist + item — one finding is one task, so a gap that both a requirement and a done-but-absent + task point at is appended once, with the whole trace. `` is one of `missing`, `partial`, `contradicts`, `unrequested`. diff --git a/templates/commands/taskstoissues.md b/templates/commands/taskstoissues.md index 484b3bfe8e..3f26b6d3a9 100644 --- a/templates/commands/taskstoissues.md +++ b/templates/commands/taskstoissues.md @@ -64,10 +64,10 @@ git config --get remote.origin.url > [!CAUTION] > ONLY PROCEED TO NEXT STEPS IF THE REMOTE IS A GITHUB URL -1. **Fetch existing issues for deduplication**: Before creating anything, build the set of task IDs you are about to process from `tasks.md` (each is a `T` followed by **at least** three digits, e.g. `T001` — `__SPECKIT_COMMAND_CONVERGE__` assigns new IDs with `T{M+1:03d}`, which is a floor rather than a cap, so once a file has more than 999 tasks the IDs are four digits or longer). Then use the GitHub MCP server's `list_issues` tool to look for issues that already cover those IDs. Do not pass a `state` value, since omitting it makes the tool return both open and closed issues. Request `perPage: 100` to keep the number of calls down, and since the tool uses cursor-based pagination, request pages with the `after` parameter (using the `endCursor` from the previous response). For each issue title, match it against the task ID pattern `\bT\d{3,}\b` (the `{3,}` accepts four-digit and longer IDs — with `\d{3}` a title containing `T1000` would not match at all, because the trailing `\b` cannot fall between two digits, so that task would be silently neither deduplicated nor created; word boundaries still stop a token like `ST001` from matching, and force the whole digit run to be consumed so `T100` can never match inside `T1000`; this also recognises titles written as `T001 ...`, `T001: ...` or `[T001] ...`) and, when it matches one of your task IDs, mark that ID as already having an issue **only if the title also carries this feature's identifier** (see below). Task IDs restart at `T001` in every feature's `tasks.md`, so an unscoped match means the first feature to reach the tracker permanently suppresses `T001` for every later feature -- a silent gap in exactly the multi-feature repos this command is for. Stop paginating as soon as every task ID has been matched, or when there are no more pages, so you do not keep fetching the whole repository's issue history once all task IDs are accounted for. This bounds the number of calls on repos with large issue histories and still prevents duplicates when the command is re-run after `tasks.md` is regenerated or the skill is re-invoked. +1. **Fetch existing issues for deduplication**: Before creating anything, build the set of task IDs you are about to process from `tasks.md` (each is a `T` followed by **at least** three digits, e.g. `T001` — `__SPECKIT_COMMAND_CONVERGE__` assigns new IDs with `T{M+1:03d}`, which is a floor rather than a cap, so once a file has more than 999 tasks the IDs are four digits or longer). Then use the GitHub MCP server's `list_issues` tool to look for issues that already cover those IDs. Do not pass a `state` value, since omitting it makes the tool return both open and closed issues. Request `perPage: 100` to keep the number of calls down, and since the tool uses cursor-based pagination, request pages with the `after` parameter (using the `endCursor` from the previous response). For each issue title, match it against the task ID pattern `\bT\d{3,}\b` (the `{3,}` accepts four-digit and longer IDs — with `\d{3}` a title containing `T1000` would not match at all, because the trailing `\b` cannot fall between two digits, so that task would be silently neither deduplicated nor created; word boundaries still stop a token like `ST001` from matching, and force the whole digit run to be consumed so `T100` can never match inside `T1000`; this also recognises titles written as `T001 ...`, `T001: ...` or `[T001] ...`) and, when it matches one of your task IDs, mark that ID as already having an issue **only if the title also carries this feature's identifier** (see below). Task IDs restart at `T001` in every feature's `tasks.md`, so an unscoped match means the first feature to reach the tracker permanently suppresses `T001` for every later feature -- a silent gap in exactly the multi-feature repos this command is for. Stop paginating early **only** when every task ID has a scoped match; otherwise exhaust the pages before classifying any ID as bare-only. A bare match is not an answer, it is the question below, and the scoped issue that answers it may sit on a later page — one early page carrying bare `T001: ...` titles for every ID would otherwise end the search and send every one of them to the user as bare-only, prompting about issues that were already matched and risking a duplicate for each. Only "no more pages" makes bare-only a fact. So stop when every ID is scoped, or when the pages run out, so you do not keep fetching the whole repository's issue history once all task IDs are genuinely accounted for. This bounds the number of calls on repos with large issue histories and still prevents duplicates when the command is re-run after `tasks.md` is regenerated or the skill is re-invoked. 1. For each task in the list, use the GitHub MCP server to create a new issue in the repository that is representative of the Git remote. Task lines in `tasks.md` start with a markdown checkbox, so first strip the leading `- [ ]` (and any `[P]` / `[US#]` markers) to recover the task ID and its description. Create the issue with a single canonical title of the form `[] T001: `, where `` is the basename of FEATURE_DIR parsed in step 1 (the `NNN-name` spec directory, e.g. `002-billing`), followed by the ID written once and then the task description (for example, the line `- [ ] T001 Create project structure` in feature `002-billing` becomes the title `[002-billing] T001: Create project structure`). The ID keeps its own word boundaries, so the `\bT\d{3,}\b` matching above is unchanged by the prefix. - **Skip** a task only when an existing issue matches **both** this feature's identifier and the task ID, and report it (for example, `[002-billing] T001 already has an issue, skipping`). A `T001` belonging to another feature is a different task and must not suppress this one. - - Issues created before this scoping exists carry a bare `T001: ...` title, and a bare title carries no feature identity: it cannot show which feature's `T001` it tracks, and the absence of a scoped title for that ID does not show it either. So never decide a bare match on your own, in either direction -- skipping silently drops this feature's task when the issue belongs to another feature, and creating duplicates it when the issue is this feature's. Before creating anything, list every task ID that matched only a bare title, each with the issue number, the issue title and this feature's description for that task, and ask the user which of those issues already track this feature's tasks. Skip the ones the user confirms and create the rest with the scoped title. Then offer to retitle each confirmed issue to the scoped form (`[] T001: ...`) so later runs match it without asking, and retitle only the issues the user agrees to. + - Issues created before this scoping exists carry a bare `T001: ...` title, and a bare title carries no feature identity: it cannot show which feature's `T001` it tracks, and the absence of a scoped title for that ID does not show it either. So never decide a bare match on your own, in either direction -- skipping silently drops this feature's task when the issue belongs to another feature, and creating duplicates it when the issue is this feature's. Before creating anything — and only once the pages have run out, per the stop rule above, so the list cannot contain an ID whose scoped issue was simply never fetched — list every task ID that matched only a bare title, each with the issue number, the issue title and this feature's description for that task, and ask the user which of those issues already track this feature's tasks. Skip the ones the user confirms and create the rest with the scoped title. Then offer to retitle each confirmed issue to the scoped form (`[] T001: ...`) so later runs match it without asking, and retitle only the issues the user agrees to. - Only create issues for tasks that do not yet have a matching issue. > [!CAUTION] diff --git a/tests/unit/test_converge_finding_coalescing.py b/tests/unit/test_converge_finding_coalescing.py new file mode 100644 index 0000000000..9f9e421672 --- /dev/null +++ b/tests/unit/test_converge_finding_coalescing.py @@ -0,0 +1,103 @@ +"""One gap must produce one appended task, however many inventory items reach it. + +Converge now builds a task inventory as well as a requirements inventory, and the two +overlap by construction: `T017` exists because `FR-003` asked for it. Code that is absent +is therefore a gap under both, and a finding per item would append two remediation tasks +for the same work — each with half the trace, and both back again on the next run, which +is the duplication (#4269) the prerequisite gate was added to stop, reintroduced one layer +up. + +So the overlap is coalesced into a single finding that keeps every reference. These pin +that, and pin the append contract to one checklist item per finding, so a later edit to +either step cannot quietly split them again. +""" + +from __future__ import annotations + +import re +from pathlib import Path + +import pytest + +PROJECT_ROOT = Path(__file__).resolve().parent.parent.parent +TEMPLATE = PROJECT_ROOT / "templates" / "commands" / "converge.md" + + +@pytest.fixture(scope="module") +def template_text() -> str: + assert TEMPLATE.is_file(), f"missing command template: {TEMPLATE}" + return TEMPLATE.read_text(encoding="utf-8") + + +def _section(text: str, heading: str) -> str: + """The body of the `### . ` section, up to the next `### `.""" + match = re.search( + rf"^### \d+\. {re.escape(heading)}.*?(?=^### |\Z)", text, re.MULTILINE | re.DOTALL + ) + assert match, f"no section headed {heading!r} any more" + return match.group(0) + + +def _prose(section: str) -> str: + """*section* with its wrapping collapsed, so a phrase may span a line break.""" + return re.sub(r"\s+", " ", section) + + +def test_step_4_coalesces_findings_that_share_the_same_work(template_text: str) -> None: + """Two inventory items pointing at one gap must yield one finding, not two.""" + step4 = _section(template_text, "Assess the Codebase and Classify Findings") + assert re.search(r"one finding per piece of work", _prose(step4), re.IGNORECASE), ( + "Step 4 no longer tells the agent to coalesce, so a requirement and the task " + "that implements it each produce their own finding for the same absent code" + ) + assert re.search(r"coalesce", _prose(step4), re.IGNORECASE), step4 + assert re.search(r"`FR-\d+, T\d+`", _prose(step4)), ( + "the coalescing rule should show the combined form it produces, or the agent has " + "to guess how to keep both references" + ) + + +def test_a_coalesced_finding_keeps_every_reference(template_text: str) -> None: + """Collapsing to one reference loses what the other one carried. + + The requirement says what is owed; the task ID says where the bookkeeping went + wrong. A remediation task that names only one of them cannot be traced back to the + other, which is the whole point of appending it. + """ + step4 = _section(template_text, "Assess the Codebase and Classify Findings") + assert "source-refs" in step4, ( + "a Finding records a single `source-ref` again, so a coalesced finding has " + "nowhere to keep the references it was coalesced from" + ) + assert re.search(r"one or more", _prose(step4)), step4 + assert re.search(r"same work when one change", _prose(step4)), ( + "the rule must say what makes two items the same work, or unrelated findings " + "that merely sit in the same file get merged too" + ) + + +def test_the_append_contract_emits_one_task_per_finding(template_text: str) -> None: + """One finding, one checklist item — the coalescing must survive into `tasks.md`.""" + step7 = _section(template_text, "Append Convergence Tasks (or report converged)") + assert "one checklist item per actionable finding" in _prose(step7), step7 + assert re.search(r"- \[ \] T042 per ", step7), ( + "the appended-task shape still names a single ``, so a coalesced " + "finding cannot be written without either splitting it or dropping a reference" + ) + assert re.search(r"comma-separated", _prose(step7)), ( + "the append contract should say how several origins are written in the one item" + ) + assert re.search(r"in \*\*one\*\* checklist item", _prose(step7)), ( + "nothing stops the agent from emitting one item per origin of a coalesced " + "finding, which is the duplicate this coalescing exists to prevent" + ) + + +def test_the_task_inventory_points_at_the_coalescing_rule(template_text: str) -> None: + """The inventory that creates the overlap has to name where it is resolved.""" + inventory = _section(template_text, "Build the Intent Inventory") + assert re.search(r"Task inventory", inventory), inventory + assert re.search(r"coalesce", inventory, re.IGNORECASE), ( + "the task inventory introduces the second path to the same gap without saying " + "that Step 4 merges them, so the agent can reasonably report both" + ) diff --git a/tests/unit/test_taskstoissues_feature_scope.py b/tests/unit/test_taskstoissues_feature_scope.py index 9120b63210..cd2bea2825 100644 --- a/tests/unit/test_taskstoissues_feature_scope.py +++ b/tests/unit/test_taskstoissues_feature_scope.py @@ -86,6 +86,39 @@ def test_a_bare_title_is_never_decided_on_the_agents_own(template_text: str) -> ) +def test_the_early_exit_requires_a_scoped_match_for_every_id(template_text: str) -> None: + """A bare match must not end the search that could still have scoped it. + + Scoping split "matched" into two outcomes, and only one of them is an answer. The + early exit still read "every task ID has been matched", so a first page carrying + bare `T001: ...` titles for every ID ends pagination — and the scoped + `[002-billing] T001: ...` issues on the next page are never seen. Every ID is then + classified bare-only: the user is asked about issues that were already matched, and + a wrong answer creates a duplicate for each. + + So a bare-only classification is only a fact once the pages have run out. + """ + stop_rule = _line_containing(template_text, "Stop paginating") + assert re.search(r"only\b.*\bscoped match", stop_rule), ( + "the early exit fires on any match again, so bare matches on an early page end " + f"the search before the scoped issues on a later one are seen:\n {stop_rule.strip()}" + ) + assert re.search(r"exhaust the pages before classifying", stop_rule), ( + "nothing requires pagination to finish before an ID is called bare-only, which " + f"is the misclassification itself:\n {stop_rule.strip()}" + ) + assert not re.search(r"as soon as every task ID has been matched", template_text), ( + "the unconditional early exit is back verbatim" + ) + + # And the question to the user must be gated on the same fact, where it is asked. + legacy_rule = _line_containing(template_text, "before this scoping exists") + assert re.search(r"once the pages have run out", legacy_rule), ( + "the bare-only list is assembled without waiting for pagination to finish, so " + f"it can name an ID whose scoped issue was never fetched:\n {legacy_rule.strip()}" + ) + + def test_confirmed_legacy_issues_are_only_retitled_with_consent(template_text: str) -> None: """Retitling is what stops the question recurring, and it edits the user's issues.""" legacy_rule = _line_containing(template_text, "before this scoping exists")