diff --git a/skills/work-plan/lib/drift.py b/skills/work-plan/lib/drift.py index 456e5b8..9c4d42b 100644 --- a/skills/work-plan/lib/drift.py +++ b/skills/work-plan/lib/drift.py @@ -23,8 +23,13 @@ def detect_drift(body: str, github_issues: list[dict]) -> list[dict]: continue gh_state = state_by_num[num] looks_closed = any(k in body_status for k in ("✅", "shipped", "merged", "closed")) - looks_open = "🔲" in body_status or "open" in body_status + # Asymmetric by design: CLOSED is terminal, so a closed issue whose + # row doesn't explicitly read closed (open marker, ambiguous, or empty) + # is drift. OPEN is not terminal — an open issue legitimately sits in + # many states (in-progress, blocked, todo), so only an explicit closed + # marker contradicts it. A broad open-side check (`not looks_open`) + # would false-positive every in-progress row, which is why we don't. if gh_state == "CLOSED" and not looks_closed: drift.append({"issue": num, "body_status": body_status, "github_state": "CLOSED"}) elif gh_state == "OPEN" and looks_closed: diff --git a/skills/work-plan/tests/test_drift.py b/skills/work-plan/tests/test_drift.py index d117faa..68293be 100644 --- a/skills/work-plan/tests/test_drift.py +++ b/skills/work-plan/tests/test_drift.py @@ -33,6 +33,38 @@ def test_drift_when_open_in_md_closed_in_github(self): def test_no_table_returns_empty(self): self.assertEqual(detect_drift("# No table\n", [{"number": 1, "state": "CLOSED"}]), []) + # --- OPEN-side + ambiguous-body cases: pin the *intentional asymmetry* ---- + # CLOSED is terminal → broad check (anything not-closed drifts). OPEN is not + # terminal → narrow check (only an explicit closed marker drifts). These + # guard against someone "restoring symmetry" with a `not looks_open` open-side + # check, which would false-positive every in-progress row. + + def _body(self, status: str) -> str: + return ("| # | Title | Status |\n" + "|---|---|---|\n" + f"| #1 | foo | {status} |\n") + + def test_drift_when_closed_in_md_open_in_github(self): + # OPEN-side condition (was untested): body says shipped, GitHub reopened it. + drift = detect_drift(self._body("✅ Shipped"), [{"number": 1, "state": "OPEN"}]) + self.assertEqual(len(drift), 1) + self.assertEqual(drift[0]["github_state"], "OPEN") + + def test_no_drift_when_open_in_md_open_in_github(self): + self.assertEqual(detect_drift(self._body("🔲 Open"), [{"number": 1, "state": "OPEN"}]), []) + + def test_open_with_ambiguous_status_is_NOT_drift(self): + # The deliberate narrow OPEN-side: an in-progress row must not be flagged. + self.assertEqual( + detect_drift(self._body("🚧 In progress"), [{"number": 1, "state": "OPEN"}]), []) + + def test_closed_with_ambiguous_status_IS_drift(self): + # The deliberate broad CLOSED-side: a closed issue whose row doesn't read + # closed (here: ambiguous) is drift. + drift = detect_drift(self._body("🚧 In progress"), [{"number": 1, "state": "CLOSED"}]) + self.assertEqual(len(drift), 1) + self.assertEqual(drift[0]["github_state"], "CLOSED") + if __name__ == "__main__": unittest.main()