diff --git a/.gitattributes b/.gitattributes index 94009e8..5a786f2 100644 --- a/.gitattributes +++ b/.gitattributes @@ -12,6 +12,5 @@ /codeception.dist.yml export-ignore /codeception.slic.yml export-ignore /cspell.json export-ignore -/engineering-plan.md export-ignore /phpstan-cache export-ignore /phpstan.neon.dist export-ignore diff --git a/CLAUDE.md b/CLAUDE.md index 49ad964..94223d0 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -17,7 +17,19 @@ Safety rests on two independent, single-purpose config keys per sub-plugin: deactivate it. Deliberately out of scope: version negotiation, any opinion on toggle UI or storage, and any -production dependency on another StellarWP library. +production dependency on another StellarWP library. Version negotiation is the one that gets +re-litigated, so: it existed in the prior art only because of a single LearnDash ProPanel quirk — +v2.x was absorbed while a completely different plugin was rebranded ProPanel v3.0 — and a callable +`conflict_policy` already covers that as config. Do not reshape the architecture around it. + +**Prior art.** Two ad-hoc implementations this is distilled from. LearnDash `sfwd-lms` copied each +addon into `includes//` with a per-addon loader +(`src/Core/Modules/Course_Grid/Legacy/Loader.php` and siblings); the three modules diverged in class +shape, in guard-constant type, and in whether version negotiation existed at all — the exact +inconsistency this library removes. `kadence-shop-kit`'s `inc/Common/Features/` is the more polished +config-array Strategy pattern, but its Provider → Repository → Resolver → Strategy indirection is +heavier than this needs and it is coupled to Kadence's option store and DI52 container; here one +load path handles every sub-plugin. The `wp_admin_notice_markup` rewrite follows LearnDash 4.21.4. ## Commands @@ -77,7 +89,9 @@ seams a host may rebind: The rest — `Boot\Scheduler`, `Loader`, `Registry_Reader`, `Conflict\Detector`, `Conflict\Gatekeeper`, `Conflict\Redirector`, `Conflict\Rewriter`, `Notices\Store`, `Notices\Renderer`, `Notices\Presenter` — are bound as concrete classes. A host that wants one of them different rebinds the class name; there -is no interface because nothing in the library dispatches on one. +is no interface because nothing in the library dispatches on one. `Provider` also binds the container +under `ContainerInterface::class`, first and before anything else, so that a container which builds +unbound classes reflectively can still satisfy the collaborators that take one. An interface belonging to a folder-scoped concern lives in that folder's `Contracts\`, not beside its implementation and not in the top-level `src/Contracts/`. `src/Contracts/` is for the interfaces whose @@ -97,15 +111,14 @@ that never checked. **The container is required.** `Config::get_container()` throws `Config_Exception` when unset, which is what `uplink`, `telemetry`, `schema` and `harbor` all do; `has_container()` stays as the probe. -Optional was the outlier — of nineteen vendored StellarWP packages exactly one falls back to `new`, -and we had modelled ourselves on it. One requirement is what that outlier costs. "Container binding -when bound, `new $default_class` otherwise" forces every default class to be constructible with no -arguments, which forces `?Peer $peer = null` constructor parameters, which forces a `protected` -accessor per peer falling back to a static. `Container\Resolution` was the class holding that chain -together, and the chain is deleted with it. The hosts qualify: `learndash-core` ships a container -implementing this very contract, exposes it as `App::container()`, and already hands it to Telemetry, -Validation and Harbor. The plugins with no container are the add-ons being absorbed, not the hosts -doing the absorbing. +Optional was the outlier — of nineteen vendored StellarWP packages exactly one falls back to `new`. +Do not reintroduce a container-less path: "container binding when bound, `new $default_class` +otherwise" forces every default class to be constructible with no arguments, which forces +`?Peer $peer = null` constructor parameters, which forces a `protected` accessor per peer falling +back to a static — a service locator wearing a constructor signature, arrived at one reasonable step +at a time. The hosts qualify: `learndash-core` ships a container implementing this very contract, +exposes it as `App::container()`, and already hands it to Telemetry, Validation and Harbor. The +plugins with no container are the add-ons being absorbed, not the hosts doing the absorbing. **The container is typed as `container-contract`'s `ContainerInterface`, not `stellarwp/foundation-container`'s.** Not a rejection of Foundation — the two are the same target. @@ -169,8 +182,8 @@ the plugin to ask about, and the collaborator does the asking. ### What exists today -Every behaviour described above is built; nothing in `src/` is still owed. What is left in the plan -is the end-to-end suite and the release pass. +The library is feature-complete for 1.0.0. Every behaviour described above is built, and the suite +that drives the whole of it against a real WordPress is `tests/unit/Scenario/`. | Path | What | |---|---| @@ -195,8 +208,8 @@ is the end-to-end suite and the release pass. ``` Config::set_hook_prefix( 'give' ); Config::set_container( $container ); // required -Absorber::register( [ …config… ] ); // once per sub-plugin; a duplicate slug throws -Absorber::boot(); // idempotent +Absorber::register( [ …config… ] ); // once per sub-plugin; an unusable config throws here +Absorber::boot(); // idempotent → Provider::register() // every binding → Boot\Scheduler // every hook, as a closure over the container @@ -213,8 +226,15 @@ below priority 5 wires cleanly. It is priority 0 rather than plugin-file scope b priority 0 — anything that grabbed the container earlier holds an orphan whose bindings are discarded. Its own block rather than a service provider, for the same reason: a provider runs whenever the host's bootstrap happens to run it. This is also why `Absorber::register()` buffers and -resolves nothing — registration at plugin-file scope, which the spec sanctions, would otherwise -register into the throwaway. +resolves nothing — registration at plugin-file scope is a shape a host is entitled to use, and it +would otherwise register into the throwaway. + +**A duplicate slug is `Registrar::register()`'s exception, not `Absorber::register()`'s.** What +`Absorber::register()` throws is config validation, from the `Sub_Plugin` constructor, in the call +the host can see in its own stack trace. The buffer reaches the registrar at the first read — +`plugins_loaded` priority 5 on a request that passes the gatekeeper, priority 6 otherwise — so the +collision surfaces from inside a core action. Both are `Config_Exception`; only one of them can name +the line the host wrote. **The too-late barrier measures against the first step in the sequence, not the last.** `Boot\Scheduler` compares the priority `plugins_loaded` is already dispatching against the lowest @@ -233,11 +253,15 @@ standalone that survives the conflict defines the guard constant as it loads and see that. `load_all()` gates each sub-plugin in order, skipping on the first failure: enabled → not already -loaded → dependencies met → file exists → `should_load` filter → `require_once` → activation -callback (only after a *successful* require). - -The activation callback is the last of those and runs through `Activator`, which `Loader` takes -as a constructor argument like the writer and the registry reader. Last, because a bundled plugin is included rather +loaded → dependencies met → file is a readable file → `should_load` filter → `require_once` → +activation callback (only after a *successful* require). The file gate is `is_file() && +is_readable()`, not `file_exists()`: that last is true for a directory and for a file with no read +permission, and `require_once` fatals on both. Only the dependency gate queues a notice; an +unreadable file is a broken build in the host plugin and reports through `_doing_it_wrong()`. + +The activation callback is the last of those and runs through `Activator_Interface`, which `Loader` +takes as a constructor argument like the writer and the registry reader. Last, because a bundled +plugin is included rather than activated: `register_activation_hook()` never fires for it, so the callback stands in for whatever that hook would have done, and it has to run with the plugin's own code already in memory. Only after a require that happened, because creating tables and seeding options for a sub-plugin @@ -249,7 +273,7 @@ rather than marked done. The guard constant is checked **before** the dependency check, not after. It is one `defined()`, it carries the whole re-declaration guarantee, and it is the only gate meaning "this plugin is already running" — warning that requirements are unmet for a plugin the admin can watch working would send -them after the wrong problem. `docs/filters.md` and the spec agree. +them after the wrong problem. `docs/filters.md` says the same. `Registry_Reader::all()` narrows to `Sub_Plugin` instances itself, so no caller repeats that guard. A host may bind a registrar returning anything, and PHP 7.4 cannot express `array` in @@ -308,12 +332,13 @@ discards the work behind `update.php?action=upgrade-plugin` exactly as it would `plugins.php?action=activate` is the request `plugin_sandbox_scrape()` replays, so exiting there makes core report the plugin being activated as fatal. Any action arg at all, never a list of the dangerous ones: half of wp-admin takes an `action`, plugins add their own, and a known-safe list -would have to stay right about every one of them forever. `user_may_resolve()` asks for -`manage_network_plugins` on multisite and `activate_plugins` otherwise — `plugins_loaded` runs -before `auth_redirect()`, so an unauthenticated GET of an admin URL gets that far, and the -capability has to match the reach of the act: `deactivate_plugins()` left at the default -`$network_wide` takes the standalone out of the *network's* active plugins, which a single site's -administrator holds no authority to do. +would have to stay right about every one of them forever. `Traits\Guards_Hook_Prefix` is the last of +the three, because it is the only one that reports to the developer and a missing prefix logged from +every front-end request would bury it. `user_may_resolve()` asks for `manage_network_plugins` on +multisite and `activate_plugins` otherwise — `plugins_loaded` runs before `auth_redirect()`, so an +unauthenticated GET of an admin URL gets that far, and the capability has to match the reach of the +act: `deactivate_plugins()` left at the default `$network_wide` takes the standalone out of the +*network's* active plugins, which a single site's administrator holds no authority to do. `Boot\Scheduler::sequence()` asks the two halves either side of `Conflict\Detector::has_conflict()`, and that order is the point. `current_user_can()` resolves and caches the current user, so asking it @@ -364,8 +389,12 @@ runnable inline as well as wirable. ### Keys -- Filters: `{$hook_prefix}/plugin_absorber/should_load`, `{$hook_prefix}/plugin_absorber/conflict_policy` -- Options: `{$option_prefix}_plugin_absorber_activations`, `{$option_prefix}_plugin_absorber_notices` +- Filters: `{$hook_prefix}/plugin_absorber/should_load` (`Loader`), + `{$hook_prefix}/plugin_absorber/conflict_policy`, + `{$hook_prefix}/plugin_absorber/conflict_notice_message` and + `{$hook_prefix}/plugin_absorber/dependency_notice_message` (all three `Sub_Plugin`) +- Options: `{$option_prefix}_plugin_absorber_activations` (`Activator`), + `{$option_prefix}_plugin_absorber_notices` (`Notices\Store`) Both are built in `Config` — `get_hook_name()` and `get_option_name()` — so nothing else assembles the segment between the host's prefix and the key's own name. The two differ in one respect: @@ -381,8 +410,16 @@ so it is a network option on multisite — matching `deactivate_plugins()`, whic ## Conventions **Namespace is `Nexcess\PluginAbsorber\`, not `StellarWP\`.** The Composer package is -`stellarwp/plugin-absorber`; the two deliberately do not match. Tests are `…\Tests\`, support -classes `…\Tests\Support\`. +`stellarwp/plugin-absorber`; the two deliberately do not match, and Composer does not require them +to. Packagist protects a vendor prefix once anyone publishes under it, and `nexcess` is already held +by an unrelated package outside our control, while `stellarwp` is ours across 38 packages — so the +package ships under the vendor we hold and the namespace stays what the code is. Tests are +`…\Tests\`, support classes `…\Tests\Support\`. + +**The name is `plugin-absorber`, not `plugin-loader`.** In WordPress, "plugin loader" is established +jargon for mu-plugin autoloading — Packagist's top results for it are all that, and +`plugin-absorber` collides with nothing. `Sub_Plugin` stays the value-object name regardless: +"sub-plugin" names the registered thing, "absorb" names what the library does to it. **Floors:** PHP `>=7.4`, WordPress 6.4 (for the `wp_admin_notice_markup` filter). WordPress is not a Composer dependency, so its floor lives in the README only. @@ -399,6 +436,11 @@ API it serves. Nexcess\PluginAbsorber`, and every method a docblock with `@since 1.0.0`. This binds `src/` only — test and support classes keep the file-level docblock but need no `@since`. +**`declare( strict_types=1 );` sits immediately after the file-level docblock**, in `src/` and +`tests/` alike — a coercing `int` parameter that silently accepts `"3 plugins"` is a bug this library +would report as a successful load. The files predating the rule get it the next time they are +touched. + **Comments describe behaviour, not the plan.** Never reference a task or plan-step number in a code comment; the code outlives the plan. Comments earn their place by explaining *why*, especially where a plausible alternative is wrong. @@ -408,19 +450,28 @@ Tabs for PHP, 4 spaces for yml/yaml/json/md (see `.editorconfig`). ### No test-only seams in `src/` Production classes do not get a `reset()` for the suite's benefit — that becomes API the library -supports forever. Tests clear static state by reflection through a helper under `tests/_support/`. -`Config` is served by `Tests\Support\Config_State::reset()`; `Registrar` and `Absorber` get the same -treatment. Any older sketch showing `Config::reset()` or `Absorber::reset()` means the support helper. +supports forever. Tests clear static state by reflection through a helper under `tests/_support/`: +`Tests\Support\Config_State::reset()` for `Config`, and `Tests\Support\Absorber_State::reset()` for +`Absorber` plus the registration buffer on `Registry_Reader`, which also unwires the hooks `boot()` +added. Any older sketch showing `Config::reset()` or `Absorber::reset()` means the support helper. +`Registrar` needs no such helper — its state is instance state behind a container binding, so a +fresh container *is* the reset. ### Testing rules -`tests/unit/` mirrors `src/`. Full detail lives in `tests/README.md`; the rules that bite hardest: +`tests/unit/` mirrors `src/`, with one exception: `tests/unit/Scenario/` is named for what its files +describe rather than for a class, and drives the library end to end through the hooks a host fires, +against real WordPress state. `Bootstrap_Test_Case.php` is the abstract parent of `LoadTest.php`, +`ConflictTest.php` and `HostTest.php`. Full detail — every scenario, with a diagram — lives in +`tests/README.md`; the rules that bite hardest: - **Never mock `exit()`.** `UopzFunctions::preventExit()` exists — do not use it. It lets a test run past the point where production would have stopped, so a test that should fail reports as passing. Instead stub the call immediately before `exit` (e.g. `wp_safe_redirect`), throw `Tests\Support\TestException` from it, catch it, and assert both a `$halted` flag *and* the - message. Dropping the flag turns "never redirected at all" into a silent pass. + message. Dropping the flag turns "never redirected at all" into a silent pass. That shape is + factored into `Traits\WithHaltedRedirects` — use `$this->capture_redirect( … )` rather than + hand-rolling it, so the flag cannot be the thing a new test forgets. - **A stub closure has no class scope.** uopz executes the replacement outside the test object, so `$this` and `self::` are fatal inside it. Bind a reference (`$x = &$this->prop;`), resolve constants to locals, `use` both, and mark the closure `static`. @@ -435,11 +486,16 @@ treatment. Any older sketch showing `Config::reset()` or `Absorber::reset()` mea - **Every test that touches a collaborator sets a container**, since there is no longer a fallback to fall back to — and it must be `Tests\Support\Test_Container`. `lucatume\DI52\Container` implements PSR-11's `ContainerInterface`, not StellarWP's, so passing it to `Config::set_container()` is a - `TypeError`. + `TypeError`. `Traits\WithContainer` is the one-line form: `set_up_container()`, `resolve()`, + `tear_down_container()`. - **Each load-path test writes its own bundled fixture file.** `require_once` caches by resolved path per PHP process, so a shared fixture makes every later test silently pass. -- **Shared fixtures go in a trait**, not a per-test helper: `tests/_support/Traits/WithSubPlugins.php` - builds `Sub_Plugin` objects for the whole suite. +- **Shared fixtures go in a trait**, not a per-test helper. `Traits\WithSubPlugins` builds + `Sub_Plugin` objects for the whole suite; `Traits\WithBundledPlugins` owns the fixture files and + their guard constants; `Traits\WithUsers` supplies a user who can activate plugins, and + `Traits\WithIncorrectUsage` asserts on `_doing_it_wrong()`. The spies — `Spy_Registrar`, + `Spy_Writer`, `Spy_Presenter`, `Spy_Resolver`, `Spy_Gatekeeper`, `Spy_Rewriter`, `Spy_Activator` — + are what gets bound into the test container in place of a real collaborator. - **Data providers are generators** returning `Generator` with named cases. - Tasks follow RED→GREEN: the failing test lands before the implementation. @@ -476,7 +532,7 @@ treatment. Any older sketch showing `Config::reset()` or `Absorber::reset()` mea one of these can only have been produced too early, and nothing in the value distinguishes it from one that was not. Refusing both costs the host a `static fn()` and fails the first time the code runs, where an eager `__()` is the `_load_textdomain_just_in_time` notice in someone else's log. - This is a deliberate departure from the spec's `string | callable`. + `string | callable` is the shape this was drafted with and it is deliberately not what shipped. - **`conflict_policy` takes either**, because a policy is usually a `Conflict_Policy` constant with nothing to defer, and is never text a user reads. `standalone_plugin_basename` is string-only: it names a file already on disk. @@ -498,6 +554,10 @@ treatment. Any older sketch showing `Config::reset()` or `Absorber::reset()` mea the latter is a common third-party shim. - **Strauss must not rewrite `plugin_loaded_constant` values.** They are shared runtime constants; prefixing them defeats the entire mechanism. +- **`enabled` is the one key with no type behind it.** It is read as a boolean when it is not + callable, so an array or an object there evaluates as enabled. That is documented rather than + validated: adding a check is fine, but do not describe the other keys' registration-time rejection + as covering this one. - **Never write a literal guard-constant *name* in `src/`.** Hosts run Strauss with a `constant_prefix` — `learndash-core` uses `LEARNDASH_`, with an empty exclude list — so a literal `'GIVE_RECURRING_VERSION'` in our source is rewritten at build time and the `defined()` check then @@ -518,15 +578,16 @@ treatment. Any older sketch showing `Config::reset()` or `Absorber::reset()` mea stays bare while nothing in its folder does the same job (`Conflict\Resolver`, not `Conflict\Standalone_Resolver`, even once `Gatekeeper` and `Redirector` sit beside it) and takes a qualifier only when a second class of the same kind lands — `Scheduler` beside a later - `Retry_Scheduler`, never a qualifier bought in advance. An abstract - `-ion`/`-ance` noun is a directory name over agent nouns — `Activation/` holding `Activator` — and - never a class name; a census of 3,343 classes across eight Nexcess/StellarWP codebases found zero. + `Retry_Scheduler`, never a qualifier bought in advance. An abstract `-ion`/`-ance` noun may name a + directory over agent nouns, the way `Boot/` holds `Scheduler`, but never a class: a census of 3,343 + classes across eight Nexcess/StellarWP codebases found zero. `Activator` sits at the root rather + than under an `Activation/` of its own, because one class is not a folder. ## Branch and PR workflow Branching is **stacked**: each branch cuts from the previous one and merges to `main` in order. -Branch names are `NN-topic` and map 1:1 to task numbers in the plan. Never open PR N+1 before PR N's -branch exists. `main` is releasable after every merge. +Branch names are `NN-topic`. Never open PR N+1 before PR N's branch exists. `main` is releasable +after every merge. - **PR size cap:** ≤10 files, tests and test infrastructure excluded. No logic-bearing PR exceeds 4 source files. @@ -557,26 +618,31 @@ branch exists. `main` is releasable after every merge. - New dev-only files belong in `.gitattributes` as `export-ignore` so they stay out of consumer installs. -## Documentation precedence - -`docs/superpowers/specs/2026-07-31-plugin-absorber-design.md` is authoritative. Where it and -`engineering-plan.md` disagree, **the spec wins** — the engineering plan is a superseded first draft -still carrying the old `stellarwp/sub-plugin-loader` package name, the `Nexcess\SubPluginLoader\` -namespace, a `Config::set_version()` that was removed, and an `ob_start()` approach replaced by the -`wp_admin_notice_markup` filter. `docs/superpowers/plans/2026-07-31-plugin-absorber.md` holds the -task-by-task breakdown, and -`docs/superpowers/plans/2026-08-12-container-required-rework.md` supersedes it wherever the two -disagree about the container, the collaborator seams or the class names — that plan is what branches -11 through 16 now implement, and the older plan still describes the optional-container design it -replaced. - -Once a task's PR merges to `main`, delete that task's section from the plan in the next branch that -touches the file; git history keeps it. A shipped task's plan describes code that already exists in -`src/`, so all it can still do is make an agent read past it to reach what is unbuilt — and since -the plan is edited on every branch of a stacked series, an oversized one is a standing -merge-conflict surface. Never renumber what survives: the numbers map 1:1 to branch names. The spec -is the durable document; the plan is scaffolding and should shrink toward empty as the series lands. +## Known, and deliberately not fixed in 1.0.0 + +**`Activator::maybe_run()` can double-run under concurrency.** It reads the option, runs the +callback, then writes. Two simultaneous first requests both see the flag unset and both invoke the +callback — which may be a `create_tables()`. Claiming the slot with `add_option()` before invoking +would close the window. The ordering is not the bug and must not be "fixed" by recording first: the +callback runs before the record so that a fatal mid-callback retries on the next request instead of +freezing a half-finished migration in place, permanently and invisibly. + +## Documentation + +**This file is the durable document.** The design spec, the task-by-task implementation plan, the +container-required rework plan and the superseded `engineering-plan.md` first draft were all deleted +once the last PR of the 1.0.0 series was written. Every decision they still carried is restated +above; git history has them in full if a rationale needs reading back. + +Do not reintroduce them, and do not write a new plan file for work that is already built. A plan +section describing code that exists in `src/` cannot do anything but make the next reader scroll past +it, and a plan edited on every branch of a stacked series is a standing merge-conflict surface. The +first draft was worse than useless by the end: it still named the package `stellarwp/sub-plugin-loader` +under a `Nexcess\SubPluginLoader\` namespace, with a `Config::set_version()` that was removed and an +`ob_start()` approach the `wp_admin_notice_markup` filter replaced. Human-facing docs are `README.md` plus `docs/installing.md`, `docs/configuration.md`, `docs/conflict-handling.md`, `docs/filters.md`, and `docs/notices.md`. Keep them short and keep -rationale here or in code comments — do not grow the README back. +rationale here or in code comments — do not grow the README back. `docs/` is `export-ignore`d and +`README.md` is not, so a link from the README into `docs/` must be an absolute repository URL; links +*between* files inside `docs/` stay relative. diff --git a/README.md b/README.md index f8e7d05..56930f1 100644 --- a/README.md +++ b/README.md @@ -13,7 +13,7 @@ composer require stellarwp/plugin-absorber **Use [Strauss](https://github.com/stellarwp/global-docs/blob/main/docs/strauss-setup.md)** — two plugins shipping different versions of this library will collide otherwise. See -[Installing](docs/installing.md) for the one prefixing rule you must not get wrong. +[Installing][installing] for the one prefixing rule you must not get wrong. ## Quick start @@ -39,23 +39,40 @@ add_action( 'plugins_loaded', function () { The container is required — any StellarWP `ContainerInterface` implementation, the one you already hand to Telemetry or Uplink. Every collaborator comes from it. -Boot before `plugins_loaded` priority 5, where conflict resolution runs: WordPress silently ignores a -callback added at or past the priority it is already dispatching. Later is reported through -`_doing_it_wrong()` and run inline, but the ordering guarantees are weaker. +Keep the `, 0`. `boot()` wires conflict resolution at `plugins_loaded` priority 5 and the load at +priority 6, and WordPress silently ignores a callback added at or past the priority it is already +dispatching — so anything below priority 5 wires cleanly, the priority 1 where several hosts wire +their container today included. Booting later is reported through `_doing_it_wrong()` and both steps +run inline instead — which on an admin page view can end the request in a redirect before `boot()` +returns. Priority 0 is the recommendation, in the block that owns your container rather than in a service provider: a host that builds one lazily and replaces it at priority 0 leaves us holding an orphan whose bindings were discarded. +A [complete bootstrap][configuration] — two sub-plugins, every optional key — closes the +configuration doc. + ## Docs -- [Installing](docs/installing.md) — Composer, Strauss, and the constants Strauss must leave alone. -- [Configuration](docs/configuration.md) — the hook prefix, the container, every sub-plugin key. -- [Conflict handling](docs/conflict-handling.md) — the policies, when they run, and the guard's limits. -- [Filters](docs/filters.md) — the runtime overrides for policies and notice text. -- [Notices](docs/notices.md) — where the queue lives, who may see it, and how to render it yourself. -- [Tests](tests/README.md) — running the suite, the fixtures and traits it offers, and every scenario - it drives the library through. +- [Installing][installing] — Composer, Strauss, and the constants Strauss must leave alone. +- [Configuration][configuration] — the hook prefix, the container, every sub-plugin key. +- [Conflict handling][conflicts] — the policies, when they run, and the guard's limits. +- [Filters][filters] — the runtime overrides for policies and notice text. +- [Notices][notices] — where the queue lives, who may see it, and how to render it yourself. +- [Tests][tests] — running the suite, the fixtures and traits it offers, and every scenario it drives + the library through. + +`docs/` and `tests/` are both `export-ignore`d, so neither is in a vendored copy of this library — +these point at the repository rather than at a path that would be missing beside the installed +source. + +[installing]: https://github.com/stellarwp/plugin-absorber/blob/main/docs/installing.md +[configuration]: https://github.com/stellarwp/plugin-absorber/blob/main/docs/configuration.md +[conflicts]: https://github.com/stellarwp/plugin-absorber/blob/main/docs/conflict-handling.md +[filters]: https://github.com/stellarwp/plugin-absorber/blob/main/docs/filters.md +[notices]: https://github.com/stellarwp/plugin-absorber/blob/main/docs/notices.md +[tests]: https://github.com/stellarwp/plugin-absorber/blob/main/tests/README.md ## License diff --git a/cspell.json b/cspell.json index 6a2ec21..08d7c31 100644 --- a/cspell.json +++ b/cspell.json @@ -1,27 +1,48 @@ { "version": "0.2", - "language": "en", + "language": "en,en-GB", "words": [ "absorber", + "assertable", "codeception", + "docblocks", "fatals", "invokable", "kadence", + "kses", "learndash", + "lucatume", "multisite", "nexcess", "packagist", "phpstan", + "phpunit", + "propanel", + "redirector", "referer", + "sapi", + "sfwd", + "singlesite", + "sitewide", "slic", "stellarwp", "strauss", + "togglable", + "unbuilt", "uncallable", "uncastable", - "unhookable", + "ungated", + "unnegated", + "unreviewed", "uopz", + "worktree", + "wpautop", "wpunit" ], + "ignoreWords": [ + "ance", + "defered" + ], "ignorePaths": [ "vendor/**", "tests/_output/**", diff --git a/docs/configuration.md b/docs/configuration.md index a01fd77..e9ca994 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -32,9 +32,13 @@ out of the five, so if your container is already built by then, anywhere below 5 ## Rebinding a collaborator -`Absorber::boot()` binds the defaults, and skips any id your container already has — so your binding -wins whether you make it before boot or after, and nothing is resolved until `plugins_loaded` -priority 5 in any case: +`Absorber::boot()` binds the defaults, and skips any *interface* your container already answers for +— so a binding against one of the ids in the table below wins whether you make it before boot or +after. A *class* id must be bound after boot: di52 reports `has()` true for any class that exists, +bound or not, so the provider cannot tell your binding from the container's own willingness to build +`Notices\Store`, `Conflict\Gatekeeper` or any other concrete collaborator, and replaces it. Booting +resolves only the two objects that do the booting; every collaborator below is built by the hook +that needs it, when it fires: ```php use Nexcess\PluginAbsorber\Contracts\Registrar_Interface; @@ -55,11 +59,11 @@ $container->singleton( Registrar_Interface::class, My_Registrar::class ); `site_option_active_sitewide_plugins` — LearnDash injects and then strips a synthetic path — because `is_plugin_active()` then does not report what is in the database. -Rebinding `Resolver_Interface` does not put you in charge of *when* resolution may run. Both gates — +Rebinding `Resolver_Interface` does not put you in charge of *when* resolution may run. The gates — [an interactive admin `GET` that carries no action, and the capability to deactivate across the network](conflict-handling.md#when-resolution-runs) — live in `Conflict\Gatekeeper`, which the hook -consults rather than the resolver, so an implementation that never thought about either is still -safe. Everything the resolver *does* — which policy branch, what the notice says, +consults before it resolves the resolver at all, so an implementation that never thought about +either is still safe. Everything the resolver *does* — which policy branch, what the notice says, where the user lands — is yours. `set_container()` is a configuration call like `set_hook_prefix()`, and order does not matter among @@ -67,12 +71,18 @@ the configuration calls: it may come before or after your `Absorber::register()` comes before boot. Registering buffers the sub-plugin and resolves nothing, so nothing is decided until the first read. -A binding that does not implement the interface it is bound to throws `Config_Exception` when it is -resolved, rather than being cached and failing later somewhere less obvious. So does a binding whose -factory throws — with the original failure kept as the previous exception. +The accessors — `Absorber::registrar()`, `notices()` and `resolver()` — check what your container +hands back and throw a `Config_Exception` naming the interface and the class that failed it, because a +binding that does not implement its interface would otherwise be a `TypeError` blaming this library +for your typo, raised inside `plugins_loaded` where nobody is looking. Whatever your container +raises for a binding it cannot build at all comes through unwrapped: that one is already yours, and +already says so. The one narrowing anywhere is `Absorber::all()`, which drops anything a rebound +registrar returns that is not a `Sub_Plugin` rather than letting it fatal inside `plugins_loaded`. -The container is **not** used to wire hooks. Those are closures that resolve when they fire, so -registering them instantiates nothing and a request that triggers none builds none. +The container does not decide when anything runs. Each hook resolves its collaborator inside the +callback, so wiring instantiates nothing and a request that reaches none of them builds none of +them. The two admin hooks are named `[ Absorber::class, … ]` callbacks precisely so you can +`remove_filter()` them; the two `plugins_loaded` steps are closures over the container. ## Sub-plugin keys @@ -84,7 +94,7 @@ registering them instantiates nothing and a request that triggers none builds no | `standalone_plugin_basename` | `string` | | The standalone's `dir/file.php` basename. Used for `is_plugin_active()` and `deactivate_plugins()`. Omit when there is no standalone. **Detection only.** | | `enabled` | `bool\|callable` | | `true` by default. A `callable( Sub_Plugin ): bool` is re-evaluated on every call, not cached. | | `conflict_policy` | `string\|callable` | | `Conflict_Policy::DEACTIVATE` by default. | -| `conflict_notice_message` | `callable` | | Shown on auto-deactivation and on a re-activation attempt. Empty by default. | +| `conflict_notice_message` | `callable` | | Used in all three places a conflict is reported — the merge notice, the still-active notice, and the rewritten activation-error screen. Each falls back to its own generic sentence naming the slug. | | `dependency_notice_message` | `callable` | | Shown when `dependency_check` fails. Defaults to a generic, untranslated sentence naming the raw slug. | | `activation_callback` | `callable( Sub_Plugin )` | | Runs **once, ever**, per slug, after a successful load. Make it idempotent. | | `dependency_check` | `callable( Sub_Plugin ): bool` | | Skips the load and queues a notice when it returns false. | @@ -100,11 +110,11 @@ at include time. Register each slug exactly once. A slug also names the sub-plugin's notices and its once-ever activation record, so a second registration under the same slug is refused with a `Config_Exception` naming both bundled files rather than quietly dropping one of the two from the -load. Registrations are buffered and nothing reads them until `plugins_loaded` — the conflict pass at -priority 5 on an admin page view, the load pass at priority 6 on everything else — so that is where -the collision surfaces, not at the second `register()` call and not at `boot()`. Whichever pass -reads first reports it with `_doing_it_wrong()`, and that request resolves no conflict and loads no -sub-plugin at all, rather than throwing out of a core hook. A config array the library cannot use is still rejected on the spot. +load. Registrations are buffered and handed to the registrar at the first read — the conflict pass +at `plugins_loaded` priority 5, or the load pass at 6 — so that collision surfaces there rather than +from the second `register()` call, reported with `_doing_it_wrong()` instead of thrown out of a core +hook; a config array the library cannot use is still rejected on the spot, in the call you can see +in your own stack trace. Register unconditionally and put anything you cannot decide up front — a licence that may not be active, a setting the site owner can change — in `enabled`, which is re-evaluated on every load. @@ -200,8 +210,50 @@ nothing to wait for. `dependency_check`, `activation_callback` and `enabled` have nothing a string could collide with, so they accept every callable form, a plain function name included. -Every key rejects a shape it cannot use at registration rather than at read time — including a -`[ class, method ]` pair naming a method that does not exist. +Every typed key rejects a shape it cannot use at registration rather than at read time — including a +`[ class, method ]` pair naming a method that does not exist. `enabled` is the exception: it is read +as a boolean if it is not callable, so an array or an object there passes registration and then +evaluates as enabled. Give it a `bool` or a `callable`, and nothing else. The [filters](filters.md) are the other way in, and they run last — after the configured value and any fallback, so they see the default text too. + +## Complete example + +```php +use Nexcess\PluginAbsorber\Config; +use Nexcess\PluginAbsorber\Conflict_Policy; +use Nexcess\PluginAbsorber\Absorber; +use Nexcess\PluginAbsorber\Sub_Plugin; + +add_action( 'plugins_loaded', function () { + Config::set_hook_prefix( 'give' ); + Config::set_container( give()->container ); + + Absorber::register( [ + 'slug' => 'give-recurring', + 'bundled_plugin_file' => GIVE_PLUGIN_DIR . 'sub-plugins/give-recurring/give-recurring.php', + 'plugin_loaded_constant' => 'GIVE_RECURRING_VERSION', + 'standalone_plugin_basename' => 'give-recurring/give-recurring.php', + 'enabled' => static fn( Sub_Plugin $sub_plugin ) => give_addon_is_licensed( $sub_plugin->get_slug() ), + 'conflict_policy' => Conflict_Policy::DEACTIVATE, + 'conflict_notice_message' => static fn() => __( 'Recurring Donations ships with Give now.', 'give' ), + 'activation_callback' => static function ( Sub_Plugin $sub_plugin ) { + \Give\Recurring\Install::create_tables(); + }, + ] ); + + Absorber::register( [ + 'slug' => 'give-stripe', + 'bundled_plugin_file' => GIVE_PLUGIN_DIR . 'sub-plugins/give-stripe/give-stripe.php', + 'plugin_loaded_constant' => 'GIVE_STRIPE_VERSION', + 'standalone_plugin_basename' => 'give-stripe/give-stripe.php', + // The standalone is still ahead of the bundled copy, so let it win for now. + 'conflict_policy' => Conflict_Policy::DEFER, + 'dependency_check' => static fn() => function_exists( 'curl_init' ), + 'dependency_notice_message' => static fn() => __( 'Stripe payments need the cURL extension.', 'give' ), + ] ); + + Absorber::boot(); +}, 0 ); +``` diff --git a/docs/conflict-handling.md b/docs/conflict-handling.md index 0f13f89..78c993b 100644 --- a/docs/conflict-handling.md +++ b/docs/conflict-handling.md @@ -6,7 +6,7 @@ When a sub-plugin's standalone counterpart is still active: | Policy | Behavior | |---|---| -| `Conflict_Policy::DEACTIVATE` | Deactivate the standalone, notify, and redirect; the bundled copy loads on the next request. **Default.** | +| `Conflict_Policy::DEACTIVATE` | Deactivate the standalone, notify, and usually redirect; the bundled copy loads on the next request. **Default.** | | `Conflict_Policy::DEFER` | Leave the standalone active; the load guard stands the bundled copy down. | | `Conflict_Policy::NOTICE_ONLY` | Leave it active and ask the user to deactivate it. | @@ -59,18 +59,23 @@ every site on a network. The gate matters at all because `plugins_loaded` fires `auth_redirect()`, so an unauthenticated GET of an admin URL reaches this code on its way to the login screen. It applies to every policy rather than only to `deactivate`, which costs nothing — the other policies just queue a notice, and a notice is neither shown nor cleared for a user without the -same capability, so nothing is consumed by waiting for one who has it. - -Both gates live in `Conflict\Gatekeeper`, and the hook asks it rather than the resolver, so binding -your own `Conflict\Contracts\Resolver_Interface` cannot drop either by omission. They are asked in -two halves either side of `Conflict\Detector::has_conflict()`, which only reports: the request-shape -gate first, then the detector, then the capability. The capability check is last because -`current_user_can()` resolves and caches the current user, and at priority 5 that lands ahead of any -`determine_current_user` filter a plugin registers from its own `plugins_loaded` callback — an SSO or -JWT plugin hooked at the default priority would never be consulted, and its users would be treated as -signed out. Asking the detector rather than the resolver keeps detection off the contract a host -rebinds, and means the resolver is built only on a request that passes both gates and has something -to resolve. +same capability. + +Both gates live in `Conflict\Gatekeeper`, along with a third that catches a host which reached +`plugins_loaded` without ever calling `Config::set_hook_prefix()` — that is reported through +`_doing_it_wrong()` and resolution stands down rather than throwing out of a core action. The hook +asks the gatekeeper *before* it resolves `Conflict\Contracts\Resolver_Interface` at all, so binding +your own resolver cannot drop any of them by omission: on a request that fails one, your +implementation is never built, let alone called. The capability is asked last, after +`Conflict\Detector::has_conflict()` has reported there is something to resolve — `current_user_can()` +resolves and caches the current user, and at priority 5 that would land ahead of any +`determine_current_user` filter an SSO or JWT plugin adds from its own `plugins_loaded` callback, +whose users would then be treated as signed out for the rest of the request. + +The deactivation itself is silent, and covers both scopes on multisite. Silent because the +standalone's own deactivation hook has already been registered by the time we run: a routine +`flush_rewrite_rules()` in that callback, at `plugins_loaded`, regenerates the rules before `init` +has declared a single post type, and every custom permalink on the site starts 404ing. ## The redirect @@ -136,11 +141,14 @@ So the library filters `wp_admin_notice_markup` and swaps that sentence for the `conflict_notice_message`, falling back to a generic one naming the slug. This is what puts the WordPress floor at 6.4: the filter does not exist before it. -It touches nothing else. The markup comes back unchanged unless all three hold — the screen is -`plugins`, or `plugins-network` in the network admin, where a super admin is the only one who can -reactivate anything; the `plugin` query arg names a standalone this library has registered; and -`_error_nonce` verifies against `plugin-activation-error_{basename}`. Another plugin's fatal is -another plugin's business. +It touches nothing else. The markup comes back untouched unless every one of these holds — the +screen is `plugins`, or `plugins-network` in the network admin, where a super admin is the only one +who can reactivate anything; the `plugin` query arg names a standalone this library has registered; +and `_error_nonce` verifies against `plugin-activation-error_{basename}`. Another plugin's fatal is +another plugin's business. (One exception, and it is not about this screen: a filter ahead of ours +that returned something other than a string is normalised to `''`, because a `string` type +declaration here would turn that plugin's mistake into a `TypeError` raised on the error screen +least able to afford a second one.) The replacement runs through `wp_kses_post()`, so a knowledge-base link survives, and it is sanitised *before* it is checked for emptiness: a message that filters down to nothing leaves core's @@ -150,4 +158,5 @@ The filter is wired by `Boot\Scheduler` under `is_admin()`, as `[ Absorber::class, 'filter_activation_error_markup' ]` — a named callback, so a host that would rather keep core's wording can `remove_filter()` it. The rewriting itself is `Conflict\Rewriter::rewrite()`, bound by class name like the rest of the conflict handling, so a host -rebinds this screen on its own — without having to supply a notice queue to get it. +can rebind this screen on its own — after `boot()`, as every class-name binding must be, and without +having to supply a notice writer to get it. diff --git a/docs/installing.md b/docs/installing.md index 6d1b4d4..49cbee3 100644 --- a/docs/installing.md +++ b/docs/installing.md @@ -4,13 +4,17 @@ composer require stellarwp/plugin-absorber ``` -Requires PHP 7.4+ and WordPress 6.4+. +Requires PHP 7.4+ and WordPress 6.4+. The WordPress floor comes from the `wp_admin_notice_markup` +filter, which does not exist before 6.4; WordPress is not a Composer dependency, so it is stated +here rather than enforced in `require`. ## Strauss Prefix this library with [Strauss](https://github.com/stellarwp/global-docs/blob/main/docs/strauss-setup.md). Two or more plugins shipping different versions of it will collide otherwise. -> **Do not let `extra.strauss.constant_prefix` rewrite a sub-plugin's `plugin_loaded_constant`.** -> Those are real, shared runtime constants — the whole safety mechanism depends on the bundled -> copy and the standalone defining the *same* name. Add them to `exclude_from_copy`. +> **Nothing may rewrite a sub-plugin's `plugin_loaded_constant`.** Those are real, shared runtime +> constants: the whole safety mechanism depends on the bundled copy and the standalone defining the +> *same* name. This library only ever reads such a name out of your config, so its own source is +> safe to prefix in full — but if your build also runs the bundled plugin's own files through +> Strauss, keep `extra.strauss.constant_prefix` away from them. diff --git a/docs/notices.md b/docs/notices.md index 465c91b..d9d0554 100644 --- a/docs/notices.md +++ b/docs/notices.md @@ -2,9 +2,10 @@ The three notices this library raises — the standalone was deactivated, the standalone is still active, a dependency check failed — are queued in a single option named -`{prefix}_plugin_absorber_notices`, where `{prefix}` is the value passed to -`Config::set_hook_prefix()`. On multisite it is a **network** option, so the queue is shared across -every site on the network. +`{option_prefix}_plugin_absorber_notices`, where `{option_prefix}` is the hook prefix lowercased +with hyphens folded to underscores — a hook prefix of `Give-Core` stores +`give_core_plugin_absorber_notices`. On multisite it is a **network** option, so the queue is shared +across every site on the network. An option and not a transient, on purpose. With a persistent object cache a transient never reaches the database, so a `wp_cache_flush()` from a deploy script or a "purge cache" button would destroy @@ -18,11 +19,10 @@ the site owner is never told their plugin was turned off. must not be shown one — a subscriber loading their profile page would otherwise silently swallow the only warning an administrator was ever going to get. -On multisite `activate_plugins` usually maps through `manage_network_plugins`, so it is normally a -network administrator, not the site administrator who installed the plugin, who sees these. Not -always, though: a network that has enabled the Plugins menu for its sites turns that mapping off, and -site administrators hold `activate_plugins` directly. Conflict resolution therefore asks for -`manage_network_plugins` by name rather than relying on the mapping — see +On multisite that is usually a network administrator rather than the site administrator who +installed the plugin: core maps `activate_plugins` through `manage_network_plugins` unless the +network has enabled the plugins menu for individual sites. Conflict resolution does not rely on that +mapping and asks for `manage_network_plugins` by name — see [conflict handling](conflict-handling.md#when-resolution-runs). ## `conflict_notice_message` is used twice @@ -37,12 +37,15 @@ fatal-error warning. ## Rendering them yourself `Absorber::notices()->option_name()` tells you where the queue is kept, so you can render it -yourself without replacing anything — and it answers for whichever queue the site is running, so a -rebound implementation keeping its notices elsewhere still gives you the right name. The value is an `array` keyed `slug:type` — `give-recurring:merge`, for -example — and the messages may contain markup; the default rendering passes them through -`wp_kses_post()`, so a link, emphasis or a list survives while scripts and event handlers are -stripped. Paragraphs come from `wpautop()`, so send the message unwrapped and let a blank line -break it — a `

` of your own is left as it is rather than nested inside another. +yourself without replacing anything — and it answers for whichever writer the site is running, so a +rebound implementation keeping its notices elsewhere still gives you the right name. The value is an +`array` keyed `slug:type`, where the type is `merge`, `conflict` or `dependency` — +`give-recurring:merge`, for example. The first two render as `notice-warning` and the third as +`notice-error`, since a dependency notice reports a plugin that did not load at all. The messages +may contain markup; the default rendering passes them through `wp_kses_post()`, so a link, emphasis +or a list survives while scripts and event handlers are stripped. Paragraphs come from `wpautop()`, +so send the message unwrapped and let a blank line break it — a `

` of your own is left as it is +rather than nested inside another. ```php use Nexcess\PluginAbsorber\Absorber; @@ -76,10 +79,12 @@ notice read and not cleared is shown on every request forever. The queue is four classes: `Notices\Writer` decides what a notice says, `Notices\Presenter` decides who may consume it and does the render-then-clear, `Notices\Store` keeps it, `Notices\Renderer` draws -it. `Writer` takes `Store` as its only constructor argument and is the one bound behind an interface, -`Writer_Interface` — the seam for a host that already runs its own notices library and wants to reword -rather than replace the plumbing. `Presenter` takes `Store` and `Renderer` and is bound by class name: -nothing in the library dispatches on it, since the trampoline on `all_admin_notices` is its only -caller. Rebinding `Notices\Renderer` replaces the markup and leaves the storage alone; rebinding -`Notices\Store` does the reverse; rebinding `Writer_Interface` replaces the wording without touching -either. +it. `Writer` takes `Store` as its only constructor argument and is the only one behind an interface, +`Notices\Contracts\Writer_Interface` — the seam for a host that already runs its own notices library +and wants to reword rather than replace the plumbing, and the id to bind rather than +`Notices\Writer`. `Presenter` takes `Store` and `Renderer` and is bound by class name like both of +them, since nothing in the library dispatches on it: the trampoline on `all_admin_notices` is its +only caller. Rebinding `Notices\Renderer` replaces the markup and leaves the storage alone, and +rebinding `Notices\Store` does the reverse — but bind either of them, or `Presenter`, *after* +`Absorber::boot()`, since the provider cannot tell a host's binding of a class it could build itself +from no binding at all and replaces it. The interface may be bound on either side of boot. diff --git a/docs/superpowers/plans/2026-07-31-plugin-absorber.md b/docs/superpowers/plans/2026-07-31-plugin-absorber.md deleted file mode 100644 index 56e287f..0000000 --- a/docs/superpowers/plans/2026-07-31-plugin-absorber.md +++ /dev/null @@ -1,3142 +0,0 @@ -# Plugin Absorber Implementation Plan - -> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. - -**Goal:** Ship `stellarwp/plugin-absorber` 1.0.0 — a dependency-light PHP library that lets a WordPress host plugin safely load formerly-standalone plugins bundled inside it, without re-declaration fatals. - -**Architecture:** A static facade (`Config` + `Loader`) over four interface-backed collaborators (`Registrar`, `Notices`, `Conflict\Resolver`, `Activation`), each resolvable from an optional PSR-style container and otherwise instantiated directly. `Loader::boot()` wires two `plugins_loaded` hooks: conflict resolution at priority 1, then the load loop at priority 2. Safety rests on two independent config keys — a load-guard constant checked with `defined()` before `require_once`, and the standalone's plugin basename used for active-detection and deactivation. - -**Tech Stack:** PHP 7.4+, WordPress 6.4+, `stellarwp/container-contract` (only production dependency), Codeception + `lucatume/wp-browser` 3.x with the WPLoader module, `uopz` for stubbing unhookable WordPress functions, PHPStan level 5 with `szepeviktor/phpstan-wordpress`, slic as the test runner. - -## Global Constraints - -Every task's requirements implicitly include this section. - -- **Composer package:** `stellarwp/plugin-absorber`. **Repository:** `github.com/stellarwp/plugin-absorber`. -- **Root namespace:** `Nexcess\PluginAbsorber\`. Tests: `Nexcess\PluginAbsorber\Tests\`. Support: `Nexcess\PluginAbsorber\Tests\Support\`. -- **PHP floor:** `>=7.4`. **WordPress floor:** 6.4 (the `wp_admin_notice_markup` filter). Stated in the README only — WordPress is not a Composer dependency, so it is not enforceable in `require`. -- **Class naming:** `Snake_Case` (`Sub_Plugin`, `Conflict_Policy`, `Config_Exception`). Methods fully spelled out and readable. Config keys descriptive and WordPress-centric. -- **Filter names:** `"{$hook_prefix}/plugin_absorber/should_load"` and `"{$hook_prefix}/plugin_absorber/conflict_policy"`. -- **Storage keys:** option `"{$option_prefix}_plugin_absorber_activations"`, option `"{$option_prefix}_plugin_absorber_notices"`. Both are assembled by `Config::get_option_name( string $name )` and nowhere else, alongside `Config::get_hook_name()` for filters. **Amended 2026-08-11:** `{$option_prefix}` is the hook prefix lowercased with hyphens folded to underscores, so `Give-Core` yields the option `give_core_plugin_absorber_notices` while still yielding the filter `Give-Core/plugin_absorber/should_load` — the prefix validator admits `A-Z` and `-`, and a hook-naming value should not reach a storage key verbatim. The two normalisations stay separate: folding case into the hook side would silently rename the host's own filters. **Amended 2026-08-03 (PR 10 review):** the notice queue was specified as a *transient* and is now an option. `set_transient()` returns before touching the database whenever an external object cache is present, so on any Redis or Memcached site the queue would live only in the cache — where a routine `wp_cache_flush()` from a deploy script or a "purge cache" button destroys it. The merge notice is raised exactly once and never re-queued, so losing it means a site owner is never told their plugin was deactivated. On multisite both this and the activation option are network options, because the resolver deactivates network-wide. -- **Production dependencies:** `stellarwp/container-contract` only. `lucatume/di52` is dev-only. No other StellarWP library. -- **PR size cap:** ≤10 files per PR, tests and test infrastructure excluded. No logic-bearing PR exceeds 4 source files. -- **PR body format** — exactly three parts, nothing else. No boilerplate headings, no restating the diff, no checklists, and no `Verify` section (dropped 2026-08-12: the commands live in `CLAUDE.md` and the coverage is in the diff, so restating them per PR is filler a reviewer learns to scroll past): - ``` - What: one line, naming every hook or entry point the PR wires. - - Usage: the snippet this PR makes possible. - - Why this way: - - **The claim, in bold.** One or two sentences: the trade-off taken, and against what. - - **The next claim.** Same again. - ``` - `Why this way` is one bold-led block per decision, never a single paragraph running several arguments together — a reviewer reads the bold leads and stops at the one they doubt. Cut the connective throat-clearing between claims, never the claims. -- **Branching:** stacked. Each branch cuts from the previous branch, and merges to `main` in order. Never open PR N+1 before PR N's branch exists. -- **Commits:** no co-author trailers, ever. -- **Every source file** carries a file-level docblock with `@package Nexcess\PluginAbsorber` and every method a docblock with `@since 1.0.0`. This binds `src/` only. Test classes and test support classes keep the file-level docblock, but their methods do not need `@since` — the test code in this plan's own tasks is written that way deliberately (ruled 2026-07-31). -- **Container tests use the test-support adapter, never di52 directly** (verified 2026-07-31 against `vendor/`). `lucatume\DI52\Container` implements `ArrayAccess` and **PSR's** `Psr\Container\ContainerInterface` — not `StellarWP\ContainerContract\ContainerInterface`; `stellarwp/container-contract` ships an adapter example at `examples/di52/Container.php` precisely because DI52 must be wrapped. Passing `new Container()` to `Config::set_container()` is a `TypeError`. Tests use `Nexcess\PluginAbsorber\Tests\Support\Test_Container`, which wraps a di52 container and implements the contract's four methods (`bind`, `get`, `has`, `singleton`); **every `use lucatume\DI52\Container;` in the task blocks below means `Test_Container`.** `Config::set_container()`'s signature is unchanged — the StellarWP contract stays the public API, per the production-dependency constraint above. -- **`Config` carries no version handling** (ruled 2026-08-11, PR 4 review). `set_version()`/`get_version()` and the `$version` property were removed: nothing in the library reads a host version, and the one scenario that would want it — telling a bundled copy apart from a standalone at a specific release — is the host's problem. This closes spec known-issue F by deletion rather than by use. -- **No test-only seams in `src/`** (ruled 2026-08-11, PR 4 review). Production classes do not carry a `reset()` for the suite's benefit — that is API the library then supports forever. Tests clear static state by reflection instead, through a helper under `tests/_support/`. `Config` is served by `Nexcess\PluginAbsorber\Tests\Support\Config_State::reset()`; **every `Config::reset()` in the task blocks below means `Config_State::reset()`.** `Loader` is served by `Tests\Support\Loader_State::reset()`, which returns each of `Loader`'s static properties to its default by reflection. There is no `Registrar_State`: `Registrar_Interface` declares only `register()` and `all()`, so there is nothing to empty a registrar with, and dropping the memo is enough for the default one. A test that binds a registrar of its own owns that instance and builds a fresh one. - -## File Structure - -``` -plugin-absorber/ -├── src/ -│ ├── Config.php # static config facade: hook prefix, container, name building -│ ├── Loader.php # static facade: resolve/register/boot/load loop -│ ├── Sub_Plugin.php # value object + every per-sub-plugin predicate -│ ├── Conflict_Policy.php # three policy string constants -│ ├── Plugin_State.php # the only file touching WordPress plugin functions -│ ├── Registrar.php # default slug => Sub_Plugin map -│ ├── Activation.php # default run-once activation tracking -│ ├── Conflict/ -│ │ ├── Contracts/ -│ │ │ └── Resolver_Interface.php -│ │ └── Resolver.php # default standalone detection/deactivation/redirect -│ ├── Contracts/ # interfaces whose implementations sit at the src/ root -│ │ ├── Activation_Interface.php -│ │ ├── Plugin_State_Interface.php -│ │ └── Registrar_Interface.php -│ ├── Notices/ -│ │ ├── Contracts/ -│ │ │ └── Queue_Interface.php -│ │ ├── Queue.php # default queue: wording + capability gate -│ │ ├── Renderer.php # markup and severity -│ │ └── Store.php # the option the queue lives in -│ └── Exceptions/ -│ └── Config_Exception.php -├── tests/ -│ ├── _bootstrap.php _support/ _data/ _output/ -│ ├── unit.suite.yml # WPLoader; singlesite + multisite envs -│ └── unit/ # mirrors src/ -├── .github/workflows/{tests-php.yml,static-analysis.yml} -├── composer.json phpstan.neon.dist cspell.json -├── codeception.dist.yml codeception.slic.yml -├── .env.testing .env.testing.slic -├── .editorconfig .gitattributes .gitignore -├── LICENSE README.md CHANGELOG.md -└── docs/ # spec + this plan; export-ignored -``` - -One responsibility per file. `Sub_Plugin` holds every predicate so the collaborators stay thin and the predicates are testable without WordPress hooks. `Loader` holds only static wiring — all behavior lives behind an interface. - ---- - -Tasks 1–6 (repo bootstrap, Codeception harness, first green CI, `Config`, static analysis in CI, -`Conflict_Policy`) shipped in PRs #1–#6 and their sections have been removed. Git history has them if -you need to read one back. - ---- - -## Task 9: `Loader` resolution and registration - -**PR 9** · branch `09-loader-resolve` from `08-registrar` · 1 source file - -**Files:** -- Create: `src/Loader.php`, `tests/_support/Loader_State.php`, `tests/_support/Spy_Registrar.php`, `tests/unit/LoaderResolveTest.php` -- Modify: `docs/configuration.md` - -**Interfaces:** -- Consumes: `Config::get_container()` (Task 4), `Registrar_Interface`/`Registrar` (Task 8), `Sub_Plugin` (Task 7). -- Produces: - - `Loader::resolve( string $interface, string $default_class ): object` — private; container-or-`new`, memoized - - `Loader::registrar(): Registrar_Interface` - - `Loader::register( array $config ): void` — validates, then buffers - - `Loader::all(): array` — drains the buffer, then reads the registrar - - `Loader::flush(): void` — private; hands the buffer to the registrar - - `Tests\Support\Loader_State::reset(): void` — clears the memo and the buffer, for the suite only - - `Tests\Support\Spy_Registrar` — a `Registrar_Interface` that records what it was handed - - Tasks 10, 12 and 13 each add one accessor alongside their own interface. - -**Registration is deferred.** `register()` resolves nothing: it builds a `Sub_Plugin` — which is what -validates the configuration — and buffers it in a private static `$pending`. The buffer drains into -the registrar on the first read, through a private `flush()` that `all()` calls before it asks the -registrar for anything. - -The reason is ordering. Resolution reads the container, and a host that registers its sub-plugins -before it calls `Config::set_container()` would pin the default registrar for the whole request and -silently ignore the binding. With registration buffered, the container is a configuration call like -every other one: it may arrive at any point before boot, rather than carrying an unwritten "before -your first `register()`" rule that fails quietly when it is broken. - -What this costs: a duplicate slug is now reported at the first read — boot — instead of at the second -`register()` call. Invalid configuration still throws from `register()`, at the call the host can see -in its own stack trace, because building the `Sub_Plugin` is what rejects it. The registrar remains -the single source of truth for duplicate-slug detection and for ordering; the buffer is a pre-store -that restates neither rule in a second dialect. - -- [ ] **Step 1: Cut the branch** - -```bash -git checkout 08-registrar && git checkout -b 09-loader-resolve -``` - -- [ ] **Step 2: Write `tests/_support/Spy_Registrar.php` and `tests/_support/Loader_State.php`** - -`Loader` is the second static facade in the library, and like `Config` it needs clearing between -tests without carrying a public `reset()` for the suite's benefit. `Loader_State` lands here, in the -task that first needs it, modelled on `Config_State`: the properties are walked by reflection and an -unknown one is a `LogicException` rather than a silent leak, so state added to `Loader` later fails -loudly instead of surviving into the next test. - -`Spy_Registrar` is a named class rather than an anonymous one repeated per test: a test that reads -`$spy->register_calls` off a value typed as `Registrar_Interface` is reading a property the interface -does not declare, and PHPStan at level 9 rightly rejects it. The call counter is what catches a -buffer that forgets to empty itself — the slug map alone cannot see the same slug registered twice. - -```php - - */ - public $sub_plugins = []; - - /** - * How many times register() was called. - * - * @var int - */ - public $register_calls = 0; - - public function register( Sub_Plugin $sub_plugin ): void { - ++$this->register_calls; - - $this->sub_plugins[ $sub_plugin->get_slug() ] = $sub_plugin; - } - - /** - * @return array - */ - public function all(): array { - return $this->sub_plugins; - } -} -``` - -```php - - */ - protected const DEFAULTS = [ - 'resolved' => [], - 'pending' => [], - ]; - - /** - * Return every static property of `Loader` to its default. - * - * @throws LogicException When `Loader` has grown a static property this helper does not know - * about, rather than leaving it to leak between tests. - * - * @return void - */ - public static function reset(): void { - $reflection = new ReflectionClass( Loader::class ); - - foreach ( $reflection->getProperties( ReflectionProperty::IS_STATIC ) as $property ) { - $name = $property->getName(); - - if ( ! array_key_exists( $name, self::DEFAULTS ) ) { - throw new LogicException( - sprintf( 'Loader::$%s has no default in %s. Add one.', $name, self::class ) - ); - } - - $property->setAccessible( true ); - $property->setValue( null, self::DEFAULTS[ $name ] ); - } - } -} -``` - -There is no `Registrar_State` helper. Emptying a registrar by reflection would only ever serve the -memoized default instance, and `Registrar_Interface` declares `register()` and `all()` and nothing -else — a container-bound singleton registrar cannot be emptied at all. A test that binds one builds -a fresh instance instead, which is the honest shape: the fake belongs to the test that made it. - -- [ ] **Step 3: Write the failing test** - -`tests/unit/LoaderResolveTest.php`, a `WPTestCase` that calls `Loader_State::reset()` and -`Config_State::reset()` in both `setUp()` and `tearDown()` — a memo stranded by a failed assertion -would otherwise be read by the next test as a real resolve. Container tests use -`Tests\Support\Test_Container`, never `lucatume\DI52\Container`, which implements PSR-11's -`ContainerInterface` rather than StellarWP's. A private `sub_plugin_config( string $slug )` builds the -raw array a host writes — the shared `WithSubPlugins` trait builds `Sub_Plugin` objects, and building -that object is the part of registration under test here — with a `_FIXTURE`-suffixed guard constant -nothing ever defines, since a `define()` lasts for the whole PHP process. - -Resolution: - -- `test_it_falls_back_to_the_default_registrar_without_a_container` -- `test_it_memoizes_the_resolved_collaborator` -- `test_it_resolves_a_bound_registrar_from_the_container` — data provider `container_binding_methods` - (`singleton`, `bind`); the memo is what makes even a `bind()`, which the container rebuilds per - call, resolve exactly once -- `test_it_ignores_a_container_with_no_binding` — asserts `has()` is false first, since DI52 reports - true for any existing *class* name and this binding must stay an interface -- `test_it_rejects_a_binding_that_does_not_implement_the_interface` — data provider - `unusable_bindings` (wrong class, the class name instead of an instance, `null`); the message names - what came back, and the bad instance must not have been memoized -- `test_it_reports_a_container_that_throws_as_a_configuration_error` — the original stays reachable - through the previous chain, walked rather than read one level deep because a container is entitled - to wrap what a factory threw -- `test_a_container_set_after_the_first_resolve_does_not_change_the_memo` - -Registration: - -- `test_register_builds_a_sub_plugin_and_stores_it` -- `test_register_rejects_an_invalid_config` -- `test_register_resolves_nothing_until_the_first_read` -- `test_a_container_set_after_register_takes_effect` -- `test_register_delegates_to_a_bound_registrar` -- `test_reading_twice_does_not_register_twice` -- `test_a_duplicate_slug_is_refused_at_the_first_read` -- `test_registrations_survive_a_container_that_throws` -- `test_all_is_empty_before_anything_is_registered` -- `test_the_state_helper_clears_buffered_registrations` - -The two that pin the deferral down: - -```php -/** - * Registering must not resolve anything, or the first register() call would pin the default - * registrar for the whole request. - */ -public function test_register_resolves_nothing_until_the_first_read(): void { - $builds = 0; - $container = new Test_Container(); - $container->singleton( - Registrar_Interface::class, - static function () use ( &$builds ): Registrar_Interface { - ++$builds; - - return new Spy_Registrar(); - } - ); - Config::set_container( $container ); - - Loader::register( $this->sub_plugin_config( 'give-recurring' ) ); - - $this->assertSame( 0, $builds, 'register() must not reach the container.' ); - - Loader::all(); - - $this->assertSame( 1, $builds, 'The first read is what resolves the registrar.' ); -} - -/** - * Deferring registration moves the duplicate-slug report from the second register() call to the - * first read. It still names both bundled files, which is what the host needs to find them. - */ -public function test_a_duplicate_slug_is_refused_at_the_first_read(): void { - Loader::register( $this->sub_plugin_config( 'give-recurring' ) ); - Loader::register( - [ - 'slug' => 'give-recurring', - 'bundled_plugin_file' => '/tmp/other/other.php', - 'plugin_loaded_constant' => 'OTHER_VERSION_FIXTURE', - ] - ); - - try { - Loader::all(); - $this->fail( 'Expected a Config_Exception.' ); - } catch ( Config_Exception $exception ) { - $this->assertStringContainsString( 'give-recurring', $exception->getMessage() ); - $this->assertStringContainsString( '/tmp/other/other.php', $exception->getMessage() ); - } -} -``` - -- [ ] **Step 4: Run it to verify it fails** - -Run: `slic run unit` -Expected: FAIL — `Class "Nexcess\PluginAbsorber\Loader" not found`. - -- [ ] **Step 5: Write `src/Loader.php`** - -Only resolution and registration in this PR. `boot()` and the load loop land in Task 11. - -```php - - */ - private static $resolved = []; - - /** - * Sub-plugins registered but not yet handed to the registrar. - * - * @var Sub_Plugin[] - */ - private static $pending = []; - - /** - * @since 1.0.0 - * - * @throws Config_Exception When the container cannot produce a usable instance. - * - * @return Registrar_Interface - */ - public static function registrar(): Registrar_Interface { - return self::resolve( Registrar_Interface::class, Registrar::class ); - } - - /** - * Register one bundled sub-plugin. Call once per sub-plugin, before boot(). - * - * The sub-plugin is buffered rather than handed straight to the registrar, so that registering - * resolves nothing. Resolution needs the container, and a host that registers before it calls - * Config::set_container() would otherwise pin the default registrar and silently ignore the - * binding. Buffering is what lets the container arrive at any point before boot, like every - * other configuration call. - * - * The configuration is still validated here: building the Sub_Plugin is what rejects it, and - * that happens at the call the host can see in its own stack trace. - * - * @since 1.0.0 - * - * @param array $config Sub-plugin configuration. - * - * @throws Config_Exception When the configuration is unusable. - * - * @return void - */ - public static function register( array $config ): void { - self::$pending[] = new Sub_Plugin( $config ); - } - - /** - * Every registered sub-plugin, keyed by slug, in registration order. - * - * @since 1.0.0 - * - * @throws Config_Exception When the container cannot produce a usable registrar, or two - * sub-plugins were registered under one slug. - * - * @return array - */ - public static function all(): array { - self::flush(); - - return self::registrar()->all(); - } - - /** - * Hand every buffered registration to the registrar. - * - * The registrar stays the single source of truth: the buffer is a pre-store that needs no - * container, and duplicate-slug detection and ordering remain the registrar's alone rather - * than being restated here in a second dialect. - * - * The buffer is emptied before the loop, so a second read cannot re-register what the - * registrar already holds and trip its duplicate-slug guard. It is emptied *after* the - * registrar resolves, so a container binding that throws leaves the registrations buffered - * for the next read rather than dropping them on the floor. - * - * @since 1.0.0 - * - * @throws Config_Exception When the container cannot produce a usable registrar, or two - * sub-plugins were registered under one slug. - * - * @return void - */ - private static function flush(): void { - if ( self::$pending === [] ) { - return; - } - - $registrar = self::registrar(); - $pending = self::$pending; - - self::$pending = []; - - foreach ( $pending as $sub_plugin ) { - $registrar->register( $sub_plugin ); - } - } - - /** - * Resolve an interface from the container when bound, else construct the default. - * - * The container is never required — with none set, every collaborator is a plain `new`, so - * every default class must be constructible with no arguments. Resolution is memoized, and - * nothing resolves until the first read, which is boot: that is what lets a host set its - * container at any point beforehand. Swapping a collaborator after that would be the worse - * behaviour, since anything already holding the old instance would keep it. - * - * @since 1.0.0 - * - * @template T of object - * - * @param class-string $interface Interface to resolve. - * @param class-string $default_class Concrete class to build when nothing is bound. - * - * @throws Config_Exception When the container throws while building the binding, or returns - * something that does not implement the interface it was asked for. - * - * @return T - */ - private static function resolve( string $interface, string $default_class ): object { - if ( isset( self::$resolved[ $interface ] ) ) { - /** @var T $memoized */ - $memoized = self::$resolved[ $interface ]; - - return $memoized; - } - - $container = Config::get_container(); - - if ( $container !== null && $container->has( $interface ) ) { - // has() true only promises the binding exists, not that it can be built: a host factory - // closure is free to throw, and a container asked for a class with an unsatisfiable - // dependency throws its own exception type. Uncaught, either one leaves the host's - // plugins_loaded with a fatal from a vendor namespace that names neither this library - // nor the binding at fault. - try { - $instance = $container->get( $interface ); - } catch ( Throwable $thrown ) { - throw new Config_Exception( - sprintf( - 'The container failed to build the binding for %s: %s', - $interface, - $thrown->getMessage() - ), - 0, - $thrown - ); - } - - // Checked before it is memoized. Without this the bad instance is cached, and every - // accessor throws a TypeError blaming this library rather than the binding. - if ( ! $instance instanceof $interface ) { - throw new Config_Exception( - sprintf( - 'The container binding for %s must implement it. Got %s.', - $interface, - is_object( $instance ) ? get_class( $instance ) : gettype( $instance ) - ) - ); - } - - self::$resolved[ $interface ] = $instance; - - return $instance; - } - - self::$resolved[ $interface ] = new $default_class(); - - return self::$resolved[ $interface ]; - } -} -``` - -> **Design notes.** -> -> *Registration is deferred on purpose.* Handing the `Sub_Plugin` straight to -> `self::registrar()->register()` is shorter and was the first sketch, but it makes the first -> `register()` call the moment the container is read, and a host that sets its container one line -> later gets the default registrar with no error to explain why its binding did nothing. The buffer -> is what makes `Config::set_container()` order-independent, at the price of moving the duplicate-slug -> report to the first read. That is the right trade: a duplicate slug is a mistake caught the first -> time the code runs either way, and it is reported with both bundled file paths wherever it fires, -> while a silently ignored container binding is a mistake that looks like it worked. -> -> *The facade has no `reset()`.* Nothing in production ever needs to un-resolve a collaborator or -> discard a registration — the memo is built once per request and dies with it — so the only caller -> would be the suite, and a public static method is a promise to every host that reads the class. -> `Tests\Support\Loader_State::reset()` does the job by reflection from `tests/_support/`, clearing -> the memo and the buffer together. -> -> *Nothing reaches into a registrar to empty it.* `Registrar_Interface` declares `register()` and -> `all()` only, so a container-bound singleton registrar has no way to be emptied and reflection into -> the shipped `Registrar` would not touch it. A test that binds a registrar of its own owns that -> fake and builds a fresh one per test — test-side code, which is exactly where this seam belongs. - -- [ ] **Step 6: Run the tests to verify they pass** - -Run: `slic run unit` -Expected: PASS — 17 test methods, 20 cases with the two data providers expanded. - -- [ ] **Step 7: Confirm static analysis is still clean** - -Run: `composer test:analysis` -Expected: `[OK] No errors`. - -- [ ] **Step 8: Append to `docs/configuration.md`** - -The container documentation goes in `docs/configuration.md`, not the README: the human docs are split -out of the README and it is not to grow back. - -```markdown -## Rebinding a collaborator - -Every collaborator is interface-backed. With a container set, bind any of them to override the -library globally; with no container, the defaults are used and nothing is required. - -```php -$container->singleton( Registrar_Interface::class, My_Registrar::class ); -Config::set_container( $container ); -``` - -| Interface | Default | Responsibility | -|---|---|---| -| `Contracts\Registrar_Interface` | `Registrar` | Holds the registered sub-plugins. | -| `Notices\Contracts\Queue_Interface` | `Notices\Queue` | Notice queue and the activation-error rewrite. | -| `Conflict\Contracts\Resolver_Interface` | `Conflict\Resolver` | Standalone detection, deactivation, redirect. | -| `Contracts\Activation_Interface` | `Activation` | Run-once activation tracking. | - -`Config::set_container()` may be called at any point before `Loader::boot()` — before or after your -`Loader::register()` calls. Nothing is resolved until the first read, so a registration made before -the container was set still reaches the bound registrar. After boot, a collaborator is fixed for the -request: swapping one then would strand whatever already holds the old instance. - -The container is **not** used to wire hooks — those stay plain static callbacks, so the container -stays genuinely optional. -``` - -- [ ] **Step 9: Commit, push, open the PR** - -```bash -git add src/Loader.php tests/_support/Loader_State.php tests/_support/Spy_Registrar.php tests/unit/LoaderResolveTest.php docs/configuration.md -git commit -m "Add Loader resolution and registration" -git push -u origin 09-loader-resolve -gh pr create --base 08-registrar --title "Loader resolution and registration" --body 'What: `Loader::resolve()`, the `registrar()` accessor, `register()`, and `all()`. - -Usage: - - Loader::register( [ "slug" => "give-recurring", ... ] ); - Loader::all(); // [ "give-recurring" => Sub_Plugin ] - - // Optional, and in any order relative to register(). - $container->singleton( Registrar_Interface::class, My_Registrar::class ); - Config::set_container( $container ); - -Why this way: one generic `resolve( $interface, $default_class )` rather than four bespoke accessors -with their own fallback logic, so adding a collaborator is one line. `register()` validates the -config by building the `Sub_Plugin` and then buffers it, so registration resolves nothing: the -container may be set at any point before boot instead of before the first `register()`. The cost is -that a duplicate slug is reported at the first read rather than at the second `register()` — against -a container binding that would otherwise be ignored with no error at all. - -The facade carries no `reset()`: nothing in production un-resolves a collaborator, so it would be a -public promise made for the test suite alone. `Tests\Support\Loader_State` clears the memo and the -buffer by reflection instead. - -Verify: `slic run unit` — 17 test methods covering both the bound and unbound paths, a container -that throws, a binding of the wrong type, and the registration order cases. Not covered here: -`boot()` and the load loop, which land in the next PR.' -``` - ---- - -## Task 11: `Loader` boot and load path - -**PR 11** · branch `11-loader-load-path` from `10-notices-queue` · 1 source file - -**Files:** -- Modify: `src/Loader.php`, `tests/_support/Loader_State.php` (the new `$booted` property needs a - default, and the hooks `boot()` added need unwiring), `README.md`, `docs/filters.md`, - `docs/configuration.md`, `docs/conflict-handling.md` -- Create: `tests/unit/LoaderBootTest.php`, `tests/unit/LoaderLoadTest.php` - -**Interfaces:** -- Consumes: `Sub_Plugin` predicates (Task 7), `Loader::all()` and its buffer drain (Task 9), - `Loader::notices()` → `Notices\Contracts\Queue_Interface`, default `Notices\Queue` (Task 10), - `Config::get_hook_name()` and `Config::get_hook_prefix()` (Task 4). -- Produces: - - `Loader::boot(): void` — idempotent - - `Loader::load_all(): void` - - `Loader::render_notices(): void` - - `Loader::load( Sub_Plugin ): void` — private - - `Loader::wiring_window_has_closed(): bool` — private - - `Loader::has_hook_prefix(): bool` — private - - `Loader::LOAD_PRIORITY` — private const, the `plugins_loaded` priority the load loop runs at - - the `"{$prefix}/plugin_absorber/should_load"` filter, args `(bool $should_load, Sub_Plugin $sub_plugin)` - - Task 12 adds the `plugins_loaded` @1 hook to `boot()`; Task 13 adds the activation call to `load()`. - -**Design note:** `boot()` wires only the @2 load hook and `all_admin_notices` in this PR. The @1 -conflict-resolution hook arrives in Task 12 with the resolver it delegates to — wiring a trampoline -to a collaborator that does not exist yet would not run. - -**The boot barrier needs no flush of its own.** `register()` buffers and `all()` flushes, which -landed in Task 9, so `load_all()` just iterates `self::all()` and the drain happens transparently on -the first read. That read is at `plugins_loaded` priority 2, which is after the host's own bootstrap -at priority 0 — so the container is set before anything resolves, and a registration made before -`Config::set_container()` still reaches the bound registrar. - -- [ ] **Step 1: Cut the branch** - -```bash -git checkout 10-notices-queue && git checkout -b 11-loader-load-path -``` - -- [ ] **Step 2: Write the failing boot test** - -`tests/unit/LoaderBootTest.php`, a `WPTestCase` that calls `Loader_State::reset()` and -`Config_State::reset()` in `tearDown()`. Behaviours to cover: - -- `boot()` wires `load_all` to `plugins_loaded` at `LOAD_PRIORITY`, and `render_notices` to - `all_admin_notices` — the latter only under `is_admin()`, so both branches need a case. Asserting - the front-end branch does *not* wire needs a working recorder, per the testing rules: assert the - hook is absent, then wire it by hand and assert the same read finds it. -- Calling `boot()` twice wires each hook exactly once — read - `$GLOBALS['wp_filter']['plugins_loaded']->callbacks` at the priority and count, since `has_action()` - cannot tell one callback from two. -- Booting too late is reported and recovered from. With `plugins_loaded` already dispatched, or - dispatching at a priority at or past `LOAD_PRIORITY`, `boot()` triggers `_doing_it_wrong()` and - loads inline instead of wiring a hook that would never fire. The inclusive comparison — booting - *from* `plugins_loaded` @2 — is its own case, because that is the near miss a host actually hits. -- Booting from `plugins_loaded` at a priority *before* `LOAD_PRIORITY` still wires normally. - -- [ ] **Step 3: Write the failing load-path test** - -`tests/unit/LoaderLoadTest.php`. Each test writes its own fixture file: `require_once` caches by -resolved path for the whole PHP process, so a shared fixture would make the second test in the run -silently pass. A fixture counts its own loads into a global and defines its guard constant inside a -`defined()` check; `tearDown()` unlinks the fixtures, drops the global, deletes the notices option -and resets both facades. Behaviours to cover: - -- A registered, enabled sub-plugin with a readable file is required exactly once, and its guard - constant is defined afterwards. Calling `load_all()` twice still loads once. -- Every registered sub-plugin loads, in registration order. -- Each gate skips: disabled; guard constant already defined; dependencies unmet — which also queues - the dependency notice through `Loader::notices()`; bundled file missing, unreadable, or a - directory, which reports through `_doing_it_wrong()` and queues *nothing*. -- The `should_load` filter is applied under the name `Config::get_hook_name( 'should_load' )`, - receives the `Sub_Plugin` as its second argument, and vetoes the load when it returns falsy. -- The gate order is itself asserted: with the guard constant defined *and* an unmet dependency, no - dependency notice is queued — the already-loaded check runs first. -- `load_all()` and `render_notices()` with no hook prefix set report through `_doing_it_wrong()` and - return, rather than throwing out of a core action. -- A registrar bound in the container that returns a non-`Sub_Plugin` entry is skipped rather than - fataling. - -- [ ] **Step 4: Run both to verify they fail** - -Run: `slic run unit` -Expected: FAIL — `Call to undefined method Nexcess\PluginAbsorber\Loader::boot()`. - -- [ ] **Step 5: Add boot and the load path to `src/Loader.php`** - -Add the priority constant and the `$booted` property beside `$resolved` and `$pending`: - -```php - /** - * plugins_loaded priority the load loop runs at. - * - * Ahead of the default priority, so a bundled plugin is in memory before the plugins that - * expect it start their own work, and low enough to leave room for earlier wiring. - * - * @since 1.0.0 - * - * @var int - */ - private const LOAD_PRIORITY = 2; - - /** - * Whether the hooks have been wired. - * - * @var bool - */ - private static $booted = false; -``` - -Then the public methods: - -```php - /** - * Wire the WordPress hooks. Idempotent — safe to call from more than one code path. - * - * Hooks are plain static trampolines rather than container callbacks, which is what keeps the - * container optional. Each trampoline delegates to the resolved collaborator, so rebinding - * still takes effect. - * - * @since 1.0.0 - * - * @return void - */ - public static function boot(): void { - if ( self::$booted ) { - return; - } - - self::$booted = true; - - if ( is_admin() ) { - // all_admin_notices, not admin_notices. WordPress dispatches admin_notices, - // network_admin_notices and user_admin_notices as mutually exclusive branches, so a - // superadmin working in the network admin -- exactly where a network-wide - // deactivation gets noticed -- would never see the queue rendered. - add_action( 'all_admin_notices', [ self::class, 'render_notices' ] ); - } - - // Adding an action at a priority the current dispatch has already passed is accepted and - // then never fires. Booting from plugins_loaded at the default priority instead of 0 -- - // the commonest hook mistake there is -- would otherwise mean nothing loads at all, with - // no warning and a site that looks entirely healthy. - if ( self::wiring_window_has_closed() ) { - _doing_it_wrong( - __METHOD__, - 'Loader::boot() must run before plugins_loaded priority 2. Loading inline instead.', - '1.0.0' - ); - - self::load_all(); - - return; - } - - add_action( 'plugins_loaded', [ self::class, 'load_all' ], self::LOAD_PRIORITY ); - } - - /** - * @since 1.0.0 - * - * @return void - */ - public static function load_all(): void { - // The load path needs the prefix for the should_load filter and for the notice store. - // Throwing out of a core action would take the whole site down over a bootstrap mistake, - // so it is reported where a developer will see it and the load is abandoned instead. - if ( ! self::has_hook_prefix() ) { - return; - } - - foreach ( self::all() as $sub_plugin ) { - // Registrar_Interface::all() only declares `array`. A host binding its own registrar - // that returns anything else would otherwise fatal inside plugins_loaded on the first - // predicate call -- the exact failure this library exists to prevent. - if ( ! $sub_plugin instanceof Sub_Plugin ) { - continue; - } - - self::load( $sub_plugin ); - } - } - - /** - * @since 1.0.0 - * - * @return void - */ - public static function render_notices(): void { - if ( ! self::has_hook_prefix() ) { - return; - } - - self::notices()->render(); - } -``` - -And the private ones, below `flush()` and `resolve()` — public, then private, and no helper above -the API it serves: - -```php - /** - * Load one sub-plugin, cheapest and most decisive check first. - * - * @since 1.0.0 - * - * @param Sub_Plugin $sub_plugin Sub-plugin to load. - * - * @throws Config_Exception When a collaborator binding is unusable. - * - * @return void - */ - private static function load( Sub_Plugin $sub_plugin ): void { - if ( ! $sub_plugin->is_enabled() ) { - return; - } - - // Ahead of the dependency check, which calls an arbitrary host callable. This is one - // defined(), it carries the whole re-declaration guarantee, and it is the only gate that - // means "the plugin is already running" -- warning that requirements are unmet for a - // plugin the admin can see working would be worse than useless. - if ( $sub_plugin->is_already_loaded() ) { - return; - } - - if ( ! $sub_plugin->are_dependencies_met() ) { - self::notices()->queue_dependency_notice( $sub_plugin ); - - return; - } - - // Not file_exists(): that is true for a directory and for a file with no read permission, - // and require_once fatals on both. A missing file is a broken build in the host plugin - // rather than anything a site owner can act on, so it goes to the developer instead of - // into the notice queue, where it would have displayed the host's own - // dependency_notice_message and sent the owner after the wrong problem entirely. - $file = $sub_plugin->get_bundled_plugin_file(); - - if ( ! is_file( $file ) || ! is_readable( $file ) ) { - _doing_it_wrong( - 'Nexcess\PluginAbsorber\Loader', - sprintf( - 'The bundled plugin file for "%s" is missing or unreadable: %s', - $sub_plugin->get_slug(), - $file - ), - '1.0.0' - ); - - return; - } - - // No type guard on the return, unlike the conflict_policy filter: there is no cast here, - // and every unexpected value is falsy-or-truthy without fataling. Anything odd skips the - // load, which is the safe direction. - $should_load = apply_filters( Config::get_hook_name( 'should_load' ), true, $sub_plugin ); - - if ( ! $should_load ) { - return; - } - - // An include takes the scope of the line it sits on, and this one is inside a method, where - // wp-settings.php includes plugins at global scope. Top-level assignments in the bundled - // file are function-local as a result -- documented for hosts, because no amount of - // wrapping here can hand a required file the global scope it would have had. - require_once $file; - } - - /** - * Whether it is already too late to wire the load hook. - * - * The comparison is inclusive. A callback added to the priority currently being dispatched is - * accepted and never reached either: WP_Hook::apply_filters() walks `$this->callbacks[$priority]` - * with a by-value foreach, so the append lands on an array the running loop has already copied. - * Booting from plugins_loaded at priority 2 is the case a host is likeliest to hit by accident, - * and an exclusive comparison would let exactly that one through unreported. - * - * @since 1.0.0 - * - * @return bool - */ - private static function wiring_window_has_closed(): bool { - if ( ! did_action( 'plugins_loaded' ) ) { - return false; - } - - if ( ! doing_action( 'plugins_loaded' ) ) { - return true; - } - - $hook = $GLOBALS['wp_filter']['plugins_loaded'] ?? null; - - return $hook instanceof WP_Hook && $hook->current_priority() >= self::LOAD_PRIORITY; - } - - /** - * Whether a hook prefix has been set, reporting to the developer when it has not. - * - * @since 1.0.0 - * - * @return bool - */ - private static function has_hook_prefix(): bool { - try { - Config::get_hook_prefix(); - } catch ( Config_Exception $exception ) { - _doing_it_wrong( 'Nexcess\PluginAbsorber\Loader', $exception->getMessage(), '1.0.0' ); - - return false; - } - - return true; - } -``` - -The filter name is built with `Config::get_hook_name( 'should_load' )` and never by concatenating -`Config::get_hook_prefix()` with the rest: `Config` owns the segment between the host's prefix and -the key's own name, and nothing else assembles it. - -- [ ] **Step 6: Teach `tests/_support/Loader_State.php` about the boot flag and the hooks** - -`Loader_State::reset()` walks `Loader`'s static properties and refuses one it has no default for, so -until `$booted` is listed every test that resets throws a `LogicException` naming it. That is the -helper doing its job: a boot flag left standing would wire the hooks once and then let every later -test's `boot()` no-op. - -Clearing the flag is not enough on its own. A `Loader` that reports itself unbooted while its -callbacks are still attached is the worse of the two states: the next `boot()` wires nothing and -still looks like it worked, and the stranded callback goes on loading sub-plugins into tests that -never registered any. So the helper unwires both hooks as well — and reads `LOAD_PRIORITY` off the -class by reflection rather than restating `2`, so it cannot go on removing a hook from a priority the -`Loader` no longer wires. - -```php - protected const DEFAULTS = [ - 'resolved' => [], - 'pending' => [], - 'booted' => false, - ]; - - public static function reset(): void { - $reflection = new ReflectionClass( Loader::class ); - - // Read rather than restated, so the helper cannot go on removing a hook from a priority - // the Loader no longer wires -- which would leave the real callback attached and every - // later test loading sub-plugins it never registered. - $load_priority = $reflection->getConstant( 'LOAD_PRIORITY' ); - - remove_action( 'plugins_loaded', [ Loader::class, 'load_all' ], (int) $load_priority ); - remove_action( 'all_admin_notices', [ Loader::class, 'render_notices' ] ); - - // ... the property walk from Task 9, unchanged. - } -``` - -There is still no `Loader::reset()`, and nothing in this PR adds one. The seam stays in -`tests/_support/`. - -- [ ] **Step 7: Run the tests to verify they pass** - -Run: `slic run unit`, then `slic run unit --env multisite`. - -- [ ] **Step 8: Confirm static analysis is still clean** - -Run: `composer test:analysis` -Expected: `[OK] No errors`. - -- [ ] **Step 9: Document it, in four places and not in one** - -The human docs are split out of the README and are not to grow back, so each piece goes where its -subject already lives. The README gets only the bootstrap, short: - -```markdown -Loader::register( [ ... ] ); -Loader::boot(); -``` - -wrapped in `add_action( 'plugins_loaded', ..., 0 )`, with a line on why the `, 0` matters: `boot()` -wires the load at priority 2, WordPress silently ignores a callback added at or past the priority it -is already dispatching, and booting later is reported and loaded inline but with weaker ordering -guarantees. - -`docs/filters.md` gets the load gate — the filter's arguments, the `add_filter()` snippet, and the -order it sits in: consulted only for a sub-plugin that would otherwise have loaded, so returning -`true` cannot force a load past the guard constant. - -`docs/configuration.md` gets two things: the `Notices\Contracts\Queue_Interface` → `Notices\Queue` -row in the collaborator table, and the global-scope caveat — top-level assignments in a bundled file -are function-local, `$GLOBALS['my_plugin'] = ...` works, and declarations, `define()`, hooks and -`__FILE__` are unaffected. - -`docs/conflict-handling.md` gets the `defined()` snippet the bundled file must wrap its guard -constant in, next to the paragraph that already says the constant must be defined at file scope. - -> **Design notes.** -> -> *The already-loaded check moved ahead of the dependency check.* The first sketch had the two the -> other way round, cheapest-first. `is_already_loaded()` is one `defined()` and the dependency check -> calls an arbitrary host callable, so cheapest-first argues the same way — but the deciding reason -> is what each gate means. A defined guard constant means the plugin is running right now; telling a -> site owner its requirements are unmet, for a plugin they can see working, sends them after a -> problem that does not exist. -> -> *The file check is `is_file()` and `is_readable()`, not `file_exists()`.* `file_exists()` is true -> for a directory and for a file the process cannot read, and `require_once` fatals on both — which -> is the exact failure this library exists to prevent. A missing bundled file is a broken build in -> the host plugin, so it is reported through `_doing_it_wrong()` and queues nothing: the notice queue -> would have shown the host's own `dependency_notice_message` and sent a site owner after the wrong -> problem. -> -> *Booting too late is reported and recovered from, not ignored.* `add_action()` accepts a callback -> at a priority the current dispatch has already passed and then never fires it, so a host that boots -> from `plugins_loaded` at the default priority would load nothing at all, silently, on a site that -> looks healthy. `wiring_window_has_closed()` compares inclusively, because booting from -> `plugins_loaded` @2 is the near miss a host actually hits and an exclusive comparison lets exactly -> that one through unreported. The recovery is an inline `load_all()`: weaker ordering, but the -> sub-plugins load. -> -> *A missing hook prefix returns instead of throwing.* Both `load_all()` and `render_notices()` run -> from core actions, and an exception out of `plugins_loaded` takes the whole site down over a -> bootstrap mistake. `_doing_it_wrong()` puts it in front of the developer who made it and the load -> is abandoned. -> -> *`Loader` still has no `reset()`.* `boot()` adds a third piece of static state and the temptation -> with it, and the answer has not changed: a public reset is API the library supports forever, and a -> host that called it mid-request would drop the registrations the load loop is about to read. -> `Tests\Support\Loader_State::reset()` clears the memo, the buffer and the boot flag by reflection, -> and unwires the two hooks in the same pass — a reset that cleared the flag alone would leave a -> `Loader` that reports itself unbooted with its callbacks still attached. - -- [ ] **Step 10: Commit, push, open the PR** - -```bash -git add src/Loader.php tests/_support/Loader_State.php tests/unit/LoaderLoadTest.php tests/unit/LoaderBootTest.php README.md docs/filters.md docs/configuration.md docs/conflict-handling.md -git commit -m "Add Loader boot and the load path" -git push -u origin 11-loader-load-path -gh pr create --base 10-notices-queue --title "Loader boot and load path" --body 'What: `boot()`, `load_all()`, the five-gate load path, and the `should_load` filter. - -Usage: - - add_action( "plugins_loaded", function () { - Config::set_hook_prefix( "give" ); - Loader::register( [ ... ] ); - Loader::boot(); // wires plugins_loaded @2 and all_admin_notices - }, 0 ); - - add_filter( "give/plugin_absorber/should_load", function ( $should_load, $sub_plugin ) { - return $should_load; - }, 10, 2 ); - -Why this way: the guard constant is checked before the dependency check rather than after it. A -defined constant means the plugin is running right now, and warning that requirements are unmet for -a plugin the admin can see working sends them after a problem that does not exist. The file check is -`is_file()` plus `is_readable()`, not `file_exists()`, which is true for a directory and for an -unreadable file and lets `require_once` fatal on both. Booting past `plugins_loaded` @2 is reported -through `_doing_it_wrong()` and loaded inline instead of wiring a hook that would never fire — -against silently loading nothing on a site that looks healthy. `boot()` wires only the @2 hook here; -the @1 conflict-resolution hook lands with the resolver it delegates to. - -Verify: `slic run unit` on both envs, and `composer test:analysis`. Covered: every gate and its -order, the boot window including the inclusive near miss at @2, and the missing-prefix path. Each -load test writes its own fixture, because `require_once` caches by resolved path for the whole PHP -process. Not covered here: conflict resolution and the activation callback, which land in Tasks 12 -and 13.' -``` - ---- - -## Task 12: `Conflict\Resolver` - -**PR 12** · branch `12-conflict-resolver` from `11-loader-load-path` · 4 source files - -**Files:** -- Create: `src/Conflict/Contracts/Resolver_Interface.php`, `src/Conflict/Resolver.php`, `tests/unit/Conflict/ResolverTest.php` -- Modify: `src/Loader.php` (add `resolver()` and the @1 hook), `README.md` - -**Interfaces:** -- Consumes: `Loader::all()` (Task 9), `Loader::notices()` (Task 10), `Sub_Plugin::is_standalone_plugin_active()` / `is_standalone_plugin_network_active()` / `get_conflict_policy()` (Task 7), `Conflict_Policy::*` (Task 6). -- Produces: - - `Conflict\Contracts\Resolver_Interface` with `resolve_all(): void` — a folder-scoped concern owns its own contract, so this lives in `src/Conflict/Contracts/`, not in the top-level `src/Contracts/` and not beside `Resolver`. Matches `Notices\Contracts\Queue_Interface` from Task 10. - - `Conflict\Resolver::redirect_destination( $referrer )` — `protected`, returns `string|false` - - `Loader::resolver(): Resolver_Interface` - - `Loader::run_conflict_resolution(): void` - -- [ ] **Step 1: Cut the branch** - -```bash -git checkout 11-loader-load-path && git checkout -b 12-conflict-resolver -``` - -- [ ] **Step 2: Write the failing test** - -```php -> - */ - private $deactivations = []; - - /** - * @var array - */ - private $redirects = []; - - public function setUp(): void { - parent::setUp(); - - Config::set_hook_prefix( 'give' ); - require_once ABSPATH . 'wp-admin/includes/plugin.php'; - - $this->deactivations = []; - $this->redirects = []; - - $this->setFunctionReturn( - 'deactivate_plugins', - function ( $plugins, $silent = false, $network_wide = null ) { - $this->deactivations[] = [ - 'plugins' => $plugins, - 'silent' => $silent, - 'network_wide' => $network_wide, - ]; - }, - true - ); - - // Throwing here stops the resolver exactly where production calls exit, - // without mocking exit itself. See tests/README.md. - $this->setFunctionReturn( - 'wp_safe_redirect', - function ( $location ) { - $this->redirects[] = $location; - - throw new TestException( self::HALTED_AT_EXIT ); - }, - true - ); - } - - public function tearDown(): void { - delete_transient( 'give_plugin_absorber_notices' ); - Loader_State::reset(); - Config::reset(); - parent::tearDown(); - } - - /** - * @param array $overrides Config overrides. - */ - private function register( array $overrides = [] ): void { - Loader::register( - array_merge( - [ - 'slug' => 'give-recurring', - 'bundled_plugin_file' => '/tmp/give-recurring.php', - 'plugin_loaded_constant' => 'GIVE_RECURRING_VERSION_RESOLVER', - 'standalone_plugin_basename' => 'give-recurring/give-recurring.php', - ], - $overrides - ) - ); - } - - private function standalone_is( bool $active, bool $network_active = false ): void { - $this->setFunctionReturn( 'is_plugin_active', $active ); - $this->setFunctionReturn( 'is_plugin_active_for_network', $network_active ); - } - - /** - * Runs the resolver, absorbing the TestException that stands in for exit(). - * - * Paths that redirect halt inside wp_safe_redirect(); paths that do not run - * to completion. Either way the assertions afterwards see the same state - * production would have left behind. - * - * @return void - */ - private function resolve(): void { - try { - ( new Resolver() )->resolve_all(); - } catch ( TestException $e ) { - $this->assertSame( self::HALTED_AT_EXIT, $e->getMessage() ); - } - } - - /** - * @return array - */ - private function queued_notices(): array { - $queue = get_transient( 'give_plugin_absorber_notices' ); - - return is_array( $queue ) ? $queue : []; - } - - public function test_the_loader_resolves_the_default_resolver(): void { - $this->assertInstanceOf( Resolver::class, Loader::resolver() ); - } - - public function test_deactivate_deactivates_notifies_and_redirects(): void { - $this->standalone_is( true ); - $this->register( [ 'conflict_policy' => Conflict_Policy::DEACTIVATE ] ); - $this->setFunctionReturn( 'wp_get_referer', false ); - - $this->resolve(); - - $this->assertCount( 1, $this->deactivations ); - $this->assertSame( 'give-recurring/give-recurring.php', $this->deactivations[0]['plugins'] ); - $this->assertArrayHasKey( 'give-recurring:merge', $this->queued_notices() ); - $this->assertCount( 1, $this->redirects ); - } - - public function test_deactivate_is_the_default_policy(): void { - $this->standalone_is( true ); - $this->register(); - $this->setFunctionReturn( 'wp_get_referer', false ); - - $this->resolve(); - - $this->assertCount( 1, $this->deactivations ); - } - - public function test_it_passes_the_network_flag_for_a_network_active_standalone(): void { - $this->standalone_is( false, true ); - $this->register(); - $this->setFunctionReturn( 'wp_get_referer', false ); - - $this->resolve(); - - $this->assertTrue( - $this->deactivations[0]['network_wide'], - 'Without $network_wide, deactivate_plugins() no-ops on a network-activated plugin and the redirect loops forever.' - ); - } - - public function test_it_omits_the_network_flag_for_a_normally_active_standalone(): void { - $this->standalone_is( true, false ); - $this->register(); - $this->setFunctionReturn( 'wp_get_referer', false ); - - $this->resolve(); - - $this->assertFalse( $this->deactivations[0]['network_wide'] ); - } - - public function test_defer_does_nothing_at_all(): void { - $this->standalone_is( true ); - $this->register( [ 'conflict_policy' => Conflict_Policy::DEFER ] ); - - $this->resolve(); - - $this->assertSame( [], $this->deactivations ); - $this->assertSame( [], $this->redirects ); - $this->assertSame( [], $this->queued_notices() ); - } - - public function test_notice_only_notifies_without_deactivating(): void { - $this->standalone_is( true ); - $this->register( [ 'conflict_policy' => Conflict_Policy::NOTICE_ONLY ] ); - - $this->resolve(); - - $this->assertSame( [], $this->deactivations ); - $this->assertSame( [], $this->redirects ); - $this->assertArrayHasKey( 'give-recurring:conflict', $this->queued_notices() ); - } - - public function test_a_callable_policy_selects_the_branch(): void { - $this->standalone_is( true ); - $this->register( - [ - 'conflict_policy' => static function ( Sub_Plugin $sub_plugin ) { - return $sub_plugin->get_slug() === 'give-recurring' - ? Conflict_Policy::DEFER - : Conflict_Policy::DEACTIVATE; - }, - ] - ); - - $this->resolve(); - - $this->assertSame( [], $this->deactivations, 'The callable chose DEFER for this slug.' ); - } - - public function test_it_skips_a_disabled_sub_plugin(): void { - $this->standalone_is( true ); - $this->register( [ 'enabled' => false ] ); - - $this->resolve(); - - $this->assertSame( [], $this->deactivations ); - } - - public function test_it_skips_when_the_standalone_is_not_active(): void { - $this->standalone_is( false, false ); - $this->register(); - - $this->resolve(); - - $this->assertSame( [], $this->deactivations ); - } - - public function test_it_skips_a_sub_plugin_with_no_standalone(): void { - $this->standalone_is( true ); - Loader::register( - [ - 'slug' => 'give-fee-recovery', - 'bundled_plugin_file' => '/tmp/give-fee-recovery.php', - 'plugin_loaded_constant' => 'GIVE_FEE_RECOVERY_VERSION_RESOLVER', - ] - ); - - $this->resolve(); - - $this->assertSame( [], $this->deactivations ); - } - - /** - * Exposes the protected redirect logic so it can be asserted directly. - * - * Defined once and reused — the four referrer cases differ only in their input. - */ - private function redirect_resolver(): Resolver { - return new class() extends Resolver { - /** - * @param string|false $referrer Referrer under test. - * - * @return string|false - */ - public function destination_for( $referrer ) { - return $this->redirect_destination( $referrer ); - } - }; - } - - public function test_it_redirects_to_the_plugins_page_without_a_referrer(): void { - $this->assertSame( admin_url( 'plugins.php' ), $this->redirect_resolver()->destination_for( false ) ); - } - - public function test_it_redirects_to_the_plugins_page_from_an_update_screen(): void { - $resolver = $this->redirect_resolver(); - - $this->assertSame( admin_url( 'plugins.php' ), $resolver->destination_for( admin_url( 'update.php?action=x' ) ) ); - $this->assertSame( admin_url( 'plugins.php' ), $resolver->destination_for( admin_url( 'update-core.php' ) ) ); - } - - public function test_it_does_not_redirect_during_an_inline_update_on_the_plugins_page(): void { - $this->assertFalse( - $this->redirect_resolver()->destination_for( admin_url( 'plugins.php' ) ), - 'Redirecting here would interrupt an inline update.' - ); - } - - public function test_it_returns_any_other_referrer_unchanged(): void { - $this->assertSame( - admin_url( 'options-general.php' ), - $this->redirect_resolver()->destination_for( admin_url( 'options-general.php' ) ) - ); - } -} -``` - -- [ ] **Step 3: Run it to verify it fails** - -Run: `slic run unit` -Expected: FAIL — `Class "Nexcess\PluginAbsorber\Conflict\Resolver" not found`. - -- [ ] **Step 4: Write `src/Conflict/Contracts/Resolver_Interface.php`** - -```php -is_enabled() || ! $sub_plugin->is_standalone_plugin_active() ) { - continue; - } - - $this->resolve( $sub_plugin ); - } - } - - /** - * @since 1.0.0 - * - * @param Sub_Plugin $sub_plugin Sub-plugin whose standalone is active. - * - * @return void - */ - protected function resolve( Sub_Plugin $sub_plugin ): void { - $policy = $sub_plugin->get_conflict_policy(); - - // A host may persist a policy in an option and a filter may return anything. Falling - // through to deactivate() would turn off a plugin the site owner deliberately activated - // on the strength of a typo, so an unrecognised policy takes the conservative branch. - if ( ! Conflict_Policy::is_valid( $policy ) ) { - $policy = Conflict_Policy::NOTICE_ONLY; - } - - switch ( $policy ) { - case Conflict_Policy::DEFER: - // The standalone wins. Its own constant makes the load path skip the bundled copy. - return; - - case Conflict_Policy::NOTICE_ONLY: - Loader::notices()->queue_conflict_notice( $sub_plugin ); - - return; - - case Conflict_Policy::DEACTIVATE: - default: - $this->deactivate( $sub_plugin ); - } - } - - /** - * @since 1.0.0 - * - * @param Sub_Plugin $sub_plugin Sub-plugin whose standalone is active. - * - * @return void - */ - protected function deactivate( Sub_Plugin $sub_plugin ): void { - if ( ! function_exists( 'deactivate_plugins' ) ) { - require_once ABSPATH . 'wp-admin/includes/plugin.php'; - } - - // The network flag is evaluated before the call, while the plugin is still active. - // Omitting it makes deactivate_plugins() a silent no-op for a network-activated plugin, - // so the next request would deactivate nothing and redirect again, forever. - deactivate_plugins( - $sub_plugin->get_standalone_plugin_basename(), - false, - $sub_plugin->is_standalone_plugin_network_active() - ); - - Loader::notices()->queue_merge_notice( $sub_plugin ); - - $destination = $this->redirect_destination( wp_get_referer() ); - - if ( $destination !== false ) { - wp_safe_redirect( $destination ); - - exit; - } - } - - /** - * Where to send the user after deactivating, or false to stay put. - * - * Never trap the user mid-update: an inline update on the plugins list must not be - * interrupted, and the update screens must not be reloaded. - * - * @since 1.0.0 - * - * @param string|false $referrer Result of wp_get_referer(). - * - * @return string|false - */ - protected function redirect_destination( $referrer ) { - if ( $referrer === false || $referrer === '' ) { - return admin_url( 'plugins.php' ); - } - - foreach ( [ admin_url( 'update.php' ), admin_url( 'update-core.php' ) ] as $update_url ) { - if ( strpos( $referrer, $update_url ) !== false ) { - return admin_url( 'plugins.php' ); - } - } - - if ( strpos( $referrer, admin_url( 'plugins.php' ) ) !== false ) { - return false; - } - - return $referrer; - } -} -``` - -- [ ] **Step 6: Add the resolver accessor and the @1 hook to `src/Loader.php`** - -Add `use Nexcess\PluginAbsorber\Conflict\Contracts\Resolver_Interface;` and `use Nexcess\PluginAbsorber\Conflict\Resolver;` to the imports, then: - -```php - /** - * @since 1.0.0 - * - * @return Resolver_Interface - */ - public static function resolver(): Resolver_Interface { - /** @var Resolver_Interface $resolver */ - $resolver = self::resolve( Resolver_Interface::class, Resolver::class ); - - return $resolver; - } - - /** - * @since 1.0.0 - * - * @return void - */ - public static function run_conflict_resolution(): void { - self::resolver()->resolve_all(); - } -``` - -And add the hook to `boot()`, **before** the existing @2 line: - -```php - add_action( 'plugins_loaded', [ self::class, 'run_conflict_resolution' ], 1 ); - add_action( 'plugins_loaded', [ self::class, 'load_all' ], 2 ); -``` - -- [ ] **Step 7: Add a boot assertion for the new hook** - -Append to `tests/unit/LoaderBootTest.php`: - -```php - public function test_it_wires_conflict_resolution_at_priority_one(): void { - Loader::boot(); - - $this->assertSame( - 1, - has_action( 'plugins_loaded', [ Loader::class, 'run_conflict_resolution' ] ) - ); - } -``` - -- [ ] **Step 8: Run the tests to verify they pass** - -Run: `slic run unit` -Expected: PASS — 15 resolver tests plus the new boot test. - -- [ ] **Step 9: Run the multisite leg** - -Run: `slic run unit --env multisite` -Expected: PASS. The network-flag tests stub `is_plugin_active_for_network`, so they assert the same way in both envs; the multisite run proves nothing else breaks under `MULTISITE`. - -- [ ] **Step 10: Confirm static analysis is still clean** - -Run: `composer test:analysis` - -- [ ] **Step 11: Append to the README** - -```markdown -### Per-sub-plugin policy override - -`conflict_policy` accepts a `callable( Sub_Plugin ): string`, so one sub-plugin can decide at -runtime without a container and without touching the library: - -```php -'conflict_policy' => static function ( Sub_Plugin $sub_plugin ) { - // Stand down if a newer standalone supersedes the bundled copy. - return my_standalone_version_at_least( $sub_plugin, '3.0.0' ) - ? Conflict_Policy::DEFER - : Conflict_Policy::DEACTIVATE; -}, -``` - -The `"{$prefix}/plugin_absorber/conflict_policy"` filter runs after that and wins: - -```php -add_filter( 'give/plugin_absorber/conflict_policy', function ( $policy, $sub_plugin ) { - return $policy; -}, 10, 2 ); -``` -``` - -- [ ] **Step 12: Commit, push, open the PR** - -```bash -git add src/Conflict/ src/Loader.php tests/unit/Conflict/ tests/unit/LoaderBootTest.php README.md -git commit -m "Add conflict resolver with network-aware deactivation" -git push -u origin 12-conflict-resolver -gh pr create --base 11-loader-load-path --title "Conflict resolver" --body 'What: the three conflict policies, network-aware deactivation, the safe redirect, and the -`plugins_loaded` @1 hook. - -Usage: - - // Automatic once booted. Per-sub-plugin override without a container: - "conflict_policy" => static function ( Sub_Plugin $sub_plugin ) { - return my_standalone_version_at_least( $sub_plugin, "3.0.0" ) - ? Conflict_Policy::DEFER - : Conflict_Policy::DEACTIVATE; - }, - -Why this way: - -**`deactivate_plugins()` receives `$network_wide`** — a change from the engineering plan, which -detects network activation and then drops the flag. Without it the call silently no-ops against a -network-activated plugin, so every admin request deactivates nothing and redirects again: an -infinite redirect loop on multisite, and exactly what the plan is own E2E criterion ("reloading the -plugins page does not loop") was meant to catch. - -**An unknown policy is its own case, never a `default:` fallthrough.** A typo like `defered` would -otherwise land on the deactivate branch and turn off a plugin the site owner deliberately enabled. - -**`redirect_destination()` returns false on a plugins.php referrer**, so an inline update is never -interrupted. update.php and update-core.php referrers are rewritten so the user is not bounced back -into an update screen. - -**`exit` is never mocked.** The stubbed `wp_safe_redirect()` throws `TestException`, halting the -resolver exactly where production calls `exit` while leaving a failing test free to report as -failing. - -**Known limitation, deliberate:** `resolve_all()` runs on front-end requests too, matching both -reference implementations. Tracked as issue B in the spec.' -``` - ---- - -## Task 13: `Activation` - -**PR 13** · branch `13-activation` from `12-conflict-resolver` · 4 source files - -**Files:** -- Create: `src/Contracts/Activation_Interface.php`, `src/Activation.php`, `tests/unit/ActivationTest.php` -- Modify: `src/Loader.php` (add `activation()` and call it from `load()`), `README.md` - -**Interfaces:** -- Consumes: `Config::get_option_name()` (Task 4), `Sub_Plugin::get_activation_callback()` / `get_slug()` (Task 7), the `WithSubPlugins` trait (Task 7) for its fixtures, `Loader::resolve()` (Task 9). -- Produces: `Activation_Interface` with `maybe_run( Sub_Plugin $sub_plugin ): void`, and `Loader::activation(): Activation_Interface`. - -**Design note:** the option key comes from `Config::get_option_name( 'activations' )`, never from -concatenating `Config::get_hook_prefix()`. The prefix validator admits `A-Z` and `-`, so a host -registering `Give-Core` would otherwise write to `Give-Core_plugin_absorber_activations`; -`get_option_name()` normalises that to `give_core_plugin_absorber_activations` while -`get_hook_name()` leaves filter names byte-for-byte as the host wrote them. `Notices\Store` uses the -same helper, so the two storage keys cannot drift apart. - -**Why this exists:** `register_activation_hook()` never fires for a `require_once`'d file, so the absorbed plugin's original activation routine would otherwise never run. Tracked per slug in one option, run exactly once ever. It is **not** a place for ongoing upgrade logic — a merged sub-plugin handles version upgrades with its own idempotent, version-gated migrations on load. - -- [ ] **Step 1: Cut the branch** - -```bash -git checkout 12-conflict-resolver && git checkout -b 13-activation -``` - -- [ ] **Step 2: Write the failing test** - -```php -assertInstanceOf( Activation::class, Loader::activation() ); - } - - public function test_it_runs_the_callback(): void { - $runs = 0; - - ( new Activation() )->maybe_run( $this->make_sub_plugin( [ 'activation_callback' => static function () use ( &$runs ) { ++$runs; } ] ) ); - - $this->assertSame( 1, $runs ); - } - - public function test_it_never_runs_the_callback_twice(): void { - $runs = 0; - $callback = static function () use ( &$runs ) { ++$runs; }; - $activation = new Activation(); - - $activation->maybe_run( $this->make_sub_plugin( [ 'activation_callback' => $callback ] ) ); - $activation->maybe_run( $this->make_sub_plugin( [ 'activation_callback' => $callback ] ) ); - - $this->assertSame( 1, $runs, 'The callback must run exactly once, ever.' ); - } - - public function test_a_fresh_instance_still_sees_the_flag(): void { - $runs = 0; - $callback = static function () use ( &$runs ) { ++$runs; }; - - ( new Activation() )->maybe_run( $this->make_sub_plugin( [ 'activation_callback' => $callback ] ) ); - ( new Activation() )->maybe_run( $this->make_sub_plugin( [ 'activation_callback' => $callback ] ) ); - - $this->assertSame( 1, $runs, 'The flag lives in an option, not in memory.' ); - } - - public function test_it_records_the_slug(): void { - ( new Activation() )->maybe_run( $this->make_sub_plugin( [ 'activation_callback' => static function () {} ] ) ); - - $this->assertSame( [ 'give-recurring' => true ], get_option( self::OPTION ) ); - } - - public function test_it_does_nothing_without_a_callback(): void { - ( new Activation() )->maybe_run( $this->make_sub_plugin() ); - - $this->assertFalse( get_option( self::OPTION ), 'No callback means no option write at all.' ); - } - - public function test_it_tracks_slugs_independently(): void { - $recurring = 0; - $fees = 0; - - $activation = new Activation(); - $activation->maybe_run( $this->make_sub_plugin( [ 'activation_callback' => static function () use ( &$recurring ) { ++$recurring; } ] ) ); - $activation->maybe_run( - $this->make_sub_plugin( - [ - 'slug' => 'give-fee-recovery', - 'activation_callback' => static function () use ( &$fees ) { ++$fees; }, - ] - ) - ); - - $this->assertSame( 1, $recurring ); - $this->assertSame( 1, $fees ); - $this->assertSame( [ 'give-recurring' => true, 'give-fee-recovery' => true ], get_option( self::OPTION ) ); - } - - public function test_it_recovers_from_a_corrupted_option(): void { - update_option( self::OPTION, 'not-an-array' ); - - $runs = 0; - ( new Activation() )->maybe_run( $this->make_sub_plugin( [ 'activation_callback' => static function () use ( &$runs ) { ++$runs; } ] ) ); - - $this->assertSame( 1, $runs ); - $this->assertSame( [ 'give-recurring' => true ], get_option( self::OPTION ) ); - } - - public function test_the_option_is_namespaced_by_hook_prefix(): void { - Config::reset(); - Config::set_hook_prefix( 'learndash' ); - - ( new Activation() )->maybe_run( $this->make_sub_plugin( [ 'activation_callback' => static function () {} ] ) ); - - $this->assertSame( [ 'give-recurring' => true ], get_option( 'learndash_plugin_absorber_activations' ) ); - $this->assertFalse( get_option( self::OPTION ) ); - - delete_option( 'learndash_plugin_absorber_activations' ); - } -} -``` - -- [ ] **Step 3: Run it to verify it fails** - -Run: `slic run unit` -Expected: FAIL — `Class "Nexcess\PluginAbsorber\Activation" not found`. - -- [ ] **Step 4: Write `src/Contracts/Activation_Interface.php`** - -```php -get_activation_callback(); - - if ( $callback === null ) { - return; - } - - $done = get_option( $this->option_name(), [] ); - $done = is_array( $done ) ? $done : []; - - if ( ! empty( $done[ $sub_plugin->get_slug() ] ) ) { - return; - } - - $callback(); - - $done[ $sub_plugin->get_slug() ] = true; - - update_option( $this->option_name(), $done, false ); - } - - /** - * @since 1.0.0 - * - * @return string - */ - private function option_name(): string { - return Config::get_option_name( 'activations' ); - } -} -``` - -- [ ] **Step 6: Wire it into `src/Loader.php`** - -Add `use Nexcess\PluginAbsorber\Contracts\Activation_Interface;`, then the accessor: - -```php - /** - * @since 1.0.0 - * - * @return Activation_Interface - */ - public static function activation(): Activation_Interface { - /** @var Activation_Interface $activation */ - $activation = self::resolve( Activation_Interface::class, Activation::class ); - - return $activation; - } -``` - -And append to `load()`, after the `require_once`: - -```php - require_once $sub_plugin->get_bundled_plugin_file(); - - self::activation()->maybe_run( $sub_plugin ); -``` - -- [ ] **Step 7: Assert the load path invokes it** - -Append to `tests/unit/LoaderLoadTest.php`: - -```php - public function test_it_runs_the_activation_callback_after_loading(): void { - $runs = 0; - - $this->register( [ 'activation_callback' => static function () use ( &$runs ) { ++$runs; } ] ); - - Loader::load_all(); - - $this->assertSame( 1, $GLOBALS['absorber_loads'] ); - $this->assertSame( 1, $runs ); - - delete_option( 'give_plugin_absorber_activations' ); - } - - public function test_it_does_not_run_the_activation_callback_when_the_load_is_skipped(): void { - $runs = 0; - - $this->register( - [ - 'enabled' => false, - 'activation_callback' => static function () use ( &$runs ) { ++$runs; }, - ] - ); - - Loader::load_all(); - - $this->assertSame( 0, $runs, 'Activation must follow a successful require, not precede it.' ); - } -``` - -- [ ] **Step 8: Run the tests to verify they pass** - -Run: `slic run unit` -Expected: PASS — 9 activation tests plus 2 new load tests. - -- [ ] **Step 9: Confirm static analysis is still clean** - -Run: `composer test:analysis` - -- [ ] **Step 10: Append to the README** - -```markdown -### Activation - -`register_activation_hook()` never fires for a `require_once`'d file, so supply the absorbed -plugin's original activation routine directly. It runs **exactly once, ever**, tracked per slug: - -```php -'activation_callback' => static function () { - \Give\Recurring\Install::create_tables(); -}, -``` - -This reproduces the original activation only. Ongoing upgrades belong in the sub-plugin's own -idempotent, version-gated migrations — not here. -``` - -- [ ] **Step 11: Commit, push, open the PR** - -```bash -git add src/Activation.php src/Contracts/Activation_Interface.php src/Loader.php tests/unit/ActivationTest.php tests/unit/LoaderLoadTest.php README.md -git commit -m "Add run-once activation tracking" -git push -u origin 13-activation -gh pr create --base 12-conflict-resolver --title "Activation" --body 'What: `Activation` and `Activation_Interface`, reachable as `Loader::activation()`, wired into the -load path as the last step after a successful require. - -Usage: - - "activation_callback" => static function () { - \Give\Recurring\Install::create_tables(); - }, - -Why this way: - -**`register_activation_hook()` never fires for a `require_once`d file**, so a plugin absorbed into -a host would never run its original install routine. One option holds a per-slug flag instead. - -**The callback fires only after a successful require** — never when the load was skipped, which -would otherwise create tables for code that is not loaded. - -**A single option rather than one per slug** keeps this to one autoloaded row no matter how many -sub-plugins a host bundles. - -**Known limitation, deliberate:** read-then-write is not atomic, so two simultaneous first requests -can both run the callback. Tracked as issue E in the spec; `add_option()` as a claim would close -it.' -``` - ---- - -## Task 14: Activation-error rewrite - -**PR 14** · branch `14-activation-error-notice` from `13-activation` · 3 source files - -When a user tries to re-activate an absorbed standalone, WordPress kills the request and reports a -generic *"Plugin could not be activated because it triggered a fatal error."* — technically true and -completely unhelpful. Swap in the sub-plugin's own explanation. - -**Files:** -- Modify: `src/Notices/Contracts/Queue_Interface.php`, `src/Notices/Queue.php`, `src/Loader.php`, `README.md` -- Create: `tests/unit/Notices/QueueActivationErrorTest.php` - -**Interfaces:** -- Consumes: `Loader::all()` (Task 9), `Sub_Plugin::get_standalone_plugin_basename()` / `get_conflict_notice_message()` (Task 7). -- Produces: - - `Notices\Contracts\Queue_Interface::filter_activation_error_markup( string $markup ): string` — **an addition to the interface shipped in Task 10** - - `Loader::filter_activation_error_markup( $markup ): string` - -**Design note (amendment A):** the engineering plan prescribed `ob_start()` on `admin_head-plugins.php`, copied from Kadence. The newer LearnDash reference (`Course_Grid/Legacy/Loader::update_legacy_plugin_activation_notice()`) uses the `wp_admin_notice_markup` filter instead. Same nonce check, same `str_replace`, but no buffering and no risk of mangling unrelated admin output — and it is testable by calling the filter directly. This is why the library requires WordPress 6.4+. - -- [ ] **Step 1: Cut the branch** - -```bash -git checkout 13-activation && git checkout -b 14-activation-error-notice -``` - -- [ ] **Step 2: Write the failing test** - -```php -wordpress_markup = '

' - // phpcs:ignore WordPress.WP.I18n.TextDomainMismatch -- matching WP's own string. - . __( 'Plugin could not be activated because it triggered a fatal error.', 'default' ) - . '

'; - - $_GET['plugin'] = self::BASENAME; - $_GET['_error_nonce'] = wp_create_nonce( 'plugin-activation-error_' . self::BASENAME ); - } - - public function tearDown(): void { - unset( $_GET['plugin'], $_GET['_error_nonce'] ); - set_current_screen( 'front' ); - Loader_State::reset(); - Config::reset(); - parent::tearDown(); - } - - /** - * @param array $overrides Config overrides. - */ - private function register( array $overrides = [] ): void { - Loader::register( - array_merge( - [ - 'slug' => 'give-recurring', - 'bundled_plugin_file' => '/tmp/give-recurring.php', - 'plugin_loaded_constant' => 'GIVE_RECURRING_VERSION_ERROR_NOTICE', - 'standalone_plugin_basename' => self::BASENAME, - 'conflict_notice_message' => 'Give Recurring is now bundled with Give.', - ], - $overrides - ) - ); - } - - public function test_it_replaces_the_fatal_error_text(): void { - $this->register(); - - $result = ( new Notices() )->filter_activation_error_markup( $this->wordpress_markup ); - - $this->assertStringContainsString( 'Give Recurring is now bundled with Give.', $result ); - $this->assertStringNotContainsString( 'fatal error', $result ); - } - - public function test_it_leaves_the_markup_alone_off_the_plugins_screen(): void { - $this->register(); - set_current_screen( 'dashboard' ); - - $this->assertSame( - $this->wordpress_markup, - ( new Notices() )->filter_activation_error_markup( $this->wordpress_markup ) - ); - } - - public function test_it_leaves_the_markup_alone_for_an_unregistered_plugin(): void { - $this->register(); - $_GET['plugin'] = 'some-other-plugin/some-other-plugin.php'; - $_GET['_error_nonce'] = wp_create_nonce( 'plugin-activation-error_some-other-plugin/some-other-plugin.php' ); - - $this->assertSame( - $this->wordpress_markup, - ( new Notices() )->filter_activation_error_markup( $this->wordpress_markup ) - ); - } - - public function test_it_leaves_the_markup_alone_with_a_bad_nonce(): void { - $this->register(); - $_GET['_error_nonce'] = 'not-a-valid-nonce'; - - $this->assertSame( - $this->wordpress_markup, - ( new Notices() )->filter_activation_error_markup( $this->wordpress_markup ) - ); - } - - public function test_it_leaves_the_markup_alone_with_no_plugin_parameter(): void { - $this->register(); - unset( $_GET['plugin'] ); - - $this->assertSame( - $this->wordpress_markup, - ( new Notices() )->filter_activation_error_markup( $this->wordpress_markup ) - ); - } - - public function test_it_leaves_the_markup_alone_without_a_configured_message(): void { - $this->register( [ 'conflict_notice_message' => '' ] ); - - $this->assertSame( - $this->wordpress_markup, - ( new Notices() )->filter_activation_error_markup( $this->wordpress_markup ), - 'With nothing to say, keep WordPress own wording rather than blanking it.' - ); - } - - public function test_it_strips_unsafe_markup_from_the_replacement_but_keeps_a_link(): void { - $this->register( - [ 'conflict_notice_message' => 'Read more' ] - ); - - $result = ( new Notices() )->filter_activation_error_markup( $this->wordpress_markup ); - - $this->assertStringNotContainsString( '