Repository navigation
Support Laravel 12 - #1366
Open
LukeTowers wants to merge 76 commits into
Open
Support Laravel 12#1366LukeTowers wants to merge 76 commits into
LukeTowers wants to merge 76 commits into
Conversation
Contributor
|
@LukeTowers Example of difference for cache key name: This may cause issues if the project shares the cache or session between projects under both versions of WinterCMS. |
austinderrick
added a commit
to austinderrick/winter
that referenced
this pull request
Jun 1, 2026
Builds on the Laravel 12 work (wintercms#1366). Bumps laravel/framework ^12 -> ^13 and PHP ^8.2 -> ^8.3 in the root and modules/{system,backend,cms}. Points winter/storm and the three modules at the Laravel 13 branch (dev-wip/1.3-laravel-13). Adds path repositories for the in-repo modules so composer resolves the Laravel 13 module manifests locally (the published split packages still pin Laravel 12), and a VCS repository for the Storm Laravel 13 branch. CI matrices dropped to PHP 8.3/8.4. No module code changes were required (all system/backend/cms suites pass on Laravel 13).
austinderrick
added a commit
to austinderrick/winter
that referenced
this pull request
Jun 1, 2026
Builds on the Laravel 12 work (wintercms#1366). Bumps laravel/framework ^12 -> ^13 and PHP ^8.2 -> ^8.3 in the root and modules/{system,backend,cms}. Points winter/storm and the three modules at the Laravel 13 branch (dev-wip/1.3-laravel-13). Adds path repositories for the in-repo modules so composer resolves the Laravel 13 module manifests locally (the published split packages still pin Laravel 12), and a VCS repository for the Storm Laravel 13 branch. CI matrices dropped to PHP 8.3/8.4. No module code changes were required (all system/backend/cms suites pass on Laravel 13).
austinderrick
added a commit
to austinderrick/winter
that referenced
this pull request
Jun 1, 2026
Builds on the Laravel 12 work (wintercms#1366). Bumps laravel/framework ^12 -> ^13 and PHP ^8.2 -> ^8.3 in the root and modules/{system,backend,cms}. Points winter/storm and the three modules at the Laravel 13 branch (dev-wip/1.3-laravel-13). Adds path repositories for the in-repo modules so composer resolves the Laravel 13 module manifests locally (the published split packages still pin Laravel 12), and a VCS repository for the Storm Laravel 13 branch. CI matrices dropped to PHP 8.3/8.4. No module code changes were required (all system/backend/cms suites pass on Laravel 13).
# Conflicts: # .github/workflows/tests.yml # .gitignore # modules/backend/controllers/Index.php # modules/system/tests/fixtures/plugins/winter/tester/components/Comments.php
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.github/workflows/manifest.yml (1)
18-18: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winScope the Composer credential and use the supported
setup-phpinput.Line [18] exposes
secrets.COMPOSER_GITHUB_TOKENto every step in the job. GitHub job-level environment variables apply to all steps. (docs.github.com)
shivammathur/setup-php@v2supports thegithub-tokeninput and deprecates theGITHUB_TOKENenvironment-variable authentication path. (github.com) Move the secret to that input, or use step-scopedCOMPOSER_AUTHfor Composer. Composer documentsCOMPOSER_AUTHfor this purpose. (getcomposer.org)Proposed fix
- env: - GITHUB_TOKEN: ${{ secrets.COMPOSER_GITHUB_TOKEN }} ... with: php-version: 8.4 extensions: curl, fileinfo, gd, mbstring, openssl, pdo, pdo_sqlite, sqlite3, xml, zip + github-token: ${{ secrets.COMPOSER_GITHUB_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/manifest.yml at line 18, Remove the job-level GITHUB_TOKEN assignment in the manifest workflow and pass secrets.COMPOSER_GITHUB_TOKEN through the setup-php step’s supported github-token input, or scope it to the Composer step via COMPOSER_AUTH. Ensure the credential is unavailable to unrelated job steps.Source: MCP tools
🤖 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/tests.yml:
- Around line 26-27: Disable persistent checkout credentials in both checkout
steps: add persist-credentials: false to the frontend job checkout at
.github/workflows/tests.yml lines 26-27 and the PHPUnit job checkout at lines
76-77.
In `@modules/system/classes/UpdateManager.php`:
- Around line 138-143: Update the SQLite version threshold in the connection
check around Schema::getConnection() from 3.35 to 3.26.0, preserving rejection
below the Laravel 12 minimum and acceptance at or above it. Add boundary
coverage confirming 3.25.x is rejected and 3.26.0 is accepted.
---
Nitpick comments:
In @.github/workflows/manifest.yml:
- Line 18: Remove the job-level GITHUB_TOKEN assignment in the manifest workflow
and pass secrets.COMPOSER_GITHUB_TOKEN through the setup-php step’s supported
github-token input, or scope it to the Composer step via COMPOSER_AUTH. Ensure
the credential is unavailable to unrelated job steps.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: ad802038-e21e-45bd-9383-89eba97ef876
📒 Files selected for processing (53)
.github/workflows/manifest.yml.github/workflows/tests.yml.gitignorecomposer.jsonconfig/app.phpconfig/hashing.phpmodules/backend/composer.jsonmodules/backend/controllers/Index.phpmodules/backend/phpunit.xmlmodules/backend/tests/classes/AuthManagerTest.phpmodules/backend/tests/classes/ControllerPostbackTest.phpmodules/backend/tests/traits/WidgetMakerTest.phpmodules/cms/classes/ComponentManager.phpmodules/cms/composer.jsonmodules/cms/phpunit.xmlmodules/cms/tests/classes/CmsObjectTest.phpmodules/cms/tests/classes/ControllerPostbackTest.phpmodules/cms/tests/classes/ControllerTest.phpmodules/cms/tests/classes/ThemeTest.phpmodules/system/classes/UpdateManager.phpmodules/system/composer.jsonmodules/system/console/WinterTest.phpmodules/system/console/asset/mix/MixCompile.phpmodules/system/console/asset/mix/MixCreate.phpmodules/system/console/asset/mix/MixInstall.phpmodules/system/console/asset/mix/MixWatch.phpmodules/system/console/asset/npm/NpmInstall.phpmodules/system/console/asset/npm/NpmRun.phpmodules/system/console/asset/npm/NpmUpdate.phpmodules/system/console/asset/npm/NpmVersion.phpmodules/system/console/asset/vite/ViteCompile.phpmodules/system/console/asset/vite/ViteCreate.phpmodules/system/console/asset/vite/ViteInstall.phpmodules/system/console/asset/vite/ViteWatch.phpmodules/system/console/scaffold/test/phpunit.stubmodules/system/phpunit.xmlmodules/system/tests/ServiceProviderTest.phpmodules/system/tests/bootstrap/PluginTestCase.phpmodules/system/tests/bootstrap/TestCase.phpmodules/system/tests/classes/MediaLibraryTest.phpmodules/system/tests/classes/SourceManifestTest.phpmodules/system/tests/classes/VersionManagerTest.phpmodules/system/tests/console/CreateCommandTest.phpmodules/system/tests/console/CreateMigrationTest.phpmodules/system/tests/console/WinterUtilTest.phpmodules/system/tests/console/asset/mix/MixCreateTest.phpmodules/system/tests/console/asset/mix/MixInstallTest.phpmodules/system/tests/console/asset/npm/NpmInstallTest.phpmodules/system/tests/console/asset/npm/NpmUpdateTest.phpmodules/system/tests/console/asset/vite/ViteCreateTest.phpmodules/system/tests/console/asset/vite/ViteInstallTest.phpmodules/system/tests/fixtures/plugins/winter/tester/components/Comments.phpphpunit.xml
💤 Files with no reviewable changes (12)
- modules/system/console/asset/mix/MixWatch.php
- modules/system/console/asset/mix/MixCompile.php
- modules/system/console/asset/mix/MixCreate.php
- modules/system/console/asset/npm/NpmInstall.php
- modules/system/console/asset/vite/ViteCompile.php
- modules/system/console/asset/vite/ViteWatch.php
- modules/system/console/asset/mix/MixInstall.php
- modules/system/console/asset/npm/NpmUpdate.php
- modules/system/console/asset/vite/ViteCreate.php
- modules/system/console/asset/npm/NpmVersion.php
- modules/system/console/asset/npm/NpmRun.php
- modules/system/console/asset/vite/ViteInstall.php
🚧 Files skipped from review as they are similar to previous changes (30)
- modules/system/tests/classes/VersionManagerTest.php
- modules/system/tests/console/asset/npm/NpmUpdateTest.php
- modules/backend/tests/classes/AuthManagerTest.php
- modules/system/tests/console/WinterUtilTest.php
- modules/system/console/scaffold/test/phpunit.stub
- phpunit.xml
- modules/cms/classes/ComponentManager.php
- modules/system/tests/console/asset/vite/ViteInstallTest.php
- modules/cms/tests/classes/CmsObjectTest.php
- modules/system/tests/classes/SourceManifestTest.php
- modules/system/tests/console/asset/vite/ViteCreateTest.php
- modules/backend/controllers/Index.php
- .gitignore
- modules/system/tests/console/CreateCommandTest.php
- modules/system/composer.json
- modules/cms/tests/classes/ControllerTest.php
- modules/system/tests/console/asset/mix/MixCreateTest.php
- modules/system/tests/bootstrap/PluginTestCase.php
- config/hashing.php
- modules/system/tests/fixtures/plugins/winter/tester/components/Comments.php
- modules/backend/phpunit.xml
- modules/system/phpunit.xml
- modules/system/tests/console/asset/mix/MixInstallTest.php
- modules/cms/composer.json
- modules/system/tests/classes/MediaLibraryTest.php
- modules/system/tests/ServiceProviderTest.php
- modules/cms/phpunit.xml
- modules/backend/composer.json
- config/app.php
- modules/system/tests/console/asset/npm/NpmInstallTest.php
Comment on lines
26
to
+27
| - name: Checkout changes | ||
| uses: actions/checkout@v3 | ||
|
|
||
| - name: Setup extension cache | ||
| id: extcache | ||
| uses: shivammathur/cache-extensions@v1 | ||
| with: | ||
| php-version: ${{ env.phpVersion }} | ||
| extensions: ${{ env.extensions }} | ||
| key: ${{ env.key }} | ||
|
|
||
| - name: Cache extensions | ||
| uses: actions/cache@v3 | ||
| with: | ||
| path: ${{ steps.extcache.outputs.dir }} | ||
| key: ${{ steps.extcache.outputs.key }} | ||
| restore-keys: ${{ steps.extcache.outputs.key }} | ||
| uses: actions/checkout@v4 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Disable persistent checkout credentials in both jobs. Both actions/checkout steps leave the workflow token in local Git configuration while later commands execute repository and dependency code.
.github/workflows/tests.yml#L26-L27: addpersist-credentials: falseto the frontend job checkout step..github/workflows/tests.yml#L76-L77: addpersist-credentials: falseto the PHPUnit job checkout step.
Proposed fix
- name: Checkout changes
uses: actions/checkout@v4
+ with:
+ persist-credentials: false📝 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.
Suggested change
| - name: Checkout changes | |
| uses: actions/checkout@v3 | |
| - name: Setup extension cache | |
| id: extcache | |
| uses: shivammathur/cache-extensions@v1 | |
| with: | |
| php-version: ${{ env.phpVersion }} | |
| extensions: ${{ env.extensions }} | |
| key: ${{ env.key }} | |
| - name: Cache extensions | |
| uses: actions/cache@v3 | |
| with: | |
| path: ${{ steps.extcache.outputs.dir }} | |
| key: ${{ steps.extcache.outputs.key }} | |
| restore-keys: ${{ steps.extcache.outputs.key }} | |
| uses: actions/checkout@v4 | |
| - name: Checkout changes | |
| uses: actions/checkout@v4 | |
| with: | |
| persist-credentials: false |
🧰 Tools
🪛 zizmor (1.29.0)
[warning] 26-27: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
📍 Affects 1 file
.github/workflows/tests.yml#L26-L27(this comment).github/workflows/tests.yml#L76-L77
🤖 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/tests.yml around lines 26 - 27, Disable persistent
checkout credentials in both checkout steps: add persist-credentials: false to
the frontend job checkout at .github/workflows/tests.yml lines 26-27 and the
PHPUnit job checkout at lines 76-77.
Source: Linters/SAST tools
Comment on lines
+138
to
+143
| $connection = Schema::getConnection(); | ||
| if ($connection->getDriverName() === 'sqlite') { | ||
| if (version_compare($connection->getServerVersion(), '3.35', '<')) { | ||
| throw new Exception("SQLite version minimum requirement not met (>= 3.35)"); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify Laravel 12's documented SQLite requirement.
curl -fsSL https://laravel.com/docs/12.x/database |
grep -oE 'SQLite 3\.26\.0\+' | head -n 1
# Locate current SQLite-version validation and coverage without executing code.
rg -n -C 4 'getServerVersion|SQLite version minimum|version_compare' \
modules/system/classes/UpdateManager.php modules/system/testsRepository: wintercms/winter
Length of output: 1041
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Repository files matching composer/lock/package manifests:"
git ls-files | rg '(^composer\.json$|^composer\.lock$|composer\..+\.json$|^modules/system/classes/UpdateManager\.php$|^modules/system/tests/.*UpdateManager.*\.php$|UpdateManagerTestCase)'
echo
echo "composer.json require hints:"
python3 - <<'PY'
import json
for path in ['composer.json', 'composer.lock']:
try:
data=json.load(open(path))
except Exception as e:
print(f"{path}: unreadable or invalid JSON ({e})")
continue
if path == 'composer.json':
print(path, data.get('require', {}).get('laravel/framework'), data.get('require-dev', {}).get('laravel/framework'))
else:
for p in data.get('packages', [])+data.get('packages-dev', []):
if p.get('name') in {'laravel/framework', 'laravel/laravel'}:
print(path, p.get('name'), p.get('version'), p.get('require',{}).get('ext-sqlite3'))
PY
echo
echo "UpdateManager relevant section:"
sed -n '1,190p' modules/system/classes/UpdateManager.php
echo
echo "Search for SQLite version tests/references:"
rg -n -C 3 '3\.35|3\.26|SQLite version|getServerVersion|version_compare' modules tests composer.json composer.lock || true
echo
echo "SQLite version_compare behavior for relevant boundary inputs:"
python3 - <<'PY'
def cmp_like_php(a,b):
import re
def parse(v):
pts=re.split(r'[-.]+', str(v))
out=[]
for i,v in enumerate(pts):
if i>=3: break
out.append(int(v))
while len(out)<3:
out.append(0)
return tuple(out)
x=parse(a); y=parse(b)
return 1 if x>y else (-1 if x<y else 0)
for v in ['3.25.9', '3.26.0', '3.34.9', '3.35.0', '3.35.1']:
print(f"{v} compared to 3.35: {cmp_like_php(v,'3.35')} ({"< 3.35" if cmp_like_php(v,'3.35') == -1 else ">= 3.35"})")
PYRepository: wintercms/winter
Length of output: 28534
Use the Laravel 12 SQLite minimum version.
This check rejects supported SQLite 3.26.0 through 3.34.x installations before migrations run. Laravel 12 documents SQLite 3.26.0+; change version_compare(..., '3.35', '<') to version_compare(..., '3.26.0', '<') and add boundary coverage for 3.25.x and 3.26.0.
🤖 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 `@modules/system/classes/UpdateManager.php` around lines 138 - 143, Update the
SQLite version threshold in the connection check around Schema::getConnection()
from 3.35 to 3.26.0, preserving rejection below the Laravel 12 minimum and
acceptance at or above it. Add boundary coverage confirming 3.25.x is rejected
and 3.26.0 is accepted.
Co-authored-by: Claude Opus 4.8 (1M context) <[email protected]>
austinderrick
added a commit
to austinderrick/winter
that referenced
this pull request
Aug 19, 2026
Builds on the Laravel 12 work (wintercms#1366). Bumps laravel/framework ^12 -> ^13 and PHP ^8.2 -> ^8.3 in the root and modules/{system,backend,cms}. Points winter/storm and the three modules at the Laravel 13 branch (dev-wip/1.3-laravel-13). Adds path repositories for the in-repo modules so composer resolves the Laravel 13 module manifests locally (the published split packages still pin Laravel 12), and a VCS repository for the Storm Laravel 13 branch. CI matrices dropped to PHP 8.3/8.4. No module code changes were required (all system/backend/cms suites pass on Laravel 13).
Add missing language keys for #1510
Added missing French translations to match English file.
Contributor
|
We will run this branch on our project and come back with our findings soon 👍 |
wip/1.3: issues found while porting a production projectFound while porting a large Winter 1.2 project to Storm
Docs
Without a PR
Dropped because nothing in core triggers them: translation overrides across several lang paths ( 🤖 Generated with Claude Code |
Member
|
@JonasPardon can you add a real link to the commit you described as "73aefbe" for the quote removal in default values ? |
@mjauvin Yes, sorry, this is a commit in another PR: wintercms/storm@ |
# Conflicts: # modules/backend/controllers/Index.php
This was referenced Oct 7, 2026
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
8 participants
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.
All tests passing. Requires wintercms/storm#207. Replaces #1094
Remaining Tasks:
Breaking changes:
array_first()andarray_last()functions that do not match the signature provided by Winter's helper functions. If you are currently using either function in your code with two or three arguments (filter & default) you will need to switch to\Winter\Storm\Support\Arr::first()or\Winter\Storm\Support\Arr:last()instead.-sshort flag for the--silentoption onmix:compile,mix:create,mix:install,mix:watch,npm:install,npm:run,npm:update,npm:version,vite:compile,vite:create,vite:install,vite:watchhas been removed as Symfony v7.2 added the--silentoption to all commands by default which conflicted with our definition. You must use the full option--silentgoing forward.static $defaultNamein Console commands to support lazy loading is no longer used in Symfony\Console 7.2, use theAsCommandclass attribute instead. We will need to update all first party plugins & modules to make use of that.Currently incompatible plugins:
All other Winter plugins have been tested and confirmed compatible (some with minor tweaks to their PHPUnit configs to remove deprecation warnings.
New Minimum Requirements:
Backwards Compatibility Fixes:
Upgrade Guides:
Summary by CodeRabbit