diff --git a/scripts/check_issue_consistency.py b/scripts/check_issue_consistency.py index dc044805e..f0dd61c58 100755 --- a/scripts/check_issue_consistency.py +++ b/scripts/check_issue_consistency.py @@ -478,6 +478,13 @@ def main() -> int: "retaining PR title issue reference.", file=sys.stderr, ) + else: + print( + "Warning: PR title uses a bare # reference with no issue context; " + "falling back to commits/headers.", + file=sys.stderr, + ) + pr_issue = None if not pr_issue: pr_issue, head_ambiguous = resolve_head_ref_issue_number(head_ref) if head_ambiguous: diff --git a/scripts/generate_suppression_guard_comment.py b/scripts/generate_suppression_guard_comment.py index e6aff874e..12ed91b52 100755 --- a/scripts/generate_suppression_guard_comment.py +++ b/scripts/generate_suppression_guard_comment.py @@ -40,6 +40,15 @@ ("marocchino/sticky-pull-request-comment", "sticky-pull-request-comment action"), ) +# Matches suppress_comments in a negation context so that an inverted guard +# like ``inputs.suppress_comments == true`` is NOT treated as a valid guard. +# Covers: != true, == false, !inputs.suppress_comments, !(inputs.suppress_comments) +_SUPPRESS_NEGATION_RE = re.compile( + r"suppress_comments\s*!=\s*true" + r"|suppress_comments\s*==\s*false" + r"|!\s*\(?\s*inputs\.suppress_comments" +) + def _normalise_keys(node: Any) -> Any: if isinstance(node, dict): @@ -112,8 +121,8 @@ def _iter_posting_steps(workflow: dict[str, Any]) -> list[tuple[str, str, list[s guarded = ( "should_post_review" in job_if_str or "should_post_review" in step_if_str - or "suppress_comments" in job_if_str - or "suppress_comments" in step_if_str + or bool(_SUPPRESS_NEGATION_RE.search(job_if_str)) + or bool(_SUPPRESS_NEGATION_RE.search(step_if_str)) ) if hints and not guarded: findings.append((str(job_id), str(name), hints)) @@ -142,11 +151,15 @@ def build_comment(workflows: Iterable[pathlib.Path], include_label: bool = False if include_label: lines.append("Label: needs-human") lines.append( - "Blocked by workflow protection: update .github/workflows/keepalive.yml and " - ".github/workflows/autofix.yml to add explicit `if:` guards on every step/job " - "that posts a PR comment or PR review so they cannot run when suppression is active. " + "Blocked by workflow protection: update " + ".github/workflows/agents-keepalive-loop.yml, " + ".github/workflows/autofix.yml, and " + ".github/workflows/reusable-18-autofix.yml to add explicit `if:` guards " + "on every step/job that posts a PR comment or PR review so they cannot " + "run when suppression is active. " "Use the suppression output key `should_post_review` (from " - ".github/scripts/should-post-review.js) to gate the posting steps." + ".github/scripts/should-post-review.js) or the `suppress_comments` " + "input to gate the posting steps." ) lines.append("") diff --git a/tests/scripts/test_generate_suppression_guard_comment.py b/tests/scripts/test_generate_suppression_guard_comment.py index 31063ef43..850622e1d 100644 --- a/tests/scripts/test_generate_suppression_guard_comment.py +++ b/tests/scripts/test_generate_suppression_guard_comment.py @@ -90,6 +90,56 @@ def test_build_comment_ignores_suppress_comments_guarded_steps( assert "post / Post comment" not in comment +def test_build_comment_flags_inverted_suppress_comments_guard( + tmp_path: Path, +) -> None: + """``suppress_comments == true`` allows posting during suppression.""" + workflow_path = tmp_path / "inverted.yml" + _write_yaml( + workflow_path, + """ + name: Inverted Guard Workflow + jobs: + post: + runs-on: ubuntu-latest + steps: + - name: Post comment + if: inputs.suppress_comments == true + run: github.rest.issues.createComment + """, + ) + + comment = build_comment([workflow_path]) + + # Inverted guard should NOT be treated as properly guarded + assert "post / Post comment" in comment + + +def test_build_comment_ignores_parenthesized_negation_guard( + tmp_path: Path, +) -> None: + """``!(inputs.suppress_comments)`` is a valid negation guard.""" + workflow_path = tmp_path / "paren.yml" + _write_yaml( + workflow_path, + """ + name: Parenthesized Negation Workflow + jobs: + post: + runs-on: ubuntu-latest + steps: + - name: Post comment + if: ${{ !(inputs.suppress_comments) }} + run: github.rest.issues.createComment + """, + ) + + comment = build_comment([workflow_path]) + + assert "No unguarded PR comment/review posting steps detected" in comment + assert "post / Post comment" not in comment + + def test_build_comment_detects_octokit_aliases(tmp_path: Path) -> None: workflow_path = tmp_path / "octokit.yml" _write_yaml( diff --git a/tests/scripts/test_issue_consistency_check.py b/tests/scripts/test_issue_consistency_check.py index 4f80be0ad..c7353accd 100644 --- a/tests/scripts/test_issue_consistency_check.py +++ b/tests/scripts/test_issue_consistency_check.py @@ -249,6 +249,31 @@ def test_main_title_hash_prefers_head_ref(monkeypatch, capsys) -> None: assert "Issue consistency check passed for #5678." in captured.out +def test_main_title_hash_without_head_ref_issue_falls_back(monkeypatch, capsys) -> None: + monkeypatch.delenv("GITHUB_EVENT_PATH", raising=False) + monkeypatch.setenv("PR_TITLE", "Update docs (#1234)") + monkeypatch.setenv("HEAD_REF", "feature/no-issue") + monkeypatch.setenv("BASE_REF", "") + monkeypatch.setenv("BASE_SHA", "") + monkeypatch.setenv("BASE_REMOTE", "origin") + monkeypatch.setattr(sys, "argv", ["check_issue_consistency.py"]) + + monkeypatch.setattr( + check_issue_consistency, + "collect_commit_messages", + lambda *args, **kwargs: (["fix: resolve issue #5678"], False), + ) + monkeypatch.setattr( + check_issue_consistency, + "collect_changed_files", + lambda *args, **kwargs: ([], False), + ) + + assert check_issue_consistency.main() == 0 + captured = capsys.readouterr() + assert "Issue consistency check passed for #5678." in captured.out + + def test_main_autofix_skips_mismatched_commits(monkeypatch, capsys) -> None: monkeypatch.delenv("GITHUB_EVENT_PATH", raising=False) monkeypatch.setenv("PR_TITLE", "Autofix #1234")