ci: enforce PR title and commit subjects in GitHub Actions (EXT-32) - #226
Closed
ruby-automation wants to merge 11 commits into
Closed
ci: enforce PR title and commit subjects in GitHub Actions (EXT-32)#226ruby-automation wants to merge 11 commits into
ruby-automation wants to merge 11 commits into
Conversation
- Insert root `mix format --check-formatted` and `mix test` (steps 3-4) between the metadata guards and the app checks in mix precommit. These cover the root project's own sources and validator tests. - Update the moduledoc to list all 11 steps with correct numbering. - Convert mix ci into a thin alias that delegates entirely to Mix.Tasks.Precommit (both run/1 and the testable run/2 passthrough). - Update precommit_test to assert all 11 steps in order. - Replace ci_test's independent step list with delegation assertions. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- .ai/prompts/implement.md: replace `mix ci` with `mix precommit` in the quality-suite step. - Readme.adoc: present `mix precommit` as the canonical full-repository quality command; retain focused single-project commands below it. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- ci.yaml: add 'edited' to pull_request trigger types so title-only
changes retrigger the required Test check.
- ci.yaml: replace skip_commit_validation boolean workflow_call input
with base_ref, pull_request_title_required, and pull_request_title so
each caller can supply the exact data its event provides.
- ci.yaml: export BASE_REF, PULL_REQUEST_TITLE_REQUIRED, and
PULL_REQUEST_TITLE as env vars (never via ${{ }} in run:) so PR titles
containing quotes, backticks, dollar signs, or Unicode are inert data.
- ci.yaml: run mix precommit instead of mix ci in the Test job, giving
the Test required check the full 11-step gate including commit-range
and PR-title validation.
- ci.yaml: exclude 'edited' from burrito_changes guard — a title edit
never produces a file diff.
- ci.yaml: update conventional_commits compatibility job condition to use
pull_request_title_required instead of skip_commit_validation; keep
emitting the Validate Commit Subjects check for PRs until EXT-33
migrates branch protection.
- main.yaml: replace skip_commit_validation: true with granular inputs —
base_ref carries github.event.before for push events and
pull_request.base.sha for closed-PR events; pull_request_title_required
is always false (no PR title to enforce post-merge).
inputs.pull_request_title_required defaults to false in GitHub Actions even for non-workflow_call triggers, so the previous condition `inputs.pull_request_title_required != false` skipped the compatibility job on direct pull_request events. Replace with `github.event_name != 'workflow_call' || inputs.pull_request_title_required` so the job runs for all direct triggers (pull_request, workflow_dispatch) and skips only when main.yaml calls the workflow with pull_request_title_required: false for a post-merge event.
BASE_REF and PULL_REQUEST_TITLE* are set in the GitHub Actions job-level env: block so mix precommit can validate commits and PR titles. But System.cmd :env only adds/overrides the listed keys — any unlisted CI var flows through from the inherited process environment and corrupts test subprocess calls. For range guard tests that don't specify an explicit BASE_REF, the CI SHA (e.g. fd2dbb8...) was used as the base; git merge-base then failed because the test's temp repo has no reachable path to that commit. For the title guard "rejects missing required title" test, PULL_REQUEST_TITLE from CI leaked through (only PULL_REQUEST_TITLE_REQUIRED was listed in the test env), so the script found the title set and validated it rather than reporting the expected missing-title error. Fix: in run/3, build an explicit ci_clear list that sets each known CI var to false (unset) unless the test has already provided a value for that key. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Elixir 1.20's System.cmd does not support {key, false} for unsetting env
vars — it calls String.to_charlist/1 on the value, which has no clause for
false. Replace the previous approach with /usr/bin/env -u KEY flags, which
properly remove each CI-level variable from the subprocess environment even
when the variable exists in the inherited process env.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- ci/validate_commit_range.sh: simplify git log format from '%P%x01%cn%x01%ce%x01%s' to '%s'; remove the parents/committer_name/ committer_email/parent_count parsing that was left behind when the merge-commit exemption was removed. Only the subject is passed to the validator, so fetching three extra fields per commit was dead code. - test/mix/tasks/precommit_test.exs: use assert_received instead of assert_receive for consistency with ci_test.exs. Messages are sent synchronously within the same process, so assert_received (which does not wait for a message to arrive) is more appropriate. - test/git_hooks_test.exs: remove unused default argument from run_with_stdin/3; every caller passes opts explicitly, so the default was never triggered (and generated a compiler warning). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
Author
|
Superseded by #234 — re-cut from current main after EXT-31 merged, removing the duplicate EXT-31 commits and spurious file deletions. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements the EXT-32 GitHub Actions event contract from Phase 14.
This PR builds on EXT-31 (PR #225, gate-approved). It branches from the EXT-31 head commit so both land together and the CI failure caused by converting
mix cito delegate tomix precommitis resolved in the same set of checks.Changes
.github/workflows/ci.yamleditedto thepull_requesttrigger — title-only changes now retrigger the requiredTestcheck so a valid-then-edited-to-invalid title is caught without needing a new commitskip_commit_validationbooleanworkflow_callinput with three granular inputs:base_ref,pull_request_title_required, andpull_request_title— each caller supplies exactly the data its event providesTestjob now runsmix precommit(full 11-step gate) instead ofmix ciBASE_REF,PULL_REQUEST_TITLE_REQUIRED, andPULL_REQUEST_TITLEare exported viaenv:, never interpolated inline inrun:— titles with quotes, backticks, dollar signs, or Unicode are treated as inert databurrito_changesguard excludeseditedevents — a title change never produces a file diffconventional_commitscompatibility job condition updated fromskip_commit_validation != truetopull_request_title_required != false— keeps emitting theValidate Commit Subjectsrequired check for PRs until EXT-33 migrates branch protection.github/workflows/main.yamlskip_commit_validation: truewith granular inputs —base_refcarriesgithub.event.beforefor push events (validating the integration commit) andpull_request.base.shafor closed-PR events;pull_request_title_requiredis alwaysfalse(no PR title to enforce post-merge)Acceptance criteria coverage
Test(PR title env is set,validate_pull_request_title.shrejects it)editedtrigger covers thisvalidate_commit_range.shruns before the app suiteref: github.event.pull_request.head.sha || github.shagithub.event.before—base_refpasses the exact SHApull_request_title_required: falsefor push/closed/dispatchTestandValidate Commit Subjectscontinue to be emitted during migrationTest plan
Testjob runsmix precommitwithBASE_REFandPULL_REQUEST_TITLEenv vars in CI logsValidate Commit Subjectscompatibility job still runs and passes🤖 Generated with Claude Code