Skip to content

fix(packages): restore a branch at its commit, and keep paths out of manager_language (3.6) - #2559

Merged
Seiger merged 3 commits into
evolution-cms:3.6.xfrom
elcreator:fix-custom-package-commit-and-language-3.6
Nov 7, 2026
Merged

Seiger merged 3 commits into
evolution-cms:3.6.xfrom
elcreator:fix-custom-package-commit-and-language-3.6

Conversation

@elcreator

@elcreator elcreator commented Nov 6, 2026 •

Copy link
Copy Markdown

Problem

1. A package on a branch could move without consent. This is the regression reported on #2551 and #2552. lockPackages() keeps only version, so a site on dev-main at commit A was pinned as plain dev-main. After a release replaced the lock, Composer resolved the branch wherever it stood by then, and unexpectedChanges() accepted that because a commit on one side only counted as equal. plan() returned ok with consent off.

Reproduced on both branches with real Composer 2.10.2, a real lock and a real record, before changing anything.

2. manager_language could hold a path. The user editor stores any value, and getUserSettings() merged it as it was. ManagerTheme normalises the setting in the manager only, so a manager on the front end kept the stored value. Several includes read it without asking ManagerLanguage::isName(): the error handler, the updater and extras plugins, the store and its installer, and the old web user form. Packages that build a path from the setting read it the same way. Writing the value takes a permission to edit users, and each include has a fixed suffix.

Change

Packages

  • lockPackages() keeps the commit of a git or hg branch. The pin becomes dev-main#<commit>, through previousVersions() and the record file.
  • A branch without a source keeps the dist reference Composer prints for it, the same way: dev-main <reference>. That is a path repository with its default reference: auto, or config, and it covers private dependencies. This reference is only compared: composerVersion() drops it before updateArguments() and the command the installer prints, so it never becomes #<reference>.
  • updateArguments() needs no change: Composer accepts that form, so the restore comes back at the commit the site ran. An older Composer that ignores the commit resolves the branch and is refused like any other change.
  • A branch whose reference is not known on both sides counts as a change, so it needs consent. An unchanged path reference passes and a changed one needs consent. A path repository with reference: none has no reference on either side and passes as before.
  • AGENTS.md records the rule.

Language

  • Core::getSettings() and getUserSettings() let only a locale name into the configuration, which also covers packages. A bad per-user value leaves the system language in place; a bad system value reads as English. A deleted locale still passes, as ManagerLanguage documents.
  • The six includes ask ManagerLanguage::isName() themselves, because the error handler can run before the settings are loaded.

Behaviour to expect

  • A record written before this change has no commit or reference, so an installer that reads such a record for a package on a branch asks for consent once, on the next update. That includes a path package with a reference. make:site reads the live lock first and is not affected.
  • If the commit is gone from the repository (a force push), the dry run cannot tell, because Composer does not check the reference there. The real update then fails with exit 1 and the update stays incomplete. The site is never moved silently.

Tests

  • New regression test from the review: a real lock at A, replaced by a release, restored at B without consent is refused, read from the lock and from the record. The same commit passes.

  • ManagerLanguageSettingTest covers the settings merge and each include; StoreLanguageTest covers a path as the store language.

  • All 21 new cases failed on unfixed 3.6.x. On 3.5.x the unfixed behaviour was shown by the Composer reproduction above, not by running these tests.

  • Added for the path-repository review, with dry runs written by Composer's own InstallOperation::format(): an unchanged path package and its private dependency restore without consent from the lock and from the record, on two updates in a row; a changed reference needs consent on either package; reference: none passes; the reference stays out of the arguments and the printed command. The 8 cases of the service failed on the previous head on both branches, and the printed-command case failed on 3.6.x without its line.

  • Real Composer with the fix, on both branches, branch moved to B: the update exits 0, the lock holds A, B's file is not in vendor, and the record keeps A.

  • Real Composer 2.11.1 with real path repositories (merge plugin, reference: auto), on both branches. The previous head refuses an unchanged package with dev-main => dev-main 5c2465e. With the fix it restores from the lock and the record, through a real restore and a second update. A changed private dependency is refused, and reference: none passes. The staged make:site dry run was not driven.

  • Full suite on Linux: 3.5.x 2346 passed, 3.6.x 2532 passed, 9 skipped on each. PHPStan reports no errors.

  • On a real site, 3.5.x and 3.6.x, each branch installed in phramark's evo-latte stack: guest and admin pass. With a path stored as manager_language for the site's user, the manager's login, edit, save and logout pass with no fatals, and the front end and manager answer 200. The stored value stays in the table and is ignored.

The manager's language page itself was not driven in a browser.

🤖 Generated with Claude Code

elcreator and others added 2 commits November 6, 2026 03:32
The lock kept only "dev-main", so a restore resolved the branch wherever it
stood by then, and treating a commit on one side as equal let that pass
without consent. The commit now travels with the pin (dev-main#<commit>),
through the lock and the record, and a branch whose commit is not known on
both sides counts as a change.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The user editor stores any value and the settings merge took it as it was.
The manager theme normalises it, but a manager on the front end kept it, and
it reached include in the error handler, the updater and extras plugins and
the store. Only a locale name now gets into the configuration, which also
covers packages that read the setting, and those includes ask for one too.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Seiger commented Nov 6, 2026

Copy link
Copy Markdown

@elcreator Please fix the path-repository regression before merging this and #2558.

Unchanged dev path packages are classified as changed. This is a static finding against 2a69122, checked against Composer 2.10.2's formatting and path-repository code.

Conditions: a custom package (or its private dependency) has a dev version, no source, and dist.type = path with a non-empty dist.reference. This is the normal default reference: auto case, not just an unusual configuration: Composer generates a config hash, or uses Git HEAD when available. reference: config also supplies a hash.

pinnedVersion() keeps only dev-main for that package, because it only reads git/hg source.reference. However, Composer's getFullPrettyVersion() falls back to dist.reference for a dev package without a source, so its lock operation is, for example:

Lock file operations: 1 install, 0 updates, 0 removals
  - Locking acme/local (dev-main 18f1a03)

After a release replaces the lock and the package needs restoring, unexpectedChanges() compares dev-main with dev-main 18f1a03. The new one-sided-reference check rejects it, so plan(..., allowChanges: false) returns ok=false, reason=changes even when the path reference is unchanged. recordInstalled() also persists only the plain version via dependencyClosure(), so accepting once does not cure this: the same false consent requirement recurs on later restores after the release lock replaces the site's lock.

Requested correction: preserve the reference used for comparison through both the live-lock and saved-record paths, including private dependencies, separately from the constraint passed to Composer. A path dist.reference is comparison metadata, not a VCS commit override to append blindly as #ref. Keep the new protection for unknown/moved git/hg commits; do not restore the old blanket rule that a missing reference equals any reference.

Please add regressions on both branches using Composer-formatted output (ideally generated by InstallOperation::format() or a real Composer dry run):

  • An unchanged dev path package with a non-empty default/config reference restores without consent, from both the original lock and the saved record.
  • After recording the successful restore and replacing the lock again, the unchanged package still restores without consent.
  • A changed path reference still needs consent; reference: none and the existing git/hg unknown/moved-commit cases remain covered.

The current dev-main vs dev-main path test only covers a no-reference configuration, such as reference: none; it does not exercise Composer's default path reference generation. Please also correct the PR description's claim that path repositories have no reference on either side.

SokilAI

Composer prints the dist reference of a dev package that has no source, so an
unchanged path package read as a change and needed consent on every update.
Keep that reference in the pin and the record to compare, and drop it before
anything reaches Composer.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@elcreator

Copy link
Copy Markdown
Author

@Seiger Confirmed and fixed in 38794e3, thanks. Composer falls back to dist.reference for a dev package with no source, and a real dry run printed dev-main 5c2465e as you described.

  • The pin now keeps that reference as dev-main <reference>, through the lock, the record and private dependencies. It is only compared: composerVersion() drops it before updateArguments() and the command the installer prints, so it never becomes #ref.
  • Unknown or moved git/hg commits are still refused, and a missing reference still does not equal any reference.
  • The new tests use dry runs written by InstallOperation::format(), and real Composer 2.11.1 with real path repositories gives the same results. An unchanged package restores from the lock and the record, also after a restore and a second update. A changed reference needs consent, also on a private dependency. reference: none passes.
  • The PR description no longer says path repositories have no reference. It also notes one thing to expect: a record written before this change has no reference, so the installers ask for consent once for such a package. make:site reads the live lock and does not ask.

@Seiger
Seiger merged commit 66b2294 into evolution-cms:3.6.x Nov 7, 2026
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