diff --git a/.ai/prompts/implement.md b/.ai/prompts/implement.md index 827699c..c0f499f 100644 --- a/.ai/prompts/implement.md +++ b/.ai/prompts/implement.md @@ -36,7 +36,7 @@ necessary for release-please to pick up our squash merge commits to main. ``` 4. Implement the changes with clean, logical commits. 5. Run the full quality suite: - - mix ci + - mix precommit 6. Fix any failures before proceeding. 7. Push the branch and create a PR: ``` diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 3b0f552..7f63640 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -28,6 +28,16 @@ jobs: steps: - 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. + 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. + ref: ${{ github.event.pull_request.head.sha || github.sha }} - uses: erlef/setup-beam@v1 with: @@ -157,6 +167,4 @@ jobs: # workflow_call), where github.event.pull_request is unset. ref: ${{ github.event.pull_request.head.sha || github.sha }} - - env: - FETCH_BASE_REF: "true" run: ./ci/validate_commit_range.sh diff --git a/Readme.adoc b/Readme.adoc index ebbb59e..22e907c 100644 --- a/Readme.adoc +++ b/Readme.adoc @@ -476,7 +476,14 @@ $ mix lc whoami $ mix lc issue list --output json ---- -The project uses ExUnit and `mix format`. Run tests with: +The project uses ExUnit and `mix format`. Run the full quality gate with: + +[source,sh] +---- +$ mix precommit +---- + +To run only the app test suite or format check directly: [source,sh] ---- diff --git a/ci/validate_commit_range.sh b/ci/validate_commit_range.sh index 523ee3e..4b9a48f 100755 --- a/ci/validate_commit_range.sh +++ b/ci/validate_commit_range.sh @@ -8,8 +8,7 @@ usage() { $0 Environment: - BASE_REF Base branch or ref. Defaults to GITHUB_BASE_REF, then origin/main. - FETCH_BASE_REF When "true", fetch BASE_REF from origin before validating. + BASE_REF Base branch or ref. Defaults to GITHUB_BASE_REF, then main. EOT } @@ -37,9 +36,7 @@ validator="$repo_top/ci/validate_conventional_subject.sh" [ -x "$validator" ] || die "validator is not executable: $validator" -base_input=${BASE_REF:-${GITHUB_BASE_REF:-}} -base_name=${base_input#refs/heads/} -base_name=${base_name#origin/} +base_input=${BASE_REF:-${GITHUB_BASE_REF:-main}} base_ref= @@ -49,20 +46,22 @@ then # already contains it, and using the immutable SHA ensures the newly # pushed main commit is validated instead of comparing main to itself. base_ref_candidates="$base_input" -elif [ -n "$base_input" ] -then - if [ "${FETCH_BASE_REF:-}" = "true" ] + + if ! git rev-parse --verify --quiet "$base_input" >/dev/null then - git_or_die fetch --no-tags origin "$base_name:refs/remotes/origin/$base_name" >/dev/null + # A shallow GitHub checkout may contain only HEAD. Fetch the precise + # pre-push SHA here rather than requiring workflow-specific setup. + git_or_die fetch --no-tags origin "$base_input" >/dev/null fi +else + base_name=${base_input#refs/heads/} + base_name=${base_name#origin/} # Prefer the remote-tracking ref. In particular, a developer pushing # directly from local main must compare against origin/main, not against # local main (HEAD), or the range would be empty and a bypassed commit-msg # hook could slip through pre-push validation. base_ref_candidates="origin/$base_name $base_input $base_name" -else - base_ref_candidates="origin/main main" fi for candidate in $base_ref_candidates @@ -74,20 +73,38 @@ do fi done +[ -n "$base_ref" ] || { + if [[ "$base_input" =~ ^[0-9a-fA-F]{40}$ ]] + then + # `git fetch origin ` normally makes the object directly + # addressable. Keep FETCH_HEAD as a fallback for Git servers that do + # not install an anonymous remote-tracking ref for a SHA request. + base_ref_candidates="$base_input FETCH_HEAD" + else + # Local clones normally already have origin/main (or their configured + # BASE_REF), so this branch is not taken locally. GitHub Actions' + # default shallow checkout does not; fetch the missing base from here + # so callers never need a workflow-level FETCH_BASE_REF switch. + git_or_die fetch --no-tags origin "$base_name:refs/remotes/origin/$base_name" >/dev/null + base_ref_candidates="origin/$base_name $base_input $base_name" + fi + + for candidate in $base_ref_candidates + do + if git rev-parse --verify --quiet "$candidate" >/dev/null + then + base_ref=$candidate + break + fi + done +} + [ -n "$base_ref" ] || die "unable to resolve commit comparison base" base_sha=$(git_or_die merge-base HEAD "$base_ref") validation_status=0 -# A commit is exempt from subject validation only when ALL three conditions hold: -# 1. It has exactly two parents (is a merge commit). -# 2. Its committer is GitHub (the trusted bot identity). -# 3. Its subject matches the canonical "Update branch" pattern. -# Ordinary contributor-created merge commits (different committer, or a -# subject that doesn't match the pattern) still go through subject validation. -github_merge_pattern="^Merge branch '[^']+' into .+" - while IFS= read -r -d '' entry do IFS=$'\x01' read -r parents committer_name committer_email subject <<< "$entry" @@ -100,14 +117,6 @@ do parent_count=0 fi - if [ "$parent_count" -eq 2 ] \ - && [ "$committer_name" = "GitHub" ] \ - && [ "$committer_email" = "noreply@github.com" ] \ - && [[ "$subject" =~ $github_merge_pattern ]] - then - continue - fi - "$validator" --subject "$subject" status=$? diff --git a/lib/mix/tasks/ci.ex b/lib/mix/tasks/ci.ex index 3b6dccc..3ddc75f 100644 --- a/lib/mix/tasks/ci.ex +++ b/lib/mix/tasks/ci.ex @@ -1,47 +1,27 @@ defmodule Mix.Tasks.Ci do - @shortdoc "Runs the complete quality gate against app/" + @shortdoc "Compatibility alias for mix precommit" @moduledoc """ #{@shortdoc}. mix ci - Runs every check `.github/workflows/ci.yaml`'s `test` job runs on a pull - request, in the same order, so a green `mix ci` locally predicts a green - CI run — and CI itself calls this task, so there's one place to fix if - either ever breaks: + Delegates to `Mix.Tasks.Precommit`, which is the canonical full-repository + quality gate. Kept for backwards compatibility with scripts and CI + configurations that call `mix ci` directly. - 1. `mix deps.get` — ensure deps are present - 2. `mix hex.audit` — reject retired or vulnerable Hex packages - 3. `mix deps.audit` — scan dependencies for known security advisories - 4. `mix format --check-formatted` — code is formatted - 5. `mix credo --strict` — static analysis (style, complexity, common bugs) - 6. `mix usage_rules.sync --check` — usage rules are in sync with deps - (catches drift introduced by a dep bump without re-running the sync; - see #79) - 7. `mix test` — all tests pass - - All steps run inside `app/`. + See `mix help precommit` for the complete step list. """ use Mix.Task - alias RepoTasks.Shell - @impl Mix.Task def run(argv) do - run(argv, &Shell.run!/3) + Mix.Tasks.Precommit.run(argv) end @doc false - def run(_argv, shell) do - shell.("mix", ["deps.get"], cd: "app") - shell.("mix", ["hex.audit"], cd: "app") - shell.("mix", ["deps.audit"], cd: "app") - shell.("mix", ["format", "--check-formatted"], cd: "app") - shell.("mix", ["credo", "--strict"], cd: "app") - shell.("mix", ["usage_rules.sync", "--check"], cd: "app") - shell.("mix", ["test"], cd: "app") - :ok + def run(argv, shell) do + Mix.Tasks.Precommit.run(argv, shell) end end diff --git a/lib/mix/tasks/precommit.ex b/lib/mix/tasks/precommit.ex index 520d0ba..c871301 100644 --- a/lib/mix/tasks/precommit.ex +++ b/lib/mix/tasks/precommit.ex @@ -14,13 +14,15 @@ defmodule Mix.Tasks.Precommit do pull request title when `PULL_REQUEST_TITLE_REQUIRED=true` 2. `ci/validate_commit_range.sh` — validate every commit since the branch diverged from its base - 3. `mix deps.get` — ensure app dependencies are present - 4. `mix hex.audit` — reject retired or vulnerable Hex packages - 5. `mix deps.audit` — scan dependencies for known security advisories - 6. `mix format --check-formatted` — check formatting - 7. `mix credo --strict` — run static analysis - 8. `mix usage_rules.sync --check` — catch usage-rule drift after dep bumps - 9. `mix test` — run the app test suite + 3. `mix format --check-formatted` — check root project formatting + 4. `mix test` — run the root project test suite (validator and task tests) + 5. `mix deps.get` — ensure app dependencies are present + 6. `mix hex.audit` — reject retired or vulnerable Hex packages + 7. `mix deps.audit` — scan dependencies for known security advisories + 8. `mix format --check-formatted` — check app formatting + 9. `mix credo --strict` — run static analysis + 10. `mix usage_rules.sync --check` — catch usage-rule drift after dep bumps + 11. `mix test` — run the app test suite Pull request metadata does not exist before a pull request is opened, so local runs skip only the title guard. GitHub Actions sets both @@ -28,7 +30,7 @@ defmodule Mix.Tasks.Precommit do a missing, empty, or non-conventional title then fails this task. Commit subjects are always validated. - All Mix quality steps run inside `app/`. + Steps 1-4 run from the repo root; steps 5-11 run inside `app/`. """ use Mix.Task @@ -44,6 +46,8 @@ defmodule Mix.Tasks.Precommit do def run([], shell) do shell.("./ci/validate_pull_request_title.sh", [], []) shell.("./ci/validate_commit_range.sh", [], []) + shell.("mix", ["format", "--check-formatted"], []) + shell.("mix", ["test"], []) shell.("mix", ["deps.get"], cd: "app") shell.("mix", ["hex.audit"], cd: "app") shell.("mix", ["deps.audit"], cd: "app") diff --git a/test/git_hooks_test.exs b/test/git_hooks_test.exs index f00a45f..4a0985c 100644 --- a/test/git_hooks_test.exs +++ b/test/git_hooks_test.exs @@ -12,13 +12,6 @@ defmodule GitHooksTest do assert {"", 0} = run(@subject_guard, ["--subject", "feat(api): add title validation"]) end - test "the shared subject guard rejects the squash title from pull request 197" do - title = "Stokowski tooling: fix Claude→Qwen routing, add lc issue comment (#197)" - - assert {output, 1} = run(@subject_guard, ["--subject", title]) - assert output =~ "Commit subject must use Conventional Commits format" - end - test "the shared subject guard rejects the squash title from pull request 196" do title = "EXT-19: isolate Burrito musl loader per user (#196)" @@ -115,6 +108,22 @@ defmodule GitHooksTest do assert {"", 0} = run(@range_guard, [], cd: worktree) end + test "the range guard fetches a missing local default base from origin" do + {worktree, _} = setup_ci_worktree!() + + # Mimic a shallow CI checkout: the remote has main, while neither a local + # main branch nor origin/main is available to resolve without a fetch. + git!(worktree, ["branch", "-m", "main", "feature"]) + git!(worktree, ["update-ref", "-d", "refs/remotes/origin/main"]) + + File.write!(Path.join(worktree, "README"), "bad commit\n", [:append]) + git!(worktree, ["add", "README"]) + git!(worktree, ["commit", "-m", "this is not conventional"]) + + assert {output, 1} = run(@range_guard, [], cd: worktree) + assert output =~ "this is not conventional" + end + test "the range guard reports all invalid subjects in a mixed commit range" do {worktree, _} = setup_ci_worktree!() @@ -136,11 +145,12 @@ defmodule GitHooksTest do refute output =~ "feat: valid commit" end - test "the range guard skips a GitHub Update-branch merge commit matching all three predicates" do + test "the range guard validates a GitHub Update-branch merge commit" do {worktree, _} = setup_ci_worktree!() add_github_merge!(worktree) - assert {"", 0} = run(@range_guard, [], cd: worktree) + assert {output, 1} = run(@range_guard, [], cd: worktree) + assert output =~ "Merge branch 'main' into feature" end test "the range guard validates when the committer name is not GitHub" do @@ -244,8 +254,8 @@ defmodule GitHooksTest do end # Creates a no-fast-forward merge commit on `main` from a throwaway `feature` - # branch. Defaults simulate GitHub's "Update branch" committer identity and - # subject so the predicate in validate_commit_range.sh matches. + # branch. Defaults simulate GitHub's "Update branch" identity and subject; + # these remain subject to validation because commit metadata is forgeable. defp add_github_merge!(worktree, opts \\ []) do committer_name = Keyword.get(opts, :committer_name, "GitHub") committer_email = Keyword.get(opts, :committer_email, "noreply@github.com") diff --git a/test/mix/tasks/ci_test.exs b/test/mix/tasks/ci_test.exs index 82e6090..78f689f 100644 --- a/test/mix/tasks/ci_test.exs +++ b/test/mix/tasks/ci_test.exs @@ -3,7 +3,7 @@ defmodule Mix.Tasks.CiTest do alias Mix.Tasks.Ci - test "runs all quality gate steps in order" do + test "delegates to mix precommit" do caller = self() shell = fn cmd, args, opts -> @@ -13,10 +13,22 @@ defmodule Mix.Tasks.CiTest do assert :ok = Ci.run([], shell) - assert_receive {:run, "mix", ["deps.get"], [cd: "app"]} - assert_receive {:run, "mix", ["hex.audit"], [cd: "app"]} - assert_receive {:run, "mix", ["format", "--check-formatted"], [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 via precommit" do + assert_raise Mix.Error, "Usage: mix precommit", fn -> + Ci.run(["unexpected"], fn _, _, _ -> :ok end) + end end end diff --git a/test/mix/tasks/precommit_test.exs b/test/mix/tasks/precommit_test.exs index c55bd49..8142be5 100644 --- a/test/mix/tasks/precommit_test.exs +++ b/test/mix/tasks/precommit_test.exs @@ -15,6 +15,8 @@ defmodule Mix.Tasks.PrecommitTest do 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"]} diff --git a/test/mix/tasks/stokowski_test.exs b/test/mix/tasks/stokowski_test.exs deleted file mode 100644 index dafea6f..0000000 --- a/test/mix/tasks/stokowski_test.exs +++ /dev/null @@ -1,40 +0,0 @@ -defmodule Mix.Tasks.StokowskiTest do - use ExUnit.Case, async: true - - test "raises when no workflow.yaml is found" do - in_tmp_dir(fn -> - assert_raise Mix.Error, ~r/No workflow\.yaml at/, fn -> - Mix.Tasks.Stokowski.run([]) - end - end) - end - - test "raises when tracker.api_key is a bare literal" do - in_tmp_dir(fn -> - File.write!("workflow.yaml", """ - tracker: - api_key: "lin_api_totally_real" - """) - - assert_raise Mix.Error, ~r/is a bare literal key/, fn -> - Mix.Tasks.Stokowski.run([]) - end - end) - end - - # Each test gets its own directory rather than sharing System.tmp_dir!() - # directly - both tests run async and would otherwise race on the same - # workflow.yaml. A cryptographic nonce avoids collisions across BEAM VM - # restarts (unlike System.unique_integer/1 which resets each run). - defp in_tmp_dir(fun) do - nonce = :crypto.strong_rand_bytes(16) |> Base.url_encode64(padding: false) - dir = Path.join(System.tmp_dir!(), "stokowski_test_#{nonce}") - File.mkdir!(dir) - - try do - File.cd!(dir, fun) - after - File.rm_rf!(dir) - end - end -end