diff --git a/README.md b/README.md
index 6eb9557..3f51ec9 100644
--- a/README.md
+++ b/README.md
@@ -39,7 +39,6 @@ Create a new GitHub Actions workflow in your project, e.g. at [.github/workflows
name: Commit Check
on:
- push:
pull_request:
branches: 'main'
@@ -50,7 +49,7 @@ jobs:
contents: read
pull-requests: write
steps:
- - uses: actions/checkout@v5
+ - uses: actions/checkout@v7
with:
fetch-depth: 0 # Required for merge-base checks
- uses: commit-check/commit-check-action@v2
@@ -60,7 +59,7 @@ jobs:
author-name: false
author-email: false
job-summary: true
- pr-comments: ${{ github.event_name == 'pull_request' }}
+ pr-comments: true
```
> [!NOTE]
@@ -231,25 +230,96 @@ or gate on individual rules.
## GitHub Action Job Summary
-By default, commit-check-action results are shown on the job summary page of the workflow.
+By default, commit-check-action results are shown on the job summary page of the
+workflow. The report below is reproduced as the action renders it, except that
+its title is a heading in the real thing — it is bold here so it stays out of
+this page's table of contents — and the footer names the version that actually
+ran.
### Success Job Summary
-
+Passing runs stay to one line, with the detail folded away:
-### Failure Job Summary
+> 
**Commit Check**
+>
+> ✅ **All 3 checks passed**
+>
+>
+> Show all 3 checks
+>
+> ```text
+> Commit message
+> ✔ PR title (feat: add login page)
+> ✔ Commit 1/2 (feat: add login page)
+> Branch
+> ✔ Branch (feature/add-login)
+> ```
+>
+>
+>
+> _commit-check <version> · [Rules reference](https://commit-check.com/rules/)_
-
+### Failure Job Summary
-## GitHub Pull Request Comments
+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:
-### Success Pull Request Comment
+> **Commit Check**
+>
+> ❌ **2 of 4 checks failed**
+>
+> | 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) |
+>
+>
+> Show all 4 checks
+>
+> ```text
+> Commit message
+> ✔ PR title (feat: add login page)
+> ✔ Commit 1/2 (feat: add login page)
+> ✖ Commit 2/2 (1 failure)
+> CC001 message
+> value: bad msg
+> The commit message should follow Conventional Commits.
+> Suggest: Use ():
+> Branch
+> ✖ Branch (1 failure)
+> CC201 branch
+> value: my-changes
+> The branch should follow Conventional Branch.
+> Suggest: Use / with allowed types
+> ```
+>
+>
+>
+> _commit-check <version> · [Rules reference](https://commit-check.com/rules/)_
-
+A scope is one thing that was checked — a commit message, the branch, the author
+— 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.
-### Failure Pull Request Comment
+## GitHub Pull Request Comments
-
+With `pr-comments: true` the same report is posted as a pull request comment.
+It is the same Markdown: the job summary and the comment are both rendered by
+`render_report`, so the two surfaces cannot disagree. See
+[Success Job Summary](#success-job-summary) and
+[Failure Job Summary](#failure-job-summary) above for what it looks like.
+
+What differs is the lifecycle rather than the content:
+
+- The comment is **edited in place** on later runs rather than added to, so a
+ pull request carries one Commit Check comment however many times CI runs. It
+ stays after the checks pass, showing the ✅ report rather than disappearing.
+- Comments are identified by a hidden `` marker, so
+ reformatting the visible text does not orphan the previous one. If several
+ marked comments somehow exist, the newest is kept and the rest deleted.
+- A comment from a version predating the marker is adopted rather than
+ duplicated — but only when a bot posted it, since the older signal was just a
+ title prefix that a person could type by hand.
## Fork PR Comments
diff --git a/docs/fork-pr-comments.md b/docs/fork-pr-comments.md
index 82ff63a..b267011 100644
--- a/docs/fork-pr-comments.md
+++ b/docs/fork-pr-comments.md
@@ -56,7 +56,7 @@ jobs:
check:
runs-on: ubuntu-latest
steps:
- - uses: actions/checkout@v5
+ - uses: actions/checkout@v7
with:
fetch-depth: 0
- uses: commit-check/commit-check-action@v2
@@ -148,7 +148,7 @@ jobs:
pull-requests: write
steps:
# SAFE: checkout the merge commit, NOT the PR head
- - uses: actions/checkout@v5
+ - uses: actions/checkout@v7
with:
fetch-depth: 0
- uses: commit-check/commit-check-action@v2
diff --git a/examples/commit-check-workflow-a.yml b/examples/commit-check-workflow-a.yml
index 9d1bece..8c6c2ba 100644
--- a/examples/commit-check-workflow-a.yml
+++ b/examples/commit-check-workflow-a.yml
@@ -16,7 +16,7 @@ jobs:
check:
runs-on: ubuntu-latest
steps:
- - uses: actions/checkout@v5
+ - uses: actions/checkout@v7
with:
fetch-depth: 0
- uses: commit-check/commit-check-action@v2
diff --git a/main.py b/main.py
index 8a76005..477b4cd 100755
--- a/main.py
+++ b/main.py
@@ -882,17 +882,25 @@ def add_pr_comments(results: list[ScopeResult]) -> int:
return 0 if all(scope.status == "pass" for scope in results) else 1
except GithubException as e:
if e.status == 403:
+ # GithubException.data is whatever the response decoded to, which
+ # is None for an empty body and a str for a non-JSON one. Reaching
+ # for .get unguarded would raise inside this handler and escape the
+ # function, turning the best-effort path into a step failure.
+ detail = e.data.get("message") if isinstance(e.data, dict) else None
print(
"::warning::Unable to post PR comment (403 Forbidden). "
- "Ensure your workflow grants 'issues: write' permission. "
- f"Error: {e.data.get('message', str(e))}",
+ "Ensure your workflow grants 'pull-requests: write' permission. "
+ f"Error: {detail or e}",
file=sys.stderr,
)
return 0
- print(f"Error posting PR comment: {e}", file=sys.stderr)
+ # Annotated, not just printed: posting the comment is best-effort and
+ # never fails the step, so without an annotation the run is green, the
+ # comment is absent, and nothing says why.
+ print(f"::warning::Unable to post PR comment: {e}", file=sys.stderr)
return 0
except Exception as e:
- print(f"Error posting PR comment: {e}", file=sys.stderr)
+ print(f"::warning::Unable to post PR comment: {e}", file=sys.stderr)
return 0
diff --git a/main_test.py b/main_test.py
index d1e6bcd..7aa330d 100644
--- a/main_test.py
+++ b/main_test.py
@@ -1085,6 +1085,105 @@ def test_skips_when_comment_is_up_to_date(self):
mock_pull_request.create_comment.assert_not_called()
+class _StubGithubException(Exception):
+ """Stands in for github.GithubException, which is mocked away in these tests.
+
+ The real class has to be a genuine exception type or the ``except`` clause
+ in add_pr_comments raises TypeError before the handler is reached.
+ """
+
+ def __init__(self, status, data=None):
+ super().__init__(f"status {status}")
+ self.status = status
+ # Stored as given, exactly as PyGithub does. Coercing a falsy payload
+ # to {} here would hide the very shapes the handler has to survive.
+ self.data = data
+
+
+class TestAddPrCommentsFailures(unittest.TestCase):
+ """Posting the comment is best-effort, but it must never fail silently.
+
+ Every branch here returns 0 so the step stays green — which is the point:
+ without an annotation the run is green, the comment is absent, and nothing
+ on the page says why.
+ """
+
+ def _run(self, side_effect):
+ mock_pull_request = MagicMock()
+ mock_pull_request.get_comments.return_value = []
+ mock_pull_request.create_comment.side_effect = side_effect
+ mock_repo = MagicMock()
+ mock_repo.get_issue.return_value = mock_pull_request
+
+ github_module = MagicMock()
+ github_module.GithubException = _StubGithubException
+ github_module.Github.return_value.get_repo.return_value = mock_repo
+
+ with (
+ patch("main.PR_COMMENTS_ENABLED", True),
+ patch("main.is_fork_pr_with_readonly_token", return_value=False),
+ patch.dict(
+ os.environ,
+ {
+ "GITHUB_TOKEN": "token",
+ "GITHUB_REPOSITORY": "owner/repo",
+ "GITHUB_REF": "refs/pull/12/merge",
+ },
+ ),
+ patch.dict(sys.modules, {"github": github_module}),
+ patch("builtins.print") as mock_print,
+ ):
+ rc = main.add_pr_comments([fail_scope()])
+ printed = [
+ call[0][0]
+ for call in mock_print.call_args_list
+ if call[0] and isinstance(call[0][0], str)
+ ]
+ return rc, printed
+
+ def test_forbidden_names_the_permission_that_actually_grants_this(self):
+ rc, printed = self._run(
+ _StubGithubException(403, {"message": "Resource not accessible"})
+ )
+ self.assertEqual(rc, 0)
+ warning = next(w for w in printed if "::warning::" in w)
+ # pull-requests, not issues: a PR comment is written with the
+ # pull-requests scope, and this hint is the only guidance a user gets.
+ self.assertIn("pull-requests: write", warning)
+ self.assertNotIn("issues: write", warning)
+ # The API's own wording is what tells the user which resource was
+ # refused, so the hint has to carry it through.
+ self.assertIn("Resource not accessible", warning)
+
+ def test_forbidden_with_a_non_mapping_payload_still_warns(self):
+ # data is whatever the body decoded to: None when empty, a str when it
+ # is not JSON. Reaching for .get on either raises inside the handler
+ # and escapes the function, which would fail the step.
+ for payload in (None, "forbidden"):
+ with self.subTest(payload=payload):
+ rc, printed = self._run(_StubGithubException(403, payload))
+ self.assertEqual(rc, 0)
+ warning = next(w for w in printed if "::warning::" in w)
+ self.assertIn("pull-requests: write", warning)
+ self.assertIn("status 403", warning)
+
+ def test_other_api_errors_are_annotated(self):
+ rc, printed = self._run(_StubGithubException(500, {"message": "boom"}))
+ self.assertEqual(rc, 0)
+ self.assertTrue(
+ any("::warning::" in w for w in printed),
+ f"a failed post must be annotated, got: {printed}",
+ )
+
+ def test_unexpected_errors_are_annotated(self):
+ rc, printed = self._run(RuntimeError("network went away"))
+ self.assertEqual(rc, 0)
+ self.assertTrue(
+ any("::warning::" in w and "network went away" in w for w in printed),
+ f"a failed post must be annotated, got: {printed}",
+ )
+
+
class TestIsForkPrWithReadonlyToken(unittest.TestCase):
def test_fork_pr_with_pull_request_event(self):
with (