From 19bd367e397b50e1d2bb25b6b771cc03aeb5acc6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ra=C3=BAl=20Negr=C3=B3n?= Date: Tue, 21 Jul 2026 11:50:46 -0400 Subject: [PATCH 01/10] chore(devex): lint PR event fanout --- .../skills/authoring-ci-workflows/SKILL.md | 2 +- .../tests/test_workflow_lint.py | 113 ++++++++++++++++++ .../hogli_commands/workflow_lint/check.py | 2 + .../workflow_lint/checks/__init__.py | 2 + .../workflow_lint/checks/pr_event_fanout.py | 112 +++++++++++++++++ .../hogli_commands/workflow_lint/cli.py | 44 ++++--- 6 files changed, 259 insertions(+), 16 deletions(-) create mode 100644 tools/hogli-commands/hogli_commands/workflow_lint/checks/pr_event_fanout.py diff --git a/.agents/skills/authoring-ci-workflows/SKILL.md b/.agents/skills/authoring-ci-workflows/SKILL.md index 35b9e77f853c..739c96f8d72e 100644 --- a/.agents/skills/authoring-ci-workflows/SKILL.md +++ b/.agents/skills/authoring-ci-workflows/SKILL.md @@ -26,7 +26,7 @@ The linters own the mechanical rules (below); this skill is the **judgment calls ## What the linters already enforce Run `bin/hogli lint:workflows` and `actionlint` before pushing — they gate CI, and they (not this list) are the source of truth for what's enforced. -Today that's: `timeout-minutes` on every job, the canonical PR concurrency block, `dorny/paths-filter` negation safety, justification for full-depth checkouts, cache-write gating, semgrep service coverage, and generic GHA correctness (bad `secrets.*` / `needs:` refs, deprecated `::set-output`, unknown runner labels). +Today that's: `timeout-minutes` on every job, the canonical PR concurrency block, an advisory repo-wide budget for unscoped PR event dispatches, `dorny/paths-filter` negation safety, justification for full-depth checkouts, cache-write gating, semgrep service coverage, and generic GHA correctness (bad `secrets.*` / `needs:` refs, deprecated `::set-output`, unknown runner labels). Third-party action digests are bumped by Renovate. ## The dispatch budget (500 runs / 10s / repo) diff --git a/tools/hogli-commands/hogli_commands/tests/test_workflow_lint.py b/tools/hogli-commands/hogli_commands/tests/test_workflow_lint.py index 3741d53d926a..cfd94099e222 100644 --- a/tools/hogli-commands/hogli_commands/tests/test_workflow_lint.py +++ b/tools/hogli-commands/hogli_commands/tests/test_workflow_lint.py @@ -13,6 +13,7 @@ import pytest +from click.testing import CliRunner from hogli_commands.workflow_lint.check import CheckResult, WorkflowCheck from hogli_commands.workflow_lint.checks import CHECKS, _build_lookup, get_check from hogli_commands.workflow_lint.checks.cache_writes import ( @@ -26,7 +27,9 @@ from hogli_commands.workflow_lint.checks.dorny_negation import DornyNegationCheck from hogli_commands.workflow_lint.checks.job_timeouts import JobTimeoutsCheck from hogli_commands.workflow_lint.checks.pr_concurrency import PrConcurrencyCheck +from hogli_commands.workflow_lint.checks.pr_event_fanout import PrEventFanoutCheck from hogli_commands.workflow_lint.checks.semgrep_services_coverage import SemgrepServicesCoverageCheck +from hogli_commands.workflow_lint.cli import cmd_lint_workflows from hogli_commands.workflow_lint.model import PR_TRIGGERS, Workflow, WorkflowParseError, read_workflows @@ -368,6 +371,71 @@ def test_skips_listed_filenames(self, tmp_path: Path) -> None: assert PrConcurrencyCheck().run(_read_all(tmp_path)).issues == [] +# --------------------------------------------------------------------------- +# PrEventFanoutCheck +# --------------------------------------------------------------------------- + + +class TestPrEventFanoutCheck: + def test_fails_when_low_frequency_workflow_adds_unscoped_pr_events(self, tmp_path: Path) -> None: + _write( + tmp_path, + "agent.yml", + """ + name: Agent + on: + pull_request: + types: [closed] + pull_request_target: + types: [opened, reopened, ready_for_review, edited] + jobs: {} + """, + ) + + result = PrEventFanoutCheck(budget={"closed": 1}).run(_read_all(tmp_path)) + + assert [issue.message for issue in result.issues] == [ + "unscoped `edited` PR dispatch fanout is 1; budget is 0", + "unscoped `opened` PR dispatch fanout is 1; budget is 0", + "unscoped `ready_for_review` PR dispatch fanout is 1; budget is 0", + "unscoped `reopened` PR dispatch fanout is 1; budget is 0", + ] + + def test_counts_default_pr_actions(self, tmp_path: Path) -> None: + _write( + tmp_path, + "new-workflow.yml", + """ + name: New workflow + on: [pull_request] + jobs: {} + """, + ) + + result = PrEventFanoutCheck(budget={}).run(_read_all(tmp_path)) + + assert [issue.message for issue in result.issues] == [ + "unscoped `opened` PR dispatch fanout is 1; budget is 0", + "unscoped `reopened` PR dispatch fanout is 1; budget is 0", + "unscoped `synchronize` PR dispatch fanout is 1; budget is 0", + ] + + def test_excludes_path_filtered_workflows(self, tmp_path: Path) -> None: + _write( + tmp_path, + "focused.yml", + """ + name: Focused + on: + pull_request: + paths: [products/example/**] + jobs: {} + """, + ) + + assert PrEventFanoutCheck(budget={}).run(_read_all(tmp_path)).issues == [] + + # --------------------------------------------------------------------------- # DornyNegationCheck # --------------------------------------------------------------------------- @@ -875,6 +943,51 @@ def test_run_returns_check_result(self, tmp_path: Path) -> None: assert isinstance(result, CheckResult) +class TestCli: + def test_advisory_check_reports_without_failing(self, tmp_path: Path) -> None: + _write( + tmp_path, + "assigned.yml", + """ + name: Assigned + on: + pull_request: + types: [assigned] + jobs: {} + """, + ) + + result = CliRunner().invoke( + cmd_lint_workflows, + ["--check", "WF007", "--workflows-dir", str(tmp_path)], + ) + + assert result.exit_code == 0 + assert "1 advisory issue(s); all blocking checks passed" in result.output + + def test_blocking_check_still_fails(self, tmp_path: Path) -> None: + _write( + tmp_path, + "missing-timeout.yml", + """ + name: Missing timeout + on: [push] + jobs: + test: + runs-on: ubuntu-latest + steps: [] + """, + ) + + result = CliRunner().invoke( + cmd_lint_workflows, + ["--check", "WF001", "--workflows-dir", str(tmp_path)], + ) + + assert result.exit_code == 1 + assert "1 blocking issue(s)" in result.output + + class TestLiveTreeSmoke: """Smoke test against the live ``.github/workflows/`` tree. diff --git a/tools/hogli-commands/hogli_commands/workflow_lint/check.py b/tools/hogli-commands/hogli_commands/workflow_lint/check.py index 6d0fbfabaf92..7f5de7ae7b49 100644 --- a/tools/hogli-commands/hogli_commands/workflow_lint/check.py +++ b/tools/hogli-commands/hogli_commands/workflow_lint/check.py @@ -44,11 +44,13 @@ class WorkflowCheck(ABC): - ``id``: stable, machine-friendly identifier (used by ``--check`` filter). - ``label``: short human-readable name shown in CLI output and GH annotations. - ``description``: one-line summary; shown by ``--list``. + - ``blocking``: whether reported issues make the command exit nonzero. """ id: str label: str description: str + blocking: bool = True @abstractmethod def run(self, workflows: list[Workflow]) -> CheckResult: ... diff --git a/tools/hogli-commands/hogli_commands/workflow_lint/checks/__init__.py b/tools/hogli-commands/hogli_commands/workflow_lint/checks/__init__.py index 4df3110b329b..3490d7dc6123 100644 --- a/tools/hogli-commands/hogli_commands/workflow_lint/checks/__init__.py +++ b/tools/hogli-commands/hogli_commands/workflow_lint/checks/__init__.py @@ -17,6 +17,7 @@ from .dorny_negation import DornyNegationCheck from .job_timeouts import JobTimeoutsCheck from .pr_concurrency import PrConcurrencyCheck +from .pr_event_fanout import PrEventFanoutCheck from .semgrep_services_coverage import SemgrepServicesCoverageCheck CHECKS: list[WorkflowCheck] = [ @@ -26,6 +27,7 @@ SemgrepServicesCoverageCheck(), CheckoutFullDepthCheck(), CacheWriteGateCheck(), + PrEventFanoutCheck(), ] diff --git a/tools/hogli-commands/hogli_commands/workflow_lint/checks/pr_event_fanout.py b/tools/hogli-commands/hogli_commands/workflow_lint/checks/pr_event_fanout.py new file mode 100644 index 000000000000..8e72cdbd1890 --- /dev/null +++ b/tools/hogli-commands/hogli_commands/workflow_lint/checks/pr_event_fanout.py @@ -0,0 +1,112 @@ +"""Cap repo-wide fanout from unscoped pull request event subscriptions. + +GitHub counts each directly triggered workflow as a separate run. Small jobs +that listen to every PR should therefore share an existing dispatcher instead +of adding another top-level ``pull_request`` or ``pull_request_target`` trigger. +Path-filtered workflows are excluded because they only dispatch for a subset of +changes. + +The per-action ceilings make any increase explicit in code review. Raising one +is allowed when a separate dispatch is justified, but it spends a shared +repo-wide budget and should not happen as a side effect of adding a small job. +""" + +from __future__ import annotations + +from collections import Counter +from collections.abc import Iterator, Mapping + +from ..check import CheckResult, Issue, WorkflowCheck +from ..model import PR_TRIGGERS, Workflow + +DEFAULT_PR_ACTIONS = frozenset({"opened", "reopened", "synchronize"}) + +PR_EVENT_FANOUT_BUDGET: Mapping[str, int] = { + "closed": 3, + "converted_to_draft": 1, + "edited": 3, + "labeled": 10, + "opened": 28, + "ready_for_review": 11, + "reopened": 24, + "review_requested": 1, + "synchronize": 28, + "unlabeled": 7, +} + + +def _trigger_configurations(on: object) -> dict[str, object]: + if isinstance(on, str): + return {on: None} + if isinstance(on, list): + return dict.fromkeys(str(trigger) for trigger in on) + if isinstance(on, dict): + return {str(trigger): config for trigger, config in on.items()} + return {} + + +def _configured_actions(config: object) -> frozenset[str]: + if not isinstance(config, dict): + return DEFAULT_PR_ACTIONS + types = config.get("types") + if isinstance(types, str): + return frozenset({types}) + if isinstance(types, list): + actions = frozenset(str(action) for action in types) + return actions or DEFAULT_PR_ACTIONS + return DEFAULT_PR_ACTIONS + + +def _has_paths_filter(config: object) -> bool: + return isinstance(config, dict) and isinstance(config.get("paths"), list) and bool(config["paths"]) + + +def _unscoped_pr_actions(workflow: Workflow) -> Iterator[str]: + triggers = _trigger_configurations(workflow.on) + for event in PR_TRIGGERS: + if event not in triggers: + continue + config = triggers[event] + if _has_paths_filter(config): + continue + yield from _configured_actions(config) + + +class PrEventFanoutCheck(WorkflowCheck): + id = "WF007-pr-event-fanout" + label = "PR event fanout" + description = "unscoped PR event subscriptions stay within the repo-wide workflow dispatch budget" + blocking = False + + def __init__(self, budget: Mapping[str, int] | None = None) -> None: + self._budget = dict(PR_EVENT_FANOUT_BUDGET if budget is None else budget) + + @property + def fix_hint(self) -> str | None: + return ( + "Avoid adding another always-fire workflow run. Fold small jobs into an existing dispatcher " + "with the same event and security context, or add a trigger-level `paths:` filter when the whole " + "workflow is skippable. If another dispatch is necessary, raise the relevant " + "`PR_EVENT_FANOUT_BUDGET` ceiling so the cost is explicit in review." + ) + + def run(self, workflows: list[Workflow]) -> CheckResult: + action_counts: Counter[str] = Counter() + for workflow in workflows: + action_counts.update(_unscoped_pr_actions(workflow)) + + result = CheckResult() + for action, count in sorted(action_counts.items()): + budget = self._budget.get(action, 0) + if count <= budget: + continue + result.issues.append( + Issue( + workflow=".github/workflows", + message=f"unscoped `{action}` PR dispatch fanout is {count}; budget is {budget}", + ) + ) + return result + + +__all__ = ["PR_EVENT_FANOUT_BUDGET", "PrEventFanoutCheck"] diff --git a/tools/hogli-commands/hogli_commands/workflow_lint/cli.py b/tools/hogli-commands/hogli_commands/workflow_lint/cli.py index 9c3a0f47d190..ae54c57ac8ed 100644 --- a/tools/hogli-commands/hogli_commands/workflow_lint/cli.py +++ b/tools/hogli-commands/hogli_commands/workflow_lint/cli.py @@ -26,16 +26,19 @@ def _gh_annotation(level: str, check_label: str, issue: Issue) -> None: click.echo(f"::{level}{file_part} title=lint:workflows ({check_label})::{issue.render()}") -def _run_one(check: WorkflowCheck, workflows: list[Workflow]) -> int: - """Run a single check, print results, return the issue count.""" - click.echo(f" {check.id} ({check.label})...") +def _run_one(check: WorkflowCheck, workflows: list[Workflow]) -> tuple[int, int]: + """Run a single check and return total and blocking issue counts.""" + mode = "" if check.blocking else ", advisory" + click.echo(f" {check.id} ({check.label}{mode})...") result = check.run(workflows) for issue in result.issues: - click.echo(f" ✗ {issue.render()}") - _gh_annotation("error", check.label, issue) + marker = "✗" if check.blocking else "!" + click.echo(f" {marker} {issue.render()}") + _gh_annotation("error" if check.blocking else "warning", check.label, issue) if not result.issues: click.echo(" ✓ ok") - return len(result.issues) + issue_count = len(result.issues) + return issue_count, issue_count if check.blocking else 0 def _default_workflows_dir() -> Path: @@ -63,7 +66,8 @@ def _default_workflows_dir() -> Path: def cmd_lint_workflows(check_id: str | None, list_checks: bool, workflows_dir: Path | None) -> None: if list_checks: for check in CHECKS: - click.echo(f"{check.id}\t{check.label} — {check.description}") + mode = "" if check.blocking else " [advisory]" + click.echo(f"{check.id}\t{check.label}{mode} — {check.description}") return if check_id is not None: @@ -84,19 +88,29 @@ def cmd_lint_workflows(check_id: str | None, list_checks: bool, workflows_dir: P click.echo(f"Linting {len(workflows)} workflow(s) with {len(selected)} check(s):\n") total_issues = 0 - failing_checks: list[WorkflowCheck] = [] + blocking_issues = 0 + checks_with_issues: list[WorkflowCheck] = [] for check in selected: - issues = _run_one(check, workflows) - total_issues += issues - if issues: - failing_checks.append(check) + check_issues, check_blocking_issues = _run_one(check, workflows) + total_issues += check_issues + blocking_issues += check_blocking_issues + if check_issues: + checks_with_issues.append(check) click.echo("") - if failing_checks: - for check in failing_checks: + if checks_with_issues: + for check in checks_with_issues: if check.fix_hint: click.echo(f"Fix for {check.id}:\n{check.fix_hint}\n") - click.echo(f"✗ {total_issues} issue(s) across {len(failing_checks)} check(s)") + + if blocking_issues: + advisory_issues = total_issues - blocking_issues + advisory_summary = f" and {advisory_issues} advisory issue(s)" if advisory_issues else "" + click.echo(f"✗ {blocking_issues} blocking issue(s){advisory_summary}") raise SystemExit(1) + if total_issues: + click.echo(f"! {total_issues} advisory issue(s); all blocking checks passed") + return + click.echo(f"✓ All {len(selected)} check(s) passed across {len(workflows)} workflow(s)") From c4094df05abf5f5251de28ffd998404ff032fe53 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ra=C3=BAl=20Negr=C3=B3n?= Date: Tue, 21 Jul 2026 13:06:54 -0400 Subject: [PATCH 02/10] chore: renumber fanout check to WF008 --- tools/hogli-commands/hogli_commands/tests/test_workflow_lint.py | 2 +- .../hogli_commands/workflow_lint/checks/pr_event_fanout.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/hogli-commands/hogli_commands/tests/test_workflow_lint.py b/tools/hogli-commands/hogli_commands/tests/test_workflow_lint.py index cfd94099e222..a16f3b710d86 100644 --- a/tools/hogli-commands/hogli_commands/tests/test_workflow_lint.py +++ b/tools/hogli-commands/hogli_commands/tests/test_workflow_lint.py @@ -959,7 +959,7 @@ def test_advisory_check_reports_without_failing(self, tmp_path: Path) -> None: result = CliRunner().invoke( cmd_lint_workflows, - ["--check", "WF007", "--workflows-dir", str(tmp_path)], + ["--check", "WF008", "--workflows-dir", str(tmp_path)], ) assert result.exit_code == 0 diff --git a/tools/hogli-commands/hogli_commands/workflow_lint/checks/pr_event_fanout.py b/tools/hogli-commands/hogli_commands/workflow_lint/checks/pr_event_fanout.py index 8e72cdbd1890..cb3a1946b0a8 100644 --- a/tools/hogli-commands/hogli_commands/workflow_lint/checks/pr_event_fanout.py +++ b/tools/hogli-commands/hogli_commands/workflow_lint/checks/pr_event_fanout.py @@ -73,7 +73,7 @@ def _unscoped_pr_actions(workflow: Workflow) -> Iterator[str]: class PrEventFanoutCheck(WorkflowCheck): - id = "WF007-pr-event-fanout" + id = "WF008-pr-event-fanout" label = "PR event fanout" description = "unscoped PR event subscriptions stay within the repo-wide workflow dispatch budget" blocking = False From 9abae711b5249f27b4313b80a39775bc0f1b3d40 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ra=C3=BAl=20Negr=C3=B3n?= Date: Tue, 21 Jul 2026 13:07:10 -0400 Subject: [PATCH 03/10] refactor: simplify lint check issue tally --- .../hogli_commands/workflow_lint/cli.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/tools/hogli-commands/hogli_commands/workflow_lint/cli.py b/tools/hogli-commands/hogli_commands/workflow_lint/cli.py index ae54c57ac8ed..15073abac9e4 100644 --- a/tools/hogli-commands/hogli_commands/workflow_lint/cli.py +++ b/tools/hogli-commands/hogli_commands/workflow_lint/cli.py @@ -26,19 +26,18 @@ def _gh_annotation(level: str, check_label: str, issue: Issue) -> None: click.echo(f"::{level}{file_part} title=lint:workflows ({check_label})::{issue.render()}") -def _run_one(check: WorkflowCheck, workflows: list[Workflow]) -> tuple[int, int]: - """Run a single check and return total and blocking issue counts.""" +def _run_one(check: WorkflowCheck, workflows: list[Workflow]) -> int: + """Run a single check, print results, return the issue count.""" mode = "" if check.blocking else ", advisory" click.echo(f" {check.id} ({check.label}{mode})...") result = check.run(workflows) + marker = "✗" if check.blocking else "!" for issue in result.issues: - marker = "✗" if check.blocking else "!" click.echo(f" {marker} {issue.render()}") _gh_annotation("error" if check.blocking else "warning", check.label, issue) if not result.issues: click.echo(" ✓ ok") - issue_count = len(result.issues) - return issue_count, issue_count if check.blocking else 0 + return len(result.issues) def _default_workflows_dir() -> Path: @@ -91,9 +90,10 @@ def cmd_lint_workflows(check_id: str | None, list_checks: bool, workflows_dir: P blocking_issues = 0 checks_with_issues: list[WorkflowCheck] = [] for check in selected: - check_issues, check_blocking_issues = _run_one(check, workflows) + check_issues = _run_one(check, workflows) total_issues += check_issues - blocking_issues += check_blocking_issues + if check.blocking: + blocking_issues += check_issues if check_issues: checks_with_issues.append(check) From 21ef9cbf11d27331ab01878124615d76c402e29d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ra=C3=BAl=20Negr=C3=B3n?= Date: Tue, 21 Jul 2026 13:43:48 -0400 Subject: [PATCH 04/10] chore: make fanout check blocking, drop advisory mode --- .../skills/authoring-ci-workflows/SKILL.md | 2 +- .../tests/test_workflow_lint.py | 26 +------------ .../hogli_commands/workflow_lint/check.py | 2 - .../workflow_lint/checks/pr_event_fanout.py | 1 - .../hogli_commands/workflow_lint/cli.py | 38 ++++++------------- 5 files changed, 15 insertions(+), 54 deletions(-) diff --git a/.agents/skills/authoring-ci-workflows/SKILL.md b/.agents/skills/authoring-ci-workflows/SKILL.md index 739c96f8d72e..c18fc3e24da2 100644 --- a/.agents/skills/authoring-ci-workflows/SKILL.md +++ b/.agents/skills/authoring-ci-workflows/SKILL.md @@ -26,7 +26,7 @@ The linters own the mechanical rules (below); this skill is the **judgment calls ## What the linters already enforce Run `bin/hogli lint:workflows` and `actionlint` before pushing — they gate CI, and they (not this list) are the source of truth for what's enforced. -Today that's: `timeout-minutes` on every job, the canonical PR concurrency block, an advisory repo-wide budget for unscoped PR event dispatches, `dorny/paths-filter` negation safety, justification for full-depth checkouts, cache-write gating, semgrep service coverage, and generic GHA correctness (bad `secrets.*` / `needs:` refs, deprecated `::set-output`, unknown runner labels). +Today that's: `timeout-minutes` on every job, the canonical PR concurrency block, a repo-wide budget for unscoped PR event dispatches, `dorny/paths-filter` negation safety, justification for full-depth checkouts, cache-write gating, semgrep service coverage, and generic GHA correctness (bad `secrets.*` / `needs:` refs, deprecated `::set-output`, unknown runner labels). Third-party action digests are bumped by Renovate. ## The dispatch budget (500 runs / 10s / repo) diff --git a/tools/hogli-commands/hogli_commands/tests/test_workflow_lint.py b/tools/hogli-commands/hogli_commands/tests/test_workflow_lint.py index a16f3b710d86..163922c15bf7 100644 --- a/tools/hogli-commands/hogli_commands/tests/test_workflow_lint.py +++ b/tools/hogli-commands/hogli_commands/tests/test_workflow_lint.py @@ -944,7 +944,7 @@ def test_run_returns_check_result(self, tmp_path: Path) -> None: class TestCli: - def test_advisory_check_reports_without_failing(self, tmp_path: Path) -> None: + def test_fanout_over_budget_fails_run(self, tmp_path: Path) -> None: _write( tmp_path, "assigned.yml", @@ -962,30 +962,8 @@ def test_advisory_check_reports_without_failing(self, tmp_path: Path) -> None: ["--check", "WF008", "--workflows-dir", str(tmp_path)], ) - assert result.exit_code == 0 - assert "1 advisory issue(s); all blocking checks passed" in result.output - - def test_blocking_check_still_fails(self, tmp_path: Path) -> None: - _write( - tmp_path, - "missing-timeout.yml", - """ - name: Missing timeout - on: [push] - jobs: - test: - runs-on: ubuntu-latest - steps: [] - """, - ) - - result = CliRunner().invoke( - cmd_lint_workflows, - ["--check", "WF001", "--workflows-dir", str(tmp_path)], - ) - assert result.exit_code == 1 - assert "1 blocking issue(s)" in result.output + assert "1 issue(s) across 1 check(s)" in result.output class TestLiveTreeSmoke: diff --git a/tools/hogli-commands/hogli_commands/workflow_lint/check.py b/tools/hogli-commands/hogli_commands/workflow_lint/check.py index 7f5de7ae7b49..6d0fbfabaf92 100644 --- a/tools/hogli-commands/hogli_commands/workflow_lint/check.py +++ b/tools/hogli-commands/hogli_commands/workflow_lint/check.py @@ -44,13 +44,11 @@ class WorkflowCheck(ABC): - ``id``: stable, machine-friendly identifier (used by ``--check`` filter). - ``label``: short human-readable name shown in CLI output and GH annotations. - ``description``: one-line summary; shown by ``--list``. - - ``blocking``: whether reported issues make the command exit nonzero. """ id: str label: str description: str - blocking: bool = True @abstractmethod def run(self, workflows: list[Workflow]) -> CheckResult: ... diff --git a/tools/hogli-commands/hogli_commands/workflow_lint/checks/pr_event_fanout.py b/tools/hogli-commands/hogli_commands/workflow_lint/checks/pr_event_fanout.py index cb3a1946b0a8..19e2033ce613 100644 --- a/tools/hogli-commands/hogli_commands/workflow_lint/checks/pr_event_fanout.py +++ b/tools/hogli-commands/hogli_commands/workflow_lint/checks/pr_event_fanout.py @@ -76,7 +76,6 @@ class PrEventFanoutCheck(WorkflowCheck): id = "WF008-pr-event-fanout" label = "PR event fanout" description = "unscoped PR event subscriptions stay within the repo-wide workflow dispatch budget" - blocking = False def __init__(self, budget: Mapping[str, int] | None = None) -> None: self._budget = dict(PR_EVENT_FANOUT_BUDGET if budget is None else budget) diff --git a/tools/hogli-commands/hogli_commands/workflow_lint/cli.py b/tools/hogli-commands/hogli_commands/workflow_lint/cli.py index 15073abac9e4..9c3a0f47d190 100644 --- a/tools/hogli-commands/hogli_commands/workflow_lint/cli.py +++ b/tools/hogli-commands/hogli_commands/workflow_lint/cli.py @@ -28,13 +28,11 @@ def _gh_annotation(level: str, check_label: str, issue: Issue) -> None: def _run_one(check: WorkflowCheck, workflows: list[Workflow]) -> int: """Run a single check, print results, return the issue count.""" - mode = "" if check.blocking else ", advisory" - click.echo(f" {check.id} ({check.label}{mode})...") + click.echo(f" {check.id} ({check.label})...") result = check.run(workflows) - marker = "✗" if check.blocking else "!" for issue in result.issues: - click.echo(f" {marker} {issue.render()}") - _gh_annotation("error" if check.blocking else "warning", check.label, issue) + click.echo(f" ✗ {issue.render()}") + _gh_annotation("error", check.label, issue) if not result.issues: click.echo(" ✓ ok") return len(result.issues) @@ -65,8 +63,7 @@ def _default_workflows_dir() -> Path: def cmd_lint_workflows(check_id: str | None, list_checks: bool, workflows_dir: Path | None) -> None: if list_checks: for check in CHECKS: - mode = "" if check.blocking else " [advisory]" - click.echo(f"{check.id}\t{check.label}{mode} — {check.description}") + click.echo(f"{check.id}\t{check.label} — {check.description}") return if check_id is not None: @@ -87,30 +84,19 @@ def cmd_lint_workflows(check_id: str | None, list_checks: bool, workflows_dir: P click.echo(f"Linting {len(workflows)} workflow(s) with {len(selected)} check(s):\n") total_issues = 0 - blocking_issues = 0 - checks_with_issues: list[WorkflowCheck] = [] + failing_checks: list[WorkflowCheck] = [] for check in selected: - check_issues = _run_one(check, workflows) - total_issues += check_issues - if check.blocking: - blocking_issues += check_issues - if check_issues: - checks_with_issues.append(check) + issues = _run_one(check, workflows) + total_issues += issues + if issues: + failing_checks.append(check) click.echo("") - if checks_with_issues: - for check in checks_with_issues: + if failing_checks: + for check in failing_checks: if check.fix_hint: click.echo(f"Fix for {check.id}:\n{check.fix_hint}\n") - - if blocking_issues: - advisory_issues = total_issues - blocking_issues - advisory_summary = f" and {advisory_issues} advisory issue(s)" if advisory_issues else "" - click.echo(f"✗ {blocking_issues} blocking issue(s){advisory_summary}") + click.echo(f"✗ {total_issues} issue(s) across {len(failing_checks)} check(s)") raise SystemExit(1) - if total_issues: - click.echo(f"! {total_issues} advisory issue(s); all blocking checks passed") - return - click.echo(f"✓ All {len(selected)} check(s) passed across {len(workflows)} workflow(s)") From f9c27f0676037bf7e23f24832f9b31225ddd8612 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ra=C3=BAl=20Negr=C3=B3n?= Date: Tue, 4 Aug 2026 11:09:11 -0400 Subject: [PATCH 05/10] chore(ci): ratchet label fanout budgets to the current tree --- .../hogli_commands/workflow_lint/checks/pr_event_fanout.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/hogli-commands/hogli_commands/workflow_lint/checks/pr_event_fanout.py b/tools/hogli-commands/hogli_commands/workflow_lint/checks/pr_event_fanout.py index 19e2033ce613..6c2959e50c90 100644 --- a/tools/hogli-commands/hogli_commands/workflow_lint/checks/pr_event_fanout.py +++ b/tools/hogli-commands/hogli_commands/workflow_lint/checks/pr_event_fanout.py @@ -25,13 +25,13 @@ "closed": 3, "converted_to_draft": 1, "edited": 3, - "labeled": 10, + "labeled": 7, "opened": 28, "ready_for_review": 11, "reopened": 24, "review_requested": 1, "synchronize": 28, - "unlabeled": 7, + "unlabeled": 4, } From cc0b38480c33afc34b2a991dac390da8f19e817e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ra=C3=BAl=20Negr=C3=B3n?= Date: Wed, 5 Aug 2026 11:24:10 -0400 Subject: [PATCH 06/10] chore(ci): parameterize fanout tests, pin paths-ignore --- .../tests/test_workflow_lint.py | 124 ++++++++++-------- .../workflow_lint/checks/pr_event_fanout.py | 5 +- 2 files changed, 72 insertions(+), 57 deletions(-) diff --git a/tools/hogli-commands/hogli_commands/tests/test_workflow_lint.py b/tools/hogli-commands/hogli_commands/tests/test_workflow_lint.py index 3f1ebf2a6d3c..8677c1835a2a 100644 --- a/tools/hogli-commands/hogli_commands/tests/test_workflow_lint.py +++ b/tools/hogli-commands/hogli_commands/tests/test_workflow_lint.py @@ -406,63 +406,77 @@ def test_skips_listed_filenames(self, tmp_path: Path) -> None: class TestPrEventFanoutCheck: - def test_fails_when_low_frequency_workflow_adds_unscoped_pr_events(self, tmp_path: Path) -> None: - _write( - tmp_path, - "agent.yml", - """ - name: Agent - on: - pull_request: - types: [closed] - pull_request_target: - types: [opened, reopened, ready_for_review, edited] - jobs: {} - """, - ) - - result = PrEventFanoutCheck(budget={"closed": 1}).run(_read_all(tmp_path)) - - assert [issue.message for issue in result.issues] == [ - "unscoped `edited` PR dispatch fanout is 1; budget is 0", - "unscoped `opened` PR dispatch fanout is 1; budget is 0", - "unscoped `ready_for_review` PR dispatch fanout is 1; budget is 0", - "unscoped `reopened` PR dispatch fanout is 1; budget is 0", - ] - - def test_counts_default_pr_actions(self, tmp_path: Path) -> None: - _write( - tmp_path, - "new-workflow.yml", - """ - name: New workflow - on: [pull_request] - jobs: {} - """, - ) - - result = PrEventFanoutCheck(budget={}).run(_read_all(tmp_path)) - - assert [issue.message for issue in result.issues] == [ - "unscoped `opened` PR dispatch fanout is 1; budget is 0", - "unscoped `reopened` PR dispatch fanout is 1; budget is 0", - "unscoped `synchronize` PR dispatch fanout is 1; budget is 0", - ] + @pytest.mark.parametrize( + "workflow,budget,expected", + [ + ( + """ + name: Agent + on: + pull_request: + types: [closed] + pull_request_target: + types: [opened, reopened, ready_for_review, edited] + jobs: {} + """, + {"closed": 1}, + [ + "unscoped `edited` PR dispatch fanout is 1; budget is 0", + "unscoped `opened` PR dispatch fanout is 1; budget is 0", + "unscoped `ready_for_review` PR dispatch fanout is 1; budget is 0", + "unscoped `reopened` PR dispatch fanout is 1; budget is 0", + ], + ), + ( + """ + name: New workflow + on: [pull_request] + jobs: {} + """, + {}, + [ + "unscoped `opened` PR dispatch fanout is 1; budget is 0", + "unscoped `reopened` PR dispatch fanout is 1; budget is 0", + "unscoped `synchronize` PR dispatch fanout is 1; budget is 0", + ], + ), + ( + """ + name: Focused + on: + pull_request: + paths: [products/example/**] + jobs: {} + """, + {}, + [], + ), + # paths-ignore usually excludes a narrow slice, so it still fires on nearly every PR. + ( + """ + name: Nearly everything + on: + pull_request: + paths-ignore: [docs/**] + jobs: {} + """, + {}, + [ + "unscoped `opened` PR dispatch fanout is 1; budget is 0", + "unscoped `reopened` PR dispatch fanout is 1; budget is 0", + "unscoped `synchronize` PR dispatch fanout is 1; budget is 0", + ], + ), + ], + ) + def test_counts_unscoped_pr_dispatches( + self, tmp_path: Path, workflow: str, budget: dict[str, int], expected: list[str] + ) -> None: + _write(tmp_path, "workflow.yml", workflow) - def test_excludes_path_filtered_workflows(self, tmp_path: Path) -> None: - _write( - tmp_path, - "focused.yml", - """ - name: Focused - on: - pull_request: - paths: [products/example/**] - jobs: {} - """, - ) + result = PrEventFanoutCheck(budget=budget).run(_read_all(tmp_path)) - assert PrEventFanoutCheck(budget={}).run(_read_all(tmp_path)).issues == [] + assert [issue.message for issue in result.issues] == expected # --------------------------------------------------------------------------- diff --git a/tools/hogli-commands/hogli_commands/workflow_lint/checks/pr_event_fanout.py b/tools/hogli-commands/hogli_commands/workflow_lint/checks/pr_event_fanout.py index 6c2959e50c90..dcfb27ab8000 100644 --- a/tools/hogli-commands/hogli_commands/workflow_lint/checks/pr_event_fanout.py +++ b/tools/hogli-commands/hogli_commands/workflow_lint/checks/pr_event_fanout.py @@ -3,8 +3,9 @@ GitHub counts each directly triggered workflow as a separate run. Small jobs that listen to every PR should therefore share an existing dispatcher instead of adding another top-level ``pull_request`` or ``pull_request_target`` trigger. -Path-filtered workflows are excluded because they only dispatch for a subset of -changes. +A trigger-level ``paths:`` allowlist is excluded, because it only dispatches for +a subset of changes. ``paths-ignore`` still counts: it usually excludes a narrow +slice, so the workflow fires on nearly every PR anyway. The per-action ceilings make any increase explicit in code review. Raising one is allowed when a separate dispatch is justified, but it spends a shared From dd5490fcd997fc3e041858c50cde40da85a20fb7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ra=C3=BAl=20Negr=C3=B3n?= Date: Wed, 5 Aug 2026 11:27:40 -0400 Subject: [PATCH 07/10] chore(ci): exempt label triggers from the raise-the-ceiling escape --- .../hogli_commands/workflow_lint/checks/pr_event_fanout.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tools/hogli-commands/hogli_commands/workflow_lint/checks/pr_event_fanout.py b/tools/hogli-commands/hogli_commands/workflow_lint/checks/pr_event_fanout.py index dcfb27ab8000..680638da1f61 100644 --- a/tools/hogli-commands/hogli_commands/workflow_lint/checks/pr_event_fanout.py +++ b/tools/hogli-commands/hogli_commands/workflow_lint/checks/pr_event_fanout.py @@ -87,7 +87,11 @@ def fix_hint(self) -> str | None: "Avoid adding another always-fire workflow run. Fold small jobs into an existing dispatcher " "with the same event and security context, or add a trigger-level `paths:` filter when the whole " "workflow is skippable. If another dispatch is necessary, raise the relevant " - "`PR_EVENT_FANOUT_BUDGET` ceiling so the cost is explicit in review." + "`PR_EVENT_FANOUT_BUDGET` ceiling so the cost is explicit in review.\n" + "\n" + "Except for `labeled` / `unlabeled` on a merge gate: AGENTS.md bans re-adding those outright, so " + "raise neither ceiling for one. GitHub cannot filter a label trigger by name, so every unrelated " + "label re-runs the full matrix against a commit CI already covered." ) def run(self, workflows: list[Workflow]) -> CheckResult: From f8f1f5e797668b3c45624a4034bce5c3ce8994d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ra=C3=BAl=20Negr=C3=B3n?= Date: Wed, 5 Aug 2026 11:34:41 -0400 Subject: [PATCH 08/10] chore(ci): tighten fanout check internals --- .../tests/test_workflow_lint.py | 5 +++++ .../workflow_lint/checks/pr_event_fanout.py | 22 ++++++------------- 2 files changed, 12 insertions(+), 15 deletions(-) diff --git a/tools/hogli-commands/hogli_commands/tests/test_workflow_lint.py b/tools/hogli-commands/hogli_commands/tests/test_workflow_lint.py index 8677c1835a2a..e8c8468e9daa 100644 --- a/tools/hogli-commands/hogli_commands/tests/test_workflow_lint.py +++ b/tools/hogli-commands/hogli_commands/tests/test_workflow_lint.py @@ -1021,6 +1021,11 @@ def test_run_returns_check_result(self, tmp_path: Path) -> None: assert isinstance(result, CheckResult) +# --------------------------------------------------------------------------- +# CLI +# --------------------------------------------------------------------------- + + class TestCli: def test_fanout_over_budget_fails_run(self, tmp_path: Path) -> None: _write( diff --git a/tools/hogli-commands/hogli_commands/workflow_lint/checks/pr_event_fanout.py b/tools/hogli-commands/hogli_commands/workflow_lint/checks/pr_event_fanout.py index 680638da1f61..38890068a5c7 100644 --- a/tools/hogli-commands/hogli_commands/workflow_lint/checks/pr_event_fanout.py +++ b/tools/hogli-commands/hogli_commands/workflow_lint/checks/pr_event_fanout.py @@ -47,14 +47,11 @@ def _trigger_configurations(on: object) -> dict[str, object]: def _configured_actions(config: object) -> frozenset[str]: - if not isinstance(config, dict): - return DEFAULT_PR_ACTIONS - types = config.get("types") + types = config.get("types") if isinstance(config, dict) else None if isinstance(types, str): return frozenset({types}) if isinstance(types, list): - actions = frozenset(str(action) for action in types) - return actions or DEFAULT_PR_ACTIONS + return frozenset(str(action) for action in types) or DEFAULT_PR_ACTIONS return DEFAULT_PR_ACTIONS @@ -63,11 +60,9 @@ def _has_paths_filter(config: object) -> bool: def _unscoped_pr_actions(workflow: Workflow) -> Iterator[str]: - triggers = _trigger_configurations(workflow.on) - for event in PR_TRIGGERS: - if event not in triggers: + for event, config in _trigger_configurations(workflow.on).items(): + if event not in PR_TRIGGERS: continue - config = triggers[event] if _has_paths_filter(config): continue yield from _configured_actions(config) @@ -89,9 +84,9 @@ def fix_hint(self) -> str | None: "workflow is skippable. If another dispatch is necessary, raise the relevant " "`PR_EVENT_FANOUT_BUDGET` ceiling so the cost is explicit in review.\n" "\n" - "Except for `labeled` / `unlabeled` on a merge gate: AGENTS.md bans re-adding those outright, so " - "raise neither ceiling for one. GitHub cannot filter a label trigger by name, so every unrelated " - "label re-runs the full matrix against a commit CI already covered." + "Never raise the `labeled` / `unlabeled` ceilings for a merge gate. AGENTS.md bans those triggers " + "outright: GitHub cannot filter a label trigger by name, so every unrelated label re-runs the " + "full matrix against a commit CI already covered." ) def run(self, workflows: list[Workflow]) -> CheckResult: @@ -111,6 +106,3 @@ def run(self, workflows: list[Workflow]) -> CheckResult: ) ) return result - - -__all__ = ["PR_EVENT_FANOUT_BUDGET", "PrEventFanoutCheck"] From 40a86e12c616ef8d28006a0ad162cb105b0da530 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ra=C3=BAl=20Negr=C3=B3n?= Date: Wed, 5 Aug 2026 12:23:09 -0400 Subject: [PATCH 09/10] chore(ci): stop budgeting label dispatches --- .../hogli_commands/tests/test_workflow_lint.py | 12 ++++++++++++ .../workflow_lint/checks/pr_event_fanout.py | 15 +++++++-------- 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/tools/hogli-commands/hogli_commands/tests/test_workflow_lint.py b/tools/hogli-commands/hogli_commands/tests/test_workflow_lint.py index e8c8468e9daa..b0b7d77c12ad 100644 --- a/tools/hogli-commands/hogli_commands/tests/test_workflow_lint.py +++ b/tools/hogli-commands/hogli_commands/tests/test_workflow_lint.py @@ -467,6 +467,18 @@ class TestPrEventFanoutCheck: "unscoped `synchronize` PR dispatch fanout is 1; budget is 0", ], ), + # Label-driven workflows are the intended use, and labels cannot burst. + ( + """ + name: Opt in by label + on: + pull_request: + types: [labeled, unlabeled] + jobs: {} + """, + {}, + [], + ), ], ) def test_counts_unscoped_pr_dispatches( diff --git a/tools/hogli-commands/hogli_commands/workflow_lint/checks/pr_event_fanout.py b/tools/hogli-commands/hogli_commands/workflow_lint/checks/pr_event_fanout.py index 38890068a5c7..7fdfdbbd0fcf 100644 --- a/tools/hogli-commands/hogli_commands/workflow_lint/checks/pr_event_fanout.py +++ b/tools/hogli-commands/hogli_commands/workflow_lint/checks/pr_event_fanout.py @@ -22,17 +22,20 @@ DEFAULT_PR_ACTIONS = frozenset({"opened", "reopened", "synchronize"}) +# Labels arrive one PR at a time, so they cannot produce the simultaneous burst this +# budget guards, and every label subscriber left in the tree wants the trigger. Merge +# gates are a separate rule that a repo-wide sum cannot express; AGENTS.md owns it. +UNBUDGETED_ACTIONS = frozenset({"labeled", "unlabeled"}) + PR_EVENT_FANOUT_BUDGET: Mapping[str, int] = { "closed": 3, "converted_to_draft": 1, "edited": 3, - "labeled": 7, "opened": 28, "ready_for_review": 11, "reopened": 24, "review_requested": 1, "synchronize": 28, - "unlabeled": 4, } @@ -65,7 +68,7 @@ def _unscoped_pr_actions(workflow: Workflow) -> Iterator[str]: continue if _has_paths_filter(config): continue - yield from _configured_actions(config) + yield from _configured_actions(config) - UNBUDGETED_ACTIONS class PrEventFanoutCheck(WorkflowCheck): @@ -82,11 +85,7 @@ def fix_hint(self) -> str | None: "Avoid adding another always-fire workflow run. Fold small jobs into an existing dispatcher " "with the same event and security context, or add a trigger-level `paths:` filter when the whole " "workflow is skippable. If another dispatch is necessary, raise the relevant " - "`PR_EVENT_FANOUT_BUDGET` ceiling so the cost is explicit in review.\n" - "\n" - "Never raise the `labeled` / `unlabeled` ceilings for a merge gate. AGENTS.md bans those triggers " - "outright: GitHub cannot filter a label trigger by name, so every unrelated label re-runs the " - "full matrix against a commit CI already covered." + "`PR_EVENT_FANOUT_BUDGET` ceiling so the cost is explicit in review." ) def run(self, workflows: list[Workflow]) -> CheckResult: From 7980c95a89f59d9c9ec23a0e988d2ef15388ff74 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ra=C3=BAl=20Negr=C3=B3n?= Date: Wed, 5 Aug 2026 12:26:39 -0400 Subject: [PATCH 10/10] fix(ci): treat empty types list as no dispatch --- .../hogli_commands/workflow_lint/checks/pr_event_fanout.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tools/hogli-commands/hogli_commands/workflow_lint/checks/pr_event_fanout.py b/tools/hogli-commands/hogli_commands/workflow_lint/checks/pr_event_fanout.py index 7fdfdbbd0fcf..f5554264a2c2 100644 --- a/tools/hogli-commands/hogli_commands/workflow_lint/checks/pr_event_fanout.py +++ b/tools/hogli-commands/hogli_commands/workflow_lint/checks/pr_event_fanout.py @@ -54,7 +54,8 @@ def _configured_actions(config: object) -> frozenset[str]: if isinstance(types, str): return frozenset({types}) if isinstance(types, list): - return frozenset(str(action) for action in types) or DEFAULT_PR_ACTIONS + # An empty list selects no activity type, so the workflow never dispatches. + return frozenset(str(action) for action in types) return DEFAULT_PR_ACTIONS