From 33b9435f9599e74b8b981a2920874119985a3908 Mon Sep 17 00:00:00 2001 From: lihognwei-ship-it Date: Sat, 19 Sep 2026 16:35:17 +0800 Subject: [PATCH] fix: scope drift issue closed-state dedup --- scripts/drift_audit.py | 4 +++- scripts/drift_watch.py | 23 ++++++++++++++--------- tests/test_release_quality_boundaries.py | 17 ++++++++++++----- 3 files changed, 29 insertions(+), 15 deletions(-) diff --git a/scripts/drift_audit.py b/scripts/drift_audit.py index 9f2cc02..10d38e1 100644 --- a/scripts/drift_audit.py +++ b/scripts/drift_audit.py @@ -154,7 +154,9 @@ def emit_issues(summaries: list[dict[str, Any]], repo: str | None) -> int: if not gh_available(): print(json.dumps({"emit_issues": "skipped", "reason": "no-github-token"}, ensure_ascii=False)) return 0 - existing = existing_open_issues("[Drift-audit]") + # Audit verdict titles intentionally dedup across closed issues: resolved + # NO_ACTIONABLE_DRIFT verdicts should not be re-opened every Saturday. + existing = existing_open_issues("[Drift-audit]", include_closed=True) opened = 0 failed: list[str] = [] for summary in summaries: diff --git a/scripts/drift_watch.py b/scripts/drift_watch.py index 2ad86ac..12a3f14 100644 --- a/scripts/drift_watch.py +++ b/scripts/drift_watch.py @@ -158,22 +158,25 @@ def gh_available() -> bool: return bool(os.environ.get("GH_TOKEN") or os.environ.get("GITHUB_TOKEN")) -def existing_open_issues(title_prefix: str) -> set[str]: - """Return titles of existing drift issues, open OR closed. - - Dedup must look at closed issues too: an author closing a resolved - "no actionable drift" issue used to erase the dedup key, so the next - weekly run re-opened a same-titled duplicate (#19 after #9). +def existing_open_issues(title_prefix: str, include_closed: bool = False) -> set[str]: + """Return titles of existing drift issues matching the prefix. + + Detection issues default to open-only so a future same-rule drift can still + notify maintainers after an older drift issue was closed. Audit verdict + issues may opt into closed-state dedup: closing a resolved "no actionable + drift" issue used to erase the dedup key, so the next weekly run re-opened + a same-titled duplicate (#19 after #9). """ + state = "all" if include_closed else "open" result = subprocess.run( - ["gh", "issue", "list", "--state", "all", "--json", "title", "--limit", "500", "--search", f"in:title {title_prefix}"], + ["gh", "issue", "list", "--state", state, "--json", "title", "--limit", "500", "--search", f"in:title {title_prefix}"], capture_output=True, check=False, text=True, ) if result.returncode: # Older gh versions / appliances without --search: fall back to the - # plain all-state listing so dedup still sees closed duplicates. + # plain state listing so dedup keeps the requested open/all semantics. result = subprocess.run( - ["gh", "issue", "list", "--state", "all", "--json", "title", "--limit", "500"], + ["gh", "issue", "list", "--state", state, "--json", "title", "--limit", "500"], capture_output=True, check=False, text=True, ) if result.returncode: @@ -193,6 +196,8 @@ def emit_issues(report: dict[str, Any], repo: str | None) -> int: if not gh_available(): print(json.dumps({"emit_issues": "skipped", "reason": "no-github-token"}, ensure_ascii=False)) return 0 + # Keep deterministic detection alerts open-only: closed historical drift + # issues must not suppress a genuinely new same-rule drift in a later week. existing = existing_open_issues("[Drift]") opened = 0 failed: list[str] = [] diff --git a/tests/test_release_quality_boundaries.py b/tests/test_release_quality_boundaries.py index 5b7fbbf..f39f7b5 100644 --- a/tests/test_release_quality_boundaries.py +++ b/tests/test_release_quality_boundaries.py @@ -240,8 +240,9 @@ def test_issue_emission_reports_notification_failures(self) -> None: report = {"platforms": [{"platform": "demo", "results": [{"rule_id": "r1", "state": "unverifiable", "url": "u", "error": "x"}]}]} with patch.object(self.module, "gh_available", return_value=False), contextlib.redirect_stdout(io.StringIO()): self.assertEqual(self.module.emit_issues(report, None), 0) - with patch.object(self.module, "gh_available", return_value=True), patch.object(self.module, "existing_open_issues", return_value={"[Drift] demo: r1 -> unverifiable"}), contextlib.redirect_stdout(io.StringIO()): + with patch.object(self.module, "gh_available", return_value=True), patch.object(self.module, "existing_open_issues", return_value={"[Drift] demo: r1 -> unverifiable"}) as existing, contextlib.redirect_stdout(io.StringIO()): self.assertEqual(self.module.emit_issues(report, None), 0) + existing.assert_called_once_with("[Drift]") failed = subprocess.CompletedProcess(args=[], returncode=1, stdout="", stderr="denied") with patch.object(self.module, "gh_available", return_value=True), patch.object(self.module, "existing_open_issues", return_value=set()), patch.object(self.module.subprocess, "run", return_value=failed), contextlib.redirect_stdout(io.StringIO()): self.assertEqual(self.module.emit_issues(report, "o/r"), 1) @@ -253,16 +254,21 @@ def test_issue_listing_and_cli_return_codes(self) -> None: with patch.dict(os.environ, {}, clear=True): self.assertFalse(self.module.gh_available()) bad = subprocess.CompletedProcess(args=[], returncode=1, stdout="", stderr="") - with patch.object(self.module.subprocess, "run", return_value=bad): + with patch.object(self.module.subprocess, "run", return_value=bad) as run_mock: self.assertEqual(self.module.existing_open_issues("x"), set()) + self.assertTrue(all("open" in call.args[0] for call in run_mock.call_args_list)) malformed = subprocess.CompletedProcess(args=[], returncode=0, stdout="bad", stderr="") with patch.object(self.module.subprocess, "run", return_value=malformed): self.assertEqual(self.module.existing_open_issues("x"), set()) valid = subprocess.CompletedProcess(args=[], returncode=0, stdout='[{"title":"one"}]', stderr="") with patch.object(self.module.subprocess, "run", return_value=valid) as run_mock: self.assertEqual(self.module.existing_open_issues("x"), {"one"}) - # Dedup must query ALL states (open + closed): a closed duplicate - # still suppresses re-opening (regression: #19 re-created after #9). + self.assertIn("--state", run_mock.call_args_list[0].args[0]) + self.assertIn("open", run_mock.call_args_list[0].args[0]) + with patch.object(self.module.subprocess, "run", return_value=valid) as run_mock: + self.assertEqual(self.module.existing_open_issues("x", include_closed=True), {"one"}) + # Audit verdict dedup may query ALL states: a closed NO_ACTIONABLE + # duplicate still suppresses re-opening (regression: #19 after #9). self.assertIn("--state", run_mock.call_args_list[0].args[0]) self.assertIn("all", run_mock.call_args_list[0].args[0]) clean = {"actionable_count": 0, "platforms": []} @@ -312,8 +318,9 @@ def test_render_issue_and_emit_failure_paths(self) -> None: with patch.object(self.module, "gh_available", return_value=False), contextlib.redirect_stdout(io.StringIO()): self.assertEqual(self.module.emit_issues([summary], None), 0) title = "[Drift-audit] demo: MANUAL_REVIEW" - with patch.object(self.module, "gh_available", return_value=True), patch.object(self.module, "existing_open_issues", return_value={title}), contextlib.redirect_stdout(io.StringIO()): + with patch.object(self.module, "gh_available", return_value=True), patch.object(self.module, "existing_open_issues", return_value={title}) as existing, contextlib.redirect_stdout(io.StringIO()): self.assertEqual(self.module.emit_issues([summary], None), 0) + existing.assert_called_once_with("[Drift-audit]", include_closed=True) failed = subprocess.CompletedProcess(args=[], returncode=1, stdout="", stderr="denied") with patch.object(self.module, "gh_available", return_value=True), patch.object(self.module, "existing_open_issues", return_value=set()), patch.object(self.module.subprocess, "run", return_value=failed), contextlib.redirect_stdout(io.StringIO()): self.assertEqual(self.module.emit_issues([summary], "o/r"), 1)