👷 ci: add PR quality gates, commit message enforcement, and QA ICU fix#4515
Merged
👷 ci: add PR quality gates, commit message enforcement, and QA ICU fix#4515
Conversation
Agent-Logs-Url: https://github.com/matecat/MateCat/sessions/fd68bba9-e500-4dc8-acc5-ca3dc0ec9264 Co-authored-by: Ostico <8008416+Ostico@users.noreply.github.com>
Agent-Logs-Url: https://github.com/matecat/MateCat/sessions/fd68bba9-e500-4dc8-acc5-ca3dc0ec9264 Co-authored-by: Ostico <8008416+Ostico@users.noreply.github.com>
- regenerate phpstan-baseline.neon to absorb 726 pre-existing @throws violations across 275 files - add phpstan-throws-backlog.txt listing all files to fix sequentially
- create `.github/PULL_REQUEST_TEMPLATE.md` to standardize pull request descriptions - add `.github/workflows/pr-readiness-check.yml` GitHub Action to validate mandatory checklist - implement checklist validation logic in `.github/scripts/pr-readiness-check.js` - include comprehensive unit tests in `.github/scripts/pr-readiness-check.test.js` Signed-off-by: domenico <domenico@translated.net>
- update instructions to only require naming the AI agent/tool used - adjust example to remove implementation + test generation details 🐛 fix(pr-checks): ensure comments are fully removed in section validation - implement loop to repeatedly strip nested HTML comments in `pr-readiness-check.js` 🔧 chore(tests): exclude `.github/scripts` from jest transformations - update `jest.config.js` to add `.github/scripts/` to `transformIgnorePatterns` Signed-off-by: domenico <domenico@translated.net>
- add testPathIgnorePatterns to jest.config.js so Jest does not pick up CI validation scripts that use Node built-in test runner
- add lib/View/APIDoc.php and lib/View/templates/_APIDoc.php to excludePaths in phpstan.neon (webpack build artifacts) - remove 5 stale baseline entries referencing excluded files
- APIDoc.php is a webpack build artifact that may not exist in CI - append (?) to its excludePaths entry so PHPStan tolerates absence
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
- validate PR title (required gate) and commit messages (hygiene gate) against emoji-prefixed conventional-commit spec - add commit-message-check.js with emoji↔type map, VS16 normalization, format/length/case/period validation, merge-commit skip - add 35 unit tests covering all 12 types, edge cases, and error collection - bypass via `commit-message-exception` label
- add requireEmoji option (default true) to validateCommitMessage so PR title stays strict while commit messages accept emoji-less conventional-commit format (e.g. "docs(qa): ...") - auto-skip GitHub web UI patterns (Update/Create/Delete/Rename) in relaxed mode - update workflow to pass requireEmoji: false for commit-messages job - add 18 new tests for relaxed mode and isGitHubWebEdit (53 total)
Contributor
There was a problem hiding this comment.
Pull request overview
This PR strengthens PR/commit quality gates in CI and fixes ICU MessageFormat validation during project creation so ICU inconsistencies are caught earlier.
Changes:
- Add ICU detection to
QAProcessor(gated by project ICU metadata) and share ICU detection logic viaICUSourceSegmentDetector. - Introduce PR template + GitHub Actions workflows for PR readiness checklist enforcement and conventional-commit validation (PR title + commits).
- Harden PHPStan configuration for
@throwscorrectness and adjust Jest config to ignore.github/scripts.
Reviewed changes
Copilot reviewed 15 out of 17 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/Model/ProjectCreation/TestableQAProcessor.php | Update test subclass to capture ICU comparator/flag passed into createQA(). |
| tests/unit/Model/ProjectCreation/QAProcessorTest.php | Add unit tests covering ICU-enabled/disabled paths and a broken-target validation case. |
| lib/Utils/LQA/ICUSourceSegmentDetector.php | New shared utility to decide whether a source segment contains valid ICU patterns. |
| lib/Utils/LQA/ICUSourceSegmentChecker.php | Refactor to delegate ICU detection to the shared detector. |
| lib/Model/ProjectCreation/QAProcessor.php | Add ICU detection/comparator wiring into QA creation during project creation processing. |
| lib/Model/ProjectCreation/ProjectManager.php | Pass project ICU-enabled metadata into QAProcessor. |
| phpstan.neon | Enable stricter @throws checks and exclude API doc artifacts. |
| phpstan-throws-backlog.txt | Add tracking list for incremental @throws cleanup work. |
| jest.config.js | Ignore .github/ paths for Jest test discovery/transforming. |
| .github/workflows/pr-readiness-check.yml | Add workflow to enforce PR template checklist + run node tests for the validator. |
| .github/workflows/commit-message-check.yml | Add workflow to enforce conventional-commit PR titles + validate commit subjects. |
| .github/scripts/pr-readiness-check.js | Implement PR checklist validator logic. |
| .github/scripts/pr-readiness-check.test.js | Add node:test coverage for PR checklist validator. |
| .github/scripts/commit-message-check.js | Implement conventional-commit validator with emoji/type mapping and relaxed mode. |
| .github/scripts/commit-message-check.test.js | Add node:test coverage for commit message validator. |
| .github/PULL_REQUEST_TEMPLATE.md | Add structured PR checklist template used by the readiness gate. |
…i/pr-quality-gates
# Conflicts: # phpstan.neon
| - name: Setup Node | ||
| uses: actions/setup-node@v4 | ||
| with: | ||
| node-version: '24' |
There was a problem hiding this comment.
Pinning actions/setup-node to node-version: '24' may make this workflow brittle if that runtime isn’t available on all GitHub-hosted runners yet. If the scripts don’t require Node 24-specific features, consider using an LTS selector (e.g. lts/*) or the repo’s standard Node version to reduce CI breakage risk.
Suggested change
| node-version: '24' | |
| node-version: 'lts/*' |
…callers Declare checked exceptions on all methods using Database::transaction(), which re-throws \Throwable on rollback. Remove 2 stale baseline entries now covered by the broader @throws declaration.
Ostico
force-pushed
the
ci/pr-quality-gates
branch
from
a7c768b to
8c40af6
Compare
- run PHPUnit with Xdebug coverage on pull requests - invoke ostico/test-guard@v1.0.1 with 80% coverage threshold - enable AI-powered test adequacy analysis
Comment on lines
+12
to
+14
| pull-requests: write | ||
| checks: write | ||
| models: read |
There was a problem hiding this comment.
Workflow-level permissions are widened to pull-requests: write, checks: write, and models: read for all runs (including pushes). Since job-level permissions in _ci-cd.yml already scope most jobs to contents: read, consider keeping the workflow-level ceiling minimal (e.g., contents: read) and moving the elevated permissions to a separate PR-only workflow (or otherwise ensuring only PR runs can ever inherit them). This reduces the blast radius if new jobs are added later without explicit permissions:.
Suggested change
| pull-requests: write | |
| checks: write | |
| models: read |
- Add Clover reporter to Jest so it writes js-coverage/clover.xml - Extract both PHP and JS coverage from test container - Pass both files to test-guard via YAML block scalar
🧪 Test-Guard Report✅ PASS — All changed source files have adequate test coverage. Coverage Analysis: ❌ FAILNo changed source files found in coverage report (threshold: 80%)
Test File Matching: ❌ FAILFile matching: 2 pass, 1 warning, 1 fail
Per-File Evaluation: ✅ PASSEvaluated 4 files: 2 via AI (2 batches), 2 via shortcuts.
Result: ✅ PASS |
riccio82
approved these changes
Apr 28, 2026
Ostico
added a commit
that referenced
this pull request
Apr 28, 2026
👷 ci: add PR quality gates, commit message enforcement, and QA ICU fix
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
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
Add PR quality gates (template + CI enforcement), conventional-commit message checks, and PHPStan hardening.
The QA ICU bug fix that was originally bundled here has been merged separately via PR #4513.
Type
feat— new user-facing featurefix— bug fixrefactor— restructure without behavior changechore— build, deps, config, docsperf— performance improvementtest— test coverageChanges
.github/PULL_REQUEST_TEMPLATE.md.github/workflows/pr-readiness-check.yml.github/scripts/pr-readiness-check.js.github/scripts/pr-readiness-check.test.js.github/workflows/commit-message-check.yml.github/scripts/commit-message-check.jsrequireEmojioption (53 tests).github/scripts/commit-message-check.test.jsphpstan.neon@throwschecking rules, excludeAPIDoc.phpbuild artifact with(?)phpstan-baseline.neon@throwsrulesphpstan-throws-backlog.txt@throwsPHPStan backlog itemsjest.config.js.github/from Jest test discoveryTesting
vendor/bin/phpunit --exclude-group=ExternalServices --no-coveragepasses./vendor/bin/phpstanpasses (0 errors, with baseline)AI Disclosure
Sisyphus (Claude claude-opus-4-6), GitHub Copilot
Notes
This PR was originally combined with the QA ICU bug fix for locked segments. That fix has been merged independently via PR #4513. This PR now contains only CI/infrastructure changes.