Repository navigation
chore(deps): upgrade to Laravel 12 - #3956
Merged
Merged
Conversation
…odule switches PermissionService, Tickets, Timesheets, Search, Ideas, Goalcanvas and ReportEngine took `?ProjectModules $projectModules = null` and skipped the module gate when it was null. A missing dependency meant module switches silently stopped being enforced (UI, JSON-RPC and MCP alike). Laravel 12's container injects a nullable parameter's default instead of resolving the class when the class has no binding. ProjectModules is a bound singleton, so the upgrade alone would not have nulled it, but the gate should not depend on that: the dependency is now required and the null branches are gone, so a missing gate is a construction error instead of a silent allow. ModuleGateWiringTest resolves each service from the container and asserts it got the real ProjectModules; tests that build the services by hand pass an all-on instance (Unit\TestCase::allModulesOn()). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
laravel/framework ^12.69 (12.70.0). Every 11.x release carries four advisories with no 11.x fix (debug page XSS, signed URL path confusion, CRLF in the email rule); the upgrade also clears the guzzle, commonmark, flysystem and http-foundation advisories in the lock. It unblocks laravel/mcp 1.0 (needs illuminate/json-schema, framework >= 11.47). Framework layer, checked against the L12 parents of what we override: - Console\Application: drop the addCommand() override. Laravel 12's own addCommand() calls setLaravel(), which is what the symfony/console 7.4 workaround did. - LoadConfig: register config_loaded_from_cache for both paths, as LoadConfiguration does. - Language::get() declares its string return. L12's trans() is return-typed, and a mocked Language returned null from the untyped method. Tests: - Query grammars take their connection in the constructor now. - Symfony 7.4.18+ counts the documentation ranges (203.0.113.0/24 etc.) as PRIVATE_SUBNETS, so the "public client can't spoof X-Forwarded-For" test uses a routable address. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- update() is typed int<0, max>, so `update(...) >= 0` was always true: run the update and return true (the result only ever meant "didn't throw"). - Timesheets: drop a discarded setTimezone() on an immutable; formatDateTimeForDb() already converts to UTC. - Wiki: iterate the row with get_object_vars(). - Api::resolveStaticAsset: use the Stringable's own contains()/startsWith(); Str:: helpers are now typed to return false for non-string haystacks. - CheckPermissions: read the route through the resolver. route() is documented as never null but is null until a route matched, so the instanceof guard stays meaningful. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
marcelfolaron
requested review from
broskees
and
a balanced review from Copilot
and removed request for
a team
October 20, 2026 16:59
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
The framework-wide upgrade needs human confirmation of acceptance CI results and coordinated validation of the companion plugin changes.
No open issues 🎉
What changed in this PR
Upgrades Leantime to Laravel 12.70 for security fixes and prepares the framework for the subsequent MCP upgrade targeting 3.13.
Changes:
- Updates framework dependencies and adapts core integrations to Laravel 12.
- Makes project-module enforcement dependencies mandatory and adds wiring coverage.
- Simplifies code flagged by sharper types and updates affected test fixtures.
| File | Description |
|---|---|
| tests/Unit/TestCase.php | Adds an all-modules-enabled test helper. |
| tests/Unit/app/Domain/Timesheets/Services/TimesheetsServiceTest.php | Supplies required module switches. |
| tests/Unit/app/Domain/Tickets/Services/TicketsServiceTest.php | Updates service construction. |
| tests/Unit/app/Domain/Tickets/Services/TicketsApiContractTest.php | Updates API test dependencies. |
| tests/Unit/app/Domain/Tickets/Services/TicketChangeEventsTest.php | Updates event test dependencies. |
| tests/Unit/app/Domain/Tickets/Repositories/StatusTypeFilterTest.php | Adapts query grammar construction. |
| tests/Unit/app/Domain/Tickets/Repositories/StatusSummaryQueryTest.php | Adapts query grammar construction. |
| tests/Unit/app/Domain/Tickets/Repositories/SearchSortTest.php | Adapts query grammar construction. |
| tests/Unit/app/Domain/Reports/Services/ReportEngineTest.php | Supplies required module switches. |
| tests/Unit/app/Domain/Ideas/Services/IdeasServiceTest.php | Supplies required module switches. |
| tests/Unit/app/Domain/Goalcanvas/Services/GoalProgressUpdatedTest.php | Updates goal-event test dependencies. |
| tests/Unit/app/Domain/Goalcanvas/Services/GoalcanvasServiceTest.php | Updates goal-service test dependencies. |
| tests/Unit/app/Core/Modules/ModuleGateWiringTest.php | Verifies container injection of module switches. |
| tests/Unit/app/Core/Middleware/RequestRateLimiterTest.php | Uses a routable public-IP fixture. |
| tests/Unit/app/Core/Auth/Permissions/PermissionServiceViewAllTest.php | Supplies required module switches. |
| tests/Unit/app/Core/Auth/Permissions/PermissionServiceModulesTest.php | Removes optional-module dependency coverage. |
| tests/Unit/app/Core/Auth/Permissions/PermissionServiceCacheTest.php | Supplies required module switches. |
| composer.lock | Locks Laravel 12.70 and updated dependencies. |
| composer.json | Requires Laravel 12.69 or newer within 12.x. |
| CLAUDE.md | Updates the documented framework version. |
| app/Domain/Wiki/Repositories/Wiki.php | Adjusts row iteration and simplifies update returns. |
| app/Domain/Timesheets/Services/Timesheets.php | Requires module switches and simplifies date formatting. |
| app/Domain/Tickets/Services/Tickets.php | Requires module switches. |
| app/Domain/Sprints/Repositories/Sprints.php | Simplifies update success handling. |
| app/Domain/Search/Services/Search.php | Removes optional module-gate bypasses. |
| app/Domain/Reports/Services/ReportEngine.php | Requires module filtering and preloading. |
| app/Domain/Projects/Repositories/Projects.php | Simplifies update success handling. |
| app/Domain/Plugins/Repositories/Plugins.php | Simplifies update success handling. |
| app/Domain/Notifications/Repositories/Notifications.php | Simplifies update success handling. |
| app/Domain/Ideas/Services/Ideas.php | Requires module filtering. |
| app/Domain/Ideas/Repositories/Ideas.php | Simplifies update success handling. |
| app/Domain/Goalcanvas/Services/Goalcanvas.php | Requires module checks and filtering. |
| app/Domain/Comments/Repositories/Comments.php | Simplifies update success handling. |
| app/Domain/Clients/Repositories/Clients.php | Simplifies update success handling. |
| app/Domain/Auth/Repositories/Auth.php | Simplifies update success handling. |
| app/Domain/Api/Services/Api.php | Uses instance methods on Stringable paths. |
| app/Core/Language.php | Declares and documents the string return type. |
| app/Core/Console/Application.php | Removes the command-registration override supplied upstream. |
| app/Core/Bootstrap/LoadConfig.php | Registers configuration-cache loading state. |
| app/Core/Auth/Permissions/PermissionService.php | Makes module-gate injection mandatory. |
| app/Core/Auth/Permissions/CheckPermissions.php | Resolves routes while retaining nullable handling. |
Review effort: Balanced
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.
Phase 0 of the MCP protocol upgrade: Laravel 12 first, as its own PR, then laravel/mcp 1.0. Target: 3.13.
Why
illuminate/json-schema, which needs framework ≥ 11.47.continue-on-error.Commits
fix(modules): requireProjectModulesin every service that enforces module switches.PermissionService, Tickets, Timesheets, Search, Ideas, Goalcanvas and ReportEngine took?ProjectModules $projectModules = nulland skipped the module gate when it was null.ProjectModulesis a bound singleton, so the upgrade alone wouldn't have nulled it (verified). The gate still shouldn't fail open: the dependency is now required.ModuleGateWiringTest, plusUnit\TestCase::allModulesOn()for tests that build these services by hand.chore(deps): upgrade to Laravel 12.Console\Application::addCommand()override: L12 ships the samesetLaravel()fix.LoadConfigregistersconfig_loaded_from_cachelike the parent does.Language::get()declares itsstringreturn (L12'strans()is return-typed).PRIVATE_SUBNETS, so the "public client can't spoof X-Forwarded-For" test now uses a routable IP.refactor: PHPStan findings from L12's sharper types. 21 new on L12, 0 on L11.update(...) >= 0was always true.setTimezone()on an immutable (formatDateTimeForDb()already converts to UTC).Str::helpers on aStringable.route()is documented as non-null but can be null before routing.Behaviour note for reviewers
With the default
LEAN_TRUSTED_PROXIES(PRIVATE_SUBNETS), Symfony's widened list now also trusts peers in 100.64.0.0/10 (CGNAT, e.g. Tailscale) and the documentation/benchmark ranges to set X-Forwarded-For. Public addresses are unaffected. SetLEAN_TRUSTED_PROXIESexplicitly if that matters for a deployment.Verification
main): identical on L11 and L12. One pre-existing PostHogEventMapTestfailure that is unrelated.-32001on the switched-off project while the same call works on another./mcpunauthenticated → 401.bin/leantime(--version,list,plugin:list,db:migrate --help).Companion plugins PR: Copilot
ExecutionControllerhad two nullable memory services that are unbound. On L12 both came out null (verified), so it needs to land with this.🤖 Generated with Claude Code