Repository navigation
Issue/575 console boot that cannot lock out core:env - #577
Merged
armanist merged 11 commits intoOct 14, 2026
Merged
Conversation
…v file exists Add a non-throwing fileExists() that tells whether the environment file the app is configured for exists. It follows the same resolution as load(): the app_env of shared/config/env.php picks .env or .env.<app_env>. The resolution is shared with resolveEnvironmentFile() through two small private helpers, and it uses App::getBaseDir() instead of the base_dir() helper, so the method can be asked before the helpers are loaded. load() behaves as before. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…ironment Add a requiresEnvironment property to CliCommand, true by default, with a public requiresEnvironment() getter, and set it to false in EnvCommand, which creates the environment file and must run when there is none. Nothing uses the getter yet; the run stage will in a following commit. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…v file is missing After the unknown command guard, RunCommandStage now asks the resolved command. A CliCommand that requires the environment is stopped with the same EnvException the boot raises when the environment file does not exist, before anything runs. Commands that declare they do not need it, such as core:env, and plain Symfony commands, such as list and help, still run. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…t is decided Add characterization tests for the boot done by the console adapter: an unknown command gets the full boot, core:env with an option still gets the reduced boot, the environment is mutable after a full boot, and the Symfony application is named from the app config after a full boot and UNKNOWN after the reduced boot. They pass on the current code and guard the following change that decides the boot by the environment file instead of the command name. The boot for a missing command name is deliberately not pinned, because that behavior is meant to change later. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…t by the command name The console adapter no longer compares the first argument with core:env. It asks Environment::fileExists() once and uses the answer for both the boot stages and the mutable environment step: with the environment file the app boots fully, whatever the command; without it only the helpers are loaded, so core:env can create the file. Commands that need the environment are stopped by RunCommandStage with the usual EnvException, and list and help now work without the file. Behavior change: core:env now gets the full boot when the environment file exists. Three tests used core:env with the file present as the trigger for the reduced boot (BootsReducedStagesForCoreEnvCommand, ...WithOptions and NamesTheApplicationUnknownAfterTheReducedBoot); they keep their assertions and now hide the environment file instead. New tests cover the full boot of core:env with the file, the reduced boot of any command without it, and start() with and without the file. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…e dir The overwrite check of core:env used the relative path .env, which PHP resolves against the current working directory, while the copy always went to the base dir. Running the command from another directory therefore skipped the confirmation and silently overwrote the project's existing .env. The check now uses the same base dir path as the copy. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
The console run will continue after a failed boot and decide per command whether the failure matters, so the context needs to hold it. The new constructor argument is optional, so existing callers are unchanged. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…failure per command Deciding the boot by the env file could not repair a broken .env: an empty or malformed file got the full boot, which failed, so `core:env --yes` was locked out where the old name check could still repair it. The adapter now runs the whole boot in a try, keeps the failure and continues. RunCommandStage re-throws that failure only for a command that requires the environment, so `core:env`, `list` and `help` run in a failed boot and every other command reports the original error. start() throws the recorded failure when anything else fails first (for example a command constructor). The application starts as UNKNOWN and is named from the app config after a successful boot, and the environment becomes mutable only then. Replaced on purpose: the three tests that pinned the reduced boot for a missing env file, and the RunCommandStage tests that pinned the file-missing guard; both now describe the recorded boot failure. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…y it any more The console no longer asks whether the env file exists: it boots, keeps the failure and decides per command. fileExists(), its two private helpers, the getBaseDir() switch in resolveEnvironmentFile() and the tests that came with them are removed, so the Environment package is back to what master has. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 34 minutes. View limit details
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Linked Issues check |  |
Issue #575 requires boot selection from the environment-file state, a non-throwing Environment::fileExists() that shares resolution with loading, and reduced boot when the file is absent. The review… |
Implement the #575 file-based boot decision and Environment::fileExists() behavior. Add or retain tests for shared file resolution, full boot when the file exists, and reduced boot with working list and help when it does not. |
| Docstring Coverage | |
Docstring coverage is 23.44% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 64 functions across 11 files. (1 skipped:… | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title identifies the main change: console boot failures must not prevent core:envfrom running. It is specific and related to the pull request, although the wording is slightly awkward.Out of Scope Changes check The reviewed changes remain connected to #575. Boot-failure retention, command environment requirements,ConsoleContextaccess, failure-state tests, the base-directory overwrite check, and the chang…
Full details: Linked Issues check
-
Explanation
Issue
#575requires boot selection from the environment-file state, a non-throwingEnvironment::fileExists()that shares resolution with loading, and reduced boot when the file is absent. The reviewed code inConsoleAppAdapteralways runs the full boot and records a failure. It does not addEnvironment::fileExists(). TheCliCommandopt-out,EnvCommandbase-directory check, tests, and changelog cover other#575requirements, but they do not implement the required file-based boot decision.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 23.44% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 64 functions across 11 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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.
Comment @coderabbitai help to get the list of available commands.
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 58840f4357
ℹ️ 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".
…cing them The application is named from the app config after a successful boot, and `?:` also replaced an empty string or '0' with UNKNOWN. `??` still covers the null name that used to crash the console and keeps every other value. The two tests that set the app config before building the adapter now load the helpers themselves, so they also pass when run alone. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
andrey-smaelov
approved these changes
Oct 14, 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
3 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.
Closes #575
What changed
ConsoleAppAdapterno longer compares the first argument withcore:env. It runs oneBootPipelineinside atry; when the boot fails it keeps the failure and continues. The SymfonyApplicationis created once asUNKNOWNand named from the app config after a successful boot, and the environment becomes mutable only after a successful boot.RunCommandStagere-throws that original failure only for aCliCommandthat requires the environment, socore:env,listandhelpstill run in a failed boot.start()throws the recorded failure when anything else fails first, for example a command constructor.CliCommand::$requiresEnvironment(defaulttrue) withrequiresEnvironment();EnvCommandsets it tofalse.ConsoleContexttakes an optional boot failure and exposes it withgetBootFailure().EnvCommandlooks for the existing.envin the application base directory instead of the current working directory, so the overwrite confirmation is no longer skipped when the command runs from another directory.Why
The reduced boot was keyed to the string
core:env, so the adapter had to know a command name. Deciding by whether the env file exists looked like the fix, but a broken.env(empty, or a Dotenv syntax error) then got the full boot, which failed, andcore:env --yescould not repair it, where master could. The boot is now attempted, the failure is recorded and enforced per command, the same way Django does it. The change of approach is explained in the comment on #575.Breaking changes
None for applications.
CliCommandgains a property and a getter with a default that keeps today's behavior, andConsoleContextgains an optional constructor argument. The reduced boot forcore:envno longer exists:core:envgoes through the same boot as every command, and a failed boot no longer stops it.How behavior was proven
quantum/project, master against this branch: 7 project states (ok,.envmissing, empty, malformed, broken app config, broken logging config,fs.phpcallingenv()with no.env) by 5 commands (core:env --yes,list,core:version, unknown command, no command). The branch is never worse than master.core:env --yesrepairs wherever master did,listworks in the failure states where master crashed, and commands that require the environment show the same original error as master..env, a requiring and an opt-out command,list, an unknown command, and a command constructor that throws.Validation
vendor/bin/phpunit: 1680 tests, 4022 assertions, 28 errors (Redis/Memcached connections, the same 28 as on master), 4 skipped (optional Twig not installed)composer cs:checkandcomposer phpstanpassedNotes for reviewers
core:envruns in whatever state a failed boot leaves, so it must not rely on config.fs.phpthat callsenv()with no.envstill cannot be repaired. The same happens on master..envno longer crashes: the app runs with the nameUNKNOWN.ErrorHandleris a separate change.quantum-php-docs:cli/qt-command-line-tool.md,cli/console-commands.md,packages/console/*,packages/environment/*,packages/app/adapters.md,packages/app/architecture.md.Summary by CodeRabbit
.envfile in the application directory, even when run from another working directory.