Skip to content

Reduce CI cost and latency across the reusable workflows - #274

Merged
swissspidy merged 6 commits into
mainfrom
claude/wp-cli-actions-optimization-mozclo
Aug 7, 2026
Merged

Reduce CI cost and latency across the reusable workflows#274
swissspidy merged 6 commits into
mainfrom
claude/wp-cli-actions-optimization-mozclo

Conversation

@swissspidy

@swissspidy swissspidy commented Aug 7, 2026

Copy link
Copy Markdown
Member

Optimisation pass over the reusable workflows and the sync workflow. Grouped into reviewable commits.

Sync workflow

sync-workflows.yml ran on a */10 * * * * cron with no concurrency group. Measured from the Actions API:

  • 34,461 runs to date.
  • GitHub throttles high-frequency schedules; across the last 30 runs it actually fired every ~48 minutes (min 23, max 82), so the ten-minute cadence was never real.
  • Without a concurrency group the runs overlapped. Recent ones took 15, 15, 84, 126 and 141 minutes while pushing to the same ~45 target repositories at once, and roughly one in six failed.

Serialised with a concurrency group, bounded with a timeout, cron dropped to daily as a drift check, and a paths filter added so unrelated commits no longer start a sync.

Test matrix

  • The ten soft-failing legs (six MariaDB, two PHP nightly, macOS, Windows) run with continue-on-error and cannot gate a merge, so they now run only on the nightly schedule and manual dispatch. Behat goes 51 → 41 and PHPUnit 12 → 9 per pull request; the scheduled run still covers all 63.
  • get-matrix, prepare-unit and prepare-functional folded into one prepare job. They were three serial dispatches before any test started, and two checked out the whole repository just to test for a file.
  • Documentation-only changes skip the matrix entirely. paths filters on the caller would not work here, because testing.yml lives in each consumer repository and carries its per-repository inputs, so the equivalent is done inside the reusable workflow. It is a deny list, not an allow list: the default is to test, and every failure path falls back to testing.
  • Dropped a duplicated matrix entry that unique had been absorbing, and switched the matrix output to jq -c with a quoted expansion — it previously relied on unquoted word splitting to flatten pretty-printed JSON.

Per-job overhead

  • Ghostscript ships with the hosted images, but every Behat leg ran apt-get update && apt-get install ghostscript regardless, ~25s a leg. Now skipped when gs is already present, with the install kept as a fallback.
  • Weekly Composer cache suffix. These packages commit no lock file, so composer update resolves on every run while the cache key stays pinned to composer.json and never changes on its own.
  • timeout-minutes on every job. None had one, so a hung Behat run could hold a runner for the full six-hour default.
  • actionlint pinned by digest instead of :latest; --fail added to the two curl calls; Node version pinned for the Gherkin lint job.
  • Repositories can select a larger runner through a RUNNERS_NAME repository variable without forking these workflows.

Run view

The calling job's name is what the run view groups by, and it was the static string Behat, so all 41 legs landed in one flat group and the checks read test / Behat / Behat | PHP 8.3 | WP latest | MySQL. The grouping key moved into the calling job, as wordpress-develop does with PHP ${{ matrix.php }}.

This also fixed an ambiguity: the leg name rendered every non-SQLite database as plain MySQL, so mysql-5.6, 5.7, 8.0 and 8.4 were indistinguishable and seven pairs of check runs shared an identical name. All 41 Behat check names are now unique.

Before merging

Check names change. Any required status check configured against the old names needs updating.

ubuntu-22.04 is untouched. The image is being retired, but MySQL 5.6/5.7 on 24.04 needs a real matrix run to validate. Worth a separate pull request.

Because consumer repositories pin @main, a pull request here does not exercise these changes. Testing them means pointing a caller at this branch.

claude added 5 commits August 6, 2026 21:35
The sync workflow ran on a `*/10 * * * *` cron with no concurrency group. Two
things went wrong with that:

* GitHub throttles high-frequency schedules. Across the last 30 runs the
  workflow actually fired every ~48 minutes (min 23, max 82), so the ten
  minute cadence was never real to begin with.
* Without a concurrency group, runs overlapped. Several recent runs took
  15, 84, 126 and 141 minutes while pushing to the same ~45 target
  repositories at once, and roughly one in six failed.

Serialise the workflow, put a bound on how long a run may take, and let the
push trigger do the work it was already doing. The cron drops to daily and
now only serves as a drift check for out-of-band edits. The push trigger
also gains a paths filter so unrelated commits no longer start a sync.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Jy4dmjymj9VmoTBaqrV4iX
Ten of the 51 Behat legs run with `continue-on-error` — the six MariaDB
combinations, both PHP nightly entries, macOS and Windows. They cannot gate a
merge, so running them on every pull request spent runner time without
producing a signal, and the Windows leg in particular sat on the critical
path.

Flag those entries with `"nightly": true` and filter them out unless the run
came from the nightly schedule or a manual dispatch. Entries supplied through
the `matrix` input carry no such flag and are always kept, so per-repository
customisation is unaffected.

Per pull request this takes Behat from 51 jobs to 41 and PHPUnit from 12 to 9.
The scheduled run still covers all 63.

Also fold `get-matrix`, `prepare-unit` and `prepare-functional` into a single
`prepare` job. They were three serial job dispatches before any test started,
and two of them checked out the whole repository just to test for the presence
of a file.

Along the way:

* Drop a duplicated `{php: 7.4, wp: trunk, mysql: mysql-8.0}` entry that the
  `unique` filter had been quietly absorbing.
* Emit the matrix with `jq -c` and quote the expansion. It previously relied
  on unquoted word splitting to flatten pretty printed JSON into one line.
* Group the concurrency key by event, so a push to the default branch and the
  nightly schedule no longer cancel one another.

The matrix JSON is reindented by two spaces as a result of moving into the new
job; its contents are otherwise unchanged apart from the flags noted above.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Jy4dmjymj9VmoTBaqrV4iX
Ghostscript ships with the GitHub hosted images, but every Behat leg ran
`apt-get update && apt-get install ghostscript` regardless — roughly 25
seconds a leg, across 41 legs, in every repository. Skip it when `gs` is
already on PATH and keep the install as a fallback. Same treatment for the
macOS leg, where `brew install` is slower still.

WP-CLI packages do not commit a lock file, so `composer update` resolves
dependencies on every run while the cache key stays pinned to composer.json.
A cache populated months ago therefore keeps serving `dev-*` requirements
indefinitely. Rotate the key weekly, as wordpress-develop does. `%Y-%W` is
used rather than the GNU-only `date --date='last Mon'` because these jobs
also run on macOS and Windows.

Other changes:

* Give every job a `timeout-minutes`. None of them had one, so a hung Behat
  run could occupy a runner for the full six hour default.
* Pin the actionlint image by digest instead of `:latest`, so the linter
  cannot change underneath ~45 repositories without a commit here.
* Add `--fail` to the two curl calls. Without it curl writes the error page
  to the output file and the tools then run against garbage.
* Pin the Node version used for the Gherkin lint job.
* Let repositories with a heavy suite select a larger runner through a
  `RUNNERS_NAME` repository variable, without forking these workflows. The
  `os` inputs now default to an empty string so that fallback chain applies
  to direct callers too; the effective default is still ubuntu-22.04.
* Skip checkout progress output unless the run is in debug mode.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Jy4dmjymj9VmoTBaqrV4iX
A pull request that only touches a README currently runs the full matrix.
wordpress-develop solves this with `paths` filters on the caller workflow,
but that would not work here: `testing.yml` lives in each of the ~45
consumer repositories and carries their per-repository `minimum-php`,
`minimum-wp` and `matrix` inputs, so it cannot be centrally synced without
overwriting that configuration.

Do the equivalent inside the reusable workflow instead, where it is already
centrally controlled. The `prepare` job diffs the change against its base and
emits empty matrices when nothing test-relevant was touched, which the
existing `if: needs.prepare.outputs.* != ''` guards already turn into skipped
jobs.

This is deliberately a deny list rather than an allow list. The reusable
workflow cannot know how a given package lays out its source, so the default
is to test, and only changes that are provably irrelevant — Markdown, issue
templates, LICENSE, editor and git metadata — are skipped. Every failure path
falls back to testing as well: a non-diffable event, a newly created branch
reporting an all-zero base, or a base commit that cannot be resolved all run
the full matrix.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Jy4dmjymj9VmoTBaqrV4iX
The calling job's name is what the Actions run view groups by, and it was the
static string "Behat". All 41 legs therefore landed in one flat group, and
because the called workflow repeated the same word the checks read
"test / Behat / Behat | PHP 8.3 | WP latest | MySQL".

Move the grouping key into the calling job, as wordpress-develop does with
"PHP ${{ matrix.php }}", and leave the called workflow to identify the leg
within its group. The run view now shows one collapsible entry per PHP
version.

This also fixes an ambiguity. The leg name rendered every non-SQLite database
as plain "MySQL", so mysql-5.6, 5.7, 8.0 and 8.4 were indistinguishable —
seven pairs of check runs shared an identical name and a failure could not be
attributed to a database version without opening the job. Spell the version
out instead. All 41 Behat check names are now unique.

Note for anyone with branch protection: these check names change, so any
required status check configured against the old names needs updating.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Jy4dmjymj9VmoTBaqrV4iX
@swissspidy
swissspidy requested a review from a team as a code owner August 7, 2026 07:14
Copilot AI lite review requested due to automatic review settings August 7, 2026 07:14
@coderabbitai

coderabbitai Bot commented Aug 7, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The pull request updates reusable GitHub Actions workflows with configurable runners, timeouts, checkout output, Composer cache rotation, stricter downloads, shared test-matrix preparation, documentation-only filtering, and serialized workflow synchronization.

Changes

Test matrix flow

Layer / File(s) Summary
Shared test-matrix preparation
.github/workflows/reusable-testing.yml
The workflow uses one prepare job to inspect changes, build unit and functional matrices, filter nightly entries, and pass matrix outputs to test jobs.

Runner, cache, and execution controls

Layer / File(s) Summary
Runner, timeout, and cache controls
.github/workflows/reusable-code-quality.yml, .github/workflows/reusable-functional.yml, .github/workflows/reusable-unit.yml
The workflows add runner fallback rules, conditional checkout progress, job timeouts, Ghostscript availability checks, weekly Composer cache suffixes, and stricter Gherkin and actionlint downloads.

Workflow synchronization

Layer / File(s) Summary
Workflow synchronization scheduling
.github/workflows/sync-workflows.yml
Push path filters, daily scheduling, serialized concurrency, and 20-minute job timeouts are added.

Estimated code review effort: 4 (Complex) | ~45 minutes

Suggested labels: scope:testing, scope:meta

Suggested reviewers: copilot, schlessera

Sequence Diagram(s)

sequenceDiagram
  participant GitHubEvent
  participant prepare
  participant UnitMatrix
  participant FunctionalMatrix
  participant TestJobs
  GitHubEvent->>prepare: start workflow and inspect changed files
  prepare->>UnitMatrix: apply file and documentation filters
  prepare->>FunctionalMatrix: apply version and coverage filters
  UnitMatrix->>TestJobs: provide unit matrix output
  FunctionalMatrix->>TestJobs: provide functional matrix output
  TestJobs->>TestJobs: run grouped matrix jobs
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main workflow changes that reduce CI cost and latency.
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/wp-cli-actions-optimization-mozclo

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
.github/workflows/reusable-testing.yml (2)

436-465: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Empty include array still produces a non-empty output and fails the job.

The step emits an empty output only when FILE_EXISTS is false or DOCS_ONLY is true. If the jq filter removes every entry, for example when inputs.minimum-php excludes all listed versions or the caller's exclude rules match everything, the output becomes {"include":[]}. That string is not empty, so the unit job runs and GitHub Actions fails the job because the matrix contains no vectors.

Emit an empty output when the filtered include array is empty.

🐛 Proposed fix
           if [[ $FILE_EXISTS == 'true' && $DOCS_ONLY != 'true' ]]; then
-            echo "matrix=$(jq -c \
+            FILTERED=$(jq -c \
               --argjson with_coverage_flag "${WITH_COVERAGE}" \
               --arg minimum_php "${INPUTS_MINIMUM_PHP}" \
               --arg minimum_wp "${INPUTS_MINIMUM_WP}" \
               '
               ...
-              ' <<< "$BASE_MATRIX")" >> "$GITHUB_OUTPUT"
+              ' <<< "$BASE_MATRIX")
+            if [ "$(jq -r '.include | length' <<< "$FILTERED")" -eq 0 ]; then
+              echo "matrix=" >> "$GITHUB_OUTPUT"
+            else
+              echo "matrix=${FILTERED}" >> "$GITHUB_OUTPUT"
+            fi
           else
             echo "matrix=" >> "$GITHUB_OUTPUT"
           fi
🤖 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/reusable-testing.yml around lines 436 - 465, Update the
matrix-generation step around the jq filter so it emits an empty GITHUB_OUTPUT
value when the filtered `.include` array has no entries. Preserve the existing
matrix output for non-empty results and the current FILE_EXISTS/DOCS_ONLY
handling.

476-509: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Same empty-include risk as the unit matrix step.

This step also emits {"include":[]} when the filter removes every entry, which fails the functional job. Apply the same guard as in the Set unit test matrix step.

🤖 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/reusable-testing.yml around lines 476 - 509, The matrix
generation in the functional testing step must handle filters that remove every
entry. Update the jq pipeline around the matrix output to apply the same
empty-include guard used by the “Set unit test matrix” step, emitting the
expected empty matrix representation instead of `{"include":[]}` when no entries
remain.
🧹 Nitpick comments (3)
.github/workflows/reusable-testing.yml (1)

511-525: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low value

Consider passing secrets explicitly instead of secrets: inherit.

reusable-unit.yml and reusable-functional.yml use only CODECOV_TOKEN. Passing that secret explicitly limits the exposure of every other organization and repository secret to the called workflow. Static analysis flags this line for the same reason.

♻️ Proposed change
     uses: ./.github/workflows/reusable-unit.yml
-    secrets: inherit
+    secrets:
+      CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}
🤖 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/reusable-testing.yml around lines 511 - 525, Replace
secrets: inherit in the unit reusable-workflow invocation with an explicit
CODECOV_TOKEN secret mapping, and apply the same change to the
reusable-functional.yml invocation. Preserve the existing workflow inputs while
limiting each called workflow to only the secret it uses.

Source: Linters/SAST tools

.github/workflows/reusable-unit.yml (1)

56-64: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoff

The weekly Composer cache-suffix step is copied into five jobs. Each copy repeats the same comment block and the same date -u +%Y-%W command. A change to the rotation period requires five edits, and the copies can drift. One option is a small composite action in this repository that the reusable workflows call; another is a single env value derived once per workflow.

  • .github/workflows/reusable-unit.yml#L56-L64: replace the inline step with the shared composite action.
  • .github/workflows/reusable-functional.yml#L140-L143: replace the inline step with the shared composite action.
  • .github/workflows/reusable-code-quality.yml#L64-L66: replace the inline step in the lint job with the shared composite action.
  • .github/workflows/reusable-code-quality.yml#L174-L176: replace the inline step in the phpcs job with the shared composite action.
  • .github/workflows/reusable-code-quality.yml#L228-L230: replace the inline step in the phpstan job with the shared composite action.
🤖 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/reusable-unit.yml around lines 56 - 64, Replace the
duplicated weekly Composer cache-suffix steps with one shared composite action
that produces the same output consumed by ramsey/composer-install. Apply this in
.github/workflows/reusable-unit.yml lines 56-64, reusable-functional.yml lines
140-143, and reusable-code-quality.yml lines 64-66, 174-176, and 228-230;
preserve each job’s existing custom-cache-suffix wiring while removing the
repeated date command and comment blocks.
.github/workflows/reusable-code-quality.yml (1)

134-135: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Pin the gherkin-lint version.

npx --yes gherkin-lint installs the newest published version on every run. A new release can then fail this job across all consuming repositories without a commit here. This contradicts the pinning applied to the actions and to the actionlint image in the same PR.

♻️ Proposed change
       - name: Run linter
-        run: npx --yes gherkin-lint -c "$RUNNER_TEMP/.gherkin-lintrc"
+        run: npx --yes gherkin-lint@3.1.4 -c "$RUNNER_TEMP/.gherkin-lintrc"
🤖 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/reusable-code-quality.yml around lines 134 - 135, Update
the “Run linter” step to invoke an explicitly pinned gherkin-lint version
instead of allowing npx to install the newest release, matching the existing
dependency pinning approach while preserving the current configuration file
usage.
🤖 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.

Inline comments:
In @.github/workflows/reusable-functional.yml:
- Around line 43-46: Update the Ubuntu-specific Install Ghostscript and Change
ImageMagick policy steps to run only on Linux runners, using runner.os rather
than checking inputs.os; additionally require the ImageMagick policy file to
exist before running the sed modification.

In @.github/workflows/reusable-testing.yml:
- Around line 52-54: Update the fetch-depth expression near the checkout
configuration to use string operands for the conditional result, such as string
equivalents of 0 and 1, so the pull_request and push branches resolve to depth 0
rather than falling through to 1. Preserve depth 1 for all other event types.

In @.github/workflows/sync-workflows.yml:
- Line 20: Update the on.push.paths entry for
.github/workflows/sync-workflows.yml to align with FILE_PATTERNS: remove the
self-trigger path to prevent unsynchronizable runs, unless self-synchronization
is explicitly intended, in which case add the same file to FILE_PATTERNS.

---

Outside diff comments:
In @.github/workflows/reusable-testing.yml:
- Around line 436-465: Update the matrix-generation step around the jq filter so
it emits an empty GITHUB_OUTPUT value when the filtered `.include` array has no
entries. Preserve the existing matrix output for non-empty results and the
current FILE_EXISTS/DOCS_ONLY handling.
- Around line 476-509: The matrix generation in the functional testing step must
handle filters that remove every entry. Update the jq pipeline around the matrix
output to apply the same empty-include guard used by the “Set unit test matrix”
step, emitting the expected empty matrix representation instead of
`{"include":[]}` when no entries remain.

---

Nitpick comments:
In @.github/workflows/reusable-code-quality.yml:
- Around line 134-135: Update the “Run linter” step to invoke an explicitly
pinned gherkin-lint version instead of allowing npx to install the newest
release, matching the existing dependency pinning approach while preserving the
current configuration file usage.

In @.github/workflows/reusable-testing.yml:
- Around line 511-525: Replace secrets: inherit in the unit reusable-workflow
invocation with an explicit CODECOV_TOKEN secret mapping, and apply the same
change to the reusable-functional.yml invocation. Preserve the existing workflow
inputs while limiting each called workflow to only the secret it uses.

In @.github/workflows/reusable-unit.yml:
- Around line 56-64: Replace the duplicated weekly Composer cache-suffix steps
with one shared composite action that produces the same output consumed by
ramsey/composer-install. Apply this in .github/workflows/reusable-unit.yml lines
56-64, reusable-functional.yml lines 140-143, and reusable-code-quality.yml
lines 64-66, 174-176, and 228-230; preserve each job’s existing
custom-cache-suffix wiring while removing the repeated date command and comment
blocks.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 8b7dcb93-ca5b-4f0c-bcdb-1e79398f6445

📥 Commits

Reviewing files that changed from the base of the PR and between 1c5b66a and 5b96c48.

📒 Files selected for processing (5)
  • .github/workflows/reusable-code-quality.yml
  • .github/workflows/reusable-functional.yml
  • .github/workflows/reusable-testing.yml
  • .github/workflows/reusable-unit.yml
  • .github/workflows/sync-workflows.yml

Comment thread .github/workflows/reusable-functional.yml
Comment thread .github/workflows/reusable-testing.yml Outdated
Comment thread .github/workflows/sync-workflows.yml

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the .github automation to prevent overlapping workflow-sync runs and to reduce unnecessary CI work by tightening triggers, adding concurrency controls, and introducing additional workflow hardening (timeouts, runner selection, caching adjustments) across reusable workflows.

Changes:

  • Serialize the sync-workflows workflow (concurrency group) and reduce its cron schedule to daily; add a push paths filter.
  • Add/adjust concurrency grouping and matrix preparation logic in reusable testing, including a “docs-only change” fast-path.
  • Add runner/timeout/caching/logging tweaks across reusable unit/functional/code-quality workflows (e.g., configurable runner via RUNNERS_NAME, weekly Composer cache suffix rotation, added job timeouts, pinned actionlint image digest).

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 8 comments.

Show a summary per file
File Description
.github/workflows/sync-workflows.yml Adds concurrency + timeouts; lowers cron frequency and adds push paths filtering to avoid overlapping sync runs.
.github/workflows/reusable-unit.yml Adds runner-selection fallback, timeouts, checkout progress toggle, and weekly Composer cache suffix rotation.
.github/workflows/reusable-testing.yml Refactors matrix preparation, adds docs-only detection, and updates concurrency grouping and job naming.
.github/workflows/reusable-functional.yml Adds runner-selection fallback, timeouts, checkout progress toggle, Ghostscript install optimization, and weekly Composer cache suffix rotation.
.github/workflows/reusable-code-quality.yml Adds timeouts, checkout progress toggle, curl hardening, node-version pinning, actionlint image digest pinning, and weekly Composer cache suffix rotation.
Suppressed comments (5)

.github/workflows/reusable-code-quality.yml:56

  • runner.debug is already a truthy/falsey context value; comparing it to the string '1' can cause this to always evaluate to false depending on how GitHub renders the context. If the intent is “only show checkout progress in debug”, you can rely on runner.debug directly.
          show-progress: ${{ runner.debug == '1' && 'true' || 'false' }}

.github/workflows/reusable-code-quality.yml:122

  • runner.debug is already a truthy/falsey context value; comparing it to the string '1' can cause this to always evaluate to false depending on how GitHub renders the context. If the intent is “only show checkout progress in debug”, you can rely on runner.debug directly.
          show-progress: ${{ runner.debug == '1' && 'true' || 'false' }}

.github/workflows/reusable-code-quality.yml:146

  • runner.debug is already a truthy/falsey context value; comparing it to the string '1' can cause this to always evaluate to false depending on how GitHub renders the context. If the intent is “only show checkout progress in debug”, you can rely on runner.debug directly.
          show-progress: ${{ runner.debug == '1' && 'true' || 'false' }}

.github/workflows/reusable-code-quality.yml:166

  • runner.debug is already a truthy/falsey context value; comparing it to the string '1' can cause this to always evaluate to false depending on how GitHub renders the context. If the intent is “only show checkout progress in debug”, you can rely on runner.debug directly.
          show-progress: ${{ runner.debug == '1' && 'true' || 'false' }}

.github/workflows/reusable-code-quality.yml:220

  • runner.debug is already a truthy/falsey context value; comparing it to the string '1' can cause this to always evaluate to false depending on how GitHub renders the context. If the intent is “only show checkout progress in debug”, you can rely on runner.debug directly.
          show-progress: ${{ runner.debug == '1' && 'true' || 'false' }}

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread .github/workflows/sync-workflows.yml Outdated
Comment thread .github/workflows/sync-workflows.yml Outdated
Comment thread .github/workflows/reusable-testing.yml Outdated
Comment thread .github/workflows/reusable-unit.yml
Comment thread .github/workflows/reusable-functional.yml
Comment thread .github/workflows/reusable-code-quality.yml
Comment thread .github/workflows/reusable-testing.yml
Comment thread .github/workflows/reusable-unit.yml
Three fixes from review:

* `fetch-depth` was always 1. In GitHub expressions `A && 0 || 1` yields 1 even
  when A holds, because 0 is falsy and the `||` falls through. Pull request and
  push runs were therefore checking out a shallow clone, the diff against the
  base failed, and the documentation-only detection silently fell back to
  testing everything on every run. Quote the operands so the true branch is
  truthy. Confirmed against zizmor's unsound-ternary audit, which flags the old
  form and accepts the new one.

* The Ubuntu-only steps keyed off `inputs.os` being empty, which no longer
  implies ubuntu-22.04 now that `RUNNERS_NAME` can select the image. Key them on
  `runner.os` instead, and skip the ImageMagick policy edit when the file is
  absent — `sed -i` on a missing file exits non-zero and would fail the leg.

* Group the concurrency key on `github.ref` rather than `github.sha`. The event
  name in the key is already what keeps a push to the default branch and the
  nightly schedule apart, so keying on the commit only prevented a new push from
  superseding the run it replaces.

Also raise the sync timeouts from 20 to 60 minutes. They are meant to bound a
pathological run rather than pace a healthy one, and the sync is idempotent, so
a run cut short is completed by the next push or the daily schedule.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Jy4dmjymj9VmoTBaqrV4iX
swissspidy pushed a commit to wp-cli/automated-tests that referenced this pull request Aug 7, 2026
Carries over a review fix from wp-cli/.github#274, which introduced the same
expression. The event name in the key is already what keeps a push to the
default branch and the nightly schedule apart, so keying on the commit only
prevented a new push from superseding the run it replaces — the opposite of
what a 155-job fan-out wants.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Jy4dmjymj9VmoTBaqrV4iX
@swissspidy swissspidy changed the title Stop the workflow sync from overlapping itself Reduce CI cost and latency across the reusable workflows Aug 7, 2026

Copy link
Copy Markdown
Member Author

Went through all eleven review threads. Six fixed, five declined with reasons below.

Fixed

fetch-depth always evaluated to 1 — correct, and the most serious of the batch. A && 0 || 1 yields 1 even when A holds, because 0 is falsy and the || falls through. Pull request and push runs were checking out a shallow clone, the diff against the base failed, and the documentation-only detection silently fell back to testing everything. The feature was dead code. Operands are now quoted; verified against zizmor's unsound-ternary audit, which flags the old form and accepts the new one.

RUNNERS_NAME could select an image where the Ubuntu-specific steps break — correct. An empty os input no longer implies ubuntu-22.04. Both steps now key on runner.os == 'Linux', and the ImageMagick policy edit is skipped when the file is absent, since sed -i on a missing file exits non-zero.

Concurrency keyed on github.sha prevented pushes from superseding each other — correct. The event name in the key is already what separates a push to the default branch from the nightly schedule, so keying on the commit only stopped a new push from cancelling the run it replaces. Now keyed on github.ref. The same expression appeared in wp-cli/automated-tests#69 and has been corrected there too.

Sync timeouts too low (both jobs) — raised from 20 to 60 minutes. They exist to bound a pathological run rather than pace a healthy one. Worth noting the sync is idempotent, so a run cut short is completed by the next push or the daily schedule.

PR description did not reflect the scope — fair, the description was the first commit message. Rewritten to cover the whole change.

Declined

runner.debug == '1' should rely on runner.debug directly (four threads) — the comparison is correct as written. runner.debug is documented as "set only if debug logging is enabled, and always has the value of 1" — a string, not a boolean, and unset otherwise. runner.debug == '1' is the canonical form; wordpress-develop uses this exact expression in 28 places. Leaving as is.

Remove sync-workflows.yml from its own paths filter — keeping it, deliberately. TARGET_REPOS and FILE_PATTERNS both live in that file, so editing it to add a repository or a new synced file is precisely when an immediate sync is wanted. Dropping the path would delay that by up to a day.


Generated by Claude Code

@swissspidy
swissspidy merged commit d6520f3 into main Aug 7, 2026
12 checks passed
@swissspidy
swissspidy deleted the claude/wp-cli-actions-optimization-mozclo branch August 7, 2026 08:51
swissspidy pushed a commit that referenced this pull request Aug 7, 2026
Addresses findings from a security review of this repository. These workflows
are consumed by ~45 repositories in the organization, so changes here apply
org-wide.

Rebased onto #274, which independently pinned actionlint by digest and added
`--fail` to the two curl calls in the code quality workflow. Those parts of the
review are covered by that change and are dropped here; #274's actionlint pin
resolves to the linux/amd64 manifest of v1.7.11, which is correct for the
GitHub-hosted runners these jobs use.

Add SECURITY.md
  This repository provides the organization's default community health files,
  but had no security policy, so no wp-cli repository surfaced a "Report a
  vulnerability" path. Points at the WP-CLI handbook and the WordPress
  HackerOne program rather than restating policy that lives elsewhere.

Scope `actions: write` to the job that needs it
  It was declared at workflow level, so it also reached `triage-new-item`,
  which runs on `pull_request_target` and processes pull request titles and
  bodies written by anyone who can open a PR. `actions: write` permits
  dispatching workflows and deleting caches and artifacts, a known
  lateral-movement path. Only `triage-unlabeled-items` needs it, and that job
  is `workflow_dispatch`-only.

  The grant has to stay in the caller, since a caller can only cap a reusable
  workflow's permissions and never raise them; removing it there is what broke
  dispatching in #271 and prompted the revert in #272. Comments in both files
  record this so the next attempt narrows rather than removes.

Pin gherkin-lint
  `npx --yes gherkin-lint` resolved and executed the newest publish on every
  run. The package was last released in December 2023 and has two maintainers,
  so a single account compromise would reach CI in every repository.

Correct the actions/setup-node pin comment
  The pinned SHA is v7.0.0, but the trailing comment read v6. The pin itself is
  immutable and current; the comment is what reviewers read, so a wrong one
  quietly defeats the point of the convention.

Fail the WP-CLI download loudly
  `curl -O` without `-f` writes the error body to the file and still exits 0,
  so an outage installed an HTML page as /usr/local/bin/wp and surfaced as a
  confusing failure much later.

Reduce blast radius of the workflow sync
  SKIP_DELETE stops a pattern change from deleting files across every target
  repository. The FILE_PATTERNS regexes now escape their dots so they match
  only the intended paths. Both jobs drop to `contents: read`: the sync
  authenticates with ACTIONS_BOT, and the action reads that token from its
  `with:` input and makes no API calls, so GITHUB_TOKEN is never used at all.
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.

3 participants