From d857acbf27cb4c55f23f0537eb80bb37894c1d0b Mon Sep 17 00:00:00 2001 From: Amri Toufali Date: Tue, 4 Aug 2026 13:30:47 -0700 Subject: [PATCH 1/4] fix(investigate): check dismiss before npm_bump so dismiss_unaffected fires for npm Fixes #112 Co-Authored-By: Claude Opus 4.8 (1M context) --- scripts/post_alert_action.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/scripts/post_alert_action.py b/scripts/post_alert_action.py index d412461..19165f2 100644 --- a/scripts/post_alert_action.py +++ b/scripts/post_alert_action.py @@ -470,6 +470,14 @@ def main() -> None: print(f" Existing PR #{existing_pr} covers this package.") comment_on_pr(repo, existing_pr, reason, dry_run) action = "existing_pr" + elif dismiss_enabled and severity.lower() not in DISMISS_BLOCKED_SEVERITIES: + # Checked before bump_pr: dismissing is the intended outcome for an + # unaffected low/medium alert, and it must take precedence over the + # npm bump path (which is npm-only and never fired dismiss for npm + # alerts — see #112). + print(" Unaffected + dismiss enabled (low/medium). Dismissing alert.") + dismiss_alert(repo, alert_number, reason, dry_run) + action = "dismissed" elif recommended == "bump_pr": if ecosystem == "npm" and patched_version: print(" npm ecosystem — deferring to npm_bump workflow step.") @@ -511,10 +519,6 @@ def main() -> None: action = "noop" else: action = "noop" - elif dismiss_enabled and severity.lower() not in DISMISS_BLOCKED_SEVERITIES: - print(" Unaffected + dismiss enabled. Dismissing alert.") - dismiss_alert(repo, alert_number, reason, dry_run) - action = "dismissed" elif dismiss_enabled: print( f" Unaffected but severity is {severity}." From 746e30c758e9942a91ebca74b09e6f15fdd983ae Mon Sep 17 00:00:00 2001 From: Amri Toufali Date: Tue, 4 Aug 2026 13:48:03 -0700 Subject: [PATCH 2/4] test(investigate): unaffected low/medium dismissed before npm_bump (#112) Co-Authored-By: Claude Opus 4.8 (1M context) --- tests/scripts/test_post_alert_action.py | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/tests/scripts/test_post_alert_action.py b/tests/scripts/test_post_alert_action.py index cb7f576..684b4ae 100644 --- a/tests/scripts/test_post_alert_action.py +++ b/tests/scripts/test_post_alert_action.py @@ -363,6 +363,29 @@ def test_npm_bump_outputs_action( mock_repo.get_contents.assert_not_called() mock_repo.create_pull.assert_not_called() + def test_dismiss_precedes_npm_bump_for_unaffected( + self, verdict_file, tmp_path, monkeypatch + ): + """Unaffected low/medium alert is dismissed, not routed to npm_bump (#112).""" + verdict_file(SAMPLE_VERDICT) # affected=False, recommended_action=bump_pr + mock_repo = MagicMock() + mock_repo.full_name = "owner/repo" + mock_repo.get_pulls.return_value = [] # no existing PR + + output_file = str(tmp_path / "github_output") + open(output_file, "w").close() + + self._run_main( + verdict_file, tmp_path, monkeypatch, mock_repo, + ALERT_ECOSYSTEM="npm", ALERT_SEVERITY="medium", + DISMISS_UNAFFECTED="true", DRY_RUN="true", + GITHUB_OUTPUT=output_file, + ) + + outputs = open(output_file).read() + assert "action=dismissed" in outputs + assert "action=npm_bump" not in outputs + def test_npm_bump_no_patched_version( self, verdict_file, tmp_path, monkeypatch ): From 48b13635a96be1ed875f958753bb8e792e6f44cc Mon Sep 17 00:00:00 2001 From: Amri Toufali Date: Wed, 5 Aug 2026 15:20:59 -0700 Subject: [PATCH 3/4] style(investigate): drop inline rationale comment per review Reasoning for ordering dismiss before bump_pr already lives in commit d857acb and the PR description; keeping it out of the code per reviewer preference. --- scripts/post_alert_action.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/scripts/post_alert_action.py b/scripts/post_alert_action.py index 19165f2..0aeca62 100644 --- a/scripts/post_alert_action.py +++ b/scripts/post_alert_action.py @@ -471,10 +471,6 @@ def main() -> None: comment_on_pr(repo, existing_pr, reason, dry_run) action = "existing_pr" elif dismiss_enabled and severity.lower() not in DISMISS_BLOCKED_SEVERITIES: - # Checked before bump_pr: dismissing is the intended outcome for an - # unaffected low/medium alert, and it must take precedence over the - # npm bump path (which is npm-only and never fired dismiss for npm - # alerts — see #112). print(" Unaffected + dismiss enabled (low/medium). Dismissing alert.") dismiss_alert(repo, alert_number, reason, dry_run) action = "dismissed" From 1c79ff869f72dee79a25f2e7cb04406af99c9b75 Mon Sep 17 00:00:00 2001 From: Amri Toufali Date: Wed, 5 Aug 2026 19:00:19 -0700 Subject: [PATCH 4/4] fix(investigate): don't auto-dismiss unaffected alerts of unknown severity MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The dismiss guard only blocked high/critical, so an unaffected alert with an empty or 'unknown' severity (the workflow input defaults to '') slipped through and got auto-dismissed — even though it could really be high/ critical. Also refuse to dismiss empty/unknown severities; add a regression test. Flagged by Copilot on #118. --- scripts/post_alert_action.py | 7 ++++++- tests/scripts/test_post_alert_action.py | 17 +++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/scripts/post_alert_action.py b/scripts/post_alert_action.py index 0aeca62..c694e88 100644 --- a/scripts/post_alert_action.py +++ b/scripts/post_alert_action.py @@ -42,6 +42,7 @@ BLENDER_NAME = "BLEnder" DISMISS_BLOCKED_SEVERITIES = {"critical", "high"} +DISMISS_UNKNOWN_SEVERITIES = {"", "unknown"} VERDICT_FILE = ".blender-alert-verdict.json" REQUIRED_KEYS = { "affected", @@ -470,7 +471,11 @@ def main() -> None: print(f" Existing PR #{existing_pr} covers this package.") comment_on_pr(repo, existing_pr, reason, dry_run) action = "existing_pr" - elif dismiss_enabled and severity.lower() not in DISMISS_BLOCKED_SEVERITIES: + elif ( + dismiss_enabled + and severity.lower() not in DISMISS_BLOCKED_SEVERITIES + and severity.lower() not in DISMISS_UNKNOWN_SEVERITIES + ): print(" Unaffected + dismiss enabled (low/medium). Dismissing alert.") dismiss_alert(repo, alert_number, reason, dry_run) action = "dismissed" diff --git a/tests/scripts/test_post_alert_action.py b/tests/scripts/test_post_alert_action.py index 684b4ae..4e32bca 100644 --- a/tests/scripts/test_post_alert_action.py +++ b/tests/scripts/test_post_alert_action.py @@ -332,6 +332,23 @@ def test_dismiss_skips_high_severity( mock_repo._requester.requestJsonAndCheck.assert_not_called() + def test_dismiss_skips_unknown_severity( + self, verdict_file, tmp_path, monkeypatch + ): + """Empty/unknown severity is not auto-dismissed (could be high/critical).""" + verdict = {**SAMPLE_VERDICT, "recommended_action": "none"} + verdict_file(verdict) + mock_repo = MagicMock() + mock_repo.full_name = "owner/repo" + mock_repo.get_pulls.return_value = [] + + self._run_main( + verdict_file, tmp_path, monkeypatch, mock_repo, + DISMISS_UNAFFECTED="true", ALERT_SEVERITY="", + ) + + mock_repo._requester.requestJsonAndCheck.assert_not_called() + def test_npm_bump_outputs_action( self, verdict_file, tmp_path, monkeypatch ):