Repository navigation
fix(zip): let Shopware install the UCP SDK instead of bundling it - #248
Conversation
…archive The 1.3.0 archive vendors the UCP SDK together with the autoloader Composer generates for it, and the plugin requires that file itself -- because Shopware never loads a plugin's own vendor/autoload.php. Every shop that installed the archive therefore ran two Composer registries. FroshTools reports it as `2 autoloaders registered: shopware/agentic-commerce, shopware/production` (FriendsOfShopware/FroshTools#469, closed as this plugin's problem), and the concern behind that check is real: InstalledVersions::getAllRawData() aggregates every registered ClassLoader that carries an installed.php, so the plugin's copy can answer version lookups the shop's own registry should own. Dropping the bundle is not an option. RequirementsValidator::validateShippedDependencies() satisfies the ucp-php-sdk requirements by reading the plugin's own vendor/composer/installed.json, and without it a zip install fails with MissingRequirementException on 6.5.8, 6.6 and 6.7 alike. executeComposerCommands() is worse: it needs Packagist reachable at install time, rewrites the merchant's composer.json, and is a silent no-op on cluster setups -- it is also what broke 1.2.x. What Shopware does register is a plugin's own autoload.psr-4 (KernelPluginLoader::registerPluginNamespaces, unchanged across all three lines), out of the whole autoload block PluginService::refreshPlugins() stores in the plugin.autoload column. So the build now declares the bundled packages' own namespaces there, pointing into the vendored tree, and strips everything Composer generated except installed.json. The archive ships the same files as before and registers nothing. The plugin keeps a fallback, narrowed to the two cases the manifest cannot cover: plugin:install -r and plugin:update-all refresh the plugin.autoload column inside a kernel that is already booted, so in that one process the archive's prefixes are a boot behind; and a development lane vendors the SDK into the plugin's own vendor/ and relies on the autoloader Composer generates there. A plain spl_autoload_register closure serves the first, which keeps it out of Composer's registry too. Guards, because both halves of this have shipped broken before: - bin/ci-assert-zip-vendors-sdk.sh now checks the shipped manifest -- psr-4 targets that are really in the archive, config.vendor-dir, the SDK still in require -- and fails on any Composer runtime under vendor/ other than installed.json. - bin/test-zip-install.sh resolves the SDK the way Shopware resolves it instead of requiring the plugin's autoloader, and asserts the installed plugin registers nothing and that Composer reports a single autoloader dataset. - package-zip.yml gains a zip-install job that installs the built archive on 6.5.x, 6.6.x and trunk, against a shop ci-smoke.sh has stripped of the plugin and both SDK packages. Nothing in CI ever installed the vendored archive before; that is how the 1.3.0 defect reached the store.
`shopware-cli` unmarshals `autoload.psr-4` values into a Go string, so a JSON array makes
every `shopware-cli extension` command fail outright before it does anything:
FATAL newPlatformPlugin: json: cannot unmarshal array into Go struct field
.autoload.psr-4 of type string
Composer and Shopware both accept either form -- `KernelPluginLoader` normalises a string
into a one-element array -- which is why nothing local caught it and the packaging job did,
on the first command it ran.
One path per namespace as a plain string, then, and the build fails loudly if a bundled
package ever declares more than one path for a namespace rather than emitting a list
shopware-cli cannot read. The plugin's own fallback keeps accepting both shapes, because a
plugin composer.json legitimately may carry either, and the unit test now covers both.
ci-smoke.sh refuses the combination outright -- a run that installs no plugin has no smoke assertions left to make -- so all three lanes failed in ten seconds before booting anything.
`extension zip --disable-git` copies the working tree verbatim, and neither .tools/ -- where the source manifest points Composer -- nor the root node_modules/ was excluded. CI never hit it because the packaging job flips vendor-dir before its first install and never runs npm at the root, but a local build in a developer's checkout shipped PHPUnit, PHPStan, php-cs-fixer and the whole npm tree. The .sdk/ guard is now a loop over all three.
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. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 55ccd51399
ℹ️ 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 (@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 (@codex) address that feedback".
Two findings from the Codex review of #248. **The install harness claimed a restoration it had not performed.** The clean-lane case this change added leaves `plugin_moved` at 0, so the EXIT handler put nothing back and still said the lane was restored -- on a lane a developer had deliberately cleaned, the store archive stayed installed and active. Restore now takes the archive back out when it was the thing that put it there: deactivate, uninstall, remove the directory, refresh. Each of the three outcomes reports what it actually did instead of one message for all of them. **The new docblock narrated the incident instead of the invariant.** AGENTS.md sets `src/` at about 0.24 comment lines per code line and says the story belongs in the commit message; the diff measured 0.45. It now measures 0.24, keeping the two things a reader could otherwise undo -- prefixes arrive from the `plugin.autoload` column a boot late during in-process lifecycle commands, and requiring a bundled `vendor/autoload.php` registers a second ClassLoader into Composer's runtime registry -- with the account of what shipped in 1.3.0 left in the commit that fixed it and in README.md.
|
Codex (Codex (@codex)) review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5424b504b6
ℹ️ 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 (@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 (@codex) address that feedback".
The cleanup added in 5424b50 was armed one step too late. `archive_installed` was set after the install status check, and a non-204 install exits before it -- so the one outcome this harness exists to catch left the extracted plugin directory and its refreshed record behind on a lane that started with neither. The flag is now set before the upload, where extraction actually happens, and renamed `archive_present` to say what it means. That also covers the sibling nobody reported: the `extension/refresh` call between upload and install, which under `set -e` would have exited at the same point with the archive already on disk. Every step of the removal was already best-effort, so an uninstall of something that never installed is a no-op. Exercised rather than argued: an archive carrying an unsatisfiable requirement uploads with 204, fails install with 424 `Required plugin/package "acme/definitely-not-installed ^9.9" is missing`, and the lane is left with no plugin rows, storefront 200 and `/.well-known/ucp` 404. The script still exits 1 through the trap, so CI keeps failing the job.
|
Codex (Codex (@codex)) review |
|
Codex Review: Didn't find any major issues. Nice work! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "Codex (@codex) address that feedback". |
…it is missing
The archive stops shipping the UCP SDK. `executeComposerCommands()` returns true, so Shopware
runs `composer require shopware/agentic-commerce:<version>` against the project when the
extension is installed or updated; `shopware/production` declares `custom/plugins/*` as a path
repository, so that resolves the extracted archive locally and pulls the pinned SDK from
Packagist into the shop's own vendor. One autoloader, nothing bundled, and 619 KB instead of
1.5 MB.
That alone would have bricked every shop already running 1.3.0. An update extracts the new
files one request before Shopware runs Composer, and the old bundled SDK leaves with the
directory the extractor renames -- so an active plugin boots with its dependency gone, and this
one could not boot at all:
SdkNotAvailableException: Unable to load the UCP SDK Symfony bundle from Composer dependencies
at SwagAgenticCommerce->getAdditionalBundles() <- Kernel->registerBundles()
and behind it, once that was survivable:
InvalidArgumentException: There is no extension able to load the configuration for "ucp_sdk"
at src/Resources/config/services.php(176) <- Kernel->buildContainer()
Storefront included, until someone installed the SDK by hand.
So the extension now switches itself off instead. `SdkAvailability` asks whether the shop has
the SDK this release pins -- comparing the installed version against the constraint rather than
calling `class_exists`, because a shop still holding the previous SDK passes that test and then
fails on the first class the new code needs. When the answer is no, `services.php` and
`routes.php` register nothing and `getAdditionalBundles()` returns none, so the container
compiles without the plugin rather than half of it. `build()` writes what happened and the one
command that fixes it to the shop's `var/log/swag-agentic-commerce.log`. Activation bootstraps
the SDK schema as well as install and update, so a shop that recovers later ends up complete.
Verified on 6.5, 6.6 and trunk lanes, against archives built by shopware-cli:
- fresh install: 204/204, SDK resolved from the shop's own vendor, no plugin-local vendor,
Composer reports a single autoloader dataset, UCP answers 2026-08-25
- update from bundled 1.3.0: the window degrades instead of failing -- storefront 200, UCP 404,
the log line written -- then the update returns 204 and UCP is back
- update where the pinned SDK moves (0.0.7 -> 0.0.6): same, and Composer swapped the SDK in the
shop's vendor to match the new pin
bin/ci-assert-zip-vendors-sdk.sh becomes bin/ci-assert-zip-no-vendor.sh and asserts the
opposite of what it used to: no vendor tree, no marker, the SDK still required, and a concrete
version in the manifest -- a path repository takes the package version from that field, and a
require of `<name>:<version>` that cannot match fails the install.
|
Changed direction after Soner (@shyim)'s point — the description is rewritten, here is what moved. The bundle is gone. One thing that does not survive on its own: an update extracts the new files a request before Composer runs, so a shop on bundled 1.3.0 boots with the SDK gone and 500s everywhere, storefront included — Verified on 6.5, 6.6 and trunk: fresh install, update from bundled 1.3.0, and an update where the pin moves |
Die beiden neuen Einträge lasen sich wie aus dem Englischen übertragen: "betrieb zwei Composer-Registries", "Request", "pinnt", "gegen ein altes SDK laufen lassen". Jetzt im Duktus der 1.3.0-Einträge -- ganze Sätze aus Sicht des Shops, deutsche Begriffe wo es welche gibt, englische nur dort, wo sie der Name der Sache sind.
Both entries carried their whole story in one clause chain each -- a 60-word sentence joined by "which is what ... -- ... and ...", and a nested aside in "writes what happened, and the one command that fixes it, to var/log". Same facts, same voice as the 1.3.0 entries, sentences that stop where a reader would.
Nothing in AGENTS.md said how the extension reaches a shop, so the same ground got rediscovered twice at a cost: 1.3.0 shipped a bundled SDK that registered a second autoloader in every shop, and removing it again would have left every shop on 1.3.0 answering 500 on every page until someone installed the SDK by hand. The new *Installation And Update* section records what has to stay true: the archive ships no dependencies because Shopware runs `composer require` itself and `custom/plugins/*` is a path repository in every project; a plugin-local `vendor/autoload.php` is what puts a second ClassLoader in the shop; an update extracts the new files one request before Composer runs, so the extension has to boot with its dependency missing or one version behind, and it does that by registering nothing rather than half a container. With a table of the five failure messages and what each one means, so the next person recognises them instead of bisecting. Also corrected two claims that had gone stale: the README said package-zip builds with the `shopware/github-actions/build-zip` action (it calls shopware-cli directly, and now installs the archive on three lanes), and the automated-release plan listed "remove bundled-SDK marker handling" as pending when it has been done, undone and redone since.
…ugins shopware/shopware#13630 names the case this PR works around -- "the flag can be set or unset from one version to the other" -- and proposes running composer into a separate vendor directory that is swapped in at the end, which removes the window an active plugin currently boots in. #13631 goes further and requires store plugins from the SBP registry instead of shipping zips. Recorded so the guard here reads as a workaround with an owner upstream, rather than as the way things must be.
|
Upstream context for the degraded-mode half of this PR, added to
Neither removes the guard here: the extension supports 6.5.8 upwards, so shops without those changes stay in scope for years. But it means what this PR adds is a workaround with an owner upstream rather than a permanent shape — worth knowing before anyone redesigns it. |
The degraded path had no test. Someone adding a service, a route, a bundle or a class the
service glob reflects on -- any of which needs the SDK and sits outside the SdkAvailability
guard -- would pass every check here and only be found by a merchant whose shop answered 500
on every page after an update. That is how 1.3.0 shipped.
bin/test-zip-install.sh now takes the SDK away from the installed, active extension, which is
the state an update leaves behind for one request, and asserts the storefront still answers
200, that UCP is switched off rather than failing, that var/log/swag-agentic-commerce.log
names the command that fixes it, and that everything returns once the SDK is back. It runs on
6.5.x, 6.6.x and trunk in the zip-install matrix.
Checked that it can fail: an archive built with the guard removed from services.php installs
and activates as normal, then exits 1 with
FAIL: the storefront answered HTTP 500 without the SDK.
An active extension must not take the shop down while composer has not run yet;
something is wired to the SDK outside the SdkAvailability guard.
…tus properly Two defects in the check added in 206dd7e, both found by CI on all three lanes. **Recovery raced a live worker.** Putting the SDK back and deleting var/cache does not change the container class name, so a PHP worker that already loaded the degraded container keeps serving it -- the timestamps show the shop answering 404 within 1.7s of the restore, from memory. It passed locally because a different worker happened to take the request. The check now toggles the extension through the API, which changes the plugin list and with it the container, and polls for up to 30s. That is also what a merchant would reach for, next to the `composer require` the log entry names. **`404000`.** `curl ... || echo "000"` concatenates when curl prints a status and then exits non-zero anyway, which trunk managed twice. A helper now keeps the code when there is one and reports `000` only when there is not. The failing branch dumps `SdkAvailability::reason()` and whether vendor/ucp-php-sdk exists, so the next failure explains itself rather than needing the run logs.
… twice The upgrade off a bundled SDK, and an upgrade where the pinned SDK moves, stay out of CI: both need a second archive in the job, and the bundled-SDK upgrade happens exactly once on a small installed base. Both were verified by hand for 1.4.0. What a merchant needs instead is a place to look, so the README gains a Troubleshooting section: what the switched-off state looks like (storefront serving, `/.well-known/ucp` answering 404, feeds quiet), the log entry that names the fix, how to recover, and what to expect on the one upgrade that has a gap -- including the 500 the upload can answer on that single request. Both changelogs point at it. AGENTS.md now records the decision rather than listing the two scenarios as gaps, so the next reader does not propose the second archive again.
Every other release section lists its bullets on consecutive lines; joining these two with a blank line rendered them as separate lists.
The troubleshooting entry pointed only at var/log/swag-agentic-commerce.log. The message also goes to PHP's error log, which is the one a hoster is more likely to have in front of them, and it is worth saying why it is not in Shopware's own prod log: the decision happens while the container is compiling, before any logger service exists.
The check failed on all three lanes with the SDK present and SdkAvailability reporting it usable: the web worker kept serving the container it had already built, for the full 30s poll and across a deactivate/activate cycle. That is when a PHP worker lets go of a compiled container -- the platform's business, not this extension's, and not something to build a test around. What it was meant to show is already shown by the install above: that run starts on a shop with no SDK and ends with UCP answering. The SDK is still put back, so the lane is left as it was found, and the load-bearing assertions stay -- storefront 200 and UCP 404 rather than 500 while the SDK is gone, which is what catches SDK wiring added outside the guard.
The markdown had no shared shape. `docs/*` wrapped around 95 columns, README and AGENTS.md ran to 800 and 184 characters on a line, and the README's own sections disagreed with each other -- `Release` had 31 of 43 prose lines over 120 characters while `QA` had none. Nothing enforced either style, so every edit picked one. Prettier now owns it: 100 columns, wrapped prose, tables aligned, every markdown file in the repository. The changelogs keep one line per entry, which is their own convention and readable in a way wrapping would not improve -- `.prettierrc.json` carries that as an override rather than as a habit. `npm run lint:md` checks, `npm run lint:md:fix` applies, and a `docs-lint` job runs the check on every pull request; it is in `expected_checks`, because a check nobody waits for is decoration. `bin/ci-assert-markdown-links.sh` joins it and asserts that every relative link resolves. Three were already dead -- `docs/manual-testing.md` linking to `docs/x.md` from inside `docs/`, which means `docs/docs/x.md`. Nothing renders an error for that. The README was 417 lines and mixed the merchant's questions with the maintainer's. It is 224 now: what the plugin is, the three feature areas, the UCP setup walkthrough, troubleshooting, and a table pointing at the rest. `Release`, `QA` and `Local Development` moved to `docs/releasing.md`, `docs/qa.md` and `docs/local-development.md`, with the trailing paragraphs sorted into whichever of the three they actually belonged to. AGENTS.md and manual-testing.md now point at the new homes instead of at README sections that no longer exist.
Dominik Grothaus (dgrothaus-sw)
left a comment
There was a problem hiding this comment.
Looks good for what the PR claims this would do.
But there's a lot of additional things going on like linting, rewriting and so on.
And there's no word of cluster setup in this PR or the comments, only about offline installs. For cluster setups PluginLifecycleService::executeComposerRequireWhenNeeded() returns false, so the plugin will miss the dependencies without raising an error.
…r-autoloader # Conflicts: # CHANGELOG.md # CHANGELOG_de-DE.md # README.md
… nothing @dgrothaus-sw on #248: cluster setups were nowhere in this PR, only offline installs, and `PluginLifecycleService::executeComposerRequireWhenNeeded()` returns early there -- so the requirements are never resolved and nothing says so. That is the one deployment where waiting does not help. Everywhere else a missing SDK is the window between extraction and `composer require`, and Composer closes it by itself; on a cluster setup the filesystem is built elsewhere and Shopware deliberately never runs Composer for a plugin, so the extension would install without error and then do nothing at all. install() and update() now refuse it, naming what is missing and what to add to the project's composer.json. The log entry was wrong for those shops too: it said Shopware installs the requirements itself when the extension is installed or updated, which on a cluster setup it never does. It now says so, and names the build-time alternative. README gains a *Cluster setups* section next to the other two troubleshooting entries, and both changelogs carry the exception.
|
Dominik Grothaus (@dgrothaus-sw) both points are fair, thank you — conflicts are resolved and the cluster case is handled in Cluster setups. You are right that nothing covered them. So Everywhere else the refusal stays off, because there a missing SDK is the window between extraction and Scope. Also fair, and it was deliberate rather than drift: the markdown formatting, the linter and the README split were asked for in this PR while it was open. If you would still rather review them apart, I can lift the docs commits into their own PR and leave this one to the SDK change — say the word. |
|
Copilot resolve the merge conflicts in this pull request |
…r-autoloader # Conflicts: # README.md Co-authored-by: BrocksiNet <3763023+BrocksiNet@users.noreply.github.com>
Co-authored-by: BrocksiNet <3763023+BrocksiNet@users.noreply.github.com>
Resolved in |
What is wrong
1.3.0 vendors the UCP SDK into the plugin's own
vendor/and loads the autoloader Composer generated for it, because Shopware does not load a plugin'svendor/autoload.php. Every shop that installs it then runs two Composer registries:That is FriendsOfShopware/FroshTools#469.
MultipleAutoloaderCheckeryields one dataset per registeredClassLoaderwhose vendor dir carries acomposer/installed.php, so the plugin's copy joins the shop's registry and can answer version lookups the shop should own.What changes
The archive ships no dependencies at all.
executeComposerCommands()returns true, so Shopware runscomposer require shopware/agentic-commerce:<version>against the project on install and update.shopware/productiondeclarescustom/plugins/*as a path repository, so that resolves the extracted archive locally and pulls the pinned SDK from Packagist into the shop's ownvendor/. One autoloader, nothing bundled, 619 KB instead of 1.5 MB.Two deployments do not get that for free, and both are handled explicitly:
shopware.deployment.cluster_setup: true) never lets Shopware run Composer for a plugin at all —executeComposerRequireWhenNeeded()returns early by design. Nothing would ever install the requirements, soinstall()andupdate()refuse there, naming what to add to the project'scomposer.json. Raised by Dominik Grothaus (@dgrothaus-sw) in review.Why it needs the second half
That change alone would have bricked every shop already on 1.3.0. An update extracts the new files one request before Shopware runs Composer, and the old bundled SDK leaves with the directory the extractor renames — so an active plugin boots with its dependency gone:
and behind it, once that is survivable:
Storefront included, until someone installs the SDK by hand.
So the extension switches itself off instead.
SdkAvailabilityasks whether the shop has the SDK this release pins — comparing the installed version against the constraint, not callingclass_exists, because a shop still holding the previous SDK passes that test and then fails on the first class the new code needs. When the answer is no:services.phpandroutes.phpregister nothing,getAdditionalBundles()returns none — the container compiles without the plugin rather than half of it. Half is not available:services.phpconfigures theucp_sdkextension, and 13 plugin classes implement SDK interfaces.build()writes the reason and the command that fixes it to PHP's error log and to the shop'svar/log/swag-agentic-commerce.log. It cannot use Shopware's own log: this is decided while the container is compiling, before any logger service exists.Verification
204, SDK resolved from the shop's own vendor, no plugin-local vendor, one autoloader dataset, UCP200serving2026-08-25, admin bundle servedzip-installjob, 6.5.x / 6.6.x / trunk200, UCP404rather than500, log entry writtenzip-installjob, all three lanes204and UCP back0.0.7→0.0.6)shopware-matrix, all three linesThe two upgrade rows are deliberately not in CI: both need a second archive in the job, and the bundled-SDK upgrade happens once on a small installed base.
AGENTS.mdrecords that decision, and the README Troubleshooting section tells a merchant what to expect.Guards
bin/ci-assert-zip-no-vendor.sh(wasci-assert-zip-vendors-sdk.sh) asserts the opposite of what it used to: no vendor tree, no marker file, the SDK still inrequire, and a concreteversionin the manifest — a path repository takes the package version from that field, and a require of<name>:<version>that cannot match fails the install. It also rejects build-only trees (.sdk,.tools,node_modules).bin/test-zip-install.shinstalls through the admin upload endpoint on a shop stripped of the plugin and the SDK, then takes the SDK away again from the installed extension and asserts the shop stays up. That is what catches SDK wiring added outside the guard — verified by building an archive with the guard removed, which fails it.package-zip.ymlgains azip-installjob running that harness on 6.5.x, 6.6.x and trunk. Nothing in CI installed the archive before — that is how the 1.3.0 defect reached the Store.Documentation, and why it is in this PR
AGENTS.mdgains an Installation And Update section: how the archive reaches a shop, why it ships no dependencies, what the update window is, the two rules that are easy to break again, and a table mapping failure messages to their cause. It also points at shopware/shopware#13630 and #13631, which would remove the window upstream.Asked for while this PR was open, and done here rather than after: the markdown had no shared shape —
docs/*wrapped near 95 columns, README and AGENTS ran to 800 and 184 characters on a line. Prettier now owns it at 100 columns with adocs-lintjob enforcing it,bin/ci-assert-markdown-links.shchecks that relative links resolve (three were already dead), and the README went from 417 to 224 lines, withRelease,QAandLocal Developmentmoved intodocs/. Happy to split that into its own PR if it reads better separately.