fix: report the version-floor check as skipped rather than passed

[bryce-lynn-nttd]

Summary

Follow-up to #94, addressing both of @aarti-joshi-nttd's review comments. Split out so #94 could merge without dismissing her approval and @rakesh-gorige-nttd's.

1. A skipped floor check no longer reports as passed

The guard ships via launch-terraform-skeleton, so it is absent until a repo picks up that update. The check step handled this by emitting a ::notice and exiting 0 — which makes steps.version_floor.outcome report success, so the commit status read version floor success when nothing was verified.

The job-log notice mitigated it only for people reading the log. Anyone reading the status got the wrong answer, and during rollout that is the majority of repos — the window where being wrong matters most. #94's own description argues that a silent no-op check is worse than none; this closes the gap between that claim and the behaviour.

The step now records what actually happened:

if [[ ! -f "${guard}" ]]; then
  echo "::notice title=Version floor check skipped::..."
  echo "result=skipped" >> "$GITHUB_OUTPUT"
  exit 0
fi
make tfmodule/check-version-floor
echo "result=success" >> "$GITHUB_OUTPUT"

and the status reflects it:

description: "Terraform lint ${{ steps.lint.outcome }}, version floor ${{ steps.version_floor.outputs.result || steps.version_floor.outcome }}"

The fallback is load-bearing: on a genuine failure the step aborts before writing result, so the expression falls through to outcome and still reads failure. Resulting descriptions:

situation	outputs.result	status description
guard absent (rollout)	skipped	version floor skipped
guard ran, passed	success	version floor success
guard ran, failed	unset	version floor failure

The status: expression is deliberately unchanged — it keys off outcome != 'failure', and a skip correctly is not a failure, so a repo without the guard still reports a green Terraform Lint overall. Only the description stops overstating.

2. Failed floor resolution is no longer silent

--print-floor had its stderr sent to /dev/null. Deferring diagnostics to the check step is intentional — this step exists only to produce a cache key — but Aarti's point stands for the case where resolution fails for a reason the check step won't explain, since that step reports its own error rather than this one.

Resolution failing stays non-fatal; it just says why now:

if ! version="$(bash "${guard}" --print-floor 2>"${RUNNER_TEMP}/floor-resolve.err")"; then
  version=""
  [[ -s "${RUNNER_TEMP}/floor-resolve.err" ]] && \
    echo "::notice title=Version floor not resolved::$(head -3 "${RUNNER_TEMP}/floor-resolve.err")"
fi

The two documented empty-value cases — guard not yet present, constraint with no lower bound — stay quiet by design, because the guard writes nothing to stderr for the first and the check step gives the real diagnostic for the second.

Validation

Both run: blocks parse under bash -n with GHA's shell defaults in mind (-eo pipefail); if ! version="$(…)" is safe under set -e, which a bare assignment would not have been.

Not yet exercised end to end against a repo that has the guard, since none have it until the skeleton pin is bumped. The three status-description outcomes above are the thing worth checking on the first real run.

Context

Worth noting for anyone sizing this: merging #94 did not change behaviour anywhere. Converted repos pin reusable-terraform-check.yml by bare SHA, so enforcement begins only when that pin is bumped in the skeleton and propagated. This lands well before that.

Generated with Cursor Agent (Opus 5)

[rakesh-gorige-nttd]

Approve. Clean follow-up to #94.

Fixes the misleading version floor success during rollout: write result=skipped|success, reflect it in the status description via outputs.result || outcome, and leave overall status: unchanged so skips stay green. Resolve-step stderr capture is a good addition; if ! version="$(…)" is correct under set -e.

Nits (non-blocking): multiline stderr in ::notice:: may need whitespace collapsing or %0A; PR description slightly overstates quietness for the no-lower-bound case.

First run with guard: absent → skipped; valid → success; bad → failure.

[ben-vaughan-nttd]

Approve. Correct follow-up to #94 — addresses both of Aarti's inline comments.

Skipped vs passed: result=skipped|success plus outputs.result || outcome in the status description fixes the rollout bug where absent guard + exit 0 advertised version floor success. Overall status staying green on skip is correct.

Resolve diagnostics: Captured stderr + conditional notice is better than 2>/dev/null; if ! version="$(…)" is safe under set -e.

Nit (non-blocking): multiline stderr in ::notice:: may need whitespace collapsing; if lint fails, floor step is skipped and description may still say version floor skipped — pre-existing, not introduced here.