From 2928f3ed20380f3e04695aaf29839948cd056aae Mon Sep 17 00:00:00 2001
From: fOuttaMyPaint <154358121+TMHSDigital@users.noreply.github.com>
Date: Fri, 24 Apr 2026 20:12:35 -0400
Subject: [PATCH] fix: composite action propagates checker exit code to caller
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The drift-check composite action was swallowing rc=1 (findings exist),
causing tool-repo CI jobs to pass green even when drift was detected.
This made the check informational rather than enforcing.

Fixes the exit code logic to propagate the checker's actual rc, so
drift findings cause the calling job to fail. Tool-repo PRs will
now correctly fail CI when they introduce or fail to fix drift.

Implementation: adds a final "Propagate checker exit code" step that
runs unconditionally (`if: always()`) and exits with the rc captured
into `steps.run.outputs.exit-code`. Restructured so the optional
sticky-issue step still runs after a rc=1 finding (meta-repo
aggregation case), then the propagate step surfaces the failure to
the calling workflow. The sticky step's own RC is intentionally NOT
propagated — it is best-effort and shouldn't gate the action on a
transient gh API hiccup. Run step still hard-aborts on rc=2 (tool
error) so we don't try to publish a sticky for a checker crash.

Adds three regression-guard tests in test_composite_action_shape.py:
- final step exists and uses if: always()
- final step is positioned last so its exit code wins
- sticky step does not propagate its own RC

Discovered during Phase 2 Session D D-0 audit. Required for the
drift-check job to function as a real gate in tool-repo validate.yml
once Session D rollout lands.

See TMHSDigital/Developer-Tools-Directory#1 Phase 2 Session D.

Signed-off-by: 154358121+TMHSDigital@users.noreply.github.com
Made-with: Cursor
---
 .github/actions/drift-check/action.yml | 43 ++++++++++++++++++----
 VERSION                                |  2 +-
 tests/test_composite_action_shape.py   | 50 ++++++++++++++++++++++++++
 3 files changed, 87 insertions(+), 8 deletions(-)

diff --git a/.github/actions/drift-check/action.yml b/.github/actions/drift-check/action.yml
index 54b7f2c..dd76c55 100644
--- a/.github/actions/drift-check/action.yml
+++ b/.github/actions/drift-check/action.yml
@@ -162,10 +162,12 @@ runs:
             --meta-commit "${{ steps.meta.outputs.sha }}"
         fi
 
-        # Action exit code is decoupled from drift-checker exit code; we
-        # surface drift via outputs so the calling workflow chooses how
-        # to react. Composite action exits 0 unless the checker hit a
-        # tool error (rc=2).
+        # Tool errors (rc=2) abort immediately and skip the sticky-issue
+        # step — there's nothing meaningful to publish if the checker
+        # itself failed. Drift findings (rc=1) are NOT propagated here so
+        # the optional sticky-issue step still runs; the final
+        # "Propagate checker exit code" step at the end of this composite
+        # surfaces rc=1 to the caller, failing the calling job on drift.
         if [ "$FINDINGS_RC" = "2" ]; then
           exit 2
         fi
@@ -209,7 +211,34 @@ runs:
         # Parse the "Sticky issue: <action>" line emitted by cli.py.
         ACTION=$(echo "$OUT" | grep -oE 'Sticky issue: (created|updated|reopened|closed|no_op)' | head -1 | awk '{print $3}')
         echo "action=${ACTION:-unknown}" >> "$GITHUB_OUTPUT"
-        if [ "$RC" = "2" ]; then
-          exit 2
-        fi
+        # Sticky-step rc is best-effort: don't gate the action on a
+        # transient gh-API hiccup. The final "Propagate checker exit
+        # code" step uses the original drift-checker rc captured by the
+        # run step, which is the authoritative pass/fail signal.
         exit 0
+
+    - name: Propagate checker exit code
+      # Always run so the action's exit code mirrors the drift checker's
+      # actual rc, regardless of whether the optional sticky step ran or
+      # what its rc was. This is what turns the composite into a real
+      # gate: rc=1 (drift) makes the calling job fail. rc=0 (clean) lets
+      # it pass. rc=2 (tool error) was already surfaced by the run step
+      # exiting 2; we mirror it here for completeness. If the run step
+      # never executed (e.g., checkout failed earlier), exit 0 so we
+      # don't mask the earlier failure that already failed the action.
+      if: always()
+      shell: bash
+      run: |
+        RC="${{ steps.run.outputs.exit-code }}"
+        if [ -z "$RC" ]; then
+          echo "drift checker did not run; nothing to propagate"
+          exit 0
+        fi
+        echo "drift checker exit code: $RC"
+        case "$RC" in
+          0) echo "no drift detected" ;;
+          1) echo "::error::drift detected — failing job per composite-action contract" ;;
+          2) echo "::error::tool error in drift checker" ;;
+          *) echo "::warning::unexpected exit code from drift checker: $RC" ;;
+        esac
+        exit "$RC"
diff --git a/VERSION b/VERSION
index bbf649f..3511591 100644
--- a/VERSION
+++ b/VERSION
@@ -1 +1 @@
-1.7.3
\ No newline at end of file
+1.7.4
\ No newline at end of file
diff --git a/tests/test_composite_action_shape.py b/tests/test_composite_action_shape.py
index b7d5a09..31e21e8 100644
--- a/tests/test_composite_action_shape.py
+++ b/tests/test_composite_action_shape.py
@@ -92,3 +92,53 @@ def test_workflow_token_has_fallback(workflow_doc):
     token = check_step["with"]["github-token"]
     assert "DRIFT_CHECK_TOKEN" in token
     assert "GITHUB_TOKEN" in token
+
+
+def test_action_propagates_checker_exit_code(action_doc):
+    """Final step must propagate the drift checker's rc to the caller.
+
+    Regression guard for the rc=1 swallowing bug fixed in v1.7.4: prior
+    to the fix the action exited 0 unless rc=2, so tool-repo CI passed
+    green even when drift was detected.
+    """
+    steps = action_doc["runs"]["steps"]
+    propagate = next(
+        (s for s in steps if s.get("name") == "Propagate checker exit code"),
+        None,
+    )
+    assert propagate is not None, "missing 'Propagate checker exit code' step"
+    # Must run unconditionally so it always surfaces the recorded rc.
+    assert propagate.get("if", "").strip().startswith("always()"), (
+        f"propagate step must use if: always(); got {propagate.get('if')!r}"
+    )
+    body = propagate.get("run", "")
+    assert "steps.run.outputs.exit-code" in body, (
+        "propagate step must read exit-code from the run step output"
+    )
+    assert 'exit "$RC"' in body, (
+        "propagate step must exit with the recorded rc, not a hard-coded value"
+    )
+
+
+def test_action_propagate_step_is_last(action_doc):
+    """The propagate step must be the last step so it determines the
+    action's exit code regardless of what happens before it."""
+    steps = action_doc["runs"]["steps"]
+    assert steps[-1].get("name") == "Propagate checker exit code", (
+        "propagate must be the last step; otherwise its exit code can be "
+        "overridden by a subsequent step"
+    )
+
+
+def test_action_sticky_step_does_not_swallow_rc(action_doc):
+    """Sticky-step is best-effort and should always exit 0; the propagate
+    step is the single source of truth for the action's rc."""
+    steps = action_doc["runs"]["steps"]
+    sticky = next((s for s in steps if s.get("id") == "sticky"), None)
+    assert sticky is not None, "missing sticky step"
+    body = sticky.get("run", "")
+    # No `exit "$RC"` or `exit $RC` in sticky — those would gate the
+    # action on a transient gh-API hiccup.
+    assert "exit \"$RC\"" not in body and "exit $RC" not in body, (
+        "sticky step must not propagate its own RC; that's the propagate step's job"
+    )