Consolidate linting infrastructure with reusable workflows - #47
Merged
Conversation
- Refactored `.php-cs-fixer.config.php` for improved directory handling. - Updated `.php-stan.config.neon` comment for clarity. - Modified `composer.json` to streamline linting commands. - Added new scripts for lint checking and formatting. Co-authored-by: Cursor Agent <cursoragent@cursor.com>
… linting - Deleted `.php-cs-fixer.config.php` and `.php-stan.config.neon` files from multiple packages. - Updated `composer.json` in each package to replace linting commands with new script references. - Removed unused development dependencies related to PHP CS Fixer and PHPStan. Co-authored-by: Cursor Agent <cursoragent@cursor.com>
This is an optimization change due to changes on linting configuration, which migrated from per-package config files to root-level ones. Co-authored-by: Cursor Agent <cursoragent@cursor.com>
Co-authored-by: Cursor Agent <cursoragent@cursor.com>
- Updated PHPStan configuration to exclude vendor paths. - Refactored linting commands in composer.json files across multiple packages to include package names. - Improved linting scripts to handle package directories and resolve paths more effectively. Co-authored-by: Cursor Agent <cursoragent@cursor.com>
- Standardized the `lint:ci` command in multiple package `composer.json` files to use `--dry-run` options. - Enhanced the linting scripts to support a new `dryRun` parameter for better control during lint checks and formatting. Co-authored-by: Cursor Agent <cursoragent@cursor.com>
- Updated `actions/checkout` from v6.0.2 to v6.0.3 in multiple workflows for improved performance and security. - Upgraded `toshimaru/auto-author-assign` from v3.0.1 to v3.0.2 for better author assignment functionality. - Updated `webfactory/ssh-agent` from v0.9.1 to v0.10.0 to leverage new features and fixes.
- Introduced `.lint.yml` and `.test.yml` workflows to standardize linting and testing processes across packages. - Updated `ci.yml` to utilize these new workflows, improving maintainability and consistency in CI processes. - Configured inputs for PHP versions and package names to enhance flexibility in running jobs. Co-authored-by: Cursor Agent <cursoragent@cursor.com>
- Replaced `php run lint` with `composer run lint:ci` for consistency with the new CI workflows. - Updated `php run test` to `composer run test` to align with the standardized testing process.
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds monorepo helpers and CLI lint scripts, centralizes lint/test into reusable GitHub workflows, removes per-package lint wrappers, refactors staged linting to use shared utilities, bumps workflow action pins, and adds a release workflow plus a release-note generator. ChangesMonorepo Linting & CI
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
4 tasks
coderabbitai
Bot
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scripts/lint-staged.php (1)
46-66:⚠️ Potential issue | 🟠 Major | ⚡ Quick winissue (blocking): Return a non-zero status when PhpStan fails.
This branch logs the PhpStan failure and then still exits
0after running php-cs-fixer, so the hook passes even when static analysis found blocking errors.Suggested fix
$phpStanResult = $this->runPhpStan($phpFiles); if ($phpStanResult !== 0) { - echo "❌ PhpStan found issues. Fixing with php-cs-fixer...\n"; + echo "❌ PhpStan found issues on staged files.\n"; + return $phpStanResult; } echo "\n🔧 Running php-cs-fixer on staged files...\n";🤖 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 `@scripts/lint-staged.php` around lines 46 - 66, The script currently logs PhpStan failures but still returns 0; update the control flow so that when runPhpStan($phpFiles) yields a non-zero $phpStanResult the hook ultimately returns a non-zero exit code (e.g., return $phpStanResult or 1) instead of returning 0. Concretely, after calling runPhpStan($phpFiles) and optionally running runPhpCsFixer($phpFiles) and addFilesToStaging($phpFiles), ensure the function checks $phpStanResult and returns a non-zero status when it failed; keep runPhpCsFixer and adding fixed files but do not override the failing exit code from runPhpStan.
🤖 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/.lint.yml:
- Around line 36-40: The lint workflow unnecessarily enables Xdebug via the
shivammathur/setup-php action (see uses: shivammathur/setup-php@... and
coverage: xdebug), which adds overhead but no coverage is collected; update the
workflow by disabling coverage for the lint matrix—either remove the coverage:
xdebug key or set it to none—so the setup-php step for the matrix.php-version
runs without enabling Xdebug.
In @.github/workflows/.test.yml:
- Around line 44-48: The workflow currently enables Xdebug via the setup-php
step (coverage: xdebug) even though the job only runs composer test and does not
publish coverage; remove or conditionally disable the coverage setting in the
shivammathur/setup-php action (the block containing php-version, extensions,
coverage) so Xdebug is not installed by default — e.g., delete or change
coverage: xdebug to not set coverage (or set to none) unless a
coverage-publishing job is added, ensuring the setup-php invocation no longer
forces Xdebug across the test matrix.
- Around line 24-27: Update the reusable workflow secret contract so the
api-token secret is required by changing the secrets.api-token.required setting
to true (so dispatch fails early when API_TOKEN is missing) and remove the
coverage: xdebug option from the shivammathur/setup-php action block in
.github/workflows/.test.yml since this workflow runs composer test (phpunit)
without enabling coverage; search for the secrets.api-token key and the
shivammathur/setup-php step to apply these two edits.
In @.github/workflows/license-update.yml:
- Line 18: The actions/checkout step currently uses
actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 (v6.0.3) and relies on
the default persist-credentials: true; update both checkout occurrences to
explicitly set persist-credentials: false to avoid leaving write-capable
credentials in the job environment (i.e., add persist-credentials: false under
each actions/checkout step so later git commands won’t inherit the
GITHUB_TOKEN).
In @.github/workflows/subtrees-sync.yml:
- Line 18: Update the pinned commit for actions/checkout@v6.0.3 to the correct
SHA: replace the incorrect df4cb1c069e1874edd31b4311f1884172cec0e10 with
9f698171ed81b15d1823a05fc7211befd50c8ae0 where actions/checkout@v6.0.3 is used
(also verify and update the other occurrence of the same pin noted in the diff);
leave the webfactory/ssh-agent@v0.10.0 pin as-is since its SHA already matches.
In `@scripts/helpers.php`:
- Around line 146-175: split_script_arguments currently forwards every
dash-prefixed argument, allowing callers to override important flags; fix this
by restricting forwarded tool flags to an allowlist inside lint_tool_options:
update lint_tool_options to define an allowlist array of safe long/short flags
(e.g. exclude --config, --configuration, -c, --rules, --generate-baseline,
etc.), filter $splitArguments['options'] against that allowlist (only keep
allowed flags), then proceed with the existing dry-run merging/unique logic and
return the filtered list; reference the split_script_arguments and
lint_tool_options symbols to locate where to apply the change.
- Around line 178-257: resolve_lint_paths() currently defaults only to
packages/* and package_directory_for_absolute()/group_lint_paths_by_package()
reject anything outside packages/, which prevents linting repo-root files
(scripts/, config files). Update resolve_lint_paths to include monorepo_root()
(or a distinct "root" target like Path::join(monorepo_root()) or '') when no
path arguments are provided; modify package_directory_for_absolute to return
monorepo_root() (or a stable "root" directory value) for paths that are inside
the repo but not under packages/ instead of null; and change
group_lint_paths_by_package to accept that root directory as a valid group (do
not exit when package_directory is the monorepo root), mapping repo-root
relative paths into that group so scripts/* and root config files are included
in lint runs.
In `@scripts/lint-staged.php`:
- Around line 129-136: The addFilesToStaging method currently catches
RuntimeException from run_git(['add', $file]) and only echoes a warning,
allowing the script to continue and incorrectly report success; change it to
propagate failures by either rethrowing the caught RuntimeException or tracking
any failed files and exiting non-zero after the loop. Specifically update
addFilesToStaging to (a) call run_git(['add', $file]) inside the try block and
on catch either throw $exception; or mark the file as failed and after
iterating, if any failures exist call exit(1) or throw a RuntimeException so the
caller does not report "Fixed files added to the staging area." Ensure
references to addFilesToStaging and run_git are used so the change is applied in
the correct location.
- Around line 85-91: The current check skips any staged file when
package_directory_for($file) returns null, which unintentionally excludes
root-level PHP files (e.g., scripts/*.php and repo config files); update the
conditional in scripts/lint-staged.php so that files are only skipped when
package_directory_for($file) === null AND the file is not a PHP file (use
pathinfo($file, PATHINFO_EXTENSION) === 'php') and not in known root folders
(e.g., check strpos($file, 'scripts/') !== 0 or match config filenames). In
short, change the early continue so PHP files and root scripts/config files are
allowed through (using the existing package_directory_for($file) call and the
$file variable to locate the affected logic).
---
Outside diff comments:
In `@scripts/lint-staged.php`:
- Around line 46-66: The script currently logs PhpStan failures but still
returns 0; update the control flow so that when runPhpStan($phpFiles) yields a
non-zero $phpStanResult the hook ultimately returns a non-zero exit code (e.g.,
return $phpStanResult or 1) instead of returning 0. Concretely, after calling
runPhpStan($phpFiles) and optionally running runPhpCsFixer($phpFiles) and
addFilesToStaging($phpFiles), ensure the function checks $phpStanResult and
returns a non-zero status when it failed; keep runPhpCsFixer and adding fixed
files but do not override the failing exit code from runPhpStan.
🪄 Autofix (Beta)
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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2c58200b-1d75-4f5a-93aa-8d8f648128b5
⛔ Files ignored due to path filters (1)
composer.lockis excluded by!**/*.lock
📒 Files selected for processing (54)
.captainhook.config.json.github/workflows/.lint.yml.github/workflows/.test.yml.github/workflows/ci.yml.github/workflows/license-update.yml.github/workflows/pr-author-assign.yml.github/workflows/subtrees-sync.yml.github/workflows/subtrees-tags-sync.yml.php-cs-fixer.config.php.php-stan.config.neoncomposer.jsonpackages/br-utils/.php-cs-fixer.config.phppackages/br-utils/.php-stan.config.neonpackages/br-utils/composer.jsonpackages/cnpj-dv/.php-cs-fixer.config.phppackages/cnpj-dv/.php-stan.config.neonpackages/cnpj-dv/composer.jsonpackages/cnpj-fmt/.php-cs-fixer.config.phppackages/cnpj-fmt/.php-stan.config.neonpackages/cnpj-fmt/composer.jsonpackages/cnpj-gen/.php-cs-fixer.config.phppackages/cnpj-gen/.php-stan.config.neonpackages/cnpj-gen/composer.jsonpackages/cnpj-utils/.php-cs-fixer.config.phppackages/cnpj-utils/.php-stan.config.neonpackages/cnpj-utils/composer.jsonpackages/cnpj-val/.php-cs-fixer.config.phppackages/cnpj-val/.php-stan.config.neonpackages/cnpj-val/composer.jsonpackages/cpf-dv/.php-cs-fixer.config.phppackages/cpf-dv/.php-stan.config.neonpackages/cpf-dv/composer.jsonpackages/cpf-fmt/.php-cs-fixer.config.phppackages/cpf-fmt/.php-stan.config.neonpackages/cpf-fmt/composer.jsonpackages/cpf-gen/.php-cs-fixer.config.phppackages/cpf-gen/.php-stan.config.neonpackages/cpf-gen/composer.jsonpackages/cpf-utils/.php-cs-fixer.config.phppackages/cpf-utils/.php-stan.config.neonpackages/cpf-utils/composer.jsonpackages/cpf-val/.php-cs-fixer.config.phppackages/cpf-val/.php-stan.config.neonpackages/cpf-val/composer.jsonpackages/utils/.php-cs-fixer.config.phppackages/utils/.php-stan.config.neonpackages/utils/composer.jsonscripts/helpers.phpscripts/lint-check.phpscripts/lint-format.phpscripts/lint-staged.phpscripts/phpstan-staged.phpscripts/phpunit-sync-configs.phpscripts/phpunit.common.xml
💤 Files with no reviewable changes (26)
- packages/cnpj-fmt/.php-stan.config.neon
- packages/utils/.php-stan.config.neon
- packages/cnpj-gen/.php-cs-fixer.config.php
- packages/cnpj-fmt/.php-cs-fixer.config.php
- packages/br-utils/.php-cs-fixer.config.php
- packages/cnpj-utils/.php-cs-fixer.config.php
- packages/cnpj-val/.php-cs-fixer.config.php
- packages/cnpj-dv/.php-cs-fixer.config.php
- packages/cpf-gen/.php-cs-fixer.config.php
- scripts/phpunit-sync-configs.php
- packages/cpf-dv/.php-cs-fixer.config.php
- packages/cnpj-utils/.php-stan.config.neon
- packages/cpf-val/.php-cs-fixer.config.php
- packages/cnpj-gen/.php-stan.config.neon
- packages/cpf-utils/.php-stan.config.neon
- packages/cpf-val/.php-stan.config.neon
- packages/utils/.php-cs-fixer.config.php
- packages/cpf-utils/.php-cs-fixer.config.php
- packages/br-utils/.php-stan.config.neon
- packages/cpf-fmt/.php-cs-fixer.config.php
- packages/cpf-dv/.php-stan.config.neon
- packages/cnpj-val/.php-stan.config.neon
- packages/cnpj-dv/.php-stan.config.neon
- packages/cpf-fmt/.php-stan.config.neon
- packages/cpf-gen/.php-stan.config.neon
- scripts/phpunit.common.xml
4 tasks
FIx as per @coderabbitai review comment at #47 (comment). Co-authored-by: CodeRabbit AI <noreply@coderabbit.ai>
juliolmuller
added a commit
to LacusSolutions/br-utils-js
that referenced
this pull request
Jun 3, 2026
Adjustment as per @coderabbitai review comment at LacusSolutions/br-utils-php#47 (comment). Co-authored-by: CodeRabbit AI <noreply@coderabbit.ai>
juliolmuller
added a commit
to LacusSolutions/br-utils-py
that referenced
this pull request
Jun 3, 2026
Adjustment as per @coderabbitai review comment at LacusSolutions/br-utils-php#47 (comment). Co-authored-by: CodeRabbit AI <noreply@coderabbit.ai>
Co-authored-by: Cursor Agent <cursoragent@cursor.com>
Co-authored-by: Cursor Agent <cursoragent@cursor.com>
Contributor
There was a problem hiding this comment.
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 (1)
composer.json (1)
13-73:⚠️ Potential issue | 🟠 Major | ⚡ Quick winissue (blocking): Missing root
testscript breaks pre-push and root-level test invocations.
.captainhook.config.jsonrunscomposer run teston pre-push, but this file only definestest:*andtest:all. Without atestalias, pre-push will fail before running package tests.Proposed fix
"scripts": { @@ + "test": "`@test`:all", "test:all": [ "`@test`:formatters", "`@test`:generators", "`@test`:validators", "`@test`:utils" ] },🤖 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 `@composer.json` around lines 13 - 73, The root composer scripts lack a "test" entry so captainhook's pre-push which runs "composer run test" fails; add a top-level "test" script to composer.json that forwards to the existing umbrella script (e.g., "test": "`@test`:all") so that "composer run test" runs the defined "`@test`:all" sequence (verify the change touches the "scripts" object where "test:all" and other "test:*" entries are defined).
🤖 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/release.yml:
- Around line 39-47: The prepare-release job currently runs without a needs
dependency so it can execute in parallel with lint and test; modify the workflow
to add needs: [lint, test] (referencing the prepare-release job name) so
prepare-release waits for both the lint and test jobs to succeed before running;
update any downstream expectations if needed but the primary change is adding
the needs dependency to the prepare-release job to prevent releases/tags from
being created when checks fail.
- Around line 55-67: Don't interpolate `${{ inputs.package }}` and `${{
inputs.version }}` directly into the `run:` script; instead set them as
environment variables and reference those safe vars inside the script. Add an
`env:` mapping (e.g., `PKG: ${{ inputs.package }}` and `VERSION: ${{
inputs.version }}`) for the step that runs `php scripts/release.php`, then in
the script use the existing `PKG` and `VERSION` shell variables (quoted as
`"$PKG"` and `"$VERSION"`) and test `if [ -n "$VERSION" ]` to build
`OUTPUT_PATH=$(php scripts/release.php "$PKG" --version "$VERSION")` or fallback
to `OUTPUT_PATH=$(php scripts/release.php "$PKG")`, keeping `set -euo pipefail`
and proper quoting to prevent command injection.
- Around line 24-37: Update the reusable-workflow inputs and job dependencies:
change the php-versions input for the lint and test jobs to use a literal, vars,
or workflow input (not env) so expressions evaluate correctly (modify the with:
php-versions used by the lint and test jobs); add prepare-release to the needs:
list for the create-release job (or stop referencing needs.prepare-release.*) so
outputs/env lookups use an actual dependency; add needs dependencies so
prepare-release runs after lint and test to gate the release; and in the
prepare-release job, stop interpolating inputs directly into run: shell
commands—pass inputs.package and inputs.version via env: or validate/sanitize
them before use to avoid shell injection.
---
Outside diff comments:
In `@composer.json`:
- Around line 13-73: The root composer scripts lack a "test" entry so
captainhook's pre-push which runs "composer run test" fails; add a top-level
"test" script to composer.json that forwards to the existing umbrella script
(e.g., "test": "`@test`:all") so that "composer run test" runs the defined
"`@test`:all" sequence (verify the change touches the "scripts" object where
"test:all" and other "test:*" entries are defined).
🪄 Autofix (Beta)
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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: f7911bc7-bde3-4516-bb73-52c333d551a3
📒 Files selected for processing (8)
.github/workflows/.lint.yml.github/workflows/.test.yml.github/workflows/license-update.yml.github/workflows/release.yml.github/workflows/subtrees-tags-sync.yml.gitignorecomposer.jsonscripts/release.php
💤 Files with no reviewable changes (1)
- .github/workflows/subtrees-tags-sync.yml
Adjustment as per @coderabbitai review comment at #47 (comment). Co-authored-by: CodeRabbit AI <noreply@coderabbit.ai>
Fix as per @coderabbitai review comment at #47 (comment). Co-authored-by: CodeRabbit AI <noreply@coderabbit.ai>
Fix as per @coderabbitai review comment at #47 (comment). Co-authored-by: CodeRabbit AI <noreply@coderabbit.ai> Co-authored-by: Cursor Agent <cursoragent@cursor.com>
…ram is passed along Fix as per @coderabbitai review comment at #47 (comment). Co-authored-by: CodeRabbit AI <noreply@coderabbit.ai> Co-authored-by: Cursor Agent <cursoragent@cursor.com>
Fix as per @coderabbitai review comment at #47 (comment). Co-authored-by: CodeRabbit AI <noreply@coderabbit.ai> Co-authored-by: Cursor Agent <cursoragent@cursor.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
.github/workflows/release.yml (3)
28-28:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winissue (blocking):
envcontext unavailable in reusable workflowwith:block.This will fail with "context 'env' is not allowed here" since
envis not among the allowed contexts (github,inputs,matrix,needs,strategy,vars) atjobs.<job>.with.Proposed fix
lint: uses: ./.github/workflows/.lint.yml with: packages: ${{ format('["{0}"]', inputs.package) }} - php-versions: ${{ format('["{0}"]', env.PHP_VERSION) }} + php-versions: '["8.2"]' test: uses: ./.github/workflows/.test.yml with: packages: ${{ format('["{0}"]', inputs.package) }} - php-versions: ${{ format('["{0}"]', env.PHP_VERSION) }} + php-versions: '["8.2"]'Also applies to: 34-34
🤖 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/release.yml at line 28, The reusable-workflow `with:` block is using an unsupported context `env` for `php-versions`; change it to an allowed context (e.g. `inputs` or `vars`) and update callers accordingly: replace uses like php-versions: ${{ format('["{0}"]', env.PHP_VERSION) }} with php-versions: ${{ format('["{0}"]', inputs.PHP_VERSION) }} (or vars.PHP_VERSION) in the `with` block of the reusable workflow and ensure the workflow caller passes `PHP_VERSION` via `with`/inputs (repeat the same fix for the other occurrence). Ensure the reusable workflow declares the corresponding input (name `PHP_VERSION`) so callers can provide the value.
55-68:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winissue (blocking): Shell injection risk from direct input interpolation.
${{ inputs.package }}and${{ inputs.version }}are interpolated directly into the shell script, allowing command injection via crafted input values.Proposed fix
- id: release + env: + INPUT_PACKAGE: ${{ inputs.package }} + INPUT_VERSION: ${{ inputs.version }} run: | set -euo pipefail - PKG="${{ inputs.package }}" + PKG="$INPUT_PACKAGE" echo "📦 Preparing release for package '$PKG'..." - if [ -n "${{ inputs.version }}" ]; then - echo "📌 Using requested version: ${{ inputs.version }}" - OUTPUT_PATH=$(php scripts/release.php "$PKG" --version "${{ inputs.version }}") + if [ -n "$INPUT_VERSION" ]; then + echo "📌 Using requested version: $INPUT_VERSION" + OUTPUT_PATH=$(php scripts/release.php "$PKG" --version "$INPUT_VERSION") else🤖 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/release.yml around lines 55 - 68, The script directly interpolates `${{ inputs.package }}` and `${{ inputs.version }}` into the shell, enabling shell injection; fix by treating inputs as data: capture them into safely-sanitized variables (e.g. PKG and VERSION) using printf/sed or a strict whitelist regex to strip/validate any characters other than expected (letters, digits, dot, underscore, hyphen), and always quote expansions and pass them as separate arguments to php (e.g. PKG="$(printf '%s' '...sanitized...' )"; VERSION="$(printf '%s' '...sanitized...' )"; then call php scripts/release.php "$PKG" --version "$VERSION" or php scripts/release.php "$PKG" when VERSION is empty); reference the PKG, VERSION, OUTPUT_PATH variables and the php scripts/release.php invocation when applying the changes.
161-163:⚠️ Potential issue | 🟠 Major | ⚡ Quick winissue (blocking): Release not gated on
lint/testjob success.The
needsfix correctly referencesprepare-release, butcreate-release(and downstreamsync-tag) will proceed even iflintortestfail since neither is in any dependency chain. Running lint/test jobs that don't gate the release provides false confidence without enforcement.Proposed fix
create-release: name: Create GitHub Release needs: + - lint + - test - prepare-release - validate🤖 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/release.yml around lines 161 - 163, The create-release (and downstream sync-tag) job is not gated on lint/test; update the job dependencies so create-release needs both lint and test (in addition to prepare-release/validate) and ensure sync-tag inherits that chain (or explicitly needs create-release); modify the needs list for the create-release job (and sync-tag if present) to include the unique job names "lint" and "test" so failures in those jobs will block the release flow.
🤖 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 @.php-cs-fixer.config.php:
- Around line 8-27: The php-cs-fixer cache file is currently rooted at the
package working directory because $cacheFile is built from $vendorDir which uses
getcwd(); change it so the Finder still uses the package-scoped $dir/$vendorDir
for search/exclude but the cache is placed under the monorepo root vendor
directory instead. Introduce a new $rootVendorDir = __DIR__ .
DIRECTORY_SEPARATOR . 'vendor' . DIRECTORY_SEPARATOR and set $cacheFile to
"{$rootVendorDir}.php-cs-fixer.cache" while leaving $vendorDir and the
Finder::create()->in($searchDirs) logic unchanged.
---
Duplicate comments:
In @.github/workflows/release.yml:
- Line 28: The reusable-workflow `with:` block is using an unsupported context
`env` for `php-versions`; change it to an allowed context (e.g. `inputs` or
`vars`) and update callers accordingly: replace uses like php-versions: ${{
format('["{0}"]', env.PHP_VERSION) }} with php-versions: ${{ format('["{0}"]',
inputs.PHP_VERSION) }} (or vars.PHP_VERSION) in the `with` block of the reusable
workflow and ensure the workflow caller passes `PHP_VERSION` via `with`/inputs
(repeat the same fix for the other occurrence). Ensure the reusable workflow
declares the corresponding input (name `PHP_VERSION`) so callers can provide the
value.
- Around line 55-68: The script directly interpolates `${{ inputs.package }}`
and `${{ inputs.version }}` into the shell, enabling shell injection; fix by
treating inputs as data: capture them into safely-sanitized variables (e.g. PKG
and VERSION) using printf/sed or a strict whitelist regex to strip/validate any
characters other than expected (letters, digits, dot, underscore, hyphen), and
always quote expansions and pass them as separate arguments to php (e.g.
PKG="$(printf '%s' '...sanitized...' )"; VERSION="$(printf '%s'
'...sanitized...' )"; then call php scripts/release.php "$PKG" --version
"$VERSION" or php scripts/release.php "$PKG" when VERSION is empty); reference
the PKG, VERSION, OUTPUT_PATH variables and the php scripts/release.php
invocation when applying the changes.
- Around line 161-163: The create-release (and downstream sync-tag) job is not
gated on lint/test; update the job dependencies so create-release needs both
lint and test (in addition to prepare-release/validate) and ensure sync-tag
inherits that chain (or explicitly needs create-release); modify the needs list
for the create-release job (and sync-tag if present) to include the unique job
names "lint" and "test" so failures in those jobs will block the release flow.
🪄 Autofix (Beta)
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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: effbcf50-0700-4133-b117-19c50d38868d
📒 Files selected for processing (5)
.github/workflows/license-update.yml.github/workflows/release.yml.php-cs-fixer.config.phpscripts/helpers.phpscripts/lint-staged.php
Fix as per @coderabbitai review comment at #47 (comment). Co-authored-by: CodeRabbit AI <noreply@coderabbit.ai> Co-authored-by: Cursor Agent <cursoragent@cursor.com>
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
scripts/lint-staged.php (1)
48-72:⚠️ Potential issue | 🟠 Major | ⚡ Quick winissue (blocking): This hook can exit successfully even when PhpStan or php-cs-fixer fails.
run()never propagates a non-zero$phpStanResult, andrunPhpCsFixer()downgrades fixer failures tocontinue. A staged file can therefore fail analysis or fail formatting altogether, yet the script still returns0and lets the commit proceed as if the staged set was validated.Also applies to: 92-128
🤖 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 `@scripts/lint-staged.php` around lines 48 - 72, The run() flow currently swallows failures: ensure non-zero results from runPhpStan() and runPhpCsFixer() propagate to a non-zero exit; specifically, after calling runPhpStan($phpFiles) check if $phpStanResult !== 0 and immediately return that non-zero code (or return 1) instead of just echoing and continuing, and modify runPhpCsFixer($phpFiles) handling so that if it returns a failure status (not just "hasChanges" boolean or an internal downgrade), run() returns a non-zero exit code; update both places where php-cs-fixer is invoked (the block shown and the similar block around 92-128) to treat fixer failures as fatal (return 1) while preserving the existing addFilesToStaging($phpFiles) error handling.scripts/helpers.php (1)
238-247:⚠️ Potential issue | 🟠 Major | ⚡ Quick winissue (blocking): Default lint runs still skip PHP files that live at the repo or package root.
resolve_lint_paths([])seeds package roots, butnormalize_package_lint_paths()immediately narrows empty scopes tosrc/tests(orscriptsfor the monorepo root). That means normal lint/check runs still miss root-level PHP config files like.php-cs-fixer.config.phpand package-level config scripts such aspackages/cpf-dv/.php-cs-fixer.config.phpunless they are passed explicitly.Also applies to: 336-359
🤖 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 `@scripts/helpers.php` around lines 238 - 247, The current resolve_lint_paths() + normalize_package_lint_paths() flow narrows an empty scope to subdirs (like src/tests or scripts) and thus skips root-level PHP files; fix by ensuring package roots and the repo root are included when no explicit paths are passed. Specifically, in resolve_lint_paths() keep seeding the package root paths (Path::join('packages', $name)) and also add the package root itself (i.e., the package directory without a suffix) and the monorepo root ('.' or 'scripts' as appropriate) to the returned list so normalize_package_lint_paths() does not drop root-level files; apply the same change to the duplicate logic at the other occurrence referenced (lines ~336-359) so root-level config files are always covered.
🤖 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.
Outside diff comments:
In `@scripts/helpers.php`:
- Around line 238-247: The current resolve_lint_paths() +
normalize_package_lint_paths() flow narrows an empty scope to subdirs (like
src/tests or scripts) and thus skips root-level PHP files; fix by ensuring
package roots and the repo root are included when no explicit paths are passed.
Specifically, in resolve_lint_paths() keep seeding the package root paths
(Path::join('packages', $name)) and also add the package root itself (i.e., the
package directory without a suffix) and the monorepo root ('.' or 'scripts' as
appropriate) to the returned list so normalize_package_lint_paths() does not
drop root-level files; apply the same change to the duplicate logic at the other
occurrence referenced (lines ~336-359) so root-level config files are always
covered.
In `@scripts/lint-staged.php`:
- Around line 48-72: The run() flow currently swallows failures: ensure non-zero
results from runPhpStan() and runPhpCsFixer() propagate to a non-zero exit;
specifically, after calling runPhpStan($phpFiles) check if $phpStanResult !== 0
and immediately return that non-zero code (or return 1) instead of just echoing
and continuing, and modify runPhpCsFixer($phpFiles) handling so that if it
returns a failure status (not just "hasChanges" boolean or an internal
downgrade), run() returns a non-zero exit code; update both places where
php-cs-fixer is invoked (the block shown and the similar block around 92-128) to
treat fixer failures as fatal (return 1) while preserving the existing
addFilesToStaging($phpFiles) error handling.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: cf560d65-34a1-4db8-8b18-e4093de038bb
📒 Files selected for processing (7)
.php-cs-fixer.config.phpscripts/helpers.phpscripts/lint-check.phpscripts/lint-format.phpscripts/lint-staged.phpscripts/phpstan-staged.phpscripts/release.php
…bles Fix as per @coderabbitai review comment at #47 (comment). Co-authored-by: CodeRabbit AI <noreply@coderabbit.ai>
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/release.yml (1)
261-267:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon't force-push subtree version tags.
This job never checks whether
refs/tags/$PACKAGE_VERSIONalready exists on the subtree remote, andgit push ... --forcewill rewrite it if it does. That makes published package tags mutable and can silently repoint consumers to a different commit.Suggested guard
- git tag "$PACKAGE_VERSION" "refs/tags/$TAG_NAME" -f + if git ls-remote --exit-code --tags "$REMOTE_URL" "refs/tags/$PACKAGE_VERSION" >/dev/null 2>&1; then + echo "❌ Remote tag '$PACKAGE_VERSION' already exists in $REMOTE_URL" + exit 1 + fi + + git tag "$PACKAGE_VERSION" "refs/tags/$TAG_NAME" @@ - git push "remote-repo-${PACKAGE_NAME}" "refs/tags/$PACKAGE_VERSION" --force + git push "remote-repo-${PACKAGE_NAME}" "refs/tags/$PACKAGE_VERSION"🤖 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/release.yml around lines 261 - 267, The workflow is force-pushing subtree tags (git push ... --force) which can rewrite published tags; change the logic around the tag push to first check the remote for an existing ref for refs/tags/$PACKAGE_VERSION (use git ls-remote or similar against remote-repo-${PACKAGE_NAME}), and if the tag already exists either skip pushing or fail the job instead of using --force; remove the --force flag from the git push of refs/tags/$PACKAGE_VERSION so published tags remain immutable and only push when the remote tag is absent.
🤖 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.
Outside diff comments:
In @.github/workflows/release.yml:
- Around line 261-267: The workflow is force-pushing subtree tags (git push ...
--force) which can rewrite published tags; change the logic around the tag push
to first check the remote for an existing ref for refs/tags/$PACKAGE_VERSION
(use git ls-remote or similar against remote-repo-${PACKAGE_NAME}), and if the
tag already exists either skip pushing or fail the job instead of using --force;
remove the --force flag from the git push of refs/tags/$PACKAGE_VERSION so
published tags remain immutable and only push when the remote tag is absent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: a908705c-5080-44f3-afa0-73091b02bae1
📒 Files selected for processing (3)
.github/workflows/.lint.yml.github/workflows/.test.yml.github/workflows/release.yml
juliolmuller
added a commit
to LacusSolutions/br-utils-js
that referenced
this pull request
Jun 10, 2026
Adjustment as per @coderabbitai review comment at LacusSolutions/br-utils-php#47 (comment). Co-authored-by: CodeRabbit AI <noreply@coderabbit.ai>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Chores
Style