Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 47 additions & 0 deletions .github/actions/required-check/action.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
name: Required check
description: >-
Report a protected check from the job that did the work. Branch protection
does not accept a skipped check as satisfied, so a tool-scoped PR cannot
merge while the other tool's jobs skip. A wrapper using this action always
runs and passes on that job's behalf.

inputs:
job:
description: Name of the job whose outcome is being reported.
required: true
result:
description: result of that job (needs.<job>.result).
required: true
gate-result:
description: >-
result of the job whose outputs decide whether the work runs. A skip is
only meaningful if this succeeded; when it did not, the work was skipped
for an unrelated reason and nothing was verified.
required: true

runs:
using: composite
steps:
- shell: bash
env:
JOB: ${{ inputs.job }}
RESULT: ${{ inputs.result }}
GATE: ${{ inputs.gate-result }}
run: |
set -euo pipefail
if [ "$GATE" != "success" ]; then
echo "::error::change detection did not succeed (result: $GATE), so $JOB was skipped for an unrelated reason and nothing was verified"
exit 1
fi
case "$RESULT" in
success)
echo "$JOB ran and passed"
;;
skipped)
echo "$JOB had no relevant changes; nothing to verify"
;;
*)
echo "::error::$JOB result: $RESULT"
exit 1
;;
esac
93 changes: 87 additions & 6 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ jobs:
- 'Makefile'

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

File-level note: .github/workflows/ci.yml

The gate decision table (7 cases: success/skipped/failure/cancelled combinations of a -run job's result crossed with detect-changes' result) and the actionlint pass the PR cites as verification were both run manually and are not captured anywhere in CI. Nothing in this repo's workflows runs actionlint, and the decision table exists only as prose in the PR body, not as a script/test under version control. This is exactly the class of bug this PR fixes (a workflow-YAML gating mistake that silently breaks required-check enforcement) and it will now recur invisibly: a future edit to required-check/action.yml or to any of the six near-identical wrapper blocks can regress the gate logic (e.g. drop the gate-result check, mis-wire needs, or flip a case) with no CI signal, only manual review to catch it. Add an actionlint step to CI (or a pre-commit/make lint target already run in CI) so workflow syntax is checked automatically, and commit the decision-table cases as a small bats/shell test that exercises .github/actions/required-check/action.yml's script directly so a regression fails the build instead of requiring another manual re-verification.

Reply inline to this comment.

- '.github/workflows/ci.yml'

build-test-cfl:
build-test-cfl-run:
needs: detect-changes
if: needs.detect-changes.outputs.cfl == 'true' || needs.detect-changes.outputs.shared == 'true'
runs-on: ubuntu-latest
Expand Down Expand Up @@ -72,7 +72,7 @@ jobs:
- name: Test cfl
run: go test -v -race -coverprofile=coverage-cfl.out ./tools/cfl/...

build-test-jtk:
build-test-jtk-run:
needs: detect-changes
if: needs.detect-changes.outputs.jtk == 'true' || needs.detect-changes.outputs.shared == 'true'
runs-on: ubuntu-latest
Expand Down Expand Up @@ -106,7 +106,7 @@ jobs:
- name: Test jtk
run: go test -v -race -coverprofile=coverage-jtk.out ./tools/jtk/...

lint-cfl:
lint-cfl-run:
needs: detect-changes
if: needs.detect-changes.outputs.cfl == 'true' || needs.detect-changes.outputs.shared == 'true'
runs-on: ubuntu-latest
Expand All @@ -120,7 +120,7 @@ jobs:
working-directory: tools/cfl
version: v2.12.2

lint-jtk:
lint-jtk-run:
needs: detect-changes
if: needs.detect-changes.outputs.jtk == 'true' || needs.detect-changes.outputs.shared == 'true'
runs-on: ubuntu-latest
Expand Down Expand Up @@ -180,7 +180,7 @@ jobs:
# packaging/identity.yml matches its tool-native files. working-directory is
# the tool root; repo-root defaults to "." so the root-relative
# goreleaser_config resolves (distribution.md §8.3 / .github#15).
identity-check-cfl:
identity-check-cfl-run:
needs: detect-changes
if: needs.detect-changes.outputs.cfl == 'true' || needs.detect-changes.outputs.shared == 'true'
runs-on: ubuntu-latest
Expand All @@ -190,7 +190,7 @@ jobs:
with:
working-directory: tools/cfl

identity-check-jtk:
identity-check-jtk-run:
needs: detect-changes
if: needs.detect-changes.outputs.jtk == 'true' || needs.detect-changes.outputs.shared == 'true'
runs-on: ubuntu-latest
Expand All @@ -207,3 +207,84 @@ jobs:
- uses: open-cli-collective/.github/actions/pr-title@v1
with:
title: ${{ github.event.pull_request.title }}

# Required-check wrappers (INT-726). Branch protection requires these
# names, and GitHub does not accept a skipped check as satisfied, so a PR
# touching one tool could not merge without an admin bypass. Each wrapper
# always runs and reports on behalf of the job that does the work, so the
# -run jobs stay free of per-step guards.
#
# detect-changes is in needs so a skip can be told apart from a skip
# caused by change detection failing, which would otherwise pass every
# required check with nothing verified.
build-test-cfl:
needs: [detect-changes, build-test-cfl-run]
if: always()
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- uses: ./.github/actions/required-check
with:
job: build-test-cfl-run
result: ${{ needs.build-test-cfl-run.result }}
gate-result: ${{ needs.detect-changes.result }}

build-test-jtk:
needs: [detect-changes, build-test-jtk-run]
if: always()
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- uses: ./.github/actions/required-check
with:
job: build-test-jtk-run
result: ${{ needs.build-test-jtk-run.result }}
gate-result: ${{ needs.detect-changes.result }}

lint-cfl:
needs: [detect-changes, lint-cfl-run]
if: always()
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- uses: ./.github/actions/required-check
with:
job: lint-cfl-run
result: ${{ needs.lint-cfl-run.result }}
gate-result: ${{ needs.detect-changes.result }}

lint-jtk:
needs: [detect-changes, lint-jtk-run]
if: always()
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- uses: ./.github/actions/required-check
with:
job: lint-jtk-run
result: ${{ needs.lint-jtk-run.result }}
gate-result: ${{ needs.detect-changes.result }}

identity-check-cfl:
needs: [detect-changes, identity-check-cfl-run]
if: always()
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- uses: ./.github/actions/required-check
with:
job: identity-check-cfl-run
result: ${{ needs.identity-check-cfl-run.result }}
gate-result: ${{ needs.detect-changes.result }}

identity-check-jtk:
needs: [detect-changes, identity-check-jtk-run]
if: always()
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- uses: ./.github/actions/required-check
with:
job: identity-check-jtk-run
result: ${{ needs.identity-check-jtk-run.result }}
gate-result: ${{ needs.detect-changes.result }}
Loading