diff --git a/README.md b/README.md index 01eea15..824bca5 100644 --- a/README.md +++ b/README.md @@ -234,10 +234,29 @@ Structured check results as JSON, available to downstream steps via Use `dry-run` (or `continue-on-error`) when a downstream step is meant to read the result and decide for itself. -Each scope carries the check outcomes (`rule_id`, `check`, `status`, `value`, -`error`, `suggest`, `docs_url`) exactly as produced by -`commit-check --format json`, so downstream jobs can build their own reports -or gate on individual rules. +The top-level `status` is one of: + +| `status` | Meaning | Exit code | +|---|---|---| +| `pass` | every check passed | 0 | +| `warn` | nothing failed, but a rule listed under the config's `warn` found something | 0 | +| `skip` | every check declined to run (for example the author is in `ignore_authors`) | 0 | +| `fail` | at least one check failed | 1 (0 with `dry-run`) | + +Only `fail` is ever non-zero; `warn` exists so a downstream step can react to a +bent-but-not-broken policy without the run turning red: + +```yaml +- if: fromJSON(steps.commit-check.outputs.result).status == 'warn' + run: echo "passed with warnings" +``` + +Each entry in `scopes` has a `label` (`PR title`, `Commit 2/3`, `Branch`, ...), +a `status` like the ones above, a `sha` (the full hash of the commit a +`Commit N/M` or `Commit message` scope checked; empty for the others) and the +check outcomes (`rule_id`, `check`, `status`, `value`, `error`, `suggest`, +`fix`, `docs_url`) exactly as produced by `commit-check --format json`, so +downstream jobs can build their own reports or gate on individual rules. ## GitHub Action Job Summary @@ -261,7 +280,7 @@ Passing runs stay to one line, with the detail folded away: > ```text > Commit message > ✔ PR title (feat: add login page) -> ✔ Commit 1/2 (feat: add login page) +> ✔ Commit 1/2 (d87faca) (feat: add login page) > Branch > ✔ Branch (feature/add-login) > ``` @@ -273,7 +292,8 @@ Passing runs stay to one line, with the detail folded away: ### Failure Job Summary Failures open with a count, then a table of only the scopes that failed — every -rule ID links to its documentation — with the full tree still one click away: +rule ID links to its documentation, and every commit to itself — with the full +tree still one click away: > **Commit Check** > @@ -281,8 +301,8 @@ rule ID links to its documentation — with the full tree still one click away: > > | Scope | Checked value | Failed checks | > |---|---|---| -> | Commit 2/2 | `bad msg` | [CC001 message](https://commit-check.com/rules/#cc001) | -> | Branch | `my-changes` | [CC201 branch](https://commit-check.com/rules/#cc201) | +> | [Commit 2/2 (5584f46)](https://github.com/acme/widgets/commit/5584f462cc3c947b2ba8d3d1a5735571803ee159) | `bad msg` | [CC001 message](https://commit-check.com/rules/#cc001) | +> | Branch | `Feature/Add-Login` | [CC201 branch](https://commit-check.com/rules/#cc201) | > >
> Show all 4 checks @@ -290,8 +310,8 @@ rule ID links to its documentation — with the full tree still one click away: > ```text > Commit message > ✔ PR title (feat: add login page) -> ✔ Commit 1/2 (feat: add login page) -> ✖ Commit 2/2 (1 failure) +> ✔ Commit 1/2 (d87faca) (feat: add login page) +> ✖ Commit 2/2 (5584f46) (1 failure) > CC001 message > value: bad msg > The commit message should follow Conventional Commits. @@ -299,9 +319,10 @@ rule ID links to its documentation — with the full tree still one click away: > Branch > ✖ Branch (1 failure) > CC201 branch -> value: my-changes +> value: Feature/Add-Login > The branch should follow Conventional Branch. -> Suggest: Use / with allowed types +> Suggest: Rename the branch to "feature/Add-Login" (git branch -m feature/Add-Login) +> Fix: feature/Add-Login > ``` > >
@@ -312,6 +333,26 @@ A scope is one thing that was checked — a commit message, the branch, the auth — not one rule evaluation, so the total matches the ✔/✖ lines you can count and does not grow with the number of rules in your config. +A commit scope names its commit by short hash, and the table row links to it, +so a reviewer can jump from a failed row straight to the offending commit. +`Fix:` is the corrected text commit-check proposes whenever the correction is +mechanical (a capitalised subject, a dropped WIP marker, a missing sign-off +trailer); when the suggestion is nothing more than "use the fix", only `Fix:` +is shown. + +The step log prints the same tree, then one annotation per finding — shown in +the run summary and on the Files changed tab — whose message carries the +commit, the checked value, the suggestion and the fix on separate lines: + +```text +::error title=CC001 message::Commit 2/2 (5584f46): The commit message should follow Conventional Commits.%0Avalue: bad msg%0ASuggest: Use (): +::error title=CC201 branch::Branch: The branch should follow Conventional Branch.%0Avalue: Feature/Add-Login%0ASuggest: Rename the branch to "feature/Add-Login" (git branch -m feature/Add-Login)%0AFix: feature/Add-Login +✖ commit-check: 2 of 4 checks failed +``` + +The verdict is a plain line rather than another `::error`, so the run's error +count equals the number of findings. + ### Skipped Job Summary Some runs validate nothing at all — most commonly when the commit author is @@ -383,7 +424,8 @@ A warned scope is marked `⚠`, never `✖`, and a real failure elsewhere still fails the run — the verdict then reads `❌ **N of M checks failed**, K warnings` and both tables appear. In the step log, a warning becomes a `::warning` annotation rather than `::error`, so it never counts toward the -run's error count. +run's error count. The [`result`](#result) output reports the run as +`"status": "warn"`, with exit code 0. This needs commit-check 2.17.0 or newer, which reports `"status": "warn"` in its JSON. Against an older engine, or a config with no `warn` list, no check diff --git a/action.yml b/action.yml index 22441e4..f0566de 100644 --- a/action.yml +++ b/action.yml @@ -39,7 +39,7 @@ inputs: default: false outputs: result: - description: Structured check results as JSON (status + per-scope checks). Consume with fromJSON(steps..outputs.result). + description: Structured check results as JSON (status pass/warn/skip/fail + per-scope label, sha and checks). Consume with fromJSON(steps..outputs.result). # Composite actions do not forward step outputs automatically: without this # mapping (and the step id it refers to) the output is always the empty # string, and fromJSON('') fails the calling workflow. diff --git a/main.py b/main.py index ded9b96..32b9209 100755 --- a/main.py +++ b/main.py @@ -5,7 +5,7 @@ results (rule IDs, error messages, suggestions, docs links), then renders them to three output surfaces: -* **step log** — grouped sections with ``::error`` annotations per rule +* **step log** — grouped sections, then one ``::error`` annotation per finding * **job summary** — a Markdown policy report table * **PR comment** — a compact Markdown summary (idempotently updated) """ @@ -93,11 +93,27 @@ class ScopeResult: ``checks`` holds the parsed JSON check outcomes (only set when the CLI produced valid JSON); ``raw_text`` holds the raw CLI output when parsing failed (a defensive fallback so unexpected output is never swallowed). + + ``sha`` is the full hash of the commit a message scope checked, or ``""`` + for scopes that have no commit (PR title, branch, author). It is kept + apart from ``label`` on purpose: ``label`` ("Commit 2/3") is what the + ``result`` output has always carried, so downstream steps can keep + matching on it, and the hash is appended only where a person reads it. """ label: str checks: list[dict[str, str]] = field(default_factory=list) raw_text: str = "" + sha: str = "" + + @property + def display_label(self) -> str: + """The label as a reader sees it: ``Commit 2/3 (5584f46)``. + + Seven characters, like ``git log --oneline``; the full hash goes + into the link and the ``result`` output, where it is not read by eye. + """ + return f"{self.label} ({self.sha[:7]})" if self.sha else self.label @property def status(self) -> str: @@ -138,21 +154,28 @@ def warnings(self) -> list[dict[str, str]]: def overall_status(results: list[ScopeResult]) -> str: - """Reduce scope statuses to one of ``pass``/``fail``/``skip``. + """Reduce scope statuses to one of ``pass``/``fail``/``warn``/``skip``. One function, used by every completion path, because the alternative is what this replaced: four separate ``all(... == "pass")`` tests, each correct only while exactly two statuses existed. The moment ``skip`` appeared they all silently reclassified a skipped run as a failure. - ``skip`` requires at least one scope and all of them skipped. A ``warn`` - scope is neither a failure nor a skip, so it falls through to ``pass`` - here — reported in full, but it never fails the workflow. + Precedence is fail, then skip, then warn, then pass. ``skip`` requires + at least one scope and all of them skipped: nothing was validated, and a + warning cannot have been found where nothing ran. ``warn`` means at + least one scope carries a finding the config listed under ``warn`` and + nothing failed. It is distinct from ``pass`` so a downstream step can + act on a bent-but-not-broken policy without walking every scope; the + exit code does not distinguish the two (see ``exit_code_for``), because + the whole point of ``warn`` is that it never fails the workflow. """ if any(scope.status == "fail" for scope in results): return "fail" if results and all(scope.status == "skip" for scope in results): return "skip" + if any(scope.status == "warn" for scope in results): + return "warn" return "pass" @@ -340,19 +363,36 @@ def get_pr_title() -> str | None: return None -def parse_commit_messages(output: str) -> list[str]: - """Split git log output into individual commit messages.""" +#: One commit to check: ``(full sha, message)``. +Commit = tuple[str, str] + +#: ``git log`` format that yields, per commit, the full hash and the raw +#: message as two NUL-terminated fields (``%x00`` is git's spelling of +#: COMMIT_MESSAGE_DELIMITER; a literal NUL cannot be passed as an argument). +#: NUL cannot appear in a hash or a message, so the split is unambiguous +#: however many blank lines the message holds. +COMMIT_LOG_FORMAT = "--pretty=format:%H%x00%B%x00" + + +def parse_commit_messages(output: str) -> list[Commit]: + """Split ``git log`` output (see ``COMMIT_LOG_FORMAT``) into commits. + + The fields alternate hash, message, hash, message; git puts a newline + between commits, which lands on the front of the next hash and is + stripped along with the message's trailing newline. Commits are paired + before empty messages are dropped, so a blank message never shifts the + hashes of the commits after it. + """ + fields = [f.strip("\n") for f in output.split(COMMIT_MESSAGE_DELIMITER)] return [ - message.strip("\n") - for message in output.split(COMMIT_MESSAGE_DELIMITER) - if message.strip("\n") + (sha, message) for sha, message in zip(fields[0::2], fields[1::2]) if message ] -def _messages_in_range(revision_range: str) -> list[str]: - """Commit messages in ``revision_range``, oldest first, or ``[]``.""" +def _messages_in_range(revision_range: str) -> list[Commit]: + """Commits in ``revision_range`` as ``(sha, message)``, oldest first, or ``[]``.""" result = subprocess.run( - ["git", "log", "--pretty=format:%B%x00", "--reverse", revision_range], + ["git", "log", COMMIT_LOG_FORMAT, "--reverse", revision_range], stdout=subprocess.PIPE, stderr=subprocess.PIPE, encoding="utf-8", @@ -363,7 +403,26 @@ def _messages_in_range(revision_range: str) -> list[str]: return [] -def get_messages_from_event_range() -> list[str]: +def head_sha() -> str: + """The full hash of HEAD, or ``""`` when git cannot say. + + Only decoration for the non-PR ``Commit message`` scope, so a failure + here never fails the run: the message is still checked, just unlabelled. + """ + try: + result = subprocess.run( + ["git", "rev-parse", "HEAD"], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + encoding="utf-8", + check=False, + ) + except OSError: + return "" + return result.stdout.strip() if result.returncode == 0 else "" + + +def get_messages_from_event_range() -> list[Commit]: """Read PR commit messages between the payload's base and head commits. ``pull_request.base.sha`` and ``pull_request.head.sha`` name the pull @@ -379,7 +438,7 @@ def get_messages_from_event_range() -> list[str]: return _messages_in_range(f"{base_sha}..{head_sha}") -def get_messages_from_merge_ref() -> list[str]: +def get_messages_from_merge_ref() -> list[Commit]: """Read PR commit messages from GitHub's synthetic merge commit. Only meaningful on a ``pull_request`` checkout, where HEAD is @@ -392,13 +451,13 @@ def get_messages_from_merge_ref() -> list[str]: return _messages_in_range("HEAD^1..HEAD^2") -def get_messages_from_head_ref(base_ref: str) -> list[str]: +def get_messages_from_head_ref(base_ref: str) -> list[Commit]: """Read PR commit messages when the workflow checks out the head SHA.""" return _messages_in_range(f"origin/{base_ref}..HEAD") -def get_pr_commit_messages() -> list[str]: - """Get all commit messages for the current PR workflow. +def get_pr_commit_messages() -> list[Commit]: + """Get all commits, as ``(sha, message)``, for the current PR workflow. The event payload's ``base.sha..head.sha`` is tried first: it names the pull request exactly, whatever was checked out. On a ``pull_request`` @@ -455,22 +514,28 @@ def run_check_json( def check_scope( - label: str, args: list[str], input_text: str | None = None + label: str, args: list[str], input_text: str | None = None, sha: str = "" ) -> ScopeResult: - """Run commit-check for one scope and wrap the outcome in a ScopeResult.""" + """Run commit-check for one scope and wrap the outcome in a ScopeResult. + + ``sha`` names the commit a message scope checked; it rides along on both + outcomes so an unparsable CLI response still says which commit it was. + """ _rc, data, raw = run_check_json(args, input_text=input_text) if isinstance(data, dict): - return ScopeResult(label=label, checks=data.get("checks", [])) - return ScopeResult(label=label, raw_text=raw) + return ScopeResult(label=label, checks=data.get("checks", []), sha=sha) + return ScopeResult(label=label, raw_text=raw, sha=sha) -def run_pr_message_checks(pr_messages: list[str]) -> list[ScopeResult]: +def run_pr_message_checks(pr_commits: list[Commit]) -> list[ScopeResult]: """Check each PR commit message individually via commit-check --message.""" results: list[ScopeResult] = [] - total = len(pr_messages) - for index, msg in enumerate(pr_messages, start=1): + total = len(pr_commits) + for index, (sha, msg) in enumerate(pr_commits, start=1): results.append( - check_scope(f"Commit {index}/{total}", ["--message"], input_text=msg) + check_scope( + f"Commit {index}/{total}", ["--message"], input_text=msg, sha=sha + ) ) return results @@ -526,11 +591,11 @@ def run_commit_check() -> tuple[int, list[ScopeResult]]: # ---- 2. Commit message checks ----------------------------------------- if MESSAGE_ENABLED: - pr_messages = get_pr_commit_messages() - if pr_messages: + pr_commits = get_pr_commit_messages() + if pr_commits: # In PR context: check each commit individually to avoid # only validating the synthetic merge commit at HEAD. - results.extend(run_pr_message_checks(pr_messages)) + results.extend(run_pr_message_checks(pr_commits)) args = [a for a in args if a != "--message"] elif is_pr_event(): # Falling through to HEAD validates the synthetic merge commit, @@ -543,7 +608,7 @@ def run_commit_check() -> tuple[int, list[ScopeResult]]: # ---- 3. Remaining checks (branch, author, etc.) ----------------------- # Outside a PR, check the HEAD commit message directly. if "--message" in args: - results.append(check_scope("Commit message", ["--message"])) + results.append(check_scope("Commit message", ["--message"], sha=head_sha())) args = [a for a in args if a != "--message"] rev = None if is_pr_event() and any(flag in AUTHOR_FLAGS for flag in args): @@ -607,22 +672,50 @@ def _grouped(results: list[ScopeResult]) -> list[tuple[str, list[ScopeResult]]]: return groups +def _finding_lines(check: dict[str, str], include_error: bool) -> list[str]: + """The detail a reader needs to act on one finding, one item per line. + + ``value:`` (what was checked), the error (when ``include_error``), + ``Suggest:`` and ``Fix:`` — each only when the CLI filled it in. This is + the single source for both the tree and the annotation payload, so the + two cannot drift; the error is optional because the annotation carries + its first line in the headline instead. + + ``Fix:`` is the corrected text itself, ready to paste. When a rule has a + fix but no bespoke advice the CLI sets ``suggest`` to ``Use ""``, + so printing both would say the same thing twice in a row; in exactly + that case only ``Fix:`` is shown. A multi-line fix (a signed-off body) + takes one row per line so the trailer lands where it would in the + message. + """ + lines: list[str] = [] + if check.get("value"): + lines.append(f"value: {check['value']}") + if include_error: + lines.extend(check.get("error", "").splitlines()) + fix = check.get("fix", "") + suggest = check.get("suggest", "") + if suggest and suggest != f'Use "{fix}"': + lines.append(f"Suggest: {suggest}") + if fix: + first, *rest = fix.splitlines() + lines.append(f"Fix: {first}") + lines.extend(rest) + return lines + + def _render_findings(checks: list[dict[str, str]], include_docs: bool) -> list[str]: """Render the indented detail lines for a list of check entries. Shared by the failure and warning branches of ``_render_scopes`` — the - same rule label / value / error / suggestion / docs layout, whichever - list it is called with. + same rule label / value / error / suggestion / fix / docs layout, + whichever list it is called with. """ lines: list[str] = [] for check in checks: lines.append(f" {_rule_label(check)}") - if check.get("value"): - lines.append(f" value: {check['value']}") - for line in check.get("error", "").splitlines(): - lines.append(f" {line}") - if check.get("suggest"): - lines.append(f" Suggest: {check['suggest']}") + detail = _finding_lines(check, include_error=True) + lines.extend(f" {line}" for line in detail) if include_docs and check.get("docs_url"): lines.append(f" Docs: {check['docs_url']}") return lines @@ -642,18 +735,19 @@ def _render_scopes(scopes: list[ScopeResult], include_docs: bool) -> list[str]: """ lines: list[str] = [] for scope in scopes: + label = scope.display_label if scope.status == "skip": # Deliberately not a ✔. Nothing was validated here, and a tick # claiming otherwise is what made a bypassed policy look enforced. - lines.append(f" ⊘ {scope.label} (skipped)") + lines.append(f" ⊘ {label} (skipped)") continue if scope.status == "pass": value = _scope_value(scope) - lines.append(f" ✔ {scope.label}{f' ({value})' if value else ''}") + lines.append(f" ✔ {label}{f' ({value})' if value else ''}") continue if scope.raw_text and not scope.checks: # Defensive fallback: commit-check produced unexpected output. - lines.append(f" ✖ {scope.label}") + lines.append(f" ✖ {label}") lines.extend(f" {ln}" for ln in scope.raw_text.strip().splitlines()) continue # A scope's status names its worst outcome (fail beats warn), but the @@ -664,12 +758,12 @@ def _render_scopes(scopes: list[ScopeResult], include_docs: bool) -> list[str]: failures = scope.failures if failures: count = f" ({len(failures)} failure{'s' if len(failures) != 1 else ''})" - lines.append(f" ✖ {scope.label}{count}") + lines.append(f" ✖ {label}{count}") lines.extend(_render_findings(failures, include_docs)) warnings = scope.warnings if warnings: count = f" ({len(warnings)} warning{'s' if len(warnings) != 1 else ''})" - lines.append(f" ⚠ {scope.label}{count}") + lines.append(f" ⚠ {label}{count}") lines.extend(_render_findings(warnings, include_docs)) return lines @@ -692,6 +786,23 @@ def _annotation_escape(text: str) -> str: return text.replace("%", "%25").replace("\r", "%0D").replace("\n", "%0A") +def _annotation_body(scope: ScopeResult, check: dict[str, str]) -> str: + """The message of one finding's annotation, before escaping. + + First line: which commit and what rule said, ``Commit 2/3 (5584f46): + Subject must start with a capital letter``. Then the same value / + Suggest / Fix rows the tree prints, so the annotation on the Files + changed tab is enough to act on without opening the step log. GitHub + renders the escaped newlines as line breaks. + """ + error = check.get("error", "") + fallback = "check warning" if check.get("status") == "warn" else "check failed" + first_line = error.splitlines()[0] if error else fallback + lines = [f"{scope.display_label}: {first_line}"] + lines.extend(_finding_lines(check, include_error=False)) + return "\n".join(lines) + + def render_step_log(results: list[ScopeResult]) -> None: """Print results to the step log, then emit one annotation per finding. @@ -699,9 +810,11 @@ def render_step_log(results: list[ScopeResult]) -> None: ``::warning`` command renders as a line of its own wherever it is printed, so emitting one inside the indented listing broke the tree apart, and its ``title=`` \u2014 which is what carries the rule ID \u2014 is only shown in the - annotations UI, never inline. Printing the detail once in the listing and - the annotations after all the groups keeps the log readable and still - surfaces findings in the run summary and on the Files changed tab. + annotations UI, never inline. Printing the listing first and the + annotations after all the groups keeps the log readable and still + surfaces findings in the run summary and on the Files changed tab — where + the annotation is all a reader has, so its message repeats the value, + suggestion and fix from the tree (``_annotation_body``), one per line. A failure becomes an ``::error``, a warning a ``::warning`` \u2014 GitHub renders the two differently, and only the errors count toward the @@ -727,17 +840,13 @@ def render_step_log(results: list[ScopeResult]) -> None: continue if scope.raw_text and not scope.checks: errors.append( - (f"commit-check: {scope.label}", "output could not be parsed") + (f"commit-check: {scope.display_label}", "output could not be parsed") ) continue for check in scope.failures: - error = check.get("error", "") - first_line = error.splitlines()[0] if error else "check failed" - errors.append((_rule_label(check), f"{scope.label}: {first_line}")) + errors.append((_rule_label(check), _annotation_body(scope, check))) for check in scope.warnings: - error = check.get("error", "") - first_line = error.splitlines()[0] if error else "check warning" - warnings.append((_rule_label(check), f"{scope.label}: {first_line}")) + warnings.append((_rule_label(check), _annotation_body(scope, check))) level = "warning" if DRY_RUN_ENABLED else "error" for title, message in errors: @@ -751,12 +860,22 @@ def render_step_log(results: list[ScopeResult]) -> None: f"::{_annotation_escape(message)}" ) - if errors and DRY_RUN_ENABLED: + # The verdict is a plain line, never an ::error: the findings above are + # already one annotation each, and a second, untitled ::error for the + # total inflated the run's error count and told the reader nothing new. + if errors: failed, total = _check_counts(results) - print( - f"commit-check (dry-run): {failed} of {total} checks failed; " - "not failing the job" - ) + if DRY_RUN_ENABLED: + print( + f"commit-check (dry-run): {failed} of {total} checks failed; " + "not failing the job" + ) + else: + verdict = f"✖ commit-check: {failed} of {total} checks failed" + warned = _warn_count(results) + if warned: + verdict += f", {warned} warning{'s' if warned != 1 else ''}" + print(verdict) if not errors: skipped, warned, total = ( _skip_count(results), @@ -799,11 +918,6 @@ def _check_counts(results: list[ScopeResult]) -> tuple[int, int]: return failed, len(results) -def _failure_count(results: list[ScopeResult]) -> int: - """Number of scopes that failed.""" - return _check_counts(results)[0] - - def _skip_count(results: list[ScopeResult]) -> int: """Number of scopes that never ran. @@ -857,10 +971,36 @@ def _markdown_table( links = "_output could not be parsed \u2014 see details_" else: links = " \u00b7 ".join(_rule_markdown_link(check) for check in entries) - rows.append(f"| {scope.label} | {value_display} | {links} |") + rows.append(f"| {_scope_markdown_label(scope)} | {value_display} | {links} |") return "\n".join(rows) +def _commit_url(sha: str) -> str: + """Link to a commit on the GitHub instance the workflow runs against. + + ``GITHUB_SERVER_URL`` is what makes this right on GitHub Enterprise + Server; without ``GITHUB_REPOSITORY`` there is nothing to link into, so + the caller falls back to plain text rather than guess. + """ + repository = os.getenv("GITHUB_REPOSITORY", "") + if not (sha and repository): + return "" + server = os.getenv("GITHUB_SERVER_URL") or "https://github.com" + return f"{server.rstrip('/')}/{repository}/commit/{sha}" + + +def _scope_markdown_label(scope: ScopeResult) -> str: + """The table's Scope cell: the label, linked to the commit when there is one. + + The link lives here rather than in the tree because the tree is a fenced + code block, where Markdown does not render; the table row is the one + place a reader can click through to the commit. + """ + url = _commit_url(scope.sha) + label = scope.display_label + return f"[{label}]({url})" if url else label + + def _markdown_details(results: list[ScopeResult]) -> str: """Render the collapsible details block listing every scope. @@ -920,7 +1060,7 @@ def _scope_value(scope: ScopeResult, max_len: int = 60) -> str: # ```text # Commit message # ✔ PR title (feat: add login page) -# ✔ Commit 1/11 (feat: add user auth) +# ✔ Commit 1/11 (d87faca) (feat: add user auth) # Branch # ✔ Branch (feature/add-login) # Author @@ -932,6 +1072,9 @@ def _scope_value(scope: ScopeResult, max_len: int = 60) -> str: # # _commit-check 2.13.1 · [Rules reference](https://commit-check.com/rules/)_ # +# A commit scope names its commit by short hash after the label, on every +# surface; the label itself ("Commit 1/11") stays bare in the `result` output. +# # Skipped (every rule declined to run — e.g. the author is in ignore_authors): # # ⊘ **All 5 checks skipped** — nothing was validated @@ -994,11 +1137,12 @@ def _scope_value(scope: ScopeResult, max_len: int = 60) -> str: # # ## Commit Check # -# ❌ **1 of 5 checks failed** +# ❌ **2 of 5 checks failed** # # | Scope | Checked value | Failed checks | # |---|---|---| -# | Commit 2/11 | `bad msg` | [CC001 message](https://commit-check.com/rules/#cc001) | +# | [Commit 2/11 (5584f46)](https://github.com/acme/widgets/commit/5584f46…) | `bad msg` | [CC001 message](https://commit-check.com/rules/#cc001) | +# | [Commit 3/11 (37d6def)](https://github.com/acme/widgets/commit/37d6def…) | `feat: add login page` | [CC002 subject-capitalized](https://commit-check.com/rules/#cc002) | # #
# Show all 5 checks @@ -1006,11 +1150,16 @@ def _scope_value(scope: ScopeResult, max_len: int = 60) -> str: # ```text # Commit message # ✔ PR title (feat: add login page) -# ✖ Commit 2/11 (1 failure) +# ✖ Commit 2/11 (5584f46) (1 failure) # CC001 message # value: bad msg # The commit message should follow Conventional Commits. # Suggest: Use (): +# ✖ Commit 3/11 (37d6def) (1 failure) +# CC002 subject-capitalized +# value: feat: add login page +# Subject must start with a capital letter +# Fix: feat: Add login page # Branch # ✔ Branch (feature/add-login) # ``` @@ -1019,7 +1168,22 @@ def _scope_value(scope: ScopeResult, max_len: int = 60) -> str: # # _commit-check 2.13.1 · [Rules reference](https://commit-check.com/rules/)_ # +# The step log prints the same tree (plus a `Docs:` line per finding), then one +# annotation per finding whose message is the tree's detail joined on `%0A`: +# +# ::error title=CC002 subject-capitalized::Commit 3/11 (37d6def): Subject must start with a capital letter%0Avalue: feat: add login page%0AFix: feat: Add login page +# ✖ commit-check: 2 of 5 checks failed +# +# The verdict is a plain line, not an `::error`, so the run's error count is +# the number of findings. +# # Notes: +# - The table's Scope cell links to the commit (GITHUB_SERVER_URL, so it is +# right on GitHub Enterprise Server too); the tree cannot, being a code block. +# - `Fix:` is the corrected text the CLI proposes, when the correction is +# mechanical (CC002 capitalisation, CC010 WIP marker, CC012 sign-off, ...). +# When the CLI's `suggest` is just `Use ""`, only `Fix:` is printed. A +# multi-line fix takes one row per line. # - One check is one thing that was checked — a commit message, the branch, the # author — not one rule evaluation. The total therefore matches the number of # ✔/✖ lines the reader can count in the details block, and does not grow with @@ -1141,7 +1305,12 @@ def set_result_output(results: list[ScopeResult]) -> None: payload = { "status": overall_status(results), "scopes": [ - {"label": scope.label, "status": scope.status, "checks": scope.checks} + { + "label": scope.label, + "sha": scope.sha, + "status": scope.status, + "checks": scope.checks, + } for scope in results ], } @@ -1338,12 +1507,16 @@ def add_pr_comments(results: list[ScopeResult]) -> int: return 0 -def log_error_and_exit(ret_code: int, results: list[ScopeResult]) -> None: - """Logs a summary error to GitHub Actions and exits with the given code.""" - if ret_code != 0 and results: - failures = _failure_count(results) - unit = "failure" if failures == 1 else "failures" - print(f"::error::commit-check found {failures} {unit}.") +def log_error_and_exit(ret_code: int) -> None: + """Exit with the given code. + + This used to print ``::error::commit-check found N failures.`` first. + That was a second ``::error`` annotation on top of the one-per-finding + annotations ``render_step_log`` had already emitted, so GitHub counted + one error more than there were findings and listed an untitled entry + that only restated the titled ones. The verdict now lives in + ``render_step_log`` as a plain line, next to the passing verdicts. + """ sys.exit(ret_code) @@ -1362,7 +1535,7 @@ def main(): if DRY_RUN_ENABLED: ret_code = 0 - log_error_and_exit(ret_code, results) + log_error_and_exit(ret_code) if __name__ == "__main__": diff --git a/main_test.py b/main_test.py index d2a0674..e6a585a 100644 --- a/main_test.py +++ b/main_test.py @@ -36,6 +36,7 @@ def make_check( value: str = "", error: str = "", suggest: str = "", + fix: str = "", docs_url: str = "", ) -> dict[str, str]: """Build a single check outcome dict as produced by commit-check JSON.""" @@ -46,10 +47,21 @@ def make_check( "value": value, "error": error, "suggest": suggest, + "fix": fix, "docs_url": docs_url, } +#: Full hashes for scopes that carry a commit; the first seven characters +#: are what a reader sees. +SHA_A = "d87faca811e7017bbaa82f5c53eade0a97c108d8" +SHA_B = "5584f462cc3c947b2ba8d3d1a5735571803ee159" + +#: ``git log`` output in COMMIT_LOG_FORMAT for two commits: hash, NUL, message +#: (with its trailing newline), NUL, then git's own newline before the next. +TWO_COMMITS_LOG = f"{SHA_A}\x00fix: first\n\x00\n{SHA_B}\x00feat: second\n\x00" + + def json_output(*checks) -> str: """Serialize checks to the CLI JSON output shape.""" status = "fail" if any(c["status"] == "fail" for c in checks) else "pass" @@ -60,9 +72,10 @@ def pass_scope(label: str = "Branch", value: str = "") -> main.ScopeResult: return main.ScopeResult(label=label, checks=[make_check("branch", value=value)]) -def fail_scope(label: str = "Commit 1/1") -> main.ScopeResult: +def fail_scope(label: str = "Commit 1/1", sha: str = "") -> main.ScopeResult: return main.ScopeResult( label=label, + sha=sha, checks=[ make_check( "message", @@ -77,6 +90,27 @@ def fail_scope(label: str = "Commit 1/1") -> main.ScopeResult: ) +def fix_scope(label: str = "Commit 2/3", sha: str = SHA_B) -> main.ScopeResult: + """A CC002 failure as commit-check 2.17 reports it: a mechanical fix, and + a ``suggest`` the engine derived from that same fix.""" + return main.ScopeResult( + label=label, + sha=sha, + checks=[ + make_check( + "subject_capitalized", + status="fail", + rule_id="CC002", + value="feat: add login page", + error="Subject must start with a capital letter", + suggest='Use "feat: Add login page"', + fix="feat: Add login page", + docs_url="https://commit-check.com/rules/#cc002", + ) + ], + ) + + class TestEnvFlag(unittest.TestCase): def test_true_value(self): with patch.dict(os.environ, {"FEATURE_FLAG": "true"}): @@ -161,8 +195,28 @@ def test_message_and_branch(self): class TestParseCommitMessages(unittest.TestCase): def test_splits_messages_and_trims_surrounding_newlines(self): - result = main.parse_commit_messages("\nfix: first\n\x00\nfeat: second\n\n\x00") - self.assertEqual(result, ["fix: first", "feat: second"]) + result = main.parse_commit_messages( + f"{SHA_A}\x00\nfix: first\n\x00\n{SHA_B}\x00\nfeat: second\n\n\x00" + ) + self.assertEqual(result, [(SHA_A, "fix: first"), (SHA_B, "feat: second")]) + + def test_real_git_log_layout(self): + self.assertEqual( + main.parse_commit_messages(TWO_COMMITS_LOG), + [(SHA_A, "fix: first"), (SHA_B, "feat: second")], + ) + + def test_the_format_asks_git_for_the_hash_and_the_body(self): + """A literal NUL cannot be an argument; git spells it %x00.""" + self.assertEqual(main.COMMIT_LOG_FORMAT, "--pretty=format:%H%x00%B%x00") + self.assertNotIn("\x00", main.COMMIT_LOG_FORMAT) + + def test_an_empty_message_does_not_shift_the_hashes_after_it(self): + output = f"{SHA_A}\x00\n\x00\n{SHA_B}\x00feat: second\n\x00" + self.assertEqual(main.parse_commit_messages(output), [(SHA_B, "feat: second")]) + + def test_empty_output_is_no_commits(self): + self.assertEqual(main.parse_commit_messages(""), []) class TestGetPrTitle(unittest.TestCase): @@ -309,10 +363,12 @@ class TestRunPrMessageChecks(unittest.TestCase): def test_single_message_pass(self): mock_result = MagicMock(returncode=0, stdout=json_output(make_check("message"))) with patch("main.subprocess.run", return_value=mock_result) as mock_run: - scopes = main.run_pr_message_checks(["fix: something"]) + scopes = main.run_pr_message_checks([(SHA_A, "fix: something")]) self.assertEqual(len(scopes), 1) self.assertEqual(scopes[0].status, "pass") self.assertEqual(scopes[0].label, "Commit 1/1") + self.assertEqual(scopes[0].sha, SHA_A) + self.assertEqual(scopes[0].display_label, "Commit 1/1 (d87faca)") self.assertEqual( mock_run.call_args[0][0], ["commit-check", "--format", "json", "--message"], @@ -325,9 +381,17 @@ def test_failed_message_marks_scope_failed(self): stdout=json_output(make_check("message", status="fail")), ) with patch("main.subprocess.run", return_value=mock_result): - scopes = main.run_pr_message_checks(["bad commit"]) + scopes = main.run_pr_message_checks([(SHA_A, "bad commit")]) self.assertEqual(scopes[0].status, "fail") self.assertEqual(len(scopes[0].failures), 1) + self.assertEqual(scopes[0].sha, SHA_A) + + def test_unparsable_output_still_names_the_commit(self): + mock_result = MagicMock(returncode=1, stdout="unexpected output") + with patch("main.subprocess.run", return_value=mock_result): + scopes = main.run_pr_message_checks([(SHA_A, "bad commit")]) + self.assertEqual(scopes[0].raw_text, "unexpected output") + self.assertEqual(scopes[0].sha, SHA_A) def test_labels_commits_in_order(self): results = [ @@ -339,10 +403,15 @@ def test_labels_commits_in_order(self): MagicMock(returncode=0, stdout=json_output(make_check("message"))), ] with patch("main.subprocess.run", side_effect=results): - scopes = main.run_pr_message_checks(["ok", "bad", "ok"]) + scopes = main.run_pr_message_checks( + [("a" * 40, "ok"), ("b" * 40, "bad"), ("c" * 40, "ok")] + ) + # The label stays the bare index for the ``result`` output; the hash + # is appended only where a person reads it. self.assertEqual( [s.label for s in scopes], ["Commit 1/3", "Commit 2/3", "Commit 3/3"] ) + self.assertEqual([s.sha for s in scopes], ["a" * 40, "b" * 40, "c" * 40]) self.assertEqual(scopes[1].status, "fail") def test_empty_list(self): @@ -466,18 +535,16 @@ def test_exception_returns_empty(self): class TestGitMessageReaders(unittest.TestCase): def test_get_messages_from_merge_ref(self): - mock_result = MagicMock( - returncode=0, stdout="fix: first\n\x00feat: second\n\x00" - ) + mock_result = MagicMock(returncode=0, stdout=TWO_COMMITS_LOG) with ( patch.dict(os.environ, {"GITHUB_EVENT_NAME": "pull_request"}), patch("main.subprocess.run", return_value=mock_result) as mock_run, ): result = main.get_messages_from_merge_ref() - self.assertEqual(result, ["fix: first", "feat: second"]) + self.assertEqual(result, [(SHA_A, "fix: first"), (SHA_B, "feat: second")]) self.assertEqual( mock_run.call_args[0][0], - ["git", "log", "--pretty=format:%B%x00", "--reverse", "HEAD^1..HEAD^2"], + ["git", "log", main.COMMIT_LOG_FORMAT, "--reverse", "HEAD^1..HEAD^2"], ) def test_merge_ref_is_never_read_on_pull_request_target(self): @@ -496,7 +563,7 @@ def run(command, **_kwargs): commands.append(command) if command[:2] == ["git", "rev-parse"]: return MagicMock(returncode=0, stdout="x\n") - return MagicMock(returncode=0, stdout="fix: first\n\x00feat: second\n\x00") + return MagicMock(returncode=0, stdout=TWO_COMMITS_LOG) with ( patch("main.get_pr_base_sha", return_value="base111"), @@ -504,9 +571,9 @@ def run(command, **_kwargs): patch("main.subprocess.run", side_effect=run), ): result = main.get_messages_from_event_range() - self.assertEqual(result, ["fix: first", "feat: second"]) + self.assertEqual(result, [(SHA_A, "fix: first"), (SHA_B, "feat: second")]) self.assertIn( - ["git", "log", "--pretty=format:%B%x00", "--reverse", "base111..head222"], + ["git", "log", main.COMMIT_LOG_FORMAT, "--reverse", "base111..head222"], commands, ) @@ -532,22 +599,38 @@ def test_a_failing_git_log_yields_no_messages(self): self.assertEqual(main.get_messages_from_head_ref("main"), []) def test_get_messages_from_head_ref(self): - mock_result = MagicMock(returncode=0, stdout="fix: first\n\x00") + mock_result = MagicMock(returncode=0, stdout=f"{SHA_A}\x00fix: first\n\x00") with patch("main.subprocess.run", return_value=mock_result) as mock_run: result = main.get_messages_from_head_ref("main") - self.assertEqual(result, ["fix: first"]) + self.assertEqual(result, [(SHA_A, "fix: first")]) self.assertEqual( mock_run.call_args[0][0], [ "git", "log", - "--pretty=format:%B%x00", + main.COMMIT_LOG_FORMAT, "--reverse", "origin/main..HEAD", ], ) +class TestHeadSha(unittest.TestCase): + def test_returns_the_full_hash(self): + mock_result = MagicMock(returncode=0, stdout=f"{SHA_A}\n") + with patch("main.subprocess.run", return_value=mock_result) as mock_run: + self.assertEqual(main.head_sha(), SHA_A) + self.assertEqual(mock_run.call_args[0][0], ["git", "rev-parse", "HEAD"]) + + def test_no_head_is_empty(self): + with patch("main.subprocess.run", return_value=MagicMock(returncode=128)): + self.assertEqual(main.head_sha(), "") + + def test_missing_git_is_empty(self): + with patch("main.subprocess.run", side_effect=OSError("no git")): + self.assertEqual(main.head_sha(), "") + + class TestRunCommitCheck(unittest.TestCase): def test_pr_path_checks_each_commit(self): with ( @@ -636,6 +719,7 @@ def test_non_pr_message_check_uses_commit_message_scope(self): patch("main.AUTHOR_EMAIL_ENABLED", False), patch("main.get_pr_commit_messages", return_value=[]), patch("main.run_pr_message_checks") as mock_pr, + patch("main.head_sha", return_value=SHA_A), patch( "main.check_scope", return_value=pass_scope("Commit message") ) as mock_scope, @@ -644,7 +728,8 @@ def test_non_pr_message_check_uses_commit_message_scope(self): rc, results = main.run_commit_check() self.assertEqual(rc, 0) mock_pr.assert_not_called() - mock_scope.assert_called_once_with("Commit message", ["--message"]) + # HEAD is the commit that was checked, so the scope names it too. + mock_scope.assert_called_once_with("Commit message", ["--message"], sha=SHA_A) def test_message_flag_removed_before_other_checks_in_pr(self): captured_args = [] @@ -691,6 +776,7 @@ def test_pr_without_enumerable_commits_warns_and_checks_head(self): patch("main.AUTHOR_EMAIL_ENABLED", False), patch("main.is_pr_event", return_value=True), patch("main.get_pr_commit_messages", return_value=[]), + patch("main.head_sha", return_value=""), patch( "main.check_scope", return_value=pass_scope("Commit message") ) as mock_scope, @@ -699,7 +785,7 @@ def test_pr_without_enumerable_commits_warns_and_checks_head(self): rc, results, output = self._run_capturing_stdout() self.assertEqual(rc, 0) self.assertIn(self.SHALLOW_PR_WARNING, output) - mock_scope.assert_called_once_with("Commit message", ["--message"]) + mock_scope.assert_called_once_with("Commit message", ["--message"], sha="") def test_push_without_pr_commits_does_not_warn(self): with ( @@ -1061,13 +1147,159 @@ def test_failure_prints_group_and_error_annotation(self): self.assertIn("value: bad message", output) self.assertIn("Suggest: Use (): ", output) self.assertIn("Docs: https://commit-check.com/rules/#cc001", output) - # The annotation names the scope, which the title alone cannot carry. + # The annotation names the scope, which the title alone cannot carry, + # then the value and the suggestion, so it can be acted on where it + # is shown (the Files changed tab) without opening the step log. self.assertIn( "::error title=CC001 message::Commit 1/1: The commit message should " - "follow Conventional Commits.", + "follow Conventional Commits." + "%0Avalue: bad message" + "%0ASuggest: Use (): \n", + output, + ) + + def test_annotation_names_the_commit_and_carries_the_fix(self): + output = self._run([fix_scope("Commit 2/3", sha=SHA_B)]) + self.assertIn( + "::error title=CC002 subject-capitalized::Commit 2/3 (5584f46): " + "Subject must start with a capital letter" + "%0Avalue: feat: add login page" + "%0AFix: feat: Add login page\n", + output, + ) + + def test_annotation_and_tree_print_the_same_detail_lines(self): + """One helper feeds both surfaces, so they cannot drift.""" + check = fix_scope().checks[0] + tree = main._finding_lines(check, include_error=True) + annotation = main._finding_lines(check, include_error=False) + self.assertEqual( + tree, + [ + "value: feat: add login page", + "Subject must start with a capital letter", + "Fix: feat: Add login page", + ], + ) + self.assertEqual([ln for ln in tree if ln in annotation], annotation) + + def test_annotation_without_an_error_still_names_the_kind_of_finding(self): + scope = main.ScopeResult( + label="Branch", + checks=[ + make_check("branch", status="fail", rule_id="CC201"), + make_check("merge_base", status="warn", rule_id="CC202"), + ], + ) + output = self._run([scope]) + self.assertIn("::error title=CC201 branch::Branch: check failed\n", output) + self.assertIn( + "::warning title=CC202 merge-base::Branch: check warning\n", output + ) + + def test_fix_is_printed_once_when_suggest_only_repeats_it(self): + """commit-check sets suggest to ``Use ""`` when a rule has a + fix and no advice of its own; printing both says it twice.""" + output = self._run([fix_scope()]) + self.assertIn(" Fix: feat: Add login page\n", output) + self.assertNotIn("Suggest:", output) + + def test_bespoke_suggest_and_fix_are_both_printed(self): + scope = main.ScopeResult( + label="Commit 1/1", + checks=[ + make_check( + "allow_wip_commits", + status="fail", + rule_id="CC010", + value="wip: stuff", + error="WIP commits are not allowed", + suggest='Drop the WIP marker: "stuff"', + fix="stuff", + ) + ], + ) + output = self._run([scope]) + listing = output.split("::endgroup::")[0] + self.assertIn( + " WIP commits are not allowed\n" + ' Suggest: Drop the WIP marker: "stuff"\n' + " Fix: stuff\n", + listing, + ) + self.assertIn( + "%0ASuggest: Drop the WIP marker: %22stuff%22%0AFix: stuff\n".replace( + "%22", '"' + ), + output, + ) + + def test_a_multi_line_fix_takes_one_row_per_line(self): + fix = "feat: add login page\n\nSigned-off-by: Jane Doe " + scope = main.ScopeResult( + label="Commit 1/1", + sha=SHA_A, + checks=[ + make_check( + "require_signed_off_by", + status="fail", + rule_id="CC012", + value="feat: add login page", + error="Signed-off-by not found in latest commit", + suggest=f'Use "{fix}"', + fix=fix, + ) + ], + ) + output = self._run([scope]) + listing = output.split("::endgroup::")[0] + self.assertIn( + " Fix: feat: add login page\n" + " \n" + " Signed-off-by: Jane Doe \n", + listing, + ) + # In the annotation the same rows ride on %0A, so the trailer lands + # where it would in the message. + self.assertIn( + "%0AFix: feat: add login page%0A%0ASigned-off-by: " + "Jane Doe \n", + output, + ) + + def test_tree_lines_carry_the_short_sha(self): + results = [ + pass_scope("Commit 1/3", value="fix: first"), + fix_scope("Commit 2/3", sha=SHA_B), + main.ScopeResult(label="Commit 3/3", sha=SHA_A, raw_text="garbage"), + ] + results[0].sha = "c" * 40 + output = self._run(results) + self.assertIn(" ✔ Commit 1/3 (ccccccc) (fix: first)\n", output) + self.assertIn(" ✖ Commit 2/3 (5584f46) (1 failure)\n", output) + self.assertIn(" ✖ Commit 3/3 (d87faca)\n", output) + self.assertIn( + "::error title=commit-check: Commit 3/3 (d87faca)::output could not " + "be parsed\n", output, ) + def test_failure_verdict_is_a_plain_line_not_an_annotation(self): + """One ::error per finding, and a verdict in words — never a second + annotation for the total, which GitHub counted as one more error.""" + output = self._run([fail_scope("Commit 1/2"), fix_scope("Commit 2/2")]) + self.assertEqual(output.count("::error"), 2) + self.assertNotIn("::error::", output) + self.assertIn("\n✖ commit-check: 2 of 2 checks failed\n", output) + self.assertNotIn("passed", output) + + def test_failure_verdict_counts_scopes_and_names_the_warnings(self): + output = self._run( + [fail_scope("Commit 1/1"), pass_scope("PR title"), warn_scope()] + ) + self.assertIn("\n✖ commit-check: 1 of 3 checks failed, 1 warning\n", output) + self.assertNotIn("dry-run", output) + def test_failure_reason_is_printed_once(self): """The listing and the annotation must not both print the reason. @@ -1152,6 +1384,7 @@ def test_dry_run_downgrades_annotations_and_prints_verdict(self): output, ) self.assertNotIn("all checks passed", output) + self.assertNotIn("✖ commit-check", output) def test_dry_run_without_failures_prints_the_usual_verdict(self): with patch("main.DRY_RUN_ENABLED", True): @@ -1236,6 +1469,93 @@ def test_failure_golden_output(self): f"{FOOTER}", ) + @pin_version + def test_failure_golden_output_with_commits(self): + """Pin the report for a pull request's commits: the table row links + to the commit, the tree names it, and a mechanical fix is shown.""" + results = [ + pass_scope("PR title", value="feat: Add login page"), + fail_scope("Commit 1/3", sha=SHA_A), + fix_scope("Commit 2/3", sha=SHA_B), + pass_scope("Commit 3/3", value="fix: Handle timeout"), + pass_scope("Branch", value="feature/add-login"), + ] + results[3].sha = "37d6def82ffcc6dbfa0af2ea1ec90512d89979ae" + env = {"GITHUB_REPOSITORY": "acme/widgets", "GITHUB_SERVER_URL": ""} + with patch.dict(os.environ, env): + body = main.render_report(results) + self.assertEqual( + body, + f"{main.COMMENT_MARKER}\n" + f"{main.REPORT_TITLE}\n" + "\n" + "❌ **2 of 5 checks failed**\n" + "\n" + "| Scope | Checked value | Failed checks |\n" + "|---|---|---|\n" + f"| [Commit 1/3 (d87faca)](https://github.com/acme/widgets/commit/{SHA_A})" + " | `bad message` | " + "[CC001 message](https://commit-check.com/rules/#cc001) |\n" + f"| [Commit 2/3 (5584f46)](https://github.com/acme/widgets/commit/{SHA_B})" + " | `feat: add login page` | " + "[CC002 subject-capitalized](https://commit-check.com/rules/#cc002) |\n" + "\n" + "
\n" + "Show all 5 checks\n" + "\n" + "```text\n" + "Commit message\n" + " ✔ PR title (feat: Add login page)\n" + " ✖ Commit 1/3 (d87faca) (1 failure)\n" + " CC001 message\n" + " value: bad message\n" + " The commit message should follow Conventional Commits.\n" + " Suggest: Use (): \n" + " ✖ Commit 2/3 (5584f46) (1 failure)\n" + " CC002 subject-capitalized\n" + " value: feat: add login page\n" + " Subject must start with a capital letter\n" + " Fix: feat: Add login page\n" + " ✔ Commit 3/3 (37d6def) (fix: Handle timeout)\n" + "Branch\n" + " ✔ Branch (feature/add-login)\n" + "```\n" + "\n" + "
\n" + "\n" + f"{FOOTER}", + ) + + def test_table_link_honours_the_server_url(self): + """GitHub Enterprise Server is not github.com.""" + env = { + "GITHUB_REPOSITORY": "acme/widgets", + "GITHUB_SERVER_URL": "https://ghe.example.com/", + } + with patch.dict(os.environ, env): + body = main.render_report([fix_scope("Commit 1/1", sha=SHA_B)]) + self.assertIn( + f"| [Commit 1/1 (5584f46)](https://ghe.example.com/acme/widgets/commit/{SHA_B}) |", + body, + ) + + def test_table_has_no_link_without_a_repository_to_link_into(self): + with patch.dict(os.environ, {"GITHUB_REPOSITORY": ""}): + body = main.render_report([fix_scope("Commit 1/1", sha=SHA_B)]) + self.assertIn("| Commit 1/1 (5584f46) | `feat: add login page` |", body) + self.assertNotIn("/commit/", body) + + def test_scopes_without_a_commit_are_never_linked(self): + with patch.dict(os.environ, {"GITHUB_REPOSITORY": "acme/widgets"}): + body = main.render_report([fail_scope("Branch")]) + self.assertIn("| Branch | `bad message` |", body) + self.assertNotIn("/commit/", body) + + def test_skipped_scope_carries_the_sha_too(self): + scope = skip_scope("Commit 1/1") + scope.sha = SHA_A + self.assertIn(" ⊘ Commit 1/1 (d87faca) (skipped)", main.render_report([scope])) + def test_a_check_is_a_thing_checked_not_a_rule_evaluation(self): """The total counts scopes, so it tracks the policy, not the PR size. @@ -1419,6 +1739,22 @@ def test_writes_heredoc_json(self): self.assertIn('"label": "Commit 1/1"', content) self.assertTrue(content.strip().endswith("EOF")) + def test_scopes_carry_the_full_sha_and_an_unchanged_label(self): + """Downstream steps keep matching on ``label``; the hash is a new + field beside it, full length so it can be passed back to the API.""" + output_path = os.path.join(tempfile.mkdtemp(), "output.txt") + with patch.dict(os.environ, {"GITHUB_OUTPUT": output_path}): + main.set_result_output([fix_scope("Commit 2/3", sha=SHA_B), pass_scope()]) + with open(output_path, encoding="utf-8") as file_obj: + body = file_obj.read().removeprefix("result<