From eabd856d562c0f54a2a2bf7a906b463bbd3435b7 Mon Sep 17 00:00:00 2001 From: Nguyen Thanh Dat Date: Wed, 26 Aug 2026 10:07:36 +0700 Subject: [PATCH] fix(converge): do not re-append work an unchecked task already tracks The append step computed the next task ID but never asked whether a finding was already represented in tasks.md. So converge run twice, or run before implement had worked through the list, appended the same remediation work again under fresh IDs -- and the second run cannot tell its own previous output apart from the plan's original tasks, so traceability for one piece of work splits across two entries. Findings are now matched against existing unchecked tasks before anything is appended, on the work described and the paths or source-ref it names rather than on wording, and scanning outside code fences like the other commands do. A finding that is already tracked is reported as such rather than silently discarded, and an all-tracked run takes the converged path saying why -- so the operator reads "the work is known", not "the codebase is complete". Closes #4269 --- templates/commands/converge.md | 17 ++++++++ tests/unit/test_converge_idempotency.py | 57 +++++++++++++++++++++++++ 2 files changed, 74 insertions(+) create mode 100644 tests/unit/test_converge_idempotency.py diff --git a/templates/commands/converge.md b/templates/commands/converge.md index eadb96ee58..30a1eca241 100644 --- a/templates/commands/converge.md +++ b/templates/commands/converge.md @@ -201,6 +201,23 @@ Append to the **end** of `tasks.md`, per the append contract: 1. Scan all existing task IDs; let `M` be the maximum. Determine the next phase number `N` (highest existing phase + 1). + + **Drop findings that existing unchecked tasks already cover.** Compare each actionable + finding against every `- [ ]` task already in `tasks.md` — outside code fences, the same + rule `__SPECKIT_COMMAND_CLARIFY__` applies — matching on the work described and the file + paths or `` it names, not on wording. A finding that an unchecked task + already represents is **not** appended: it is unbuilt work that is already tracked, and + appending it again splits one piece of work across two IDs. + + This is what makes the command idempotent. Converge run twice, or run before + `__SPECKIT_COMMAND_IMPLEMENT__` has worked through the list, would otherwise append the + same remediation tasks under fresh IDs each time — the second run cannot tell its own + previous output apart from the plan's original tasks. + + Report the dropped ones in the summary rather than silently discarding them: + `F3 — already tracked by T017, not appended`. If **every** finding is already tracked, + there is nothing to append: take the `converged` path below and say why, so the operator + sees "the work is known" rather than "the codebase is complete". 2. Write a single new section header `## Phase N: Convergence`. 3. Emit one checklist item per actionable finding, ordered CRITICAL/HIGH first, assigning zero-padded IDs `T{M+1:03d}, T{M+2:03d}, …`: diff --git a/tests/unit/test_converge_idempotency.py b/tests/unit/test_converge_idempotency.py new file mode 100644 index 0000000000..1cd8510cc2 --- /dev/null +++ b/tests/unit/test_converge_idempotency.py @@ -0,0 +1,57 @@ +"""`/speckit-converge` must not append work that an unchecked task already tracks. + +The command's only write is appending remediation tasks to `tasks.md`. It knew how to +compute the next task ID, but not whether a finding was already represented — so running +it twice, or running it before `/speckit-implement` had worked through the list, appended +the same work again under fresh IDs and split one piece of work across two entries +(#4269). The dedup rule is what makes the command idempotent, so it is pinned here rather +than left to survive on prose alone. +""" + +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 test_the_append_step_still_exists(template_text: str) -> None: + """Guard against the assertions below passing vacuously if the step is renamed away.""" + assert re.search(r"Append Convergence Tasks", template_text), ( + "the append step is gone; the rules below no longer describe this command" + ) + + +def test_findings_are_compared_against_existing_unchecked_tasks(template_text: str) -> None: + assert re.search(r"unchecked task", template_text, re.IGNORECASE), ( + "converge no longer says to compare findings against existing unchecked tasks, so " + "a second run appends the same work again under new IDs" + ) + assert re.search(r"`- \[ \]`", template_text), ( + "the comparison should name the marker it scans for, so the rule is executable" + ) + + +def test_the_comparison_skips_code_fences(template_text: str) -> None: + """Same rule the other commands apply — an example checkbox is not a tracked task.""" + assert re.search(r"outside\s+(?:of\s+)?code\s+fences", template_text, re.IGNORECASE), ( + "the scan must exclude fenced blocks, or a checklist documenting the checkbox " + "format reads as tracked work" + ) + + +def test_already_tracked_findings_are_reported_not_silently_dropped(template_text: str) -> None: + assert re.search(r"already tracked", template_text, re.IGNORECASE), ( + "a finding dropped as already-tracked must be reported; dropping it silently is " + "indistinguishable from not having found it" + )