Skip to content

Modernize DevTools for Phalcon 6 - #1551

Open
romeomanzur wants to merge 15 commits into
phalcon:5.0.xfrom
romeomanzur:feature/modernize-ci-tooling
Open

romeomanzur wants to merge 15 commits into
phalcon:5.0.xfrom
romeomanzur:feature/modernize-ci-tooling

Conversation

@romeomanzur

@romeomanzur romeomanzur commented Sep 25, 2026 •

Copy link
Copy Markdown
  • Type: new feature | code quality | documentation
  • Link to issue: N/A

In raising this pull request, I confirm the following:

  • I have read and understood the Contributing Guidelines
  • I have checked that another pull request for this purpose does not exist
  • I wrote tests for this PR

Summary

This pull request modernizes Phalcon DevTools for the Phalcon 6 ecosystem while using the current 5.0.x branch as the integration base.

The changes update the runtime, generated code, migrations integration, testing infrastructure, static analysis, CI, dependencies, documentation, and release tooling so DevTools can operate with the Composer-based Phalcon 6 packages.

Motivation

Phalcon 6 is distributed through Composer and no longer relies on the legacy ext-phalcon extension.

The existing DevTools codebase still contained assumptions and dependencies inherited from the Phalcon 4/5 extension-based architecture, including outdated generated code, migration handling, test infrastructure, CI configuration, and runtime dependencies.

This PR updates those areas while preserving the existing DevTools command structure and development workflow where possible.

Requirements and compatibility

  • PHP >= 8.2 < 9.0
  • Phalcon 6 via phalcon/phalcon
  • The legacy ext-phalcon extension is no longer required
  • phalcon/migrations v4 integration

Main changes

  • Added Phalcon 6 runtime support using phalcon/phalcon.
  • Updated the minimum PHP requirement to PHP 8.2.
  • Modernized model generation for Phalcon 6.
  • Fixed model metadata namespace handling.
  • Improved relation alias generation to avoid duplicate aliases.
  • Updated project and scaffold generation for Phalcon 6.
  • Updated WebTools compatibility.
  • Integrated phalcon/migrations v4.
  • Updated migration configuration handling to use Phalcon\Migrations\Utils\Config.
  • Aligned migration options such as no-auto-increment, description, reference-schema handling, and foreign-key checks with migrations v4.
  • Added migration database fixtures and console coverage.
  • Modernized the test suite to Codeception 5 and PHPUnit 11.
  • Updated PHPStan, Psalm, and PHP_CodeSniffer tooling.
  • Added PHP 8.2, 8.3, and 8.4 CI coverage.
  • Adjusted remote acceptance coverage handling for CI environments where coverage collection is not applicable.
  • Cleaned runtime dependencies so development-only tooling is not installed in consumer projects.
  • Updated README and contribution documentation for Phalcon 6.
  • Modernized the PHAR release workflow to build with PHP 8.2 and the Composer-managed Box dependency.

Runtime dependencies

The runtime dependency set was reviewed separately from development tooling.

Development-only packages such as Codeception, PHPUnit, PHPStan, Psalm, PHP_CodeSniffer, Box, and PHPDoc tooling remain in require-dev and are not installed in consumer projects.

A separate consumer installation smoke test was performed to verify that DevTools installs and runs without its development dependency set.

Migrations

The migration command previously passed the regular application Phalcon\Config\Config instance directly into phalcon/migrations.

Migrations v4 expects Phalcon\Migrations\Utils\Config, so DevTools now converts the application configuration before invoking the migrations API while preserving the original configuration for DevTools-specific options.

Migration generation, listing, and execution were tested against MySQL, including migration state tracking.

Validation

The updated branch has been validated with:

  • Codeception unit, functional, console, and acceptance suites.
  • MySQL acceptance tests.
  • PostgreSQL acceptance tests.
  • PHPStan 2.
  • Psalm 6.
  • PHP_CodeSniffer.
  • Composer validation.
  • Box configuration validation.
  • Consumer installation smoke testing without development dependencies.
  • PHAR compilation using Box 4.7.
  • PHAR runtime smoke tests with --version and --help.

The generated PHAR successfully reports:

Phalcon DevTools (6.0.0alpha1)
Phalcon Version: 6.0.0RC2

@niden

niden commented Oct 1, 2026

Copy link
Copy Markdown
Member

@romeomanzur I have not forgotten about this. I will check it sometime this week coming or earlier

@codecov

codecov Bot commented Oct 1, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0.85837% with 231 lines in your changes missing coverage. Please review.
✅ Project coverage is 6.57%. Comparing base (2e16ea6) to head (cb18b95).
⚠️ Report is 11 commits behind head on 5.0.x.

Additional details and impacted files
@@            Coverage Diff             @@
##             5.0.x   #1551      +/-   ##
==========================================
+ Coverage     4.45%   6.57%   +2.11%     
- Complexity       0     788     +788     
==========================================
  Files           82      48      -34     
  Lines         3995    2692    -1303     
==========================================
- Hits           178     177       -1     
+ Misses        3817    2515    -1302     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@niden

niden commented Oct 26, 2026

Copy link
Copy Markdown
Member

@romeomanzur Thank you again for this work. There are some questions though and some issues.

A bit of background here. We have tried to make devtools work for v5 a long time ago and it was really a mess. The decision was short of a full rewrite, the project would end up half done at best, so it was never high in our priority. At least today with AI we can revisit this and see if we can fix this.

In the meantime, we have started a brand new project called crest (phalcon/crest) which will be the next devtools and will support the functionality that developers seek from devtools including additional features such as dockerized environment, web server etc. That project is still under development but we are getting there slowly. The ADR handling and generation are done and going through the MVC models etc. now. Last will be the CLI and Micro. This is just for information, that the community does not think we have completely abandoned these developer tools that are essential.

The review for the PR is below. I had an AI agent run though it and verified the findings.

@niden

niden commented Oct 26, 2026

Copy link
Copy Markdown
Member

The main consideration is whether this PR is going to add value or not, and also the aftermath i.e. what do we do after we merge it.

As mentioned above, we are creating a new tool that will offer developers the functionality they need. Is it worth keeping two tools for the same task, even in this AI era. Honestly, I don't want us to do that and the direction the team chose is with crest. However nothing stops the community itself for pursuing this project and enhancing it to work with v5/6 etc.

Having written that, there review of the code identified some issues that I am posting for your attention. The main issue that I found also is that the PR is very bit and it should not be. I would split this work into smaller manageable chunks of code that are as independent as possible, adding tests along the way where they do not exist (again if possible) and then in the end you merge into one branch which will be the branch that will be merged into master. That way we have a much lower chance of missing something. Food for thought.

The issues identified are as follows:

Issues identified:

  1. @column(type="int") and type="bool" break annotation metadata (src/Builder/Component/Model.php:806-814, src/Generator/Snippet.php:179)
  • getPHPType() now returns int/float/bool. That value goes into both the @var docblock and the @column(type=…) annotation.
  • In Phalcon 6, MetaData\Strategy\Annotations (src/Mvc/Model/MetaData/Strategy/Annotations.php:148-207) knows integer and boolean, but not int or bool. Those fall to TYPE_VARCHAR and BIND_PARAM_STR, and the column is not marked numeric.
  • Failure: a model made with --annotate binds integer keys as strings, and PostgreSQL boolean saves fail.
  • The fixture tests/_data/console/app/models/files/TestModel.php:15 now expects type="int", so the tests lock the bug in.
  • Fix A: keep getPHPType() for @var only, and add a separate annotation-type map (integer, boolean, double, …) for @column.
  • Fix B: go back to the old getPHPType() values and convert to int/bool/float only where the docblock type is written.

  1. Consumers cannot install the package (composer.json:40-43, 80-83)
  • phalcon/migrations: dev-master, phalcon/volt: 1.0.x-dev and phalcon/annotations: 1.0.x-dev are in require. Packagist has no tagged release for any of them (migrations only has v3.0.1).
  • @rc on phalcon/phalcon has no effect in a non-root package. Consumers also never inherit the repositories entry.
  • Failure: composer require phalcon/devtools fails in any project with the default minimum-stability: stable.
  • Fix A: tag migrations v4 and volt/annotations (alpha/RC is enough), then require ^4.0@…-style ranges.
  • Fix B: keep the dev constraints only on a dev branch, and document that a stable DevTools tag is blocked until those packages are tagged. Remove the redundant repositories entry.

NOTE: This is not that big of an issue because the supporting packages have not been tagged yet. I am posting it here for awareness


  1. Only the Simple project type works with Composer-based Phalcon (src/Builder/Project/Simple.php:64,185). Only simple gets composer.json and require vendor/autoload.php. The Cli, Micro and Modules templates have no autoload at all. The PR edited templates/project/modules/services.php but did not add one there.
  • Failure: phalcon project --type=micro produces an app that stops with "Class Phalcon\Di\FactoryDefault not found".
  • Fix A: move createComposerFile() into ProjectBuilder, and add the autoload require to all four templates.
  • Fix B: limit the PR to simple, and make the other types fail with a clear message until they are migrated.

  1. The config value wins over the --descr CLI option (src/Commands/Builtin/Migration.php:82)
  • The line is $descr = $migrationsConfig->descr ?? $this->getOption('descr');. Every other option in this method gives the CLI value first: exportDataFromTables, migrations, log-in-db, ts-based.
  • Failure: with application.descr in the config, the --descr=foo value has no effect.
  • Fix A: $this->isReceivedOption('descr') ? $this->getOption('descr') : $migrationsConfig->descr.
  • Fix B: $this->getOption('descr') ?? $migrationsConfig->descr.

  1. A breaking major change targets the 5.x maintenance line (PR base 5.0.x)
  • The PR drops PHP < 8.2 and ext-phalcon 5. Any later 5.x tag would break Phalcon 5 users.
  • A 6.0.x branch already exists. It is stale: 12 commits behind 5.0.x and 0 ahead.
  • Fix A: fast-forward 6.0.x to 5.0.x and retarget the PR to 6.0.x.
  • Fix B: create a new 6.x branch from 5.0.x and retarget the PR there.

  1. The workflows use mutable tags instead of pinned SHAs (.github/workflows/tests.yml:52-187, release-phar.yml:18,23)
  • The PR uses @v4, @v5 and @v2 tags. master already pins every action by SHA ("bumping action versions" / "hardening workflow"). The release job has contents: write, and tests.yml has no permissions: block.
  • Failure: a moved tag on a third-party action can tamper with the release asset. A later merge forward also conflicts with master.
  • Fix A: copy the SHA-pinned uses: lines from master, and add permissions: contents: read to tests.yml.
  • Fix B: cherry-pick the master workflow hardening commits into this branch first, then rebase.

  1. Tests are missing for new and changed logic (Model.php:672-1040, tests/_data/model-project/, ScaffoldPhtmlCest.php:255,295)
  • About 250 lines of alias resolution have no unit test.
  • The model-project fixture and its .gitignore exceptions are not used by any test.
  • testNewAction and testEditAction submit the form but check nothing. The PR also deleted the commented-out see('samedi') check instead of enabling it. This is the path that the hidden $pk$ fix changed.
  • The PR checkbox says "I wrote tests".
  • Fix A: add unit tests for getRelationRole() and resolveRelationAliases() (the docblock examples are ready-made cases), and assert the saved values in the new/edit Cests.
  • Fix B: use the model-project fixture in a console test that generates models for a schema with duplicate FKs and asserts the aliases. Delete the fixture if no test uses it.

  1. The singularizer defeats the "canonical alias" rule (Model.php:967-1000)
  • The 'ses' rule gives courses→cours, responses→respons, purchases→purchas, and the plain s rule gives status→statu.
  • Example: with course_id + previous_course_id, the aliases become CoursesCourse / CoursesPreviousCourse instead of Courses / CoursesPrevious.
  • A: try several candidates (token, strip s, strip es, ies→y) and keep the one that matches the field parts.
  • B: drop the 'ses' rule and accept the less common statuses-style miss.

  1. The config conversion is duplicated across two modules (Migration.php:77, MigrationsController.php:231-247)
  • In Balanced Coupling terms, this is functional coupling across modules on a volatile contract (migrations dev-master). Both copies must change together.
  • The instanceof MigrationsConfig branch can never be true.
  • A: add one small adapter (for example Phalcon\DevTools\Migrations\ConfigFactory::fromConfig()) and use it from both places.
  • B: keep both copies, but make the controller call MigrationsConfig::fromArray($this->config->toArray()) directly.

  1. The alias heuristics live inside the Model builder (Model.php:672-1040)
  • That logic is hard to unit-test, because the builder needs a database.
  • The new methods are also not in alphabetical order.
  • A: extract a RelationAliasResolver class with no database dependency.
  • B: keep it in place, sort the methods, and test it through a subclass.

  1. Features were removed without a note
  • The generated email validator, the license.txt header, and the find()/findFirst() overrides are gone.
  • The public methods Snippet::getValidateEmail, getModelFind and getModelFindFirst, and the $license parameter of getClass(), are also gone. The CHANGELOG only says "Updated … model generation".
  • A: list each item under a "Breaking changes" section in the CHANGELOG.
  • B: keep the email validator and the license.txt support, because Phalcon 6 still has Validator\Email.

  1. The hidden PK in Volt is not escaped (templates/scaffold/no-forms/views/edit.volt:18)
  • The .phtml version uses htmlspecialchars. Volt does not escape output by default.
  • A: {{ $singular$.$pk$ | e }}.
  • B: turn on Volt autoescape in the generated services.

  1. The scaffold templates are now mixed
  • new/edit now use raw . index/search still use inputSubmit and tag->a, and those still pass the tests.
  • A: go back to the inputSubmit calls in new/edit, and change only the hidden field.
  • B: change all four templates to raw HTML.

  1. The Dockerfile port lines are dead and misleading (docker/php/Dockerfile:47-56)
  • The www.conf value listen = 8000 has no effect, because zz-docker.conf (9000) loads after it. nginx uses :9000. The comment and EXPOSE 8000 are wrong, and the setup works only by chance.
  • A: remove the sed line and expose 9000.
  • B: write the port to zz-docker.conf and keep nginx in sync.

  1. Duplicated fixture and inconsistent defaults
  • tests/_data/schemas/mysql/customers.sql duplicates the customers table in mysql/dump.sql.
  • The defaults in resetCustomersFixture() (mysql/devtools/password) are not the same as the config defaults (127.0.0.1/root/'').
  • A: have dump.sql include or source customers.sql.
  • B: read the connection values from the acceptance config file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants