Skip to content

Add an aggregate CI check and run every PR job from test.yml - #150

Draft
zeevmoney wants to merge 17 commits into
per-16331/deprecate-permit-exceptionfrom
per-16779/aggregate-ci-check
Draft

zeevmoney wants to merge 17 commits into
per-16331/deprecate-permit-exceptionfrom
per-16779/aggregate-ci-check

Conversation

@zeevmoney

@zeevmoney zeevmoney commented Oct 3, 2026 •

Copy link
Copy Markdown
Member

Linear issues

  • PER-16779: one aggregate required CI check for permit-python, following the Terraform provider's pattern.
  • PER-16336: the SDK upgrade guide; this PR applies its section 9, "One aggregate required check".

Stacked on #144, the top of the 3.x stack.

Why

The ruleset on main requires six job names spread across three workflows (test.yml, security.yml, pre-commit.yml). A renamed job silently stops being required, a new job is not required until someone adds it to the ruleset, and GitHub counts a job skipped by an if: as success for a required check. needs works only inside one workflow, so the jobs a pull request must pass move into test.yml, where one CI job checks all of them.

What changed

  • One workflow for PR checks. test.yml now holds pre-commit (moved from pre-commit.yml, which is deleted) and Dependency Audit, Post Audit Comment, Dependency Review, Audit Script Tests and Workflow Hardening (moved from security.yml), next to its existing jobs. Job ids and names are unchanged, so the six check names the ruleset requires today still report.
  • One copy of the audit. The audit's steps live in a local composite action, .github/actions/dependency-audit. The audit jobs in test.yml and security.yml both check out the repository and call it. The jobs keep contents: read, and the artifact is still named dependency-audit. The gate_failed job output, which nothing read, is removed.
  • security.yml runs only on the Monday 09:00 UTC schedule and on manual dispatch, with audit and notify. The Slack message is unchanged.
  • CI job, copied from the Terraform provider. It has if: always() and permissions: {}, and needs 10 jobs: api-coverage, audit, audit-scripts-test, comment, compatibility, dependency-review, migration-skill, pre-commit, pytest and workflow-hardening (EXPECTED_JOBS: 10). It exits 2 when the job results cannot be read or their count is not EXPECTED_JOBS, and 1 when any result is not success. The one exception: dependency-review skipped passes on push.
  • e2e-unpinned-pdp (the latest-PDP-image and cloud-PDP legs) stays in test.yml after pytest and does not block a PR: CI does not need it.
  • Post Audit Comment runs on every event, still after a failed audit. Its steps run only on a PR from a branch of this repository, so on pushes and fork PRs the job succeeds with its steps skipped. The comment step also skips posting when the PR head has moved past the commit the run audited.
  • Workflow Hardening gains two steps:
    • "Check that CI needs every job" fails in four cases: test.yml's jobs, minus ci and ADVISORY_JOBS, differ from ci.needs; an ADVISORY_JOBS entry is not a job, is listed twice or is also in needs; or EXPECTED_JOBS is not the number of jobs in needs. It exits 2 when it reads no jobs. ADVISORY_JOBS holds e2e-unpinned-pdp, with a comment giving the reason.
    • "Shellcheck the local actions" shellchecks each shell: bash step under .github/actions with actionlint's options, because actionlint does not lint composite actions. It exits 1 on a finding and 2 when it reads no bash step.
  • Tests. .github/scripts/test_ci_checks.py (50 tests, run by Audit Script Tests) reads the bash and env: of those three steps from test.yml with yq. It runs them the way a shell: bash step runs, against planted job results, workflows and actions.
  • Dependabot. The github-actions entry also scans /.github/actions/*.
  • Publish workflow. One comment in python-sdk-publish.yml pointed at the Trivy step in security.yml; it now points at .github/actions/dependency-audit. Nothing else in that workflow changes.
  • Docs. CONTRIBUTING.md has a new CI section. It covers CI as the one check to require, how to add a job, what the job-list check enforces, the six check names not to rename until the ruleset switch, and security.yml as the home of the weekly audit. The headers of .github/scripts/pytest.ini and skills/tests/README.md, and the Dependabot comments, no longer name the old workflows or describe only some of the tests. The repository has no CLAUDE.md or AGENTS.md.

Behaviour changes

  • The PR dependency audit now runs on every push to main/master, because test.yml has no path filter. Before, a push ran it only when pyproject.toml, security.yml, audit-deps.sh or format_audit.py changed.
  • New pushes to a PR no longer cancel the older runs of the audit, comment, dependency review, audit script tests and workflow hardening, because test.yml has no concurrency group. The audit comment is not posted from a run whose commit is no longer the PR head. Dependency Review's own on-failure summary comment can still come from a superseded run.
  • security.yml no longer runs on pull_request or push. Its concurrency group stays; the cancel-on-PR setting is dropped.
  • Post Audit Comment runs on every event and succeeds with its steps skipped where it cannot post. Same-repo PRs, Dependabot's included, get the comment as before.
  • Once the ruleset requires CI, more jobs block a merge: API Coverage, the compatibility legs, the Migration Skill Tests legs, Dependency Review on PRs and Post Audit Comment. e2e-unpinned-pdp still does not block.
  • Workflow Hardening now fails in three more cases: a test.yml job is in neither ci.needs nor ADVISORY_JOBS, EXPECTED_JOBS is wrong, or a local action's bash has a shellcheck finding.
  • Audit Script Tests also runs test_ci_checks.py, which uses bash, jq, yq and shellcheck from the runner image.
  • The audit's run steps now run as composite steps under shell: bash (bash --noprofile --norc -eo pipefail) instead of the default bash -e. Every script already sets pipefail.
  • Dependabot also opens updates for the composite action's pins. Major bumps are not grouped, so a major bump to an action pinned in both the workflows and the composite action arrives as one PR per directory, and the two pins can differ until both merge.
  • The Notify Slack check run (skipped) no longer appears on PRs.

How it was tested

  • .github/scripts suite, run as Audit Script Tests runs it: 273 passed, 50 of them in test_ci_checks.py. Local tools: bash 3.2.57, yq v4.53.3, jq, shellcheck 0.11.0.

  • Planted inputs, run locally against the step bash extracted from test.yml. Exit codes:

    • CI: all success on pull_request 0 and on push 0; pytest skipped 1; audit cancelled 1; compatibility failed 1; 9 results 2; 11 results 2; unreadable JSON 2; dependency-review skipped on push 0 and on pull_request 1; comment skipped on push 1.
    • Job-list check: committed test.yml 0; a job missing from needs 1; a needs entry that is not a job 1; a stale advisory entry 1; a duplicated advisory entry 1; an advisory job also in needs 1; EXPECTED_JOBS 9 gives 1 and 11 gives 1; no jobs read 2.
    • Local actions' shellcheck: the committed action (5 bash steps) 0; a planted echo $x 1; a missing directory 2.
  • Mutation check: I made 23 mutations. 17 were in the CI and job-list bash:

    • skipped or cancelled counted as success
    • the push exception applied on every event, or to any dependency-review result
    • no count check
    • unreadable JSON exiting 1
    • each job-list check removed in turn
    • ci not dropped from the job list
    • an empty ADVISORY_JOBS
    • EXPECTED_JOBS off by one
    • a job dropped from needs

    The other 6 were in the shellcheck step: no placeholder, no exclusions, a finding not failing, bash steps skipped, no exit 2 when nothing is read, and an unreadable action exiting 0. Each mutation failed at least one test.

  • The comment script was run under Node 24 with a mocked GitHub client:

    • head unchanged: it updates the existing comment or creates one
    • head moved: it only logs a notice
    • no report: it warns
    • report over the length limit: it posts the link body

    The script before this change updated the comment on a moved head.

  • actionlint 1.7.12: no findings. zizmor 1.30.1, offline and online: no findings (2 ignored, 17 suppressed). uv run pre-commit run --all-files passes.

  • Job ids and names, compared with the base using yq, are unchanged. The bodies of pre-commit, dependency-review, notify, pytest, e2e-unpinned-pdp, api-coverage, compatibility and migration-skill are identical. The composite action's steps match the old audit steps except for the python-version input, shell: bash, one comment and the removed gate_failed lines.

  • On GitHub:

    • The first run on this PR (bdb639c) passed, CI included, with 29 checks green. The composite action ran inside Dependency Audit, and Post Audit Comment posted the audit comment. CI didn't wait for the non-blocking e2e (latest PDP image) leg.
    • A temporary commit (6ca2789) forced Migration Skill Tests to skip. CI failed with "Jobs that did not succeed: migration-skill skipped" and exit code 1. The next commit (559241a) reverts it, so the tree is the same as at bdb639c.
    • Not yet seen for real: the audit comment's moved-head check, which first runs when a push lands while an older run is still going.

Owner actions before merge

  1. After this merges and main produces the CI check, replace the six per-job required checks in the main ruleset with CI alone. Do it only then: requiring CI earlier leaves every PR waiting. The six checks are pytest (Pydantic pydantic<2.0.0), pytest (Pydantic pydantic>=2.0.0), pre-commit, Dependency Audit, Audit Script Tests and Workflow Hardening. Then, in a follow-up, remove the note that lists those six names from CI's leading comment in test.yml and from CONTRIBUTING.md's CI section.
  2. Replace pre-commit with prek #149 (prek) edits the hooks job this PR moves. Whichever of the two merges second ports Replace pre-commit with prek #149's hooks-job changes (the prek run command and the PREK_HOME cache) into the pre-commit job in test.yml, and keeps pre-commit.yml deleted.

🤖 Generated with Claude Code

zeevmoney and others added 15 commits October 3, 2026 19:17
security.yml's audit job now checks out the repository and runs
.github/actions/dependency-audit, which holds the audit's steps, so the
PR workflow can run the same audit without a second copy. The job keeps
its read-only permissions and its gate_failed output, and the artifact
keeps its dependency-audit name.

Dependabot's github-actions entry also scans .github/actions/*, so the
pins inside the composite action stay updated.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The pre-commit job moves from pre-commit.yml, which is deleted, and the
Dependency Audit, Post Audit Comment, Dependency Review, Audit Script
Tests and Workflow Hardening jobs move from security.yml. Job ids and
names are unchanged, so the check names the ruleset requires keep
passing. test.yml's audit job runs the shared composite action.

security.yml keeps only the weekly and manual audit and its Slack
notification. Since test.yml runs on every push to main, the audit now
runs on each one instead of only when a dependency file changes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The Post Audit Comment job used to be skipped on pushes and on PRs from
forks. It now runs on every event (still after a failed audit) and its
download and comment steps run only on a pull request from a branch of
this repository, so the job succeeds with its steps skipped elsewhere.
An aggregate check can then require it to succeed. Same-repo PRs get
the comment as before.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
CI, the job named CI in test.yml, runs whatever happened to the jobs it
needs and fails when any of them failed, was cancelled or was skipped,
since GitHub counts a skipped required check as passing. Dependency
review may be skipped on a push, where it does not run. CI exits 2 when
the job results cannot be read or their count is not EXPECTED_JOBS.

CI needs every job in test.yml but e2e-unpinned-pdp, which tests PDPs
this repository does not pin and stays non-blocking. Comments that
named the per-job required checks now describe CI.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A new step in the Workflow Hardening job reads test.yml with yq and
fails when its jobs, minus CI and the ADVISORY_JOBS list, differ from
CI's needs, when an ADVISORY_JOBS entry is not a job, is listed twice
or is also in CI's needs, or when CI's EXPECTED_JOBS is not the number
of jobs CI needs. It exits 2 when it reads no jobs. ADVISORY_JOBS holds
e2e-unpinned-pdp, with the reason it does not block a PR.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
test_ci_checks.py reads both steps' bash and env from test.yml with yq
and runs them as a `shell: bash` step runs, against planted job results
(failed, cancelled and skipped jobs, a missing or extra result,
unreadable JSON, dependency review skipped on a push and on other
events) and planted workflows (a job CI does not need, a need that is
not a job, stale, repeated or needed advisory entries, a wrong
EXPECTED_JOBS, no jobs). The Audit Script Tests job runs it with the
other CI script tests.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
CONTRIBUTING.md gains a CI section: CI in test.yml is the one check to
require, how to add a job (to CI's needs with EXPECTED_JOBS, or to
ADVISORY_JOBS with the reason), what the job-list check enforces, and
that the weekly audit runs from security.yml. The e2e and CI script
test sections, the skills tests README and the Dependabot comments no
longer name security.yml as the PR audit or the per-job required
checks.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
test.yml has no concurrency group, so a newer push no longer cancels
the run of the older commit, and that run can finish last. Its comment
step now reads the PR's current head and posts only when it is the
commit the run audited, so the audit comment never goes back to an
older commit's report.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Nothing reads the audit jobs' gate_failed output, in test.yml or in
security.yml, nor the composite action's failed output that feeds it:
the comment and Slack jobs read the dependency-audit artifact and the
audit job's result. The gate step still fails the job on a fixable
HIGH or CRITICAL advisory.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
actionlint shellchecks the run blocks of workflows only, so moving the
audit's bash into .github/actions/dependency-audit took it out of CI's
shellcheck. A new Workflow Hardening step shellchecks every bash step
of the local actions with actionlint's options: expressions replaced
by a placeholder and the checks it turns off left off. It exits 1 on a
finding and 2 when it reads no bash step.

test_ci_checks.py runs the step against the committed actions and
against planted ones: a finding, an expression, a variable set by
env:, no action and no bash step.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
CONTRIBUTING.md's CI section said CI needs every other job in test.yml,
which left out e2e-unpinned-pdp, the one job in ADVISORY_JOBS.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Moving the jobs dropped the comments that said the pytest lanes' and
pre-commit's names are required checks. Until the ruleset on main
requires CI alone, it still requires six check names, so CI's leading
comment and CONTRIBUTING.md's CI section list them and say not to
rename those jobs until then.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The header named the audit formatter's and the schema drift check's
tests only, and said they need nothing but pytest and the standard
library. It now covers every test file in .github/scripts and says
that test_ci_checks.py also runs bash, jq, yq and shellcheck.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@linear-code

linear-code Bot commented Oct 3, 2026

Copy link
Copy Markdown

PER-16779

@github-actions

github-actions Bot commented Oct 3, 2026

Copy link
Copy Markdown

Dependency Security Audit

Scanned: pyproject.toml dependencies + dev group, resolved at Python 3.10 (the current resolution, and the lowest versions the published specs permit under each pydantic major)

✅ No known vulnerabilities found.

Both the resolved dependency set and the lowest versions the published specs permit are clean at HIGH and CRITICAL.

zeevmoney and others added 2 commits October 3, 2026 20:18
Reverted in the next commit.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
CI failed on it as intended: "Jobs that did not succeed: migration-skill
skipped", exit 1.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant