From f0d0ba0137bd21f3dc226f91d8a8c0c7cf1019c6 Mon Sep 17 00:00:00 2001 From: bougyman's bot Date: Fri, 4 Sep 2026 12:01:34 -0400 Subject: [PATCH 1/3] ci: gate Burrito regression and consolidate pre-push validation Refs: EXT-17, EXT-30, EXT-31, EXT-32, EXT-33 See linear for details --- AGENTS.md | 2 + documents/phase-14-plan.adoc | 543 +++++++++++++++++++++++++++++++++++ 2 files changed, 545 insertions(+) create mode 100644 documents/phase-14-plan.adoc diff --git a/AGENTS.md b/AGENTS.md index 8679014..61b202a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -16,6 +16,8 @@ and best practices for agents to follow. - Phase 11 Plan: documents/phase-11-plan.adoc - Phase 12 Plan: documents/phase-12-plan.adoc - Phase 13 Plan: documents/phase-13-plan.adoc +- Phase 14 Plan: documents/phase-14-plan.adoc +- Burrito distribution decision: documents/burrito-decision.adoc ## Project Structure diff --git a/documents/phase-14-plan.adoc b/documents/phase-14-plan.adoc new file mode 100644 index 0000000..37a43dc --- /dev/null +++ b/documents/phase-14-plan.adoc @@ -0,0 +1,543 @@ += {my-title} +Tj Vanderpoel (bougyman) +:revdate: Sep 04, 2026 +:my-title: Phase 14 plan: end-to-end Conventional Commits and pull request title enforcement +:icons: font +:env-github: +ifdef::env-github[] +:tip-caption: :bulb: +:note-caption: :information_source: +:important-caption: :heavy_exclamation_mark: +:caution-caption: :fire: +:warning-caption: :warning: +endif::[] +:toc: + +[NOTE] +==== +This plan was reconstructed on 2026-09-03 after the prompt-session database +that contained the original planning discussion became malformed. The user's +confirmed goal is to guarantee both Conventional Commit subjects and +Conventional Commit-formatted pull request titles. + +Nine draft files from that session survived untracked on branch `recover-197`, +at `f6a5e37` (then the tip of `main`). They are inventoried below. They are +implementation evidence, not a complete or approved change set. In particular, +none of the tracked callers, workflows, documentation, or old enforcement +files had been changed, and `git diff --cached` was empty when this plan was +written. The recovered validator drafts were also placed under `git-hooks/`, +but that placement is not the intended architecture: checks are first-class +standalone programs under `ci/`, while Git hooks are only lifecycle adapters. + +On 2026-09-04, `main` was pulled through `df3c769` (v2.7.0) while all nine +drafts survived untracked. That pull landed one related guidance change, +`a34a1b7` / #202, which teaches the Stokowski implementation prompt to use a +Conventional Commit-shaped PR title. It did not land any validator, hook, task, +workflow, or branch-protection enforcement from this phase. +==== + +== Goal + +Make it impossible for a commit with a non-conforming subject to enter the +protected `main` branch through the normal repository workflows, and make a +non-conforming pull request title a required-check failure before merge. + +The local hooks provide immediate feedback, while GitHub Actions plus branch +protection provide the authoritative enforcement boundary. Local hooks alone +cannot be the guarantee because Git permits `--no-verify`, hooks are not active +until `mix setup` has been run, and a pre-push hook can be bypassed by another +client entirely. + +The motivating regression is pull request #197. Its individual branch commits +were checked, but its title was: + +---- +Stokowski tooling: fix Claude→Qwen routing, add lc issue comment +---- + +GitHub used that title for the squash commit and appended `(#197)`, producing +the non-conventional `main` commit `f6a5e37`. The old CI gate only inspected the +commits already on the PR branch; it never inspected the title that would +become the integration commit's subject. + +The same gap produced a second regression immediately afterward: pull request +#196 became `8207063 EXT-19: add lc issue view command with v alias (#196)` on +`main`. Commit `a34a1b7` then corrected the agent prompt that had proposed that +issue-identifier-first title shape. Subsequent observed titles through +`df3c769` conform, but guidance and voluntary compliance are not enforcement. + +This phase closes both sides of that gap: + +* every commit subject introduced by a PR or direct protected-branch update; +* the current pull request title, including after it is edited. + +== Scope of the rule + +The machine-enforced contract is the first line (the Git commit *subject*, or +the whole single-line PR title): + +---- +[(scope)][!]: +---- + +Allowed types remain the project list in `app/usage-rules.md`: +`fix`, `feat`, `perf`, `observability`/`obs`, +`config`/`configuration`, `chore`, `ci`, `docs`, `refactor`, +`sec`/`security`, `style`, `cleanup`, and `test`. + +Commit bodies and footers remain allowed and are not syntax-linted beyond +selecting the first non-comment, non-blank line from a commit-message file. +The existing imperative-present-tense guidance remains a human review rule; +trying to classify English grammar in a shell regex would create false +guarantees rather than a useful gate. + +== Reconstructed decisions + +1. *One shared subject validator.* + `ci/validate_conventional_subject.sh` owns the format and allowed-type + regex as an independently executable check. It accepts either a + commit-message file (for `commit-msg`) or `--subject ` (for + commit-range and PR-title checks). A context value such as + `CONVENTIONAL_SUBJECT_KIND="Pull request title"` changes diagnostics, not + validation semantics. There must not be separate regex copies for commits + and titles. +2. *PR titles follow the identical format.* + `ci/validate_pull_request_title.sh` requires and validates + `PULL_REQUEST_TITLE` only when `PULL_REQUEST_TITLE_REQUIRED=true`; non-PR + events skip it explicitly. Missing, empty, malformed, or trailing-whitespace + titles fail when required. The environment switch itself accepts only + `true` or `false`, so a workflow wiring typo fails closed. +3. *Every introduced commit is checked as a range.* + `ci/validate_commit_range.sh` validates every subject after the merge + base between `HEAD` and its base. Local default-base precedence is + `origin/main`, then `main`, specifically so a direct push from local `main` + is compared with the pre-push remote instead of with itself. `BASE_REF` + accepts an exact 40-character SHA so a GitHub push event can use + `github.event.before` and validate the new `main` commit rather than an empty + `main..main` range. +4. *No exception for non-conventional merge subjects.* + Bare `Merge branch ...` subjects fail like any other non-conventional + subject. The old validator exempted GitHub's update-branch merge commits, + but that contradicts the goal that every commit subject conform. A + contributor can rebase, or merge locally with a subject such as + `chore: Merge branch 'main' into my-branch`. Documentation must call out + this consequence because GitHub's Update branch button can create a subject + that the gate deliberately rejects. +5. *Local hooks are thin lifecycle adapters, not checks.* + `git-hooks/commit-msg` only resolves the repository and `exec`s + `ci/validate_conventional_subject.sh` with Git's message-file argument. + `git-hooks/pre-push` only resolves the repository and `exec`s + `ci/validate_push_refs.sh`, preserving Git's arguments and stdin. The latter + first-class check validates the commits actually introduced by every ref + update, ignores deletions, and avoids the inaccurate claim that checking + only the currently checked-out `HEAD` covers every possible `git push` + form. New branches fall back to their merge base with `origin/main`; + existing remote branches use the outgoing update range. The remote CI gate + remains authoritative even after the local adapter is made exact. +6. *All custom checks are first-class programs under `ci/`.* + The canonical check programs are + `ci/validate_conventional_subject.sh`, + `ci/validate_pull_request_title.sh`, + `ci/validate_commit_range.sh`, and + `ci/validate_push_refs.sh`. Each has its own usable command-line/environment + contract, diagnostics, exit status, executable mode, and direct tests; none + requires invocation through a Git hook, Mix task, or workflow. Existing + `ci/validate_conventional_commit.sh` and `ci/conventional_commits.sh` + responsibilities are consolidated into those canonical programs, not moved + out of `ci/`. If an old filename must remain temporarily for compatibility, + it is only an `exec` wrapper and contains no validation logic. + + `git-hooks/` contains only Git's extensionless `commit-msg` and `pre-push` + adapters. Mix tasks and GitHub Actions call the relevant `ci/*.sh` programs + directly. `mix git_hooks` configures `core.hooksPath=git-hooks`, rejects + unexpected arguments, and retains an injectable runner for unit tests. + There must be one live implementation of each check after consolidation. +7. *`mix precommit` becomes the canonical full-repository gate.* + It runs cheap metadata checks first, then root-project checks, then app + checks. Its complete order is: + + . `ci/validate_pull_request_title.sh`; + . `ci/validate_commit_range.sh`; + . root `mix format --check-formatted`; + . root `mix test`; + . app `mix deps.get`; + . app `mix hex.audit`; + . app `mix deps.audit`; + . app `mix format --check-formatted`; + . app `mix credo --strict`; + . app `mix usage_rules.sync --check`; + . app `mix test`. + + The recovered draft omitted both root checks, which would leave the task's + own implementation and validator tests outside the gate that claimed to + cover the repository. That omission must be fixed. The shipped `mix ci` + command remains as a thin compatibility alias to `mix precommit`; it must + not retain a second independently maintained command list. +8. *One required CI result eventually owns the whole gate.* + `.github/workflows/ci.yaml`'s existing `Test` job calls `mix precommit` from + the repository root after a full-history checkout of the real PR head or + event SHA. The separate `Validate Commit Subjects` job becomes redundant + only after branch protection has been migrated safely; see the no-gap + sequence below. +9. *PR title edits retrigger the required check.* + The direct `pull_request` trigger explicitly includes `opened`, + `synchronize`, `reopened`, and `edited`. Without `edited`, a title could be + valid when CI passed and changed to an invalid value afterward without a new + required run. The title enters the shell only as a quoted environment value, + never as generated shell source. +10. *Post-merge pushes validate the integration commit too.* + `main.yaml` must stop blanket-skipping commit validation. On a `push` to + `main`, the reusable CI workflow checks `github.event.before..github.sha`. + This is defense in depth for the exact failure seen in #197 and also covers + an authorized direct update to `main`. Non-PR events skip only the PR-title + check, not commit subjects. +11. *All merge methods may remain enabled only after merge-title settings are + aligned with the guarantee.* + As re-verified on 2026-09-04, the repository allows merge, squash, and + rebase merges, but `merge_commit_title` is `MERGE_MESSAGE` and + `squash_merge_commit_title` is `COMMIT_OR_PR_TITLE`. Squash remains safe + once both inputs are enforced: GitHub chooses either a validated individual + commit subject or the validated PR title. Rebase relies on the validated + individual subjects. Merge commits are not safe while GitHub can generate + a `Merge pull request ...`-style subject, so this phase changes + `merge_commit_title` to `PR_TITLE` before declaring enforcement complete. + Disabling merge commits is the acceptable fallback if GitHub cannot retain + that setting, but silently leaving `MERGE_MESSAGE` is not. +12. *Tests that create Git repositories must survive interrupted runs.* + A prior crashed session left `/tmp/linear_cli_git_test_*` directories. + `System.unique_integer/1` restarts in a new BEAM VM, so later runs collided + with those directories and reported `nothing to commit`. Both existing app + Git tests and new root hook tests must create directories with cross-process + uniqueness (or atomically retry on `:eexist`) and clean up on exit. A stale + directory from a killed run must not make `mix precommit` red. +13. *Enforcement is prospective; do not rewrite published `main` history.* + Commits `f6a5e37` (#197) and `8207063` (#196) remain regression fixtures; + `8207063` is the last known bad integration subject as of 2026-09-04. Range + selection must exclude commits already present on the chosen base, so new + branches from current `main` are not permanently blocked by either one. + Every commit added after this phase lands must conform. + +== GitHub Actions event contract + +The workflow wiring must be explicit enough that each event validates the +correct revision and does not accidentally inspect GitHub's synthetic PR merge +commit. + +[cols="2,1,2,3", options="header"] +|=== +| Event +| Require PR title +| Commit comparison base +| Revision checked out + +| `pull_request`: `opened`, `synchronize`, `reopened`, `edited` +| yes +| exact `pull_request.base.sha` (or an equivalently fetched base ref) +| exact `pull_request.head.sha`, with full history + +| `push` to `main` through `main.yaml` / `workflow_call` +| no +| exact `github.event.before` +| `github.sha`, with full history + +| `pull_request: closed` through `main.yaml` +| no +| `pull_request.base.sha` when validating the merged result; an explicit + documented skip is acceptable only for an unmerged close whose range is + empty +| merged/default-branch `github.sha`, not the deleted PR head + +| `workflow_dispatch` +| no +| explicit input if supplied, otherwise `origin/main` +| the selected `github.sha`, with full history +|=== + +For a PR event, CI exports: + +* `PULL_REQUEST_TITLE_REQUIRED=true`; +* `PULL_REQUEST_TITLE` from `github.event.pull_request.title`; +* `BASE_REF` as the exact base SHA. + +For all other events, `PULL_REQUEST_TITLE_REQUIRED=false`. A reusable-workflow +input may be used where GitHub expression scoping makes the event values +awkward, but the behavior in the table is the contract; a generic +`skip_commit_validation: true` from `main.yaml` is not. + +== Safe branch-protection migration + +As re-verified on 2026-09-04, `main` protection is strict, applies to admins, and +requires two GitHub Actions checks: `Test` and `Validate Commit Subjects`. +Removing the latter job in the same PR without coordinating protection would +leave that PR permanently waiting for a check that can no longer be emitted. + +Use this no-gap migration: + +1. Make `Test` run the complete `mix precommit` gate, including commit subjects + and PR title, while temporarily retaining the existing required + `Validate Commit Subjects` job as a compatibility check. +2. Prove on the implementation PR that an invalid title fails `Test`, a fixed + title reruns and passes, and an invalid commit subject fails. +3. Merge with both currently-required checks green. +4. Change branch protection to require `Test` and no longer require + `Validate Commit Subjects`, preserving strictness, the GitHub Actions app + binding, admin enforcement, force-push prohibition, and deletion + prohibition. +5. Change `merge_commit_title` from `MERGE_MESSAGE` to `PR_TITLE` (or disable + merge commits if that setting cannot be retained), while preserving the + safe squash/rebase configuration. +6. Remove the compatibility job in a follow-up change. Do not reverse steps 4 + and 6. + +If branch protection is deliberately updated immediately before the +implementation PR merges instead, record that operation and verify there is no +window in which `Test` is still the old app-only gate. The compatibility-job +sequence is preferred because it has no enforcement gap. + +== Recovery status through 2026-09-04 + +The following untracked drafts survived. Preserve and rework their useful +contents, but do not add every draft at its recovered path: + +* `git-hooks/commit-msg`; +* `git-hooks/pre-push`; +* `git-hooks/validate-conventional-subject` (relocate its implementation to + `ci/validate_conventional_subject.sh`); +* `git-hooks/validate-commit-range` (relocate its implementation to + `ci/validate_commit_range.sh`); +* `git-hooks/validate-pull-request-title` (relocate its implementation to + `ci/validate_pull_request_title.sh`); +* `lib/mix/tasks/precommit.ex`; +* `test/git_hooks_test.exs`; +* `test/mix/tasks/git_hooks_test.exs`; +* `test/mix/tasks/precommit_test.exs`. + +The missing `ci/validate_push_refs.sh` must be extracted from the recovered +`pre-push` behavior. After consolidation, only `commit-msg` and `pre-push` +remain under `git-hooks/`; the recovered validators do not. + +What is already useful in those drafts: + +* one shared commit/PR-title regex and contextual diagnostics; +* the #197 regression subject (the #196/`8207063` fixture still needs adding); +* required/optional PR-title behavior; +* `origin/main` precedence for local-main validation; +* exact-SHA `BASE_REF` support for push events; +* metadata-first `mix precommit` orchestration; +* executable file modes and clean shell syntax. + +What is known incomplete: + +* `Mix.Tasks.GitHooks` is still tracked in its old form, so the two recovered + `GitHooks` tests fail with undefined `run/2`; +* `.github/workflows/ci.yaml` still calls `mix ci` and the old scripts, never + exports the PR title, and does not request `pull_request.edited`; +* `.github/workflows/main.yaml` still passes + `skip_commit_validation: true` for every event; +* `mix precommit` runs only the app checks and therefore omits the root tests + that exercise the new enforcement code; +* the old `githooks/` adapters still exist, while the recovered validators are + misplaced under `git-hooks/` and overlap the tracked checks under `ci/`; +* `AGENTS.md`, `app/usage-rules.md`, and `Readme.adoc` still describe or invoke + the old paths/commands; +* Stokowski's implementation prompt now proposes a conventional PR title after + `a34a1b7`, but still invokes `mix ci` instead of the canonical + `mix precommit` and does not itself enforce the title; +* the recovered `pre-push` wrapper checks `HEAD`, not every ref supplied by + Git's pre-push protocol; +* required-status-check migration has not happened. + +Verification performed during recovery: + +* new shell files pass `bash -n` / `sh -n`; +* root formatting passes; +* root tests are 20/22, with only the two expected missing-`GitHooks.run/2` + failures; +* the app suite was 352/352 on the original recovery base when run with an + isolated temporary directory; +* the draft `mix precommit` successfully reaches every app gate, including + both dependency audits, formatting, Credo, usage-rules sync, and tests. + +== Building blocks + +=== First-class checks and hook adapters + +* Finish direct `ci/validate_conventional_subject.sh` coverage for every + allowed header shape, scoped and breaking forms, invalid types, empty input, + trailing whitespace, message-file comments/blanks, and merge subjects. +* Finish direct `ci/validate_pull_request_title.sh` fail-closed behavior for + missing/empty title values and invalid required-switch values. +* Extend direct `ci/validate_commit_range.sh` tests to cover a feature branch, + local `main` ahead of `origin/main`, an exact base SHA, a range containing + both valid and invalid subjects, and an empty range. +* Implement and directly test `ci/validate_push_refs.sh` against Git's + pre-push stdin protocol, covering new branches, existing branches, multiple + refs, and deletion records in isolated test repositories. +* Keep `git-hooks/commit-msg` and `git-hooks/pre-push` trivial: each preserves + Git's inputs and `exec`s the corresponding `ci/` check. +* Update `Mix.Tasks.GitHooks`, its moduledoc, and `mix setup` expectations for + `git-hooks/` and argument rejection. + +=== Full validation task + +* Complete `Mix.Tasks.Precommit` with the root checks and strict no-argument + contract. +* Invoke `ci/validate_pull_request_title.sh` and + `ci/validate_commit_range.sh` directly; `mix precommit` must not route checks + through Git hook entrypoints. +* Keep the injectable `RepoTasks.Shell` boundary so ordering and working + directories are unit-testable without actually invoking the full suite. +* Convert `Mix.Tasks.Ci` into a thin compatibility alias to + `Mix.Tasks.Precommit`. No duplicate list of checks may survive. +* Retain `.ai/prompts/implement.md`'s conventional PR-title guidance landed by + `a34a1b7`, update its quality command from `mix ci` to `mix precommit`, and + make clear that scope is optional. Keep the Linear identifier in the + description or after the conventional prefix. + +=== Workflows + +* Update `ci.yaml` to the event contract above, including a full checkout, + real PR head, exact base SHA, PR-title environment, and `edited` activity. +* Call `mix precommit` from the root `Test` job. +* Have any temporary compatibility job call the appropriate canonical + `ci/*.sh` check directly; workflows do not call Git hook adapters. +* Update `main.yaml` so a push validates from `github.event.before`; remove the + blanket skip while handling closed/unmerged PR events explicitly. +* Keep the Burrito regression job's existing scope and behavior. This phase + changes its dependency/gating context only if required by the consolidated + workflow; it does not alter release packaging. +* Exercise workflow expression values with titles containing quotes, + backticks, dollar signs, and Unicode to prove the title is data rather than + executable shell text. + +=== Documentation and repository structure + +* `AGENTS.md`: retain this Phase 14 index entry; replace the `githooks/` + structural entry with `git-hooks/` and describe that directory as adapters + only; describe `ci/` as the home of first-class standalone checks as well as + build, packaging, publishing, and release scripts; update the Standards + paths. +* `app/usage-rules.md`: document local commit-time/pre-push checks, required CI + validation of every PR commit and current PR title, title-edit reruns, the + no-exception merge-subject policy, and the authoritative role of protected + CI. +* `Readme.adoc`: make Conventional Commits apply explicitly to both commit + subjects and PR titles; present `mix precommit` as the full development + command while retaining focused test commands where useful. +* `.ai/prompts/implement.md`: use `mix precommit` and a valid PR title as + described above. Search all other prompts for `mix ci`, `gh pr create`, and + issue-identifier-first title examples during implementation. +* Do not edit the historical Phase 8 plan to pretend it anticipated this + change. Phase 8 accurately records the original commit-range-only CI design; + this document records the correction prompted by #197. +* No Ash resources, actions, code interfaces, associations, or shared domain + helpers change, so `documents/ash-domain-erd.adoc` is untouched. + +== Tests and acceptance criteria + +Automated acceptance: + +* A normal `feat(api): add title validation` commit subject passes everywhere. +* PR #197's title fails both the shared validator and the PR-title wrapper. +* PR #196's `EXT-19: ...` title / resulting `8207063` subject fails too. +* A valid required PR title passes; invalid, missing, empty, or + trailing-whitespace titles fail. +* Editing a valid open PR title to an invalid title creates a new required + failing `Test` run; editing it back creates a passing run without needing a + new commit. +* A PR containing any invalid commit subject fails even when all other commits + and the PR title are valid. +* A `push` event validates the exact commits after `github.event.before`, + including the new squash/merge integration commit. +* Local `main` ahead of `origin/main` is not treated as an empty range. +* Every custom check under `ci/` is executable and testable directly, with a + documented input and exit-status contract. +* `commit-msg` and `pre-push` contain no validation logic and delegate to the + appropriate `ci/` checks while preserving arguments and stdin; bypassing + `commit-msg` and then pushing is caught by `pre-push` in normal and multi-ref + cases. +* `mix git_hooks` writes `core.hooksPath=git-hooks`; `mix setup` still invokes + it; both tasks reject unsupported arguments as documented. +* `mix precommit` runs root and app formatting/tests plus the app audit/static + checks in the documented order and exits on the first failure. +* Root Git-hook tests and app Git tests pass even when stale similarly-named + directories already exist under the system temporary directory. +* No duplicate check implementations survive. Active callers use the + canonical `ci/*.sh` programs; any retained legacy `ci/` filename is a thin + compatibility wrapper, and `git-hooks/` contains only `commit-msg` and + `pre-push`. +* `mix precommit` is green from a clean checkout. +* Repository settings report `merge_commit_title=PR_TITLE`, or merge commits + are disabled; `MERGE_MESSAGE` is not left enabled as an integration-subject + source. + +Live GitHub acceptance before declaring the phase complete: + +1. Open or use a test PR whose commits are conventional but whose title is not; + observe required `Test` failure attributed clearly to the title. +2. Edit only the title to a conforming value; observe an `edited`-event rerun + and green required check. +3. Push an invalid commit subject to a disposable test branch; observe failure + even with a valid title, then replace or reword it using a fresh/disposable + branch so the verification does not introduce force-pushing as normal + project practice. +4. Merge a conforming test PR with the project's normal merge method; confirm + the resulting `main` subject remains conventional and the `push` validation + checks it from the event's `before` SHA. +5. Complete the required-status-check migration and confirm a fresh PR is + merge-blocked when `Test` fails and mergeable when it passes. + +== Issue decomposition + +Create and implement these as separate Linear issues in dependency order. The +titles below are stable so a later session can map issue IDs back to this plan +even if links or local state are unavailable. + +1. *EXT-29 — Harden Conventional Commit validators and Git test isolation.* + Finish the shared subject/title/range behavior, add both #197 and #196 + regression fixtures, make pre-push validation cover the actual refs supplied + on stdin, and make root/app temporary Git repositories collision-proof after + interrupted runs. This is the foundation for every later issue. +2. *EXT-30 — Consolidate repository Git hooks under `git-hooks/`.* + Update `Mix.Tasks.GitHooks`, migrate the two lifecycle adapters from + `githooks/` to `git-hooks/`, keep all check implementations under `ci/`, + update structural/local-hook documentation, and leave the root hook suite + green. Depends on EXT-29. +3. *EXT-31 — Make `mix precommit` the canonical repository quality gate.* + Add root format/tests to the recovered task, retain every app quality step, + turn `mix ci` into a thin alias, and update Stokowski/development guidance. + Depends on EXT-29; it can proceed in parallel with EXT-30 once shared + validator paths are settled. +4. *EXT-32 — Enforce Conventional Commit subjects and PR titles in GitHub + Actions.* + Implement the event table, title-edit trigger, safe environment handling, + full-history real-head checkout, exact push base, and post-merge validation. + Keep the old required job temporarily for the no-gap migration. Depends on + EXT-29, EXT-30, and EXT-31. +5. *EXT-33 — Migrate repository settings and verify Phase 14 end to end.* + Change the required-check set only after the consolidated `Test` check is + live, set merge commit titles to `PR_TITLE` (or disable merge commits), run + the live invalid-title/title-edit/invalid-commit cases, remove the temporary + compatibility job, and verify final protection settings. Depends on EXT-32. + +== Sequencing + +1. *EXT-29*: harden the temporary Git-repository helpers, finish + shared-validator/range tests, and make the recovered root suite green + without workflow changes. +2. *EXT-30*: complete `Mix.Tasks.GitHooks`, migrate only the two adapters from + `githooks/` to `git-hooks/`, point them at the canonical `ci/` checks, and + update focused docs/tests. +3. *EXT-31*: complete `mix precommit` with root and app checks; reduce `mix ci` + to a thin alias; update Stokowski and development commands. This may run in + parallel with EXT-30 after EXT-29. +4. *EXT-32*: wire `ci.yaml` and `main.yaml` to the event contract while + retaining the temporary `Validate Commit Subjects` compatibility job. +5. *EXT-33*: run all local/live checks, merge with both existing required + checks green, migrate branch protection and merge-title settings, remove the + obsolete compatibility job in the prescribed order, and verify a fresh PR + against the final one-check configuration. + +Standard workflow from here: take the next unblocked Linear issue, branch from +it, commit, and open a PR with a Conventional Commits-formatted title. Do not +commit directly to `main`. Conventional commit type: `ci`. From b037ed005d331df040fcc60374cd3f6091f0bdec Mon Sep 17 00:00:00 2001 From: bougyman's bot Date: Fri, 4 Sep 2026 12:08:09 -0400 Subject: [PATCH 2/3] ci: consolidate hooks and run pre-push quality checks Refs: EXT-17, EXT-30, EXT-31, EXT-32, EXT-33 See linear for details --- AGENTS.md | 4 +- app/usage-rules.md | 18 ++-- {githooks => git-hooks}/commit-msg | 4 +- git-hooks/pre-push | 10 +++ git-hooks/validate-commit-range | 94 ++++++++++++++++++++ git-hooks/validate-conventional-subject | 86 +++++++++++++++++++ git-hooks/validate-pull-request-title | 36 ++++++++ githooks/pre-push | 9 -- lib/mix/tasks/git_hooks.ex | 29 ++++--- lib/mix/tasks/precommit.ex | 60 +++++++++++++ test/git_hooks_test.exs | 109 ++++++++++++++++++++++++ test/mix/tasks/git_hooks_test.exs | 24 ++++++ test/mix/tasks/precommit_test.exs | 32 +++++++ vendor/stokowski | 2 +- 14 files changed, 483 insertions(+), 34 deletions(-) rename {githooks => git-hooks}/commit-msg (64%) create mode 100755 git-hooks/pre-push create mode 100755 git-hooks/validate-commit-range create mode 100755 git-hooks/validate-conventional-subject create mode 100755 git-hooks/validate-pull-request-title delete mode 100755 githooks/pre-push create mode 100644 lib/mix/tasks/precommit.ex create mode 100644 test/git_hooks_test.exs create mode 100644 test/mix/tasks/git_hooks_test.exs create mode 100644 test/mix/tasks/precommit_test.exs diff --git a/AGENTS.md b/AGENTS.md index 61b202a..02462d4 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -59,7 +59,7 @@ Supporting directories (not Mix projects): `lproj`). Each one just calls `exec lc ...`. - `ci/` — Shell scripts for CI and release: conventional-commit enforcement, container build/publish, Homebrew formula bump. -- `githooks/` — `commit-msg` and `pre-push` hooks; installed by `mix setup`. +- `git-hooks/` — `commit-msg` and `pre-push` hooks; installed by `mix setup`. - `oci/` — `Containerfile` for the published container image. - `schema/` — The Linear GraphQL schema (`LinearAPI.graphql`), kept for reference. - `cinemas/` — Terminal session recordings (`.cinema.gif`) embedded in Readme.adoc. @@ -129,7 +129,7 @@ not need a structural-doc update. ## Standards - Conventional Commits: app/usage-rules.md — enforced by the `commit-msg` - and `pre-push` hooks at `githooks/` (run `mix setup` once per clone to + and `pre-push` hooks at `git-hooks/` (run `mix setup` once per clone to activate them). - Dogfooding & running `lc` locally (no MCP, no escript): app/usage-rules.md - Accessibility: app/usage-rules.md — the actual reason this project diff --git a/app/usage-rules.md b/app/usage-rules.md index 4989a4f..6a814e5 100644 --- a/app/usage-rules.md +++ b/app/usage-rules.md @@ -7,15 +7,15 @@ - Use the imperative, present tense in the description (`add`, not `added`/`adds`). - Mark breaking changes with `!` before the colon (e.g. `feat!: ...`). - Bare `Merge branch ...` subjects are rejected — reword as `chore: Merge branch ...`. -- Enforced locally by the `commit-msg` hook at `githooks/commit-msg` (each - commit's own subject, via `ci/validate_conventional_commit.sh`) and the - `pre-push` hook at `githooks/pre-push` (every commit about to be pushed, - via `ci/conventional_commits.sh` - catches anything that slipped past - `commit-msg`, e.g. a commit made before the hooks were installed) — run - `mix setup` once per clone to activate both. -- Enforced in CI across a whole PR's commit range by the same - `ci/conventional_commits.sh` the `pre-push` hook uses (skips GitHub's own - auto-generated update-branch merge commits). +- Enforced locally by the `commit-msg` hook at `git-hooks/commit-msg` (each + commit's own subject, via its shared subject validator) and the `pre-push` + hook at `git-hooks/pre-push` (`mix precommit`, which validates + every commit about to be pushed and runs the dependency security audits, + formatting, static analysis, and tests) — run `mix setup` once per clone + to activate both. +- Enforced in CI across a whole PR's commit range by + `ci/conventional_commits.sh` (skips GitHub's own auto-generated + update-branch merge commits). ## Dogfooding: use `lc`, not a Linear MCP server or skill diff --git a/githooks/commit-msg b/git-hooks/commit-msg similarity index 64% rename from githooks/commit-msg rename to git-hooks/commit-msg index 6de8de1..d17782e 100755 --- a/githooks/commit-msg +++ b/git-hooks/commit-msg @@ -1,7 +1,7 @@ #!/bin/sh # Enforces Conventional Commits on the commit subject line. # See app/usage-rules.md for the rule. Activate with: -# git config core.hooksPath githooks +# mix git_hooks repo_top=$(git rev-parse --show-toplevel) || exit 1 -exec "$repo_top/ci/validate_conventional_commit.sh" "$1" +exec "$repo_top/git-hooks/validate-conventional-subject" "$1" diff --git a/git-hooks/pre-push b/git-hooks/pre-push new file mode 100755 index 0000000..ce2d199 --- /dev/null +++ b/git-hooks/pre-push @@ -0,0 +1,10 @@ +#!/bin/sh +# Runs the same complete validation gate as CI before every push, including +# Hex's retired/vulnerable-package audit. It also validates every commit since +# the branch diverged from its base (catching a commit made before the hooks +# were installed, an amend, a rebase, etc.). Activate with: +# mix git_hooks + +repo_top=$(git rev-parse --show-toplevel) || exit 1 +cd "$repo_top" || exit 1 +exec mix precommit diff --git a/git-hooks/validate-commit-range b/git-hooks/validate-commit-range new file mode 100755 index 0000000..ebb9048 --- /dev/null +++ b/git-hooks/validate-commit-range @@ -0,0 +1,94 @@ +#!/usr/bin/env bash + +usage() { + cat <<-EOT + Validate Conventional Commits subjects in the current branch range. + + 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. +EOT +} + +die() { + printf "ERROR: %s\n\n" "$*" >&2 + usage >&2 + exit 1 +} + +git_or_die() { + output=$(git "$@" 2>&1) + status=$? + + if [ "$status" -ne 0 ] + then + printf "%s\n" "$output" >&2 + die "git $* failed" + fi + + printf "%s" "$output" +} + +repo_top=$(git_or_die rev-parse --show-toplevel) +validator="$repo_top/git-hooks/validate-conventional-subject" + +[ -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_ref= + +if [[ "$base_input" =~ ^[0-9a-fA-F]{40}$ ]] +then + # GitHub push events provide the exact pre-push commit. A full checkout + # 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" ] + then + git_or_die fetch --no-tags origin "$base_name:refs/remotes/origin/$base_name" >/dev/null + fi + + # 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 +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 + +while IFS= read -r -d '' subject +do + "$validator" --subject "$subject" + status=$? + + if [ "$status" -ne 0 ] + then + validation_status=$status + fi +done < <(git log -z --format='%s' "$base_sha..HEAD") + +exit "$validation_status" diff --git a/git-hooks/validate-conventional-subject b/git-hooks/validate-conventional-subject new file mode 100755 index 0000000..f3d5e04 --- /dev/null +++ b/git-hooks/validate-conventional-subject @@ -0,0 +1,86 @@ +#!/usr/bin/env bash + +VALID_TYPES="fix|feat|perf|observability|obs|config|configuration|chore|ci|docs|refactor|sec|security|style|cleanup|test" + +allowed_types=$VALID_TYPES +header_pattern="^(${allowed_types})(\\([A-Za-z0-9._/-]+\\))?(!)?: .+" +subject_kind=${CONVENTIONAL_SUBJECT_KIND:-Commit subject} + +usage() { + cat <<-EOT + Validate a Conventional Commits subject. + + Usage: + $0 + $0 --subject "" +EOT +} + +die() { + printf "ERROR: %s\n\n" "$*" >&2 + usage >&2 + exit 1 +} + +if [ "$#" -eq 2 ] && [ "$1" = "--subject" ] +then + subject=$2 +elif [ "$#" -eq 1 ] +then + message_file=$1 + [ -f "$message_file" ] || die "Commit message file not found: $message_file" + + subject=$( + sed -n \ + -e '/^[[:space:]]*#/d' \ + -e '/^[[:space:]]*$/d' \ + -e 'p;q' \ + "$message_file" + ) +else + die "Invalid arguments" +fi + +[ -n "${subject:-}" ] || die "$subject_kind is empty" + +if [[ "$subject" =~ [[:space:]]$ ]] +then + die "$subject_kind must not end with whitespace: $subject" +fi + +if [[ "$subject" =~ ^Merge\ branch\ .+ ]] +then + cat >&2 <<-EOT +ERROR: $subject_kind must use Conventional Commits format. + +$subject_kind: + $subject + +Recommended fix: + Reword it with chore: in front of the merge subject: + chore: $subject +EOT + exit 1 +fi + +if [[ ! "$subject" =~ $header_pattern ]] +then + cat >&2 <<-EOT +ERROR: $subject_kind must use Conventional Commits format. + +$subject_kind: + $subject + +Expected: + [(scope)][!]: + +Allowed types: + ${allowed_types//|/, } + +Examples: + docs: update wallet one-pager + feat(api): add wallet debit endpoint + fix(db)!: change ledger migration format +EOT + exit 1 +fi diff --git a/git-hooks/validate-pull-request-title b/git-hooks/validate-pull-request-title new file mode 100755 index 0000000..abf6917 --- /dev/null +++ b/git-hooks/validate-pull-request-title @@ -0,0 +1,36 @@ +#!/usr/bin/env bash + +required=${PULL_REQUEST_TITLE_REQUIRED:-false} + +case "$required" in + true) + ;; + false) + printf 'Skipping pull request title validation outside a pull request event.\n' + exit 0 + ;; + *) + printf 'ERROR: PULL_REQUEST_TITLE_REQUIRED must be "true" or "false", got: %s\n' "$required" >&2 + exit 1 + ;; +esac + +if [ -z "${PULL_REQUEST_TITLE+x}" ] +then + printf 'ERROR: PULL_REQUEST_TITLE must be set for a pull request event.\n' >&2 + exit 1 +fi + +repo_top=$(git rev-parse --show-toplevel) || exit 1 +validator="$repo_top/git-hooks/validate-conventional-subject" + +if [ ! -x "$validator" ] +then + printf 'ERROR: validator is not executable: %s\n' "$validator" >&2 + exit 1 +fi + +CONVENTIONAL_SUBJECT_KIND="Pull request title" \ + "$validator" --subject "$PULL_REQUEST_TITLE" || exit $? + +printf 'Pull request title uses Conventional Commits format: %s\n' "$PULL_REQUEST_TITLE" diff --git a/githooks/pre-push b/githooks/pre-push deleted file mode 100755 index f8828bc..0000000 --- a/githooks/pre-push +++ /dev/null @@ -1,9 +0,0 @@ -#!/bin/sh -# Enforces Conventional Commits on every commit about to be pushed, not -# just whatever commit-msg already checked at commit time (catches a -# commit made before this hook was installed, an amend, a rebase, etc.). -# See app/usage-rules.md for the rule. Activate with: -# git config core.hooksPath githooks - -repo_top=$(git rev-parse --show-toplevel) || exit 1 -exec "$repo_top/ci/conventional_commits.sh" diff --git a/lib/mix/tasks/git_hooks.ex b/lib/mix/tasks/git_hooks.ex index 6dad2fc..16daa77 100644 --- a/lib/mix/tasks/git_hooks.ex +++ b/lib/mix/tasks/git_hooks.ex @@ -1,18 +1,15 @@ defmodule Mix.Tasks.GitHooks do - @shortdoc "Installs this repo's git hooks (commit-msg, pre-push - Conventional Commits)" + @shortdoc "Installs this repo's commit-msg and pre-push validation hooks" @moduledoc """ #{@shortdoc}. mix git_hooks - Sets `core.hooksPath` to `githooks/` (this repo's own `commit-msg` and - `pre-push` hooks, both enforcing Conventional Commits via - `ci/validate_conventional_commit.sh`/`ci/conventional_commits.sh`) - - the same one-line `git config` this repo's docs already told you to run - by hand, just idempotent and easy to re-run. Safe to run repeatedly: - setting the same git config value twice is a no-op. Wired into - `mix setup` - see that task. + Sets `core.hooksPath` to `git-hooks/`: its `commit-msg` hook enforces + Conventional Commits, and its `pre-push` hook runs `mix precommit`. + This is idempotent and safe to run repeatedly: setting the same Git config + value twice is a no-op. Wired into `mix setup` - see that task. """ use Mix.Task @@ -20,9 +17,19 @@ defmodule Mix.Tasks.GitHooks do alias RepoTasks.Shell @impl Mix.Task - def run(_argv) do - Shell.run!("git", ["config", "core.hooksPath", "githooks"]) - Mix.shell().info("==> Git hooks installed (core.hooksPath = githooks)") + def run(argv) do + run(argv, &Shell.run!/3) + Mix.shell().info("==> Git hooks installed (core.hooksPath = git-hooks)") :ok end + + @doc false + def run([], shell) do + shell.("git", ["config", "core.hooksPath", "git-hooks"], []) + :ok + end + + def run(_argv, _shell) do + Mix.raise("Usage: mix git_hooks") + end end diff --git a/lib/mix/tasks/precommit.ex b/lib/mix/tasks/precommit.ex new file mode 100644 index 0000000..f6bfcd1 --- /dev/null +++ b/lib/mix/tasks/precommit.ex @@ -0,0 +1,60 @@ +defmodule Mix.Tasks.Precommit do + @shortdoc "Runs every local and CI validation for this repository" + + @moduledoc """ + #{@shortdoc}. + + mix precommit + + This is the single validation entrypoint for developers, Git hooks, and + GitHub Actions. Cheap metadata guards run first so an invalid pull request + title or commit subject fails before dependency setup and the test suite: + + 1. `git-hooks/validate-pull-request-title` — require a Conventional + Commits pull request title when `PULL_REQUEST_TITLE_REQUIRED=true` + 2. `git-hooks/validate-commit-range` — 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 + + Pull request metadata does not exist before a pull request is opened, so + local runs skip only the title guard. GitHub Actions sets both + `PULL_REQUEST_TITLE_REQUIRED=true` and `PULL_REQUEST_TITLE` from the event; + a missing, empty, or non-conventional title then fails this task. Commit + subjects are always validated. + + All Mix quality steps run inside `app/`. + """ + + use Mix.Task + + alias RepoTasks.Shell + + @impl Mix.Task + def run(argv) do + run(argv, &Shell.run!/3) + end + + @doc false + def run([], shell) do + shell.("./git-hooks/validate-pull-request-title", [], []) + shell.("./git-hooks/validate-commit-range", [], []) + 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 + end + + def run(_argv, _shell) do + Mix.raise("Usage: mix precommit") + end +end diff --git a/test/git_hooks_test.exs b/test/git_hooks_test.exs new file mode 100644 index 0000000..8f1b0c8 --- /dev/null +++ b/test/git_hooks_test.exs @@ -0,0 +1,109 @@ +defmodule GitHooksTest do + use ExUnit.Case, async: true + + @subject_guard Path.expand("../git-hooks/validate-conventional-subject", __DIR__) + @title_guard Path.expand("../git-hooks/validate-pull-request-title", __DIR__) + @range_guard Path.expand("../git-hooks/validate-commit-range", __DIR__) + + test "the shared subject guard accepts Conventional Commits" 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 pull request guard accepts a valid required title" do + env = [ + {"PULL_REQUEST_TITLE_REQUIRED", "true"}, + {"PULL_REQUEST_TITLE", "feat(ci): enforce pull request titles"} + ] + + assert {output, 0} = run(@title_guard, [], env: env) + assert output =~ "Pull request title uses Conventional Commits format" + end + + test "the pull request guard rejects an invalid required title" do + env = [ + {"PULL_REQUEST_TITLE_REQUIRED", "true"}, + {"PULL_REQUEST_TITLE", "EXT-17: isolate Burrito musl loader per user"} + ] + + assert {output, 1} = run(@title_guard, [], env: env) + assert output =~ "Pull request title must use Conventional Commits format" + end + + test "the pull request guard skips non-pull-request events" do + env = [ + {"PULL_REQUEST_TITLE_REQUIRED", "false"}, + {"PULL_REQUEST_TITLE", "not conventional"} + ] + + assert {output, 0} = run(@title_guard, [], env: env) + assert output =~ "Skipping pull request title validation" + end + + test "the range guard compares local main against origin/main" do + test_root = + Path.join(System.tmp_dir!(), "linear_cli_git_hooks_#{System.unique_integer([:positive])}") + + bare_repo = Path.join(test_root, "origin.git") + worktree = Path.join(test_root, "worktree") + + on_exit(fn -> File.rm_rf!(test_root) end) + + File.mkdir_p!(worktree) + git!(test_root, ["init", "--bare", bare_repo]) + git!(worktree, ["init", "--initial-branch", "main"]) + git!(worktree, ["config", "user.name", "Git Hooks Test"]) + git!(worktree, ["config", "user.email", "git-hooks@example.com"]) + git!(worktree, ["config", "commit.gpgsign", "false"]) + + File.write!(Path.join(worktree, "README"), "initial\n") + git!(worktree, ["add", "README"]) + git!(worktree, ["commit", "-m", "chore: create test repository"]) + git!(worktree, ["remote", "add", "origin", bare_repo]) + git!(worktree, ["push", "--set-upstream", "origin", "main"]) + {initial_sha, 0} = System.cmd("git", ["rev-parse", "HEAD"], cd: worktree) + initial_sha = String.trim(initial_sha) + + hooks_dir = Path.join(worktree, "git-hooks") + File.mkdir_p!(hooks_dir) + + for guard <- [@subject_guard, @range_guard] do + destination = Path.join(hooks_dir, Path.basename(guard)) + File.cp!(guard, destination) + File.chmod!(destination, 0o755) + end + + 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(Path.join(hooks_dir, "validate-commit-range"), [], cd: worktree) + assert output =~ "this is not conventional" + + assert {output, 1} = + run(Path.join(hooks_dir, "validate-commit-range"), [], + cd: worktree, + env: [{"BASE_REF", initial_sha}] + ) + + assert output =~ "this is not conventional" + end + + defp run(command, args, opts \\ []) do + opts = Keyword.put(opts, :stderr_to_stdout, true) + System.cmd(command, args, opts) + end + + defp git!(directory, args) do + case System.cmd("git", args, cd: directory, stderr_to_stdout: true) do + {_output, 0} -> :ok + {output, status} -> flunk("git #{Enum.join(args, " ")} failed (#{status}):\n#{output}") + end + end +end diff --git a/test/mix/tasks/git_hooks_test.exs b/test/mix/tasks/git_hooks_test.exs new file mode 100644 index 0000000..52884d5 --- /dev/null +++ b/test/mix/tasks/git_hooks_test.exs @@ -0,0 +1,24 @@ +defmodule Mix.Tasks.GitHooksTest do + use ExUnit.Case, async: true + + alias Mix.Tasks.GitHooks + + test "points core.hooksPath at the repository-owned hooks" do + caller = self() + + shell = fn cmd, args, opts -> + send(caller, {:run, cmd, args, opts}) + :ok + end + + assert :ok = GitHooks.run([], shell) + + assert_receive {:run, "git", ["config", "core.hooksPath", "git-hooks"], []} + end + + test "rejects arguments" do + assert_raise Mix.Error, "Usage: mix git_hooks", fn -> + GitHooks.run(["unexpected"], fn _, _, _ -> :ok end) + end + end +end diff --git a/test/mix/tasks/precommit_test.exs b/test/mix/tasks/precommit_test.exs new file mode 100644 index 0000000..25eb1ee --- /dev/null +++ b/test/mix/tasks/precommit_test.exs @@ -0,0 +1,32 @@ +defmodule Mix.Tasks.PrecommitTest do + use ExUnit.Case, async: true + + alias Mix.Tasks.Precommit + + test "runs metadata guards before all quality gate steps" do + caller = self() + + shell = fn cmd, args, opts -> + send(caller, {:run, cmd, args, opts}) + :ok + end + + assert :ok = Precommit.run([], shell) + + assert_receive {:run, "./git-hooks/validate-pull-request-title", [], []} + assert_receive {:run, "./git-hooks/validate-commit-range", [], []} + 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"]} + end + + test "rejects arguments" do + assert_raise Mix.Error, "Usage: mix precommit", fn -> + Precommit.run(["unexpected"], fn _, _, _ -> :ok end) + end + end +end diff --git a/vendor/stokowski b/vendor/stokowski index 2a43887..73bbbcf 160000 --- a/vendor/stokowski +++ b/vendor/stokowski @@ -1 +1 @@ -Subproject commit 2a43887511e8b3d16559e84c6e8626cef43646de +Subproject commit 73bbbcf6e4284e768263df23c673948fe85eb81c From 6bf1f50c9b8355f85128ae8a7b8684a70507005d Mon Sep 17 00:00:00 2001 From: bougyman's bot Date: Fri, 4 Sep 2026 12:24:59 -0400 Subject: [PATCH 3/3] test: isolate Git fixtures and capture expected diagnostics Refs: EXT-29, EXT-31 See linear for details --- app/test/linear_cli/api_test.exs | 8 ++++- .../linear_cli/cli/issue_commands_test.exs | 28 ++++++++-------- app/test/linear_cli/cli_test.exs | 6 +++- app/test/linear_cli/git_test.exs | 33 +++++++++---------- app/test/linear_cli/linear/issue_test.exs | 10 ++++-- 5 files changed, 49 insertions(+), 36 deletions(-) diff --git a/app/test/linear_cli/api_test.exs b/app/test/linear_cli/api_test.exs index eaf68c4..60dde94 100644 --- a/app/test/linear_cli/api_test.exs +++ b/app/test/linear_cli/api_test.exs @@ -3,6 +3,7 @@ defmodule LinearCli.ApiTest do # not per-process) LINEAR_API_KEY env var. test_helper.exs sets a default # for the rest of the suite; running this module concurrently with it would race. use ExUnit.Case, async: false + import ExUnit.CaptureLog test "returns {:ok, data} on a successful response" do Req.Test.stub(LinearCli.Api, fn conn -> @@ -33,7 +34,12 @@ defmodule LinearCli.ApiTest do }) end) - assert LinearCli.Api.call("{ issue(id: $id) { id } }") == {:ok, %{"issue" => nil}} + log = + capture_log(fn -> + assert LinearCli.Api.call("{ issue(id: $id) { id } }") == {:ok, %{"issue" => nil}} + end) + + assert log =~ "Linear API partial-success: 1 field error(s) discarded, data returned" end test "returns {:error, {:unexpected_response, body}} when there's neither data nor errors" do diff --git a/app/test/linear_cli/cli/issue_commands_test.exs b/app/test/linear_cli/cli/issue_commands_test.exs index 7bfcc74..7763c6c 100644 --- a/app/test/linear_cli/cli/issue_commands_test.exs +++ b/app/test/linear_cli/cli/issue_commands_test.exs @@ -439,20 +439,20 @@ defmodule LinearCli.CLI.IssueCommandsTest do end test "--state with an unknown type exits 1 (Optimus parse error)" do - test_pid = self() - halt = fn code -> send(test_pid, {:halted, code}) end - - # Optimus catches the bad value and calls halt.(1); with a fake halt that - # doesn't terminate the process, execution continues and eventually crashes - # (same artifact as the --help test in cli_test.exs). Rescue it so the test - # can still verify halt was called with the right code. - try do - LinearCli.CLI.main(["issue", "list", "--state", "badtype"], halt) - rescue - _ -> :ok - end + # The production halt function never returns. Throw from the test double + # too, so the parser's error path stops before it reaches the normal CLI + # dispatch and emits an unrelated exception diagnostic. + output = + capture_io(fn -> + assert catch_throw( + LinearCli.CLI.main( + ["issue", "list", "--state", "badtype"], + fn code -> throw({:halted, code}) end + ) + ) == {:halted, 1} + end) - assert_received {:halted, 1} + assert output =~ "invalid value \"badtype\" for --state option" end test "--labels filters by a single label name (case-insensitive)" do @@ -1043,8 +1043,6 @@ defmodule LinearCli.CLI.IssueCommandsTest do end test "-y/--yes with all required flags creates and self-assigns without any prompts" do - me = %User{id: "u1", name: "Ada", email: "ada@x.com"} - created_issue = issue_map(%{ "id" => "i2", diff --git a/app/test/linear_cli/cli_test.exs b/app/test/linear_cli/cli_test.exs index 6068b4b..d1d98fc 100644 --- a/app/test/linear_cli/cli_test.exs +++ b/app/test/linear_cli/cli_test.exs @@ -1,6 +1,7 @@ defmodule LinearCli.CLITest do use ExUnit.Case, async: true import ExUnit.CaptureIO + import ExUnit.CaptureLog setup do Req.Test.stub(LinearCli.Api, fn conn -> @@ -558,10 +559,13 @@ defmodule LinearCli.CLITest do output = capture_io(:stderr, fn -> - LinearCli.CLI.main(["issue", "develop", "CRY-999"], halt) + log = capture_log(fn -> LinearCli.CLI.main(["issue", "develop", "CRY-999"], halt) end) + send(test_pid, {:log, log}) end) assert_received {:halted, 66} + assert_received {:log, log} + assert log =~ "Linear API partial-success: 1 field error(s) discarded, data returned" assert output =~ "No issue found with id" refute output =~ "What the heck is this?" end diff --git a/app/test/linear_cli/git_test.exs b/app/test/linear_cli/git_test.exs index 4cec519..d129516 100644 --- a/app/test/linear_cli/git_test.exs +++ b/app/test/linear_cli/git_test.exs @@ -8,8 +8,7 @@ defmodule LinearCli.GitTest do # under System.tmp_dir!(), never against the real project working # directory. See house rule 6 in the project instructions. setup do - origin_path = tmp_path("origin") - File.mkdir_p!(origin_path) + origin_path = tmp_dir!("origin") {_output, 0} = System.cmd("git", ["init", "--bare", "-q"], cd: origin_path) # `git init --bare`'s HEAD symref follows the runner's ambient @@ -23,27 +22,29 @@ defmodule LinearCli.GitTest do {_output, 0} = System.cmd("git", ["symbolic-ref", "HEAD", "refs/heads/main"], cd: origin_path) - repo_path = tmp_path("repo") - File.mkdir_p!(repo_path) + repo_path = tmp_dir!("repo") init_repo!(repo_path) commit_file!(repo_path, "README.md", "hello") {_output, 0} = System.cmd("git", ["branch", "-M", "main"], cd: repo_path) {_output, 0} = System.cmd("git", ["remote", "add", "origin", origin_path], cd: repo_path) {_output, 0} = System.cmd("git", ["push", "-q", "-u", "origin", "main"], cd: repo_path) - on_exit(fn -> - File.rm_rf!(origin_path) - File.rm_rf!(repo_path) - end) - %{repo: repo_path, origin: origin_path} end - defp tmp_path(prefix) do - Path.join( - System.tmp_dir!(), - "linear_cli_git_test_#{prefix}_#{System.unique_integer([:positive, :monotonic])}" - ) + # `System.unique_integer/1` is unique only within the current BEAM VM. A + # fresh `mix test` process starts its sequence over, so an interrupted prior + # run can otherwise reuse its stale /tmp fixture. A cryptographic nonce makes + # the directory unique across processes as well; register cleanup before any + # Git command can fail, so failed setup does not leave another collision + # behind. + defp tmp_dir!(prefix) do + nonce = :crypto.strong_rand_bytes(16) |> Base.url_encode64(padding: false) + path = Path.join(System.tmp_dir!(), "linear_cli_git_test_#{prefix}_#{nonce}") + + File.mkdir!(path) + on_exit(fn -> File.rm_rf!(path) end) + path end defp init_repo!(path) do @@ -113,11 +114,9 @@ defmodule LinearCli.GitTest do end test "returns an error tuple when there is no origin remote" do - repo_path = tmp_path("repo_no_origin") - File.mkdir_p!(repo_path) + repo_path = tmp_dir!("repo_no_origin") init_repo!(repo_path) commit_file!(repo_path, "README.md", "hello") - on_exit(fn -> File.rm_rf!(repo_path) end) assert {:error, _reason} = Git.default_branch(cwd: repo_path) end diff --git a/app/test/linear_cli/linear/issue_test.exs b/app/test/linear_cli/linear/issue_test.exs index b8823f0..30eaeeb 100644 --- a/app/test/linear_cli/linear/issue_test.exs +++ b/app/test/linear_cli/linear/issue_test.exs @@ -1,5 +1,6 @@ defmodule LinearCli.Linear.IssueTest do use ExUnit.Case, async: true + import ExUnit.CaptureLog alias LinearCli.Linear @@ -244,8 +245,13 @@ defmodule LinearCli.Linear.IssueTest do }) end) - assert {:error, %Ash.Error.Unknown{errors: [%{value: [not_found: _id]}]}} = - Linear.issues(%{ids: ["nope"]}) + log = + capture_log(fn -> + assert {:error, %Ash.Error.Unknown{errors: [%{value: [not_found: _id]}]}} = + Linear.issues(%{ids: ["nope"]}) + end) + + assert log =~ "Linear API partial-success: 1 field error(s) discarded, data returned" end describe "create_issue/3+" do