From c755c8f7c67ae1341621ba91b28ecb2bf7aa7a16 Mon Sep 17 00:00:00 2001 From: stranske Date: Tue, 10 Feb 2026 06:10:12 +0000 Subject: [PATCH 1/4] feat: add suppress_comments guard to autofix workflow (#1414) Add suppress_comments boolean input (default: false) to reusable-18-autofix.yml, gating the three PR comment-posting steps: - Upsert consolidated PR comment - Upsert clean-mode file summary comment - Upsert safe sweep file summary comment Update generate_suppression_guard_comment.py to recognise suppress_comments as a valid guard alongside should_post_review, and fix DEFAULT_WORKFLOWS to match actual filenames. Add test coverage for the new suppress_comments guard detection. Closes remaining deferred tasks from issue #1414. --- .github/workflows/reusable-18-autofix.yml | 11 ++++++--- scripts/generate_suppression_guard_comment.py | 10 ++++++-- ...test_generate_suppression_guard_comment.py | 24 +++++++++++++++++++ 3 files changed, 40 insertions(+), 5 deletions(-) mode change 100644 => 100755 scripts/generate_suppression_guard_comment.py diff --git a/.github/workflows/reusable-18-autofix.yml b/.github/workflows/reusable-18-autofix.yml index 089a6342e..d4d64636a 100644 --- a/.github/workflows/reusable-18-autofix.yml +++ b/.github/workflows/reusable-18-autofix.yml @@ -87,6 +87,11 @@ on: required: false default: '' type: string + suppress_comments: + description: 'Suppress PR comment posting (suppression guard)' + required: false + default: false + type: boolean secrets: service_bot_pat: description: 'PAT for SERVICE_BOT to trigger workflows on autofix commits' @@ -1292,7 +1297,7 @@ jobs: scripts-path: workflows-lib/scripts - name: Upsert consolidated PR comment - if: steps.guard.outputs.skip != 'true' && steps.build_comment.outputs.should-post == 'true' + if: steps.guard.outputs.skip != 'true' && steps.build_comment.outputs.should-post == 'true' && inputs.suppress_comments != true uses: actions/github-script@v8 env: PR_NUMBER: ${{ inputs.pr_number }} @@ -1336,7 +1341,7 @@ jobs: } - name: Upsert clean-mode file summary comment - if: steps.guard.outputs.skip != 'true' && steps.clean_mode.outputs.enabled == 'true' && steps.fix_results.outputs.changed == 'true' + if: steps.guard.outputs.skip != 'true' && steps.clean_mode.outputs.enabled == 'true' && steps.fix_results.outputs.changed == 'true' && inputs.suppress_comments != true uses: actions/github-script@v8 env: PR_NUMBER: ${{ inputs.pr_number }} @@ -1381,7 +1386,7 @@ jobs: } - name: Upsert safe sweep file summary comment - if: steps.guard.outputs.skip != 'true' && steps.clean_mode.outputs.enabled != 'true' && steps.fix_results.outputs.changed == 'true' + if: steps.guard.outputs.skip != 'true' && steps.clean_mode.outputs.enabled != 'true' && steps.fix_results.outputs.changed == 'true' && inputs.suppress_comments != true uses: actions/github-script@v8 env: PR_NUMBER: ${{ inputs.pr_number }} diff --git a/scripts/generate_suppression_guard_comment.py b/scripts/generate_suppression_guard_comment.py old mode 100644 new mode 100755 index fa55062f5..e6aff874e --- a/scripts/generate_suppression_guard_comment.py +++ b/scripts/generate_suppression_guard_comment.py @@ -12,8 +12,9 @@ import yaml DEFAULT_WORKFLOWS = ( - pathlib.Path(".github/workflows/keepalive.yml"), + pathlib.Path(".github/workflows/agents-keepalive-loop.yml"), pathlib.Path(".github/workflows/autofix.yml"), + pathlib.Path(".github/workflows/reusable-18-autofix.yml"), ) SCRIPT_PATTERNS: tuple[tuple[re.Pattern[str], str], ...] = ( @@ -108,7 +109,12 @@ def _iter_posting_steps(workflow: dict[str, Any]) -> list[tuple[str, str, list[s hints.append(action_hint) step_if = step.get("if") step_if_str = step_if if isinstance(step_if, str) else "" - guarded = "should_post_review" in job_if_str or "should_post_review" in step_if_str + 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 + ) if hints and not guarded: findings.append((str(job_id), str(name), hints)) return findings diff --git a/tests/scripts/test_generate_suppression_guard_comment.py b/tests/scripts/test_generate_suppression_guard_comment.py index 741a84273..31063ef43 100644 --- a/tests/scripts/test_generate_suppression_guard_comment.py +++ b/tests/scripts/test_generate_suppression_guard_comment.py @@ -66,6 +66,30 @@ def test_build_comment_reports_unguarded_steps(tmp_path: Path) -> None: assert "post / Post comment" in comment +def test_build_comment_ignores_suppress_comments_guarded_steps( + tmp_path: Path, +) -> None: + workflow_path = tmp_path / "suppress.yml" + _write_yaml( + workflow_path, + """ + name: Suppress Comments 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]) + + 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( From 9b9c56aac1e45def23e5ffb0a7e2d0c0d79a5bd1 Mon Sep 17 00:00:00 2001 From: stranske Date: Tue, 10 Feb 2026 06:26:48 +0000 Subject: [PATCH 2/4] fix: address review comments on suppress_comments guard detection 1. Parse suppress_comments guard semantics instead of just checking token presence. A regex now verifies the expression uses a negation An inverted guard like 'suppress_comments == true' is no longer falsely treated as properly guarded. 2. Update build_comment() message to reference the actual workflow filenames (agents-keepalive-loop.yml, reusable-18-autofix.yml) instead of the non-existent keepalive.yml. 3. Add test for inverted guard detection. --- scripts/generate_suppression_guard_comment.py | 24 +++++++++++++----- ...test_generate_suppression_guard_comment.py | 25 +++++++++++++++++++ 2 files changed, 43 insertions(+), 6 deletions(-) diff --git a/scripts/generate_suppression_guard_comment.py b/scripts/generate_suppression_guard_comment.py index e6aff874e..b50cb508b 100755 --- a/scripts/generate_suppression_guard_comment.py +++ b/scripts/generate_suppression_guard_comment.py @@ -40,6 +40,14 @@ ("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. +_SUPPRESS_NEGATION_RE = re.compile( + r"suppress_comments\s*!=\s*true" + r"|suppress_comments\s*==\s*false" + r"|!\s*inputs\.suppress_comments" +) + def _normalise_keys(node: Any) -> Any: if isinstance(node, dict): @@ -112,8 +120,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 +150,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..63a9e326d 100644 --- a/tests/scripts/test_generate_suppression_guard_comment.py +++ b/tests/scripts/test_generate_suppression_guard_comment.py @@ -90,6 +90,31 @@ 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_detects_octokit_aliases(tmp_path: Path) -> None: workflow_path = tmp_path / "octokit.yml" _write_yaml( From 8f792efdf242f9d811200ef994322b9c4d6d9b7b Mon Sep 17 00:00:00 2001 From: stranske Date: Tue, 10 Feb 2026 06:33:42 +0000 Subject: [PATCH 3/4] fix: handle parenthesized negation in suppress_comments guard detection Extend _SUPPRESS_NEGATION_RE to match !(inputs.suppress_comments) in 'needs-human' findings when the guard uses parenthesized negation. Add test for the parenthesized form. --- scripts/generate_suppression_guard_comment.py | 3 ++- ...test_generate_suppression_guard_comment.py | 25 +++++++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/scripts/generate_suppression_guard_comment.py b/scripts/generate_suppression_guard_comment.py index b50cb508b..12ed91b52 100755 --- a/scripts/generate_suppression_guard_comment.py +++ b/scripts/generate_suppression_guard_comment.py @@ -42,10 +42,11 @@ # 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*inputs\.suppress_comments" + r"|!\s*\(?\s*inputs\.suppress_comments" ) diff --git a/tests/scripts/test_generate_suppression_guard_comment.py b/tests/scripts/test_generate_suppression_guard_comment.py index 63a9e326d..850622e1d 100644 --- a/tests/scripts/test_generate_suppression_guard_comment.py +++ b/tests/scripts/test_generate_suppression_guard_comment.py @@ -115,6 +115,31 @@ def test_build_comment_flags_inverted_suppress_comments_guard( 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( From c12523925486c7135349f3454f6ac6142819e1b9 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Tue, 10 Feb 2026 06:37:06 +0000 Subject: [PATCH 4/4] chore(codex-autofix): apply updates (PR #1431) --- scripts/check_issue_consistency.py | 7 ++++++ tests/scripts/test_issue_consistency_check.py | 25 +++++++++++++++++++ 2 files changed, 32 insertions(+) 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/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")