Skip to content

Conversation

@lancepioch
Copy link
Member

This pull request includes the changes for upgrading to Laravel 12.x. Feel free to commit any additional changes to the shift-164392 branch.

Before merging, you need to:

  • Checkout the shift-164392 branch
  • Review all pull request comments for additional changes
  • Run composer update (if the scripts fail, try with --no-scripts)
  • Clear any config, route, or view cache
  • Thoroughly test your application (no tests?, no CI?)

If you get stuck, never hesitate to email support. If you need more help with your upgrade, check out the Human Shifts.

@lancepioch
Copy link
Member Author

ℹ️ Starting with Laravel 10, the lang folder is no longer included in a default Laravel application. Laravel now recursively merges any customizations with framework defaults.

Shift streamlined your language files by removing options that matched the Laravel defaults and preserving your customizations. If you wish to keep the full set of language, Shift recommends running artisan lang:publish --force to get the latest configuration files from Laravel, then reapplying the customizations Shift streamlined.

@lancepioch
Copy link
Member Author

⚠️ Shift upgraded your configuration files by defaulting them and merging your true customizations. These include values which are not changeable through core ENV variables.

You should review this commit for additional customizations or opportunities to use new ENV variables. If you have a lot of customizations, you may undo this commit with git revert 42e1c488 and make the config file changes manually.

@lancepioch
Copy link
Member Author

ℹ️ Shift detected customized options within your configuration files which may be set with an ENV variable. To help keep your configuration files streamlined, you may set the following variables. Be sure adjust any values per environment.

DB_DATABASE=panel
DB_USERNAME=pelican
REDIS_CLIENT=predis

Note: some of these may simply be values which changed between Laravel versions. You may ignore any ENV variables you do not need to customize.

1 similar comment
@lancepioch
Copy link
Member Author

ℹ️ Shift detected customized options within your configuration files which may be set with an ENV variable. To help keep your configuration files streamlined, you may set the following variables. Be sure adjust any values per environment.

DB_DATABASE=panel
DB_USERNAME=pelican
REDIS_CLIENT=predis

Note: some of these may simply be values which changed between Laravel versions. You may ignore any ENV variables you do not need to customize.

@lancepioch
Copy link
Member Author

ℹ️ Laravel 12 removed the MAIL_ENCRYPTION environment variable. While a new MAIL_SCHEME environment variable was introduced, its available options are not the same. Your application will likely behave correctly with the new default values. However, if you were setting MAIL_ENCRYPTION to tls, you may want to review this Mastering Laravel Tip for more details.

@lancepioch
Copy link
Member Author

⚠️ Laravel 12 changed the default values of the CACHE_PREFIX, REDIS_PREFIX, and SESSION_COOKIE to use dashes (-) instead of underscores (_). If you were not explicitly setting these environment variables, your application will use these new default values - which may result in unexpected behavior. For example, temporary cache misses or forced log out.

If your application uses cache prefixes or session cookies and you were not explicitly setting these ENVs, you may set them to their previous defaults to avoid any unexpected behavior.

@lancepioch
Copy link
Member Author

❌ Laravel 12 removed the APP_TIMEZONE environment variable. The timezone now defaults to UTC. If you need to customize it, you may set the timezone configuration option directly within config/app.php.

Shift detected references to the APP_TIMEZONE environment variable in the following files. You should remove the environment variable and, if necessary, reference the app.timezone configuration option instead.

  • .github/workflows/ci.yaml

@lancepioch
Copy link
Member Author

ℹ️ Laravel added a composer run dev script which starts php artisan serve, php artisan queue:listen --tries=1, php artisan pail, and npm run dev all in one command.

Shift has added this script, as well as the necessary packages. Once you run composer update and npm install, you will be able to use the new composer run dev script.

@lancepioch
Copy link
Member Author

ℹ️ Laravel added a composer run setup script which runs commands like composer install, npm install, and others to set up your Laravel application. You are encouraged to customize this script for your project.

@lancepioch
Copy link
Member Author

ℹ️ Laravel added a composer run test script which runs the new php artisan config:clear command, then php artisan test. This is provided as a convenience. You are welcome to customize this script for your test suite.

@lancepioch
Copy link
Member Author

ℹ️ Shift updated your dependencies for Laravel 12. While many of the popular packages are reviewed, you may have to update additional packages in order for your application to be compatible with Laravel 12. Watch dealing with dependencies for tips on handling any Composer issues.

The following dependencies were not updated by Shift and may be incompatible. If Composer lists one of these packages as a problem, you should check for a newer version.

@lancepioch
Copy link
Member Author

ℹ️ Starting with Laravel 10, PHP type hints were added within classes included in a Laravel application. In an effort to modernize your code, Shift automated this change where Laravel strictly specifies a type. If you prefer not to type hint your code you may revert this commit by running: git revert 5db7ca50

@lancepioch
Copy link
Member Author

ℹ️ All of the underlying Symfony components used by Laravel have been upgraded to Symfony 7.2. Shift detected references to Symfony classes within your application. These are most likely type hints and can safely be ignored. If you are using Symfony classes directly or experience issues relating to Symfony, you should review the Symfony change log for any additional changes.

@lancepioch
Copy link
Member Author

ℹ️ If you are using the Laravel installer CLI tool to create new Laravel applications with laravel new, you should updated to the latest version to receive updates for Laravel 12 and the new starter kits.

You may update your installer by running:

composer global require laravel/installer

@lancepioch
Copy link
Member Author

⚠️ Laravel 12 no longer includes the SVG image type when performing image validation. If your application allows SVG images, you should review your image validation rules and update them to use the new allow_svg option.

'cover' => ['required', 'image:allow_svg'],
'avatar' => ['required', File::image(allowSvg: true)],

@lancepioch
Copy link
Member Author

❌ Several of the Schema methods changed in Laravel 12. The getTables, getViews, getTypes, getTableListing methods now returns all schemas (databases). In addition, the getTableListing method returns the schema-qualified table name (e.g. database.table).

Shift detected potential uses of these methods in the following files. You should review these instances to see if you need to preserve the original behavior. You may review the documentation on Multi-Schema Database Inspecting for more information.

  • database/migrations/2017_02_02_175548_UpdateColumnNames.php
  • database/migrations/2017_02_03_140948_UpdateNodesTable.php
  • database/migrations/2017_02_03_155554_RenameColumns.php
  • database/migrations/2017_02_05_164123_AdjustColumnNames.php
  • database/migrations/2017_02_05_164516_AdjustColumnNamesForServicePacks.php
  • database/migrations/2017_02_10_171858_UpdateAPIKeyColumnNames.php
  • database/migrations/2017_11_19_122708_MigratePubPrivFormatToSingleKey.php
  • database/migrations/2020_12_12_102435_support_multiple_docker_images_and_updates.php
  • database/migrations/2020_12_14_013707_make_successful_nullable_in_server_transfers.php
  • database/migrations/2020_12_24_092449_make_allocation_fields_json.php
  • database/migrations/2021_07_12_013420_remove_userinteraction.php
  • database/migrations/2024_03_12_154408_remove_nests_table.php
  • database/migrations/2024_07_25_072050_convert_rules_to_array.php

@coderabbitai
Copy link

coderabbitai bot commented Jan 7, 2026

📝 Walkthrough

Walkthrough

This PR updates Laravel framework to v12.45, refactors Eloquent scope declarations to use attribute-based syntax, updates database and mail configurations with new defaults and connection types, removes legacy authentication language entries, upgrades development dependencies, and adds test environment settings.

Changes

Cohort / File(s) Summary
Framework & Dependencies
composer.json, package.json
Bumped laravel/framework from ^12.37 to ^12.45; bumped laravel-vite-plugin from ^1.0 to ^2.0.0; added dev, setup, test, and pre-package-uninstall scripts
Type Annotations
app/Console/Commands/Overrides/ConfigCacheCommand.php
Added void return type annotation to handle() method
Model Scopes Refactoring
app/Models/ActivityLog.php
Refactored scopeForEvent() and scopeForActor() from public methods to protected methods with #[Scope] attribute, aligning with Laravel's attribute-based scope syntax
Database Configuration
config/database.php
Updated SQLite database path handling and added transaction_mode; changed MySQL/MariaDB/PostgreSQL defaults (database: 'panel' → 'laravel', username: 'pelican' → 'root'); added new MSSQL (sqlsrv) connection block; updated Redis client from 'predis' to 'phpredis', modified prefix pattern, and added retry/backoff configuration options (max_retries, backoff_algorithm, backoff_base, backoff_cap)
Mail Configuration
config/mail.php
Changed SMTP scheme to use MAIL_SCHEME environment variable directly; cast APP_URL to string in local_domain parsing; added retry_after (60) to failover and roundrobin mailers
Authentication Localization
lang/en/auth.php
Removed three legacy authentication messages ('failed', 'password', 'throttle')
Test Configuration
phpunit.xml
Added BROADCAST_CONNECTION and NIGHTWATCH_ENABLED environment variables for testing

Possibly related PRs

🚥 Pre-merge checks | ✅ 1 | ❌ 2
❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title 'Laravel 12.x Shift' is vague and generic, using non-descriptive terminology that doesn't convey meaningful information about the specific changeset. Consider using a more specific title like 'Upgrade to Laravel 12.x' or 'Apply Laravel 12.x automated upgrade changes' to better describe the changeset.
✅ Passed checks (1 passed)
Check name Status Explanation
Description check ✅ Passed The description is related to the changeset, providing context about the Laravel 12.x upgrade and listing required pre-merge steps.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@lancepioch
Copy link
Member Author

ℹ️ The container now respects the default value of constructor parameters when resolving a class instance. If you were previously relying on the container to set a value, you will need to pass in this value when resolving the class instead.

@lancepioch
Copy link
Member Author

🎉 Congratulations, you're now running the latest version of Laravel!

Next, you may optionally run the following Shifts to ensure your application is fully upgraded, adopts the latest Laravel conventions, and easier to maintain in the future:

  • Laravel Fixer automatically updates your code to the latest Laravel conventions.
  • Tests Generator intelligently generates model factories, HTTP Tests, and configuration for your application.
  • CI Generator intelligently generates CI jobs to lint PHP, check code style, and run tests, including Dusk.

You may also use the Shift Workbench to automate common tasks for maintaining your Laravel application.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
config/database.php (1)

5-10: Dead code: These variables are no longer used.

Lines 5–10 define $datapasePath (typo of $databasePath) and conditionally set $databasePath, but neither variable is referenced after line 44 was changed to use env('DB_DATABASE', database_path('database.sqlite')) directly.

🧹 Suggested fix: Remove unused code
 <?php

 use Illuminate\Support\Str;

-$database = env('DB_DATABASE', 'database.sqlite');
-$datapasePath = database_path($database);
-
-if (str_starts_with($database, '/') || $database === ':memory:') {
-    $databasePath = $database;
-}
-
 return [
🤖 Fix all issues with AI agents
In @config/database.php:
- Line 154: The change of the default Redis client to phpredis (see the config
entry 'client' => env('REDIS_CLIENT', 'phpredis') in config/database.php) can
break installs without the phpredis extension; update upgrade/migration notes to
call this out and either add guidance to set REDIS_CLIENT=predis in .env or add
a commented example line REDIS_CLIENT=predis to .env.example so users without
the extension know to revert to predis; ensure the docs mention commit 5331c5ab
and the composer dependency on predis (^2.3) for clarity.

In @package.json:
- Line 14: The package.json now pins "laravel-vite-plugin": "^2.0.0" which is
ESM-only and has breaking changes; update package.json and project config by
either downgrading the plugin or upgrading Vite and related plugins (vite,
@vitejs/*, vue/react plugins) to compatible majors, ensure your Node version
meets the new minimum, and modify vite.config.js (or rename to vite.config.cjs
if you must keep CommonJS) to address ESM requirements and the new server.https
shape (replace boolean server.https: true with an object form), verify any
default path changes (e.g., refresh/watch patterns like app/Livewire/**) and run
full dev and production builds to confirm no regressions.
📜 Review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6ebeb40 and ddf616b.

📒 Files selected for processing (8)
  • app/Console/Commands/Overrides/ConfigCacheCommand.php
  • app/Models/ActivityLog.php
  • composer.json
  • config/database.php
  • config/mail.php
  • lang/en/auth.php
  • package.json
  • phpunit.xml
🧰 Additional context used
🧬 Code graph analysis (1)
app/Models/ActivityLog.php (1)
app/Services/Activity/ActivityLogService.php (1)
  • actor (96-101)
🔇 Additional comments (15)
composer.json (2)

18-18: LGTM: Framework version bump looks safe.

The bump from ^12.37 to ^12.45 is a patch-level upgrade within Laravel 12.x and should maintain backward compatibility.


87-105: The composer.json scripts are correctly structured.

The dev script is well-formed with concurrently (^9.0.1) already listed in package.json devDependencies, so npx will find it without requiring a global installation. The four color codes are valid hex format and match the four process names (server, queue, logs, vite). When you run this script, verify that the concurrent processes (Laravel server, queue listener, logs, and Vite dev server) start and stop cleanly together.

config/mail.php (3)

52-52: Good defensive coding with string cast.

Casting APP_URL to string before parse_url() prevents potential type errors if the environment variable is unexpectedly null or non-string.


91-91: LGTM: Retry configuration improves resilience.

Adding retry_after to failover and roundrobin mailers provides better handling of temporary mail delivery failures with a 60-second backoff.

Also applies to: 100-100


45-45: No action needed on this line; the code is correct.

The 'scheme' => env('MAIL_SCHEME') configuration correctly reads the environment variable for Laravel 12's mail scheme. Per Laravel 12 documentation, MAIL_SCHEME accepts null (default STARTTLS on port 587), smtp, or smtps (implicit TLS on port 465). If migrating from MAIL_ENCRYPTION=tls, set MAIL_SCHEME=null or MAIL_SCHEME=smtps in your .env file—this is an environment configuration concern, not a code issue.

Likely an incorrect or invalid review comment.

phpunit.xml (1)

19-19: LGTM: Test environment configuration additions.

The new environment variables appropriately disable broadcasting and Nightwatch monitoring during test execution, preventing side effects and improving test isolation.

Also applies to: 29-29

app/Console/Commands/Overrides/ConfigCacheCommand.php (1)

12-12: Return type correctly matches Laravel 12.45 parent signature.

The void return type on line 12 matches the parent Illuminate\Foundation\Console\ConfigCacheCommand::handle() method signature in Laravel 12.45.

lang/en/auth.php (1)

16-21: LGTM — Custom authentication strings preserved correctly.

The removal of failed, password, and throttle entries aligns with Laravel 12's approach where framework defaults are recursively merged at runtime. Your custom 2FA-related entries are preserved as expected.

app/Models/ActivityLog.php (2)

11-11: LGTM — Import added for attribute-based scopes.


112-125: LGTM — Scopes correctly migrated to Laravel 12's attribute-based pattern.

The #[Scope] attribute registration with protected visibility and unprefixed method names follows the new Laravel 12 convention. The existing PHPDoc @method annotations on lines 41-42 remain accurate for IDE/static analysis support.

config/database.php (5)

44-50: SQLite configuration updated with Laravel 12 defaults.

The direct env() call for the database path and new transaction_mode setting are consistent with Laravel 12's defaults.


58-59: Default credentials changed from project-specific to Laravel defaults.

The defaults changed from panel/pelican to laravel/root. Existing deployments should be unaffected if DB_DATABASE and DB_USERNAME are set in .env, but this could cause confusion for new installations following project-specific documentation.

Per the PR comments, consider adding these to .env.example:

DB_DATABASE=panel
DB_USERNAME=pelican

Also applies to: 78-79, 98-99


108-121: New SQL Server connection added.

Standard Laravel 12 addition. The commented encrypt and trust_server_certificate options are appropriate for optional TLS configuration.


158-158: Redis prefix format changed.

Changed from _database_ suffix to -database- suffix with (string) cast. Existing Redis data will become inaccessible under the new prefix after upgrade unless REDIS_PREFIX is explicitly set to preserve the old format.


182-186: New Redis retry/backoff options added.

Good resilience improvements with sensible defaults: 3 retries using decorrelated jitter backoff (100ms base, 1000ms cap). These are additive and shouldn't affect existing behavior.

Also applies to: 214-218

'redis' => [

'client' => env('REDIS_CLIENT', 'predis'),
'client' => env('REDIS_CLIENT', 'phpredis'),
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check if predis is a required dependency in composer.json
rg -n '"predis' composer.json

# Check .env.example for REDIS_CLIENT setting
rg -n 'REDIS_CLIENT' .env.example

Repository: pelican-dev/panel

Length of output: 96


🏁 Script executed:

#!/bin/bash
# Check if .env.example exists
ls -la .env* | head -20

# Read the config/database.php file around line 154 to see full context
cat -n config/database.php | sed -n '140,170p'

# Search for REDIS_CLIENT in all .env files
find . -name ".env*" -type f | xargs grep -l "REDIS_CLIENT" 2>/dev/null || echo "No REDIS_CLIENT found in .env files"

Repository: pelican-dev/panel

Length of output: 1467


🏁 Script executed:

#!/bin/bash
# Read .env.example
cat -n .env.example

# Check git log for changes to config/database.php
git log --oneline config/database.php 2>/dev/null | head -10

# Show the last few commits mentioning REDIS or redis
git log --oneline --all 2>/dev/null | grep -i redis | head -10

Repository: pelican-dev/panel

Length of output: 762


🏁 Script executed:

#!/bin/bash
# Show the change in commit 5331c5ab (when predis was set as default)
git show 5331c5ab --stat 2>/dev/null | head -20

# Show the current state more clearly
git diff 5331c5ab HEAD -- config/database.php 2>/dev/null | head -50

Repository: pelican-dev/panel

Length of output: 2375


🏁 Script executed:

#!/bin/bash
# Show the exact change in the redis client line
git show 5331c5ab:config/database.php 2>/dev/null | grep -A 2 -B 2 "REDIS_CLIENT" | head -10

# Check what was before that commit
git show 5331c5ab~1:config/database.php 2>/dev/null | grep -A 2 -B 2 "REDIS_CLIENT" | head -10

Repository: pelican-dev/panel

Length of output: 766


Document Redis client change in upgrade notes or provide migration guidance.

The default Redis client was changed from predis to phpredis in commit 5331c5a. While predis remains available as a dependency (^2.3 in composer.json), installations without the phpredis PHP extension installed will break unless REDIS_CLIENT=predis is explicitly set in .env.

This should be documented prominently in upgrade notes, or consider adding REDIS_CLIENT=predis to .env.example as a commented default to guide users through the transition.

🤖 Prompt for AI Agents
In @config/database.php at line 154, The change of the default Redis client to
phpredis (see the config entry 'client' => env('REDIS_CLIENT', 'phpredis') in
config/database.php) can break installs without the phpredis extension; update
upgrade/migration notes to call this out and either add guidance to set
REDIS_CLIENT=predis in .env or add a commented example line REDIS_CLIENT=predis
to .env.example so users without the extension know to revert to predis; ensure
the docs mention commit 5331c5ab and the composer dependency on predis (^2.3)
for clarity.

"autoprefixer": "^10.4.21",
"concurrently": "^9.0.1",
"laravel-vite-plugin": "^1.0",
"laravel-vite-plugin": "^2.0.0",
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

laravel-vite-plugin 2.0.0 breaking changes migration guide

💡 Result:

Short migration summary for laravel-vite-plugin → 2.0.0 and what to change

Sources: [1][2][3]

What changed (breaking / notable)

  • Drops CJS build / ESM-first: plugin no longer ships a CommonJS build — projects must use ESM or .cjs config files. [2][1]
  • Node.js support bumped / older Node removed: older Node (14/15/16/17/19, etc.) support removed — run on a supported Node for the Vite version you use. [2]
  • server.https boolean removed: boolean value for vite.config server.https is no longer supported (use the object form or other TLS options). [2]
  • Depend on matching Vite major: v2 of the plugin expects newer Vite/plugin versions — you must upgrade Vite and any Vite plugins (Vue/React plugins) to compatible versions. [2]
  • Misc developer conveniences: new commands and default refresh/watch paths (e.g., app/Livewire/**) and SSR manifest output behaviour changed in prior major moves—check your refresh/watch settings if you rely on defaults. [2]

Minimum migration steps

  1. Update packages:
    • npm install --save-dev vite@^ laravel-vite-plugin@^2.0.0 and update any @vitejs plugins to their matching compatible versions. Confirm exact Vite major required by your plugin release notes. [2][1]
  2. Make package.json ESM (if not already):
    • npm pkg set type=module or keep CJS config files as .cjs when you must keep CommonJS in configs. [2]
  3. Update vite.config.*:
    • Replace any server.https: true/false boolean with the supported object form or your TLS setup.
    • Convert any require(...) usage in vite.config to ESM import, or rename file to vite.config.cjs and keep require. [2]
  4. Check Node version:
    • Run on a Node version supported by the Vite major you installed (upgrade Node if needed). [2]
  5. Verify hot/hotFile and filesystem paths:
    • If you use hotFile or hot-related features, confirm directories exist and plugin behaviour (hotFile path) after upgrade; some fixes were released after 2.0.0. [1][3]
  6. Test dev and build:
    • Run dev server and production build; confirm manifest/SSR output and asset URLs used by your Laravel views still match (update blade calls if manifest structure or file names changed).

If you want, I can:

  • fetch the exact Vite version compatibility and the plugin’s full changelog/upgrade guide (link to the plugin CHANGELOG / release notes) and produce a patch/diff for your vite.config and package.json. Which repo/version of Vite are you upgrading from?

Sources
[1] npm/jsDelivr package listing for laravel-vite-plugin (shows v2.0.0).
[2] Extracts from the plugin upgrade notes / changelog (noting CJS removal, server.https boolean removal, Node drops, and upgrade steps).
[3] Yarn/npm changelog entries and follow-up fixes referencing post-2.0.0 patch (hotFile parent directory fix).


Address laravel-vite-plugin v2.0.0 breaking changes before deploying.

The upgrade from ^1.0 to ^2.0.0 introduces several breaking changes:

  • ESM-only: Plugin no longer ships CommonJS builds—your project must use ESM or configure Vite config as .cjs files
  • Node.js version bump: Drops support for Node 14, 15, 16, 17, 19—verify your Node version matches the minimum required by your Vite version
  • server.https config change: Boolean values (e.g., server.https: true) no longer supported—use object form instead
  • Vite version compatibility: Requires upgrading Vite and related plugins (@vitejs, Vue, React) to compatible major versions
  • Default paths updated: Refresh and watch paths changed (e.g., new defaults include app/Livewire/**)

Update vite.config.js (or rename to vite.config.cjs if keeping CommonJS), upgrade Vite and plugins, verify Node version, and test both dev and production builds before merging.

🤖 Prompt for AI Agents
In @package.json at line 14, The package.json now pins "laravel-vite-plugin":
"^2.0.0" which is ESM-only and has breaking changes; update package.json and
project config by either downgrading the plugin or upgrading Vite and related
plugins (vite, @vitejs/*, vue/react plugins) to compatible majors, ensure your
Node version meets the new minimum, and modify vite.config.js (or rename to
vite.config.cjs if you must keep CommonJS) to address ESM requirements and the
new server.https shape (replace boolean server.https: true with an object form),
verify any default path changes (e.g., refresh/watch patterns like
app/Livewire/**) and run full dev and production builds to confirm no
regressions.

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ddf616b550

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines 41 to 45
'sqlite' => [
'driver' => 'sqlite',
'url' => env('DB_URL'),
'database' => $datapasePath,
'database' => env('DB_DATABASE', database_path('database.sqlite')),
'prefix' => '',

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Normalize sqlite DB_DATABASE to database_path

The sqlite connection now uses env('DB_DATABASE') directly, so a relative value like database.sqlite (the installer’s default) will resolve relative to the app root instead of database_path(). In the Docker image the persistent DB is symlinked at ./database/database.sqlite, so this change points sqlite at /var/www/html/database.sqlite, creating a new empty DB outside the volume and losing persistence across restarts. Consider restoring the database_path() wrapping or normalizing relative paths before using them.

Useful? React with 👍 / 👎.

Comment on lines 153 to 155

'client' => env('REDIS_CLIENT', 'predis'),
'client' => env('REDIS_CLIENT', 'phpredis'),

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Avoid defaulting Redis client to phpredis

Defaulting REDIS_CLIENT to phpredis will break environments that don’t have the PHP Redis extension installed. This repo’s base image doesn’t install that extension while it does include predis/predis, so if REDIS_CLIENT isn’t explicitly set, any Redis usage will throw Class Redis not found / connection errors. Consider keeping predis as the default or ensuring the extension is installed and the env is updated.

Useful? React with 👍 / 👎.

Comment on lines 43 to 46
'smtp' => [
'transport' => 'smtp',
'scheme' => env('MAIL_SCHEME', env('MAIL_ENCRYPTION') === 'ssl' || env('MAIL_ENCRYPTION') === 'tls' ? 'smtps' : 'smtp'),
'scheme' => env('MAIL_SCHEME'),
'url' => env('MAIL_URL'),

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve MAIL_ENCRYPTION fallback for SMTP

The SMTP mailer no longer derives scheme from MAIL_ENCRYPTION, so existing deployments that still set only MAIL_ENCRYPTION=tls/ssl will now send with a null scheme. On servers that require TLS, SMTP connections will fail (or downgrade to plaintext if accepted). Consider keeping the fallback or updating env/docs to require MAIL_SCHEME explicitly.

Useful? React with 👍 / 👎.

@lancepioch lancepioch self-assigned this Jan 7, 2026
Comment on lines +86 to +104
],
"dev": [
"Composer\\Config::disableProcessTimeout",
"npx concurrently -c \"#93c5fd,#c4b5fd,#fb7185,#fdba74\" \"php artisan serve\" \"php artisan queue:listen --tries=1\" \"php artisan pail --timeout=0\" \"npm run dev\" --names=server,queue,logs,vite --kill-others"
],
"setup": [
"composer install",
"@php -r \"file_exists('.env') || copy('.env.example', '.env');\"",
"@php artisan key:generate",
"@php artisan migrate --force",
"npm install",
"npm run build"
],
"test": [
"@php artisan config:clear --ansi",
"@php artisan test"
],
"pre-package-uninstall": [
"Illuminate\\Foundation\\ComposerScripts::prePackageUninstall"
Copy link
Member

@rmartinoscar rmartinoscar Jan 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
],
"dev": [
"Composer\\Config::disableProcessTimeout",
"npx concurrently -c \"#93c5fd,#c4b5fd,#fb7185,#fdba74\" \"php artisan serve\" \"php artisan queue:listen --tries=1\" \"php artisan pail --timeout=0\" \"npm run dev\" --names=server,queue,logs,vite --kill-others"
],
"setup": [
"composer install",
"@php -r \"file_exists('.env') || copy('.env.example', '.env');\"",
"@php artisan key:generate",
"@php artisan migrate --force",
"npm install",
"npm run build"
],
"test": [
"@php artisan config:clear --ansi",
"@php artisan test"
],
"pre-package-uninstall": [
"Illuminate\\Foundation\\ComposerScripts::prePackageUninstall"

This was purposely removed in 78aa7d2 and #1994 got refused aswell.

"autoprefixer": "^10.4.21",
"concurrently": "^9.0.1",
"laravel-vite-plugin": "^1.0",
"laravel-vite-plugin": "^2.0.0",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not v2.0.1 ?

Suggested change
"laravel-vite-plugin": "^2.0.0",
"laravel-vite-plugin": "^2.0.1",

"calebporzio/sushi": "^2.5",
"dedoc/scramble": "^0.12.10",
"filament/filament": "~4.0",
"gboquizosanchez/filament-log-viewer": "^2.1",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please bump this to v2.2.1

Suggested change
"gboquizosanchez/filament-log-viewer": "^2.1",
"gboquizosanchez/filament-log-viewer": "^2.2",

@rmartinoscar
Copy link
Member

Superseded by #2081

@github-actions github-actions bot locked and limited conversation to collaborators Jan 8, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants