Add GitHub Actions harness for PR gating#2
Conversation
Adds .github/workflows/harness.yml running format / compile / credo / doctor / sobelow / test+cover@70% / dialyzer on every push to main and every PR. Versions pinned via .tool-versions (Erlang 27.3.4.11, Elixir 1.18.4-otp-27) so CI and local dev stay in lockstep on mix format output. Coverage threshold set at 70% — main currently sits at 73.62%, leaving a small margin. Raise as coverage climbs. Closes the no-CI gap noted while reviewing the Task 6 PR.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a GitHub Actions workflow "Harness" that pins Erlang/Elixir via .tool-versions and runs deps install, compile (warnings-as-errors), format check, Credo, doctor, sobelow, tests with a 70% coverage gate, and Dialyzer; also updates .tool-versions and Dialyzer ignore patterns. ChangesCI Harness Setup
🎯 3 (Moderate) | ⏱️ ~20 minutes
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/harness.yml (1)
25-27: Add a job timeout to cap hung runs.Consider adding
timeout-minuteson theharnessjob so stalled runs (especially initial PLT/dialyzer) don’t consume runners indefinitely.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/harness.yml around lines 25 - 27, The harness job currently lacks a timeout and can hang indefinitely; add a timeout-minutes setting to the harness job definition (the job named "harness") to cap run time (e.g., timeout-minutes: 60 or your desired limit) so stalled steps like PLT/dialyzer won't consume runners indefinitely; update the harness job block in the workflow to include this key alongside name and runs-on.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In @.github/workflows/harness.yml:
- Around line 25-27: The harness job currently lacks a timeout and can hang
indefinitely; add a timeout-minutes setting to the harness job definition (the
job named "harness") to cap run time (e.g., timeout-minutes: 60 or your desired
limit) so stalled steps like PLT/dialyzer won't consume runners indefinitely;
update the harness job block in the workflow to include this key alongside name
and runs-on.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 84d217d1-f1c9-4afd-b141-c124096446e9
📒 Files selected for processing (2)
.github/workflows/harness.yml.tool-versions
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f0dc73f5bc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| run: mix doctor --raise | ||
|
|
||
| - name: Sobelow (honors .sobelow-conf, exit=Low) | ||
| run: mix sobelow |
There was a problem hiding this comment.
Make Sobelow fail the harness on findings
As written, this step only reports Sobelow results; it does not gate the PR when findings are present. Sobelow's task docs list --exit as the option that returns a non-zero exit status, and this repo has no .sobelow-conf to supply that option, so a PR introducing a Low/Medium/High finding would still pass this job. Add --exit Low or save an equivalent config if this harness is meant to block security findings.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This PR adds a GitHub Actions CI “harness” to gate pushes to main and all pull requests with a deterministic Elixir toolchain, and pins Erlang/Elixir versions via .tool-versions to keep CI and local formatting/compilation consistent.
Changes:
- Add
.github/workflows/harness.ymlto run format, compile (warnings-as-errors), Credo, Doctor, Sobelow, tests+coverage gate, and Dialyzer on PRs and pushes tomain. - Add
.tool-versionspinning Erlang and Elixir versions for deterministic local/CI toolchains.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
.tool-versions |
Pins Erlang/Elixir versions for deterministic CI and local development. |
.github/workflows/harness.yml |
Introduces a CI workflow to run the full Elixir quality/test/static-analysis harness on PRs and main. |
Comments suppressed due to low confidence (2)
.github/workflows/harness.yml:65
- The comment mentions
.doctor.exsand araise=falsesetting, but there is no.doctor.exsfile in the repo. Either add the configuration file being referenced, or adjust the comment to describe the actual behavior (i.e., runningmix doctor --raisewithout relying on a config override).
- name: Doctor (--raise overrides .doctor.exs raise=false to gate CI)
run: mix doctor --raise
.github/workflows/harness.yml:68
- The comment claims Sobelow "honors .sobelow-conf", but there is no
.sobelow-confin this repository. Either add the config file or update the comment to avoid pointing to a non-existent configuration.
- name: Sobelow (honors .sobelow-conf, exit=Low)
run: mix sobelow
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| env: | ||
| MIX_ENV: test | ||
| steps: |
| # so harness only fails on real Credo issues (see development-philosophy.md | ||
| # § "TODO Comment Requirements"). |
test/support/tempo_test_helpers.ex calls ExRLP.encode/1 — ExRLP is a transitive dep via onchain (and only compiles in :test env via elixirc_paths). Dialyxir's :apps_direct PLT strategy doesn't pull in transitive deps, so the call surfaces as :unknown_function in CI. Local dev env dialyzer didn't trip this because test/support isn't compiled there. CI runs with MIX_ENV=test, which exposes it. Per CLAUDE.md: ExRLP is documented as a transitive dep needing @dialyzer suppressions — same class as Jason / Req / Cartouche / Onchain / Descripex already in this file.
Summary
.github/workflows/harness.ymlrunning format / compile (warnings-as-errors) / credo --strict / doctor --raise / sobelow /mix test.json --cover --cover-threshold 70/ dialyzer on every push tomainand every PR..tool-versionsso CI and local dev never drift onmix formatoutput.Why
Closes the no-CI gap noted while reviewing the Task 6 PR (#1). Every PR push now gets the full Elixir harness as a GitHub check — visible to user, agent, and PR-review tooling.
Test plan
mix format --check-formatted— cleanmix compile --warnings-as-errors— cleanmix credo --strict --ignore TagTODO,TagFIXME— 0 issues, 17 source filesmix doctor --raise— 100% doc/moduledoc/spec coveragemix sobelow— scan complete, no findingsmix test.json --cover— 73.62% (above 70% threshold)mix dialyzer— verified passing in prior sessions viamix dialyzer.json; CI first run will build PLT (~5 min), then cachedNotes
.tool-versionsuses27.3.4.11(latest 27.x patch inasdf list erlang) —setup-beamfetches precompiled binaries for any named version.mix test.json'stest_helper.exsalready excludes:integrationtags by default; no--exclude integrationflag needed.Summary by CodeRabbit