Skip to content

ci: lint GitHub workflows with jactionlint - #13466

Open
alies-dev wants to merge 3 commits into
vimeo:masterfrom
alies-dev:alies-dev/jactionlint
Open

alies-dev wants to merge 3 commits into
vimeo:masterfrom
alies-dev:alies-dev/jactionlint

Conversation

@alies-dev

@alies-dev alies-dev commented Nov 8, 2026 •

Copy link
Copy Markdown
Contributor

Adds a jactionlint job that runs on changes under .github/ and fixes what it found on the current workflows.

The release builds (build-phar, build-docker) restored the shared composer cache, so a poisoned cache entry could end up in a published phar or image; they now install without it. bcc and pr-labels ran with the repository's default token permissions and now request only what they use. The PHP lint step in ci.yml piped git ls-files through unquoted grep patterns without pipefail, so a failing git went unnoticed; it now uses a pathspec (same 1737 files) under shell: bash. Smaller items: a concurrency group for pr-labels, quoted $GITHUB_OUTPUT, and a 7-day Dependabot cooldown.

.github/jactionlint.yaml keeps the default profile but turns off missing-timeout, duplicate-triggers (unfiltered push runs CI on fork branches on purpose) and superfluous-actions, and records why build-phar.yml keeps its concurrency in skip-duplicate-actions and why perf-comment.yml's workflow_run trigger and pull-requests: write are acceptable.

- build release phars and docker images without a restored composer cache
- set explicit token permissions in bcc and pr-labels
- run the PHP lint pipeline with pipefail and quoted pathspecs
- add concurrency groups to build-phar and pr-labels
- quote $GITHUB_OUTPUT
- add a 7-day Dependabot cooldown
composer update --no-install

- name: Cache composer cache
uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v5

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed on purpose (cache-poisoning). This job publishes the phar on releases, and it restored the Linux-composer-* cache that other workflows on the default branch also write. Composer does not verify dist checksums for most GitHub-hosted packages, so a poisoned entry there would end up inside the released phar. Without the cache, composer install downloads everything fresh, which makes the build a little slower.

composer update --no-install

- name: Cache composer cache
uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v5

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same reason as in build-phar.yml: this job pushes the image on tags, so it should not build from a restored cache.

Comment thread .github/workflows/build-phar.yml Resolved
Comment thread .github/workflows/ci.yml
- run: |
git ls-files | grep \\\.php$ | grep -v ^dictionaries/scripts/* | ./vendor/bin/parallel-lint --stdin
- shell: bash
run: git ls-files -- '*.php' ':(exclude)dictionaries/scripts/' | ./vendor/bin/parallel-lint --stdin

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same file list as the old grep chain (I diffed both: 1737 files). The old version had two problems. Without pipefail, a failing git ls-files went unnoticed and the step linted nothing. And the regexes were not quoted, so the shell could treat ^dictionaries/scripts/* as a glob. shell: bash makes Actions run with -o pipefail.

Comment thread .github/workflows/bcc.yml
cancel-in-progress: true

permissions:
contents: read

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before this, the job ran with the repository's default token permissions. It only checks out the code and runs roave/backward-compatibility-check, so contents: read is enough.

group: ${{ github.workflow }}-${{ github.event.pull_request.number }}
cancel-in-progress: true
permissions:
pull-requests: read

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mheap/github-action-required-labels at this pin calls only issues.listLabelsOnIssue. It calls listComments/deleteComment only with add_comment: true, which this workflow does not set, so read access is enough. The concurrency group drops stale runs when labels change several times in a row.

Comment thread .github/dependabot.yml
schedule:
interval: "weekly"
cooldown:
default-days: 7

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With this, Dependabot waits 7 days after an action release before proposing it (its implicit default is 3 days). A compromised release is then more likely to be found and pulled before we pin it.

Comment thread .github/jactionlint.yaml
missing-timeout: off
# Unfiltered "push" is deliberate: it runs CI on contributors' fork branches before a PR exists.
duplicate-triggers: off
superfluous-actions: off

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

superfluous-actions only flagged svenstaro/upload-release-action in build-phar.yml and suggested replacing it with gh release upload. That is a style preference, not a bug, so the rule is off. missing-timeout would mean picking an upper bound for every analysis and test job (21 jobs), and a wrong guess would kill slow but healthy runs.

jobs:
jactionlint:
name: jactionlint
runs-on: ubuntu-24.04

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The runner is pinned to ubuntu-24.04 instead of ubuntu-latest because jactionlint checks run: scripts only when shellcheck is on PATH. The 24.04 image has it, and pinning keeps that from changing silently.

- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v6
with:
persist-credentials: false
- uses: jdx/jactionlint@9e87e8f34edfc00b849a27807e26eff9d00b936d # v2.5.0

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The action pins the jactionlint version too: its version input defaults to the release the action was published with. Dependabot (with the new cooldown) will propose updates.

@github-actions

github-actions Bot commented Nov 8, 2026 •

Copy link
Copy Markdown
Contributor

✅ Performance: unchanged · Issues: unchanged

Project Memory (MB) Wall (s) Issues Weighted
✅ laravel 1 821 55.3 $\color{gray}{\textsf{-2.6\%}}$ 25 914 51 150
✅ symfony 4 002 213.6 $\color{gray}{\textsf{+0.5\%}}$ 51 079 101 098
✅ doctrine-orm 428 17.4 $\color{gray}{\textsf{-0.2\%}}$ 3 507 5 580
✅ psl 651 21.9 1 424 5 514
✅ phpunit 506 10.5 $\color{gray}{\textsf{+1.1\%}}$ 1 627 1 265
✅ php-parser 245 10.1 $\color{gray}{\textsf{+0.7\%}}$ 17 169 20 624
✅ tempest 922 21.6 $\color{gray}{\textsf{+3.3\%}}$ 5 016 9 616
✅ amphp-amp 60 2.1 $\color{gray}{\textsf{-1.4\%}}$ 32 79
✅ webmozarts-assert 272 4.8 $\color{gray}{\textsf{+0.4\%}}$ 577 2 714
✅ guzzle-promises 55 2.1 163 407
✅ monolog 161 5.4 $\color{gray}{\textsf{+0.6\%}}$ 1 004 2 722
✅ drupal 5 196 200.6 $\color{gray}{\textsf{+3.0\%}}$ 98 825 161 668
Total 14 319 565.2 $\color{gray}{\textsf{+1.1\%}}$ 206 337 362 437
How to read this table
  • Memory: Psalm's own peak figure, threads=1. More than +5 % fails, unless the PR changes composer.json.
  • Wall: base and head run on the same runner; ±10 % is noise. Above +20 % warns, never fails. re-run: a reversed second run was taken and the faster kept.
  • Issues: head count, then $\color{red}{\textsf{+added}}$ $\color{green}{\textsf{-removed}}$ against base.
  • Weighted: issues weighted by error level (levels 1–8 as is, always-errors 10, feature-specific and purity suggestions 0). The projects' own code is pinned and their dependencies resolve identically for base and head, so a change means Psalm reports more, fewer or differently serious issues. A rise is not necessarily a regression.
  • Total: sum over the projects compared on both sides. Thresholds apply per project, not to the total.
  • Colors: $\color{red}{\textsf{red}}$ past a fail or warn threshold (or more issues), $\color{green}{\textsf{green}}$ an improvement past noise (or fewer issues), $\color{gray}{\textsf{gray}}$ noise.
  • Performance: regressed if a head run crashed or timed out or a memory or wall delta is red, else improved if one is green, else unchanged. Its icon is the check result: ❌ fail · ⚠️ warn · ✅ pass; the icon next to each project is that project's result, 💥 = not comparable: base crashed or timed out (counts as ⚠️).
  • Issues (heading): the total weighted change, increased · decreased · unchanged. Informational, never fails the check.

head 81c0f02 · base 0cc606c (merge-base) · workflow run

A native concurrency group leaves the next run pending, which
skip-duplicate-actions (concurrent_skipping: always) treats as a
concurrent duplicate, so both runs could skip building the phar.
@github-actions github-actions Bot added the release:internal The PR will be included in 'Internal changes' section of the release notes label Nov 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release:internal The PR will be included in 'Internal changes' section of the release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant