Modernize tooling and update dependencies #144
Conversation
- Update all production dependencies to latest versions - @actions/core 1.2.6 → 2.0.2 - @actions/github 5.1.1 → 7.0.0 - picomatch 2.2.2 → 4.0.3 - Remove unused @octokit/webhooks dependency - Migrate from Jest to Node.js native test runner - Migrate from @vercel/ncc to esbuild for bundling - Migrate ESLint to v9 flat config - Add comprehensive unit tests for main modules - Add smoke test workflow with PHP fixture - Fix ESM compatibility issues (import.meta.dirname) - Bundle output changed from dist/index.js to dist/index.cjs Co-Authored-By: Claude Opus 4.5 <[email protected]>
📝 WalkthroughWalkthroughThis PR modernizes toolchain and CI: migrates ESLint to flat config, switches tests from Jest to Node's experimental runner, enables ESM, updates TypeScript to ES2022, adds test fixtures and suites, adjusts GitHub Actions, updates dependencies, and makes small runtime/type refinements in source. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In @.github/workflows/test.yml:
- Around line 47-52: The CI step named "Dump GitHub context" currently runs the
shell builtin "set", which prints all environment variables (potentially
exposing secrets); remove the "set" invocation or gate it behind a debug
conditional so it only runs when a safe debug flag is enabled, and if you must
log context limit output to the already-populated GITHUB_CONTEXT variable (or
whitelist specific non-sensitive fields) instead of dumping the entire
environment; update that step to either delete "set" or wrap it with a
conditional check for a new DEBUG_DUMP_ENV (false by default) and ensure secrets
are filtered before any output.
- Around line 20-43: The smoke-test step currently runs the "Smoke test bundle"
action against an intentionally invalid fixture "__fixtures__/bad-code.php",
which will cause the smoke-test job to fail; update the "smoke-test" job by
either swapping the fixture for a valid file or marking the "Smoke test bundle"
step (named "Smoke test bundle") with continue-on-error and add a follow-up
assertion step that explicitly checks the action exit status or output to assert
the expected failure, so CI remains green while still verifying the action
reports an error for the bad fixture.
- Around line 59-62: Update the "Install PHP_CodeSniffer" workflow step to
download PHPCS from the official PHAR repository (use
https://phars.phpcodesniffer.com/phpcs.phar instead of the current URL), also
download the matching .asc signature file, import or ensure the GPG key with
fingerprint D91D86963AF3A29B6520462297B02DD8E5071466 is available, and run gpg
--verify against the .asc before executing the phar; alternatively,
conditionally use GitHub attestation verification (gh attestation verify
phpcs.phar -o PHPCSStandards) for PHPCS 3.10.3+ and fail the step if
verification does not succeed.
In `@package.json`:
- Around line 28-30: The action manifest's runtime is mismatched with
package.json: update the Action YAML so the runner uses Node 22+ by changing the
runs.using value (the runs.using field in action.yml) from 'node20' to 'node22'
(or higher) to match engines.node >= 22.6.0 in package.json and ensure
compatibility with features like --experimental-strip-types; locate the
runs.using entry in action.yml and replace its value accordingly and re-run any
build/pack steps to regenerate the action bundle if required.
In `@src/get-changed-file.ts`:
- Around line 14-28: The getChangedFiles function assumes payload.pull_request
exists; add an explicit runtime guard at the start of getChangedFiles to check
that payload.pull_request is defined (and has base and head) and throw a clear
Error (or return a rejected Promise) if not, e.g. validate the
PullRequestPayload payload variable before using payload.pull_request.base.sha
so non-PR events (push) fail fast with a descriptive message instead of silently
returning empty results.
In `@src/main.ts`:
- Around line 20-23: The current use of import.meta.dirname in main.ts (used to
build matchersPath and print the phpcs matcher path via matchersPath and the
console log) will fail on Node 20.0–20.10 which lack import.meta.dirname;
replace it with a safe fallback: import fileURLToPath and compute dirname from
fileURLToPath(import.meta.url) when import.meta.dirname is undefined, then use
that computed directory to build matchersPath and the console output. Ensure the
logic references import.meta.dirname first and falls back to
fileURLToPath(import.meta.url) so the matchersPath and the `##[add-matcher]` log
continue to work across all Node 20.x versions.
🧹 Nitpick comments (4)
__fixtures__/github.ts (1)
20-22: Keepcontext.shain sync when payload is replaced.Tests that call
setPayload()could end up with a stalecontext.sha. Consider updating it from the PR head SHA when available.♻️ Suggested tweak
export function setPayload(payload: typeof context.payload): void { context.payload = payload; + const headSha = payload?.pull_request?.head?.sha; + if (headSha) context.sha = headSha; }package.json (1)
13-14: Experimental flags may change behavior in future Node.js versions.The test script relies on
--experimental-strip-types,--experimental-test-module-mocks, and--experimental-test-coverage. These experimental features may change or be removed in future Node.js releases. Consider documenting this in the README or adding a note about the Node.js version requirement.__tests__/main.test.ts (2)
13-14: Consider removing unused parameters from mock definitions.The
_filesparameters trigger ESLint warnings. Sincemock.fn()doesn't require matching the exact signature, you can simplify these.Suggested fix
-const runOnCompleteFilesMock = mock.fn((_files: string[]) => 0); -const runOnBlameMock = mock.fn(async (_files: string[]) => {}); +const runOnCompleteFilesMock = mock.fn(() => 0); +const runOnBlameMock = mock.fn(async () => {});
92-92: Same unused parameter pattern in mock implementation.Consider removing the unused
_filesparameter here as well for consistency.Suggested fix
- runOnCompleteFilesMock.mock.mockImplementation((_files: string[]) => 0); + runOnCompleteFilesMock.mock.mockImplementation(() => 0);
| - name: Dump GitHub context | ||
| env: | ||
| GITHUB_CONTEXT: ${{ toJson(github) }} | ||
| run: | | ||
| echo "$GITHUB_CONTEXT" | ||
| set |
There was a problem hiding this comment.
Avoid dumping full environment in CI logs.
set prints all env vars, which can include sensitive values or PII. Please remove or guard this behind a debug flag and limit output to non-sensitive fields.
🧹 Safer logging
- - name: Dump GitHub context
- env:
- GITHUB_CONTEXT: ${{ toJson(github) }}
- run: |
- echo "$GITHUB_CONTEXT"
- set
+ - name: Dump GitHub context (debug only)
+ if: ${{ env.DEBUG_LOGS == 'true' }}
+ env:
+ GITHUB_CONTEXT: ${{ toJson(github) }}
+ run: echo "$GITHUB_CONTEXT"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Dump GitHub context | |
| env: | |
| GITHUB_CONTEXT: ${{ toJson(github) }} | |
| run: | | |
| echo "$GITHUB_CONTEXT" | |
| set | |
| - name: Dump GitHub context (debug only) | |
| if: ${{ env.DEBUG_LOGS == 'true' }} | |
| env: | |
| GITHUB_CONTEXT: ${{ toJson(github) }} | |
| run: echo "$GITHUB_CONTEXT" |
🤖 Prompt for AI Agents
In @.github/workflows/test.yml around lines 47 - 52, The CI step named "Dump
GitHub context" currently runs the shell builtin "set", which prints all
environment variables (potentially exposing secrets); remove the "set"
invocation or gate it behind a debug conditional so it only runs when a safe
debug flag is enabled, and if you must log context limit output to the
already-populated GITHUB_CONTEXT variable (or whitelist specific non-sensitive
fields) instead of dumping the entire environment; update that step to either
delete "set" or wrap it with a conditional check for a new DEBUG_DUMP_ENV (false
by default) and ensure secrets are filtered before any output.
| - name: Install PHP_CodeSniffer | ||
| run: | | ||
| curl -OL https://squizlabs.github.io/PHP_CodeSniffer/phpcs.phar | ||
| php phpcs.phar --version |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
PHP_CodeSniffer official download verification checksum signature
💡 Result:
Short answer — official PHPCS PHARs are signed. Verify them by checking the GPG .asc signature (or using GitHub artifact attestations).
Steps (example for phpcs.phar)
-
Download PHAR and signature:
- curl -OL https://phars.phpcodesniffer.com/phpcs.phar
- curl -OL https://phars.phpcodesniffer.com/phpcs.phar.asc. [1]
-
Import the project release key (fingerprint shown on the project pages):
- key fingerprint: D91D86963AF3A29B6520462297B02DD8E5071466. [2]
- import from a keyserver or vendor copy: gpg --keyserver hkps://keyserver.ubuntu.com --recv-keys D91D86963AF3A29B6520462297B02DD8E5071466
-
Verify:
- gpg --verify phpcs.phar.asc phpcs.phar
Alternative: for PHAR provenance you can verify GitHub Artifact Attestations with the GitHub CLI (example): gh attestation verify phpcs.phar -o PHPCSStandards. [2]
Sources: PHAR download/signature page and official repo documentation. [1][2]
References
[1] https://phars.phpcodesniffer.com/
[2] PHPCS GitHub / documentation (release GPG fingerprint and gh attestation instructions).
🌐 Web query:
PHPCS squizlabs github release checksums integrity verification
💡 Result:
Short answer — PHPCS PHARs are distributed with cryptographic signatures (not simple plaintext checksums). You can verify provenance two ways:
- GPG signature verification (PHAR .asc)
- Download phpcs.phar and phpcs.phar.asc from the release/site.
- Import the release key (fingerprint shown in the project README: D91D 8696 3AF3 A29B 6520 4622 97B0 2DD8 E507 1466). Verify the .asc signature:
- gpg --keyserver keyserver.ubuntu.com --recv-keys 0xE5071466
- gpg --verify phpcs.phar.asc phpcs.phar.
(README documents the PHAR signing and fingerprint.) [1]
- GitHub Artifact Attestations (recommended for recent releases)
- PHPCS supports verifying PHAR provenance via GitHub Artifact Attestations with the GitHub CLI:
- gh attestation verify phpcs.phar -o PHPCSStandards
- The README / releases mention this attestation support as of PHPCS 3.10.3 and later. [1][2]
References
- PHPCS README (PHAR signing fingerprint + attestation note). [1]
- PHPCS releases page (release assets / attestation mention). [2]
🏁 Script executed:
# Let's check if there are any PHPCS release resources or documentation
curl -s https://api.github.com/repos/squizlabs/PHP_CodeSniffer/releases | head -100Repository: tinovyatkin/action-php-codesniffer
Length of output: 212
Use correct download URL and verify PHPCS integrity with GPG signature.
Downloading .phar files without verifying integrity poses a supply-chain risk. PHPCS provides official GPG-signed releases; use the correct URL and verify the signature before execution.
Update the download to:
- Use the official PHAR repository:
https://phars.phpcodesniffer.com/phpcs.phar - Download the corresponding
.ascsignature file - Verify the signature using the official GPG key (fingerprint:
D91D86963AF3A29B6520462297B02DD8E5071466)
Example with GPG signature verification
- name: Install PHP_CodeSniffer
run: |
- curl -OL https://squizlabs.github.io/PHP_CodeSniffer/phpcs.phar
+ curl -OL https://phars.phpcodesniffer.com/phpcs.phar
+ curl -OL https://phars.phpcodesniffer.com/phpcs.phar.asc
+ gpg --keyserver hkps://keyserver.ubuntu.com --recv-keys D91D86963AF3A29B6520462297B02DD8E5071466
+ gpg --verify phpcs.phar.asc phpcs.phar
php phpcs.phar --versionAlternatively, for PHPCS 3.10.3+, use GitHub Artifact Attestations: gh attestation verify phpcs.phar -o PHPCSStandards.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Install PHP_CodeSniffer | |
| run: | | |
| curl -OL https://squizlabs.github.io/PHP_CodeSniffer/phpcs.phar | |
| php phpcs.phar --version | |
| - name: Install PHP_CodeSniffer | |
| run: | | |
| curl -OL https://phars.phpcodesniffer.com/phpcs.phar | |
| curl -OL https://phars.phpcodesniffer.com/phpcs.phar.asc | |
| gpg --keyserver hkps://keyserver.ubuntu.com --recv-keys D91D86963AF3A29B6520462297B02DD8E5071466 | |
| gpg --verify phpcs.phar.asc phpcs.phar | |
| php phpcs.phar --version |
🤖 Prompt for AI Agents
In @.github/workflows/test.yml around lines 59 - 62, Update the "Install
PHP_CodeSniffer" workflow step to download PHPCS from the official PHAR
repository (use https://phars.phpcodesniffer.com/phpcs.phar instead of the
current URL), also download the matching .asc signature file, import or ensure
the GPG key with fingerprint D91D86963AF3A29B6520462297B02DD8E5071466 is
available, and run gpg --verify against the .asc before executing the phar;
alternatively, conditionally use GitHub attestation verification (gh attestation
verify phpcs.phar -o PHPCSStandards) for PHPCS 3.10.3+ and fail the step if
verification does not succeed.
| "engines": { | ||
| "node": ">=22.6.0" | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
GitHub Actions runners Node.js versions 2025 2026 available node20 node22
💡 Result:
Short answer: GitHub-hosted runners included Node.js 20 and 22 through 2025, and GitHub began adding Node.js 24 and migrating defaults in late‑2025/early‑2026. GitHub announced deprecation of Node 20 on Actions runners (Node 20 EOL April 2026); runners started supporting Node 24 in fall 2025, and Node 24 becomes the default on March 4, 2026 (Node 20 removal planned later summer 2026). You can still pin or choose Node 20/22/24 via actions/setup-node or the runner images' cached node builds. [1][2][3][4]
Sources:
- GitHub Changelog: deprecation of Node 20 / Node 24 migration timeline. [1]
- actions/runner-images: default Node 20 rollout (June 2025) and related issues. [2]
- actions/node-versions (node builds / versions-manifest used by runners). [3]
- actions/setup-node docs (how to select/pin Node versions in workflows). [4]
🏁 Script executed:
find . -maxdepth 2 -name "action.y*ml" -o -name "action.json" 2>/dev/null | head -20Repository: tinovyatkin/action-php-codesniffer
Length of output: 90
🏁 Script executed:
cat -n ./action.ymlRepository: tinovyatkin/action-php-codesniffer
Length of output: 1212
Update action.yml to specify Node 22+ runtime—current node20 runtime is incompatible with package.json requirement.
The action manifest specifies runs.using: 'node20' but package.json requires engines.node >= 22.6.0. This creates a critical mismatch: the bundled code uses features that require Node 22.6.0+ (notably the --experimental-strip-types flag), so the action will fail when GitHub Actions executes it with Node 20. Update action.yml line 25 to using: 'node22' or higher to match the actual minimum requirement.
🤖 Prompt for AI Agents
In `@package.json` around lines 28 - 30, The action manifest's runtime is
mismatched with package.json: update the Action YAML so the runner uses Node 22+
by changing the runs.using value (the runs.using field in action.yml) from
'node20' to 'node22' (or higher) to match engines.node >= 22.6.0 in package.json
and ensure compatibility with features like --experimental-strip-types; locate
the runs.using entry in action.yml and replace its value accordingly and re-run
any build/pack steps to regenerate the action bundle if required.
| interface PullRequestPayload { | ||
| pull_request: { | ||
| base: { sha: string }; | ||
| head: { sha: string }; | ||
| }; | ||
| } | ||
|
|
||
| export async function getChangedFiles(): Promise<ChangedFiles> { | ||
| const pattern = core.getInput('files', { | ||
| required: false, | ||
| }); | ||
| const globs = pattern.length ? pattern.split(',') : ['**.php']; | ||
| const isMatch = picomatch(globs); | ||
| console.log('Filter patterns:', globs, isMatch('src/test.php')); | ||
| const payload = github.context.payload as Webhooks.WebhookPayloadPullRequest; | ||
| const payload = github.context.payload as unknown as PullRequestPayload; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/get-changed-file.tsRepository: tinovyatkin/action-php-codesniffer
Length of output: 2784
🏁 Script executed:
find . -type f \( -name "action.yml" -o -name "*.md" -o -name ".github" \) | head -20Repository: tinovyatkin/action-php-codesniffer
Length of output: 102
🏁 Script executed:
cat -n action.yml 2>/dev/null || cat -n action.yaml 2>/dev/null || echo "No action.yml found"Repository: tinovyatkin/action-php-codesniffer
Length of output: 1212
🏁 Script executed:
find .github -type f -name "*.yml" -o -name "*.yaml" 2>/dev/null | head -10Repository: tinovyatkin/action-php-codesniffer
Length of output: 104
🏁 Script executed:
cat -n .github/workflows/test.ymlRepository: tinovyatkin/action-php-codesniffer
Length of output: 2103
🏁 Script executed:
cat -n README.md | head -50Repository: tinovyatkin/action-php-codesniffer
Length of output: 2321
Add a runtime guard for non‑PR events.
Line 28 assumes pull_request is always present, but the test workflow runs this action on both pull_request and push events. On non-PR triggers, accessing payload.pull_request.base.sha at line 44 will fail silently (returning empty results due to the try-catch). Add an explicit guard with a clear error message instead:
🔧 Suggested guard
- const payload = github.context.payload as unknown as PullRequestPayload;
+ const payload = github.context.payload as unknown as Partial<PullRequestPayload>;
+ if (!payload.pull_request?.base?.sha) {
+ throw new Error('This action must run on pull_request events.');
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| interface PullRequestPayload { | |
| pull_request: { | |
| base: { sha: string }; | |
| head: { sha: string }; | |
| }; | |
| } | |
| export async function getChangedFiles(): Promise<ChangedFiles> { | |
| const pattern = core.getInput('files', { | |
| required: false, | |
| }); | |
| const globs = pattern.length ? pattern.split(',') : ['**.php']; | |
| const isMatch = picomatch(globs); | |
| console.log('Filter patterns:', globs, isMatch('src/test.php')); | |
| const payload = github.context.payload as Webhooks.WebhookPayloadPullRequest; | |
| const payload = github.context.payload as unknown as PullRequestPayload; | |
| interface PullRequestPayload { | |
| pull_request: { | |
| base: { sha: string }; | |
| head: { sha: string }; | |
| }; | |
| } | |
| export async function getChangedFiles(): Promise<ChangedFiles> { | |
| const pattern = core.getInput('files', { | |
| required: false, | |
| }); | |
| const globs = pattern.length ? pattern.split(',') : ['**.php']; | |
| const isMatch = picomatch(globs); | |
| console.log('Filter patterns:', globs, isMatch('src/test.php')); | |
| const payload = github.context.payload as unknown as Partial<PullRequestPayload>; | |
| if (!payload.pull_request?.base?.sha) { | |
| throw new Error('This action must run on pull_request events.'); | |
| } |
🤖 Prompt for AI Agents
In `@src/get-changed-file.ts` around lines 14 - 28, The getChangedFiles function
assumes payload.pull_request exists; add an explicit runtime guard at the start
of getChangedFiles to check that payload.pull_request is defined (and has base
and head) and throw a clear Error (or return a rejected Promise) if not, e.g.
validate the PullRequestPayload payload variable before using
payload.pull_request.base.sha so non-PR events (push) fail fast with a
descriptive message instead of silently returning empty results.
| const matchersPath = path.join(import.meta.dirname, '..', '.github'); | ||
| console.log( | ||
| `##[add-matcher]${path.join(matchersPath, 'phpcs-matcher.json')}` | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Node.js 20 import.meta.dirname support version release notes
💡 Result:
import.meta.dirname (and import.meta.filename) were added in Node.js v20.11.0 (semver-minor) — released 2024-01-09 — and documented as added in v20.11.0 with stability “1.2 - Release candidate”. [1][2]
Sources:
- Node.js v20.11.0 release notes (entry: "esm: add import.meta.dirname and import.meta.filename"). [1]
- Node.js v20 API (ESM) documentation showing Added in: v20.11.0 and stability. [2]
🏁 Script executed:
# Check for workflow files and action.yml that define Node runtime
find .github -type f -name "*.yml" -o -name "*.yaml" | head -20
cat .github/workflows/* 2>/dev/null | grep -i "node\|version" || echo "No workflows found"
test -f action.yml && cat action.yml || echo "No action.yml"Repository: tinovyatkin/action-php-codesniffer
Length of output: 1325
Add fallback for import.meta.dirname to support Node 20.0–20.10.x.
import.meta.dirname was added in Node.js v20.11.0. The action's node20 runtime specification doesn't pin to a specific minor version, so it could include earlier versions without import.meta.dirname support, causing matcher registration to fail. Use fileURLToPath as a fallback for compatibility with all Node 20.x versions:
Recommended fallback
-import * as path from 'node:path';
+import * as path from 'node:path';
+import { fileURLToPath } from 'node:url';
const matchersPath = path.join(
+ path.dirname(fileURLToPath(import.meta.url)),
- import.meta.dirname,
'..',
'.github'
);🤖 Prompt for AI Agents
In `@src/main.ts` around lines 20 - 23, The current use of import.meta.dirname in
main.ts (used to build matchersPath and print the phpcs matcher path via
matchersPath and the console log) will fail on Node 20.0–20.10 which lack
import.meta.dirname; replace it with a safe fallback: import fileURLToPath and
compute dirname from fileURLToPath(import.meta.url) when import.meta.dirname is
undefined, then use that computed directory to build matchersPath and the
console output. Ensure the logic references import.meta.dirname first and falls
back to fileURLToPath(import.meta.url) so the matchersPath and the
`##[add-matcher]` log continue to work across all Node 20.x versions.
The fixture has intentional PSR-12 violations, so the action should fail. Use continue-on-error and assert the failure outcome. Co-Authored-By: Claude Opus 4.5 <[email protected]>
Summary
node --test)import.meta.dirname)dist/index.jstodist/index.cjsTest plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Chores
Tests
✏️ Tip: You can customize this high-level summary in your review settings.