diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 7f63640..fee9975 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -5,14 +5,35 @@ on: # yamllint disable-line rule:truthy workflow_dispatch: workflow_call: inputs: - skip_commit_validation: + base_ref: description: >- - Skip the commit-subject check - it only makes sense pre-merge - (should this PR be mergeable), not as a post-merge gate. + Commit comparison base: an exact 40-character SHA (for push events + and closed-PR validation), a branch name (for workflow_dispatch with + an explicit base), or empty (falls back to origin/main). The Test job + exports this as BASE_REF so ci/validate_commit_range.sh uses the + correct range instead of re-resolving it from scratch. + required: false + type: string + default: "" + pull_request_title_required: + description: >- + Set to true only for pull_request events. ci/validate_pull_request_title.sh + treats any other value as "skip" and exits 0. Pass "false" for push, + closed-PR, and workflow_dispatch events where there is no PR title to enforce. required: false type: boolean default: false + pull_request_title: + description: >- + The pull request title to validate. Only meaningful when + pull_request_title_required is true. Passed as an environment variable + so the title is never interpolated as shell code — quotes, backticks, + dollar signs, and Unicode are all safe. + required: false + type: string + default: "" pull_request: + types: [opened, synchronize, reopened, edited] permissions: contents: read @@ -29,14 +50,15 @@ jobs: - uses: actions/checkout@v7 with: - # The repository-level precommit task validates every commit since - # the merge base. Retain the full range so its shell guard can - # resolve that base without a second network fetch. + # mix precommit calls ci/validate_commit_range.sh, which validates + # every commit since the merge base. A full-history checkout lets the + # script resolve that base locally without a second network fetch. fetch-depth: 0 # pull_request otherwise checks out GitHub's synthetic test-merge - # commit. Validate the contributor's actual branch tip instead; - # the shell guard must never exempt a commit based on forgeable - # subject or committer metadata. + # commit (subject "Merge into "). Validate the + # contributor's actual branch tip instead so that commit never + # appears in the validated range. Falls back to github.sha for + # non-PR triggers (workflow_dispatch, workflow_call from push/closed). ref: ${{ github.event.pull_request.head.sha || github.sha }} - uses: erlef/setup-beam@v1 @@ -58,19 +80,44 @@ jobs: app/_build key: ${{ runner.os }}-mix-${{ hashFiles('app/mix.lock') }} - - # mix ci covers: deps.get, hex.audit, deps.audit (Elixir security - # advisories), format --check-formatted, credo (static analysis), - # usage_rules.sync --check (catches dep-bump drift, see #79), - # and mix test. Runs from the repo root; cd: app/ is handled - # inside the task itself. - run: mix ci + # mix precommit runs the full 11-step gate: + # 1. ci/validate_pull_request_title.sh — PR title (when required) + # 2. ci/validate_commit_range.sh — all commit subjects in the range + # 3-4. Root format + test (the repo-management tooling itself) + # 5-11. App deps.get, hex.audit, deps.audit, format, credo, + # usage_rules.sync, and test. + # BASE_REF carries the exact comparison base SHA so the commit-range + # validator never has to guess. For pull_request events it is the exact + # base SHA; for push events it is github.event.before; for + # workflow_dispatch and unverified closes it is empty, and the validator + # falls back to origin/main. + # PULL_REQUEST_TITLE is set via env (never ${{ }} in run:) so PR titles + # containing quotes, backticks, dollar signs, or Unicode are treated as + # inert data, not executable shell code. + name: Run full quality gate + env: + BASE_REF: >- + ${{ inputs.base_ref != '' && inputs.base_ref + || github.event.pull_request.base.sha }} + PULL_REQUEST_TITLE_REQUIRED: >- + ${{ inputs.pull_request_title_required + || github.event_name == 'pull_request' }} + PULL_REQUEST_TITLE: >- + ${{ inputs.pull_request_title != '' && inputs.pull_request_title + || github.event.pull_request.title }} + run: mix precommit working-directory: . burrito_changes: # Building Burrito is deliberately reserved for changes that affect its # dependency graph or packaging path. The release workflow still builds # every target before publishing. - if: github.event_name == 'pull_request' && github.event.action != 'closed' + # Exclude 'edited' events — a title-only change never alters file content, + # so there are no Burrito-impacting diffs to check. + if: >- + github.event_name == 'pull_request' + && github.event.action != 'closed' + && github.event.action != 'edited' name: Detect Burrito-impacting changes runs-on: ubuntu-latest outputs: @@ -152,7 +199,16 @@ jobs: run: ../ci/test_burrito_shared_loader.sh ./burrito_out/lc_linux_x86_64 conventional_commits: - if: inputs.skip_commit_validation != true + # Temporary compatibility job retained while branch protection still lists + # "Validate Commit Subjects" as a required check. The Test job above runs + # the same ci/validate_commit_range.sh via mix precommit and is the + # authoritative gate; this job will be removed in EXT-33 after the required- + # check set is migrated to "Test" only. + # Run for all direct triggers (pull_request, workflow_dispatch); skip only + # when main.yaml calls this workflow for a non-PR event and explicitly sets + # pull_request_title_required to false — those are post-merge runs where + # there is no open PR to gate on Validate Commit Subjects. + if: github.event_name != 'workflow_call' || inputs.pull_request_title_required name: Validate Commit Subjects runs-on: ubuntu-latest steps: @@ -167,4 +223,8 @@ jobs: # workflow_call), where github.event.pull_request is unset. ref: ${{ github.event.pull_request.head.sha || github.sha }} - + env: + BASE_REF: >- + ${{ inputs.base_ref != '' && inputs.base_ref + || github.event.pull_request.base.sha }} run: ./ci/validate_commit_range.sh diff --git a/.github/workflows/main.yaml b/.github/workflows/main.yaml index 03dcf3e..0e6e326 100644 --- a/.github/workflows/main.yaml +++ b/.github/workflows/main.yaml @@ -19,7 +19,23 @@ jobs: name: Validations uses: ./.github/workflows/ci.yaml with: - skip_commit_validation: true + # push: validate every commit introduced after github.event.before. + # The exact SHA lets ci/validate_commit_range.sh check the squash/merge + # integration commit itself rather than comparing main to main (empty). + # pull_request closed: validate the merged result when a PR was actually + # merged; the base SHA gives the correct start of the new-commits range. + # For unmerged closes the range would be empty, but passing the base SHA + # is harmless — git log will find no commits to validate. + # workflow_dispatch: no PR title and no exact base; the validator falls + # back to origin/main as the comparison base. + # PR title validation only applies to direct pull_request events + # (handled by ci.yaml's own trigger), not to any workflow_call here. + base_ref: >- + ${{ github.event_name == 'push' && github.event.before + || github.event_name == 'pull_request' && github.event.pull_request.base.sha + || '' }} + pull_request_title_required: false + pull_request_title: "" manage-release-pr: needs: [validate] diff --git a/ci/validate_commit_range.sh b/ci/validate_commit_range.sh index 4b9a48f..9c862c5 100755 --- a/ci/validate_commit_range.sh +++ b/ci/validate_commit_range.sh @@ -105,18 +105,8 @@ base_sha=$(git_or_die merge-base HEAD "$base_ref") validation_status=0 -while IFS= read -r -d '' entry +while IFS= read -r -d '' subject do - IFS=$'\x01' read -r parents committer_name committer_email subject <<< "$entry" - - if [ -n "$parents" ] - then - IFS=' ' read -ra parents_array <<< "$parents" - parent_count=${#parents_array[@]} - else - parent_count=0 - fi - "$validator" --subject "$subject" status=$? @@ -124,6 +114,6 @@ do then validation_status=$status fi -done < <(git log -z --format='%P%x01%cn%x01%ce%x01%s' "$base_sha..HEAD") +done < <(git log -z --format='%s' "$base_sha..HEAD") exit "$validation_status" diff --git a/test/git_hooks_test.exs b/test/git_hooks_test.exs index 4a0985c..801c318 100644 --- a/test/git_hooks_test.exs +++ b/test/git_hooks_test.exs @@ -318,7 +318,7 @@ defmodule GitHooksTest do {worktree, ci_dir} end - defp run_with_stdin(command, stdin_content, opts \\ []) do + defp run_with_stdin(command, stdin_content, opts) do nonce = :crypto.strong_rand_bytes(8) |> Base.url_encode64(padding: false) stdin_file = Path.join(System.tmp_dir!(), "linear_cli_stdin_#{nonce}") File.write!(stdin_file, stdin_content) @@ -344,8 +344,29 @@ defmodule GitHooksTest do end defp run(command, args, opts \\ []) do - opts = Keyword.put(opts, :stderr_to_stdout, true) - System.cmd(command, args, opts) + test_env = Keyword.get(opts, :env, []) + test_env_keys = MapSet.new(test_env, fn {k, _} -> k end) + + # Elixir 1.20's System.cmd :env only adds/overrides listed keys — any CI var + # not listed flows through unchanged. Elixir 1.20 also does not support + # {key, false} for unsetting. Use env(1) -u flags instead to clear CI-level + # variables (BASE_REF, PULL_REQUEST_TITLE*) that the GitHub Actions job + # environment sets and that would otherwise corrupt these test subprocess calls. + unset_flags = + ~w[BASE_REF GITHUB_BASE_REF PULL_REQUEST_TITLE PULL_REQUEST_TITLE_REQUIRED] + |> Enum.reject(&MapSet.member?(test_env_keys, &1)) + |> Enum.flat_map(&["-u", &1]) + + set_args = Enum.map(test_env, fn {k, v} -> "#{k}=#{v}" end) + + env_args = unset_flags ++ set_args ++ [command | args] + + opts = + opts + |> Keyword.delete(:env) + |> Keyword.put(:stderr_to_stdout, true) + + System.cmd("/usr/bin/env", env_args, opts) end defp git!(directory, args) do diff --git a/test/mix/tasks/precommit_test.exs b/test/mix/tasks/precommit_test.exs index 8142be5..3af847d 100644 --- a/test/mix/tasks/precommit_test.exs +++ b/test/mix/tasks/precommit_test.exs @@ -13,17 +13,17 @@ defmodule Mix.Tasks.PrecommitTest do assert :ok = Precommit.run([], shell) - assert_receive {:run, "./ci/validate_pull_request_title.sh", [], []} - assert_receive {:run, "./ci/validate_commit_range.sh", [], []} - assert_receive {:run, "mix", ["format", "--check-formatted"], []} - assert_receive {:run, "mix", ["test"], []} - assert_receive {:run, "mix", ["deps.get"], [cd: "app"]} - assert_receive {:run, "mix", ["hex.audit"], [cd: "app"]} - assert_receive {:run, "mix", ["deps.audit"], [cd: "app"]} - assert_receive {:run, "mix", ["format", "--check-formatted"], [cd: "app"]} - assert_receive {:run, "mix", ["credo", "--strict"], [cd: "app"]} - assert_receive {:run, "mix", ["usage_rules.sync", "--check"], [cd: "app"]} - assert_receive {:run, "mix", ["test"], [cd: "app"]} + assert_received {:run, "./ci/validate_pull_request_title.sh", [], []} + assert_received {:run, "./ci/validate_commit_range.sh", [], []} + assert_received {:run, "mix", ["format", "--check-formatted"], []} + assert_received {:run, "mix", ["test"], []} + assert_received {:run, "mix", ["deps.get"], [cd: "app"]} + assert_received {:run, "mix", ["hex.audit"], [cd: "app"]} + assert_received {:run, "mix", ["deps.audit"], [cd: "app"]} + assert_received {:run, "mix", ["format", "--check-formatted"], [cd: "app"]} + assert_received {:run, "mix", ["credo", "--strict"], [cd: "app"]} + assert_received {:run, "mix", ["usage_rules.sync", "--check"], [cd: "app"]} + assert_received {:run, "mix", ["test"], [cd: "app"]} end test "rejects arguments" do