INF-1484/fix: stop swallowing pre-commit hook failures

action.yaml

@@ -85,10 +85,14 @@ runs:
     - name: Run pre-commit
       id: pre-commit
       shell: bash
-      continue-on-error: true
       env:
         PIP_EXTRA_INDEX_URL: https://europe-north1-python.pkg.dev/two-artifacts/pypi-virtual/simple/
+      # The default composite shell runs bash -e, which would abort this step on
+      # hook failure before EXIT_CODE is written (and continue-on-error would
+      # mask it, so the report step's `exit ''` succeeded - failures were
+      # silently swallowed). Capture the code with -e disabled instead.
       run: |
+        set +e
         python -m pre_commit_action.run_hooks ${{ github.base_ref }} HEAD pre-commit-output
         echo "EXIT_CODE=$?" >> "$GITHUB_OUTPUT"
 
@@ -97,7 +101,6 @@ runs:
       shell: bash
       env:
         PRE_COMMIT_EXIT_CODE: ${{ steps.pre-commit.outputs.EXIT_CODE }}
-        PRE_COMMIT_OUTCOME: ${{ steps.pre-commit.outcome }}
         PRE_COMMIT_BASE_REF: ${{ github.base_ref }}
         PRE_COMMIT_ACTOR: ${{ github.actor }}
       run: python -m pre_commit_action.render_comment pre-commit-output pre-commit-comment.md
@@ -112,4 +115,5 @@ runs:
 
     - name: Report pre-commit status
       shell: bash
-      run: exit ${{ steps.pre-commit.outputs.EXIT_CODE }}
+      # Fail loud if the exit code is somehow missing rather than `exit ''` -> 0.
+      run: exit ${{ steps.pre-commit.outputs.EXIT_CODE || '1' }}

pre_commit_action/render_comment.py

@@ -18,7 +18,6 @@ def middle_truncate(text: str, max_bytes: int = MAX_OUTPUT_BYTES) -> str:
 def render_comment(
     output_path: str,
     exit_code: str,
-    outcome: str,
     base_ref: str,
     actor: str,
 ) -> str:
@@ -29,6 +28,9 @@ def render_comment(
         raw = f"(failed to read {output_path}: {err})"
 
     output = middle_truncate(raw)
+    # The step itself always succeeds (exit code is captured with -e disabled),
+    # so the hook status must come from the exit code, not the step outcome.
+    outcome = "success" if exit_code == "0" else "failure"
     emoji = "🏆" if outcome == "success" else "🚫"
 
     hint = (
@@ -66,9 +68,8 @@ def render_comment(
     output_path = sys.argv[1]
     comment_path = sys.argv[2] if len(sys.argv) > 2 else "pre-commit-comment.md"
     exit_code = os.environ["PRE_COMMIT_EXIT_CODE"]
-    outcome = os.environ["PRE_COMMIT_OUTCOME"]
     base_ref = os.environ["PRE_COMMIT_BASE_REF"]
     actor = os.environ["PRE_COMMIT_ACTOR"]
-    comment = render_comment(output_path, exit_code, outcome, base_ref, actor)
+    comment = render_comment(output_path, exit_code, base_ref, actor)
     with open(comment_path, "w") as f:
         f.write(comment)

tests/test_render_comment.py

@@ -4,7 +4,7 @@
 def test_success_shows_trophy(tmp_path):
     out = tmp_path / "output.txt"
     out.write_text("All hooks passed.")
-    result = render_comment(str(out), "0", "success", "main", "alice")
+    result = render_comment(str(out), "0", "main", "alice")
     assert "🏆" in result
     assert "@alice" in result
     assert "All hooks passed." in result
@@ -13,7 +13,7 @@ def test_success_shows_trophy(tmp_path):
 def test_failure_shows_stop_sign_and_hint(tmp_path):
     out = tmp_path / "output.txt"
     out.write_text("Hook failed.")
-    result = render_comment(str(out), "1", "failure", "main", "bob")
+    result = render_comment(str(out), "1", "main", "bob")
     assert "🚫" in result
     assert "pre-commit run --from-ref origin/main" in result
     assert "@bob" in result
@@ -22,19 +22,19 @@ def test_failure_shows_stop_sign_and_hint(tmp_path):
 def test_success_has_no_hint(tmp_path):
     out = tmp_path / "output.txt"
     out.write_text("ok")
-    result = render_comment(str(out), "0", "success", "main", "carol")
+    result = render_comment(str(out), "0", "main", "carol")
     assert "pre-commit install" not in result
 
 
 def test_exit_code_in_comment(tmp_path):
     out = tmp_path / "output.txt"
     out.write_text("done")
-    result = render_comment(str(out), "42", "failure", "main", "x")
+    result = render_comment(str(out), "42", "main", "x")
     assert "Exit code: 42" in result
 
 
 def test_missing_output_file(tmp_path):
-    result = render_comment(str(tmp_path / "nonexistent.txt"), "1", "failure", "main", "x")
+    result = render_comment(str(tmp_path / "nonexistent.txt"), "1", "main", "x")
     assert "failed to read" in result