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: " 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" + )