diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0a399aad93..c8f6527c69 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -219,24 +219,17 @@ jobs: python3 scripts/test-check-dev-skill-mirrors.py python3 scripts/check-dev-skill-mirrors.py check - - name: Validate pushed commit messages - if: github.event_name == 'push' + # Dispatch admits an integration branch. Judge the commits a merge onto + # the default branch would introduce, with the same linter a push uses. + # A push still uses the before SHA, so published history is not rejudged. + - name: Validate commit messages + if: github.event_name == 'push' || github.event_name == 'workflow_dispatch' env: + EVENT_NAME: ${{ github.event_name }} BEFORE_SHA: ${{ github.event.before }} HEAD_SHA: ${{ github.sha }} - run: | - set -euo pipefail - if [ "$BEFORE_SHA" = "0000000000000000000000000000000000000000" ]; then - if git rev-parse "${HEAD_SHA}^" >/dev/null 2>&1; then - base_sha="${HEAD_SHA}^" - else - git show --no-patch --format=%B "$HEAD_SHA" | npm run lint:commit -- - exit 0 - fi - else - base_sha="$BEFORE_SHA" - fi - node scripts/lint-commit-range.mjs --repository "$PWD" "$base_sha" "$HEAD_SHA" + DEFAULT_BRANCH: ${{ github.event.repository.default_branch }} + run: scripts/lint-ci-commits.sh macos-test-partition: name: Test macOS ${{ matrix.group }} diff --git a/scripts/lint-ci-commits.sh b/scripts/lint-ci-commits.sh new file mode 100755 index 0000000000..64a149863f --- /dev/null +++ b/scripts/lint-ci-commits.sh @@ -0,0 +1,85 @@ +#!/usr/bin/env bash +# Lint the commits a CI event is admitting. +# +# A master push lints github.event.before..HEAD, which is how an integration +# merge is judged after it lands. workflow_dispatch is the admission path for +# that integration branch, so it lints the same not-yet-on-the-default-branch +# range. Already published history is not rejudged: a dispatch of the default +# branch has an empty range. +set -euo pipefail + +script_dir=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) +project_root=$(cd "${script_dir}/.." && pwd) +repository=${REPOSITORY:-$project_root} +event=${EVENT_NAME:-} +head=${HEAD_SHA:-} +zero_sha=0000000000000000000000000000000000000000 + +if [[ -z $event || -z $head ]]; then + echo "usage: EVENT_NAME= HEAD_SHA= [BEFORE_SHA=] [DEFAULT_BRANCH=] [REPOSITORY=] $0" >&2 + exit 2 +fi + +lint_range() { + local base=$1 + node "${script_dir}/lint-commit-range.mjs" --repository "$repository" "$base" "$head" +} + +lint_root_commit() { + git -C "$repository" show --no-patch --format=%B "$head" | ( + cd "$project_root" + npm run --silent lint:commit -- + ) +} + +resolve_default_branch() { + local branch=$1 + local remote_ref="refs/remotes/origin/${branch}" + if git -C "$repository" remote get-url origin >/dev/null 2>&1; then + git -C "$repository" fetch --no-tags --quiet origin \ + "+refs/heads/${branch}:${remote_ref}" >/dev/null + fi + if git -C "$repository" rev-parse --verify --quiet "$remote_ref" >/dev/null; then + echo "$remote_ref" + return 0 + fi + if git -C "$repository" rev-parse --verify --quiet "refs/heads/${branch}" >/dev/null; then + echo "refs/heads/${branch}" + return 0 + fi + echo "commit lint: default branch ${branch} is not available" >&2 + return 1 +} + +case "$event" in + push) + before=${BEFORE_SHA:-} + if [[ -z $before ]]; then + echo "commit lint: push requires BEFORE_SHA" >&2 + exit 2 + fi + if [[ $before == "$zero_sha" ]]; then + if git -C "$repository" rev-parse --verify --quiet "${head}^" >/dev/null; then + lint_range "${head}^" + else + lint_root_commit + fi + else + lint_range "$before" + fi + ;; + workflow_dispatch) + default_branch=${DEFAULT_BRANCH:-} + if [[ -z $default_branch ]]; then + echo "commit lint: workflow_dispatch requires DEFAULT_BRANCH" >&2 + exit 2 + fi + upstream=$(resolve_default_branch "$default_branch") + base=$(git -C "$repository" merge-base "$head" "$upstream") + lint_range "$base" + ;; + *) + echo "commit lint: unsupported event ${event}" >&2 + exit 2 + ;; +esac diff --git a/scripts/test-lint-commit-range.py b/scripts/test-lint-commit-range.py index aac5532fd1..f58dd0f965 100755 --- a/scripts/test-lint-commit-range.py +++ b/scripts/test-lint-commit-range.py @@ -14,6 +14,21 @@ REPOSITORY_ROOT = Path(__file__).resolve().parent.parent LINT_RANGE = REPOSITORY_ROOT / "scripts" / "lint-commit-range.mjs" +LINT_CI = REPOSITORY_ROOT / "scripts" / "lint-ci-commits.sh" + +# Subjects from the integration fold that dispatch admitted and the following +# master push rejected. Each matches the typed-header grammar and fails only +# because the header is longer than the configured maximum. +BATCH_FOLLOWUP_SUBJECTS = ( + "fix(pr-1633): warm the diagnose fixture through the shared support helper", + "fix(pr-1740): resolve git through common::git_program and sort the mod line", + "fix(pr-1617): share the exact-arguments dispatch instead of widening CaptureTransport", +) +HYGIENIC_FOLLOWUP_MESSAGES = ( + "fix(pr-1633): warm the diagnose fixture through shared support\n\nhelper", + "fix(pr-1740): resolve git through common::git_program and sort mods\n\nthe mod line", + "fix(pr-1617): share exact-argument dispatch without widening transport\n\ninstead of widening CaptureTransport", +) def run( @@ -155,6 +170,134 @@ def test_node_startup_count_is_constant_for_a_large_range(self) -> None: f"elapsed_ms={elapsed_ms}" ) + def lint_ci( + self, + *, + event: str, + head: str, + before: str | None = None, + default_branch: str | None = None, + ) -> subprocess.CompletedProcess[str]: + environment = os.environ.copy() + environment.update( + { + "EVENT_NAME": event, + "HEAD_SHA": head, + "REPOSITORY": str(self.root), + } + ) + if before is not None: + environment["BEFORE_SHA"] = before + if default_branch is not None: + environment["DEFAULT_BRANCH"] = default_branch + return run( + ["bash", str(LINT_CI)], + cwd=self.root, + env=environment, + check=False, + ) + + def test_dispatch_rejects_batch_followup_headers_over_the_maximum(self) -> None: + base = self.commit("chore(test): establish fixture base") + master = self.commit("fix(test): keep the default branch valid", base) + run(["git", "branch", "master", master], cwd=self.root) + head = master + followups = [] + for subject in BATCH_FOLLOWUP_SUBJECTS: + self.assertGreater(len(subject), 72) + head = self.commit(subject, head) + followups.append(head) + + result = self.lint_ci( + event="workflow_dispatch", + head=head, + default_branch="master", + ) + output = result.stdout + result.stderr + + self.assertNotEqual(result.returncode, 0, output) + for sha, subject in zip(followups, BATCH_FOLLOWUP_SUBJECTS, strict=True): + self.assertIn(sha, output) + self.assertIn(subject, output) + self.assertIn("header-max-length", output) + self.assertNotIn(master, output) + + def test_dispatch_accepts_the_same_followups_once_the_header_fits(self) -> None: + base = self.commit("chore(test): establish fixture base") + master = self.commit("fix(test): keep the default branch valid", base) + run(["git", "branch", "master", master], cwd=self.root) + head = master + for message in HYGIENIC_FOLLOWUP_MESSAGES: + header = message.split("\n", 1)[0] + self.assertLessEqual(len(header), 72) + self.assertTrue(header.startswith("fix(pr-")) + head = self.commit(message, head) + + result = self.lint_ci( + event="workflow_dispatch", + head=head, + default_branch="master", + ) + + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + + def test_dispatch_of_the_default_branch_does_not_rejudge_published_history(self) -> None: + base = self.commit("chore(test): establish fixture base") + published = self.commit(BATCH_FOLLOWUP_SUBJECTS[0], base) + master = self.commit("fix(test): keep the default branch valid", published) + run(["git", "branch", "master", master], cwd=self.root) + + result = self.lint_ci( + event="workflow_dispatch", + head=master, + default_branch="master", + ) + + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + self.assertNotIn(published, result.stdout + result.stderr) + + def test_push_still_lints_the_before_sha_range(self) -> None: + base = self.commit("chore(test): establish fixture base") + head = self.commit(BATCH_FOLLOWUP_SUBJECTS[2], base) + + result = self.lint_ci(event="push", head=head, before=base) + output = result.stdout + result.stderr + + self.assertNotEqual(result.returncode, 0, output) + self.assertIn(head, output) + self.assertIn("header-max-length", output) + + def test_push_of_a_root_commit_lints_that_message(self) -> None: + valid = self.commit("chore(test): establish fixture base") + invalid = self.commit("not a conventional header") + + valid_result = self.lint_ci( + event="push", + head=valid, + before="0000000000000000000000000000000000000000", + ) + invalid_result = self.lint_ci( + event="push", + head=invalid, + before="0000000000000000000000000000000000000000", + ) + + self.assertEqual( + valid_result.returncode, + 0, + valid_result.stdout + valid_result.stderr, + ) + self.assertNotEqual(invalid_result.returncode, 0) + self.assertIn("type-empty", invalid_result.stdout + invalid_result.stderr) + + def test_unsupported_event_is_rejected(self) -> None: + head = self.commit("chore(test): establish fixture base") + + result = self.lint_ci(event="schedule", head=head) + + self.assertEqual(result.returncode, 2) + self.assertIn("unsupported event", result.stderr) + if __name__ == "__main__": unittest.main()