From 9faf13c87deb56c2805897f65e64147f87570a98 Mon Sep 17 00:00:00 2001 From: Nikolay Strikhar Date: Mon, 24 Aug 2026 15:10:44 +0200 Subject: [PATCH 1/2] Claim a merge only where the standalone really went away The merge notice was queued as the deactivation was made, and the re-check that asks whether the standalone actually went off ran afterwards and fed the redirect alone. A site whose mu-plugin filters `option_active_plugins`, or a host that bound `Plugin\Contracts\Deactivator_Interface` to a no-op -- which that interface invites by name -- therefore got no redirect, correctly, and a notice saying a plugin they can watch still running had been deactivated, re-queued on every admin GET. The re-check moves in front of the notice, so the sentence is written on the branch that confirms it. One notice per plugin lost is unchanged: it is still queued per sub-plugin, as each deactivation is confirmed, and still before the redirect that would otherwise carry it off. Nothing is reported through `report_error()` on the other branch. This is the DEFER outcome reached by another route -- the standalone is running, so its own guard constant stands the bundled copy down and nothing re-declares -- and DEFER announces nothing. Every route here is a site's own configuration rather than a mistake in the host's code, which is who `_doing_it_wrong()` addresses, and the conflict is re-detected on every admin GET, so a report would print the same sentence on every screen of a debugging site for a site behaving as its owner set it up to. --- src/Conflict/Resolver.php | 49 +++++++++++++-------- tests/unit/Conflict/ResolverTest.php | 65 ++++++++++++++++++++++++---- tests/unit/Scenario/ConflictTest.php | 50 +++++++++++++++++++++ 3 files changed, 138 insertions(+), 26 deletions(-) diff --git a/src/Conflict/Resolver.php b/src/Conflict/Resolver.php index 8f54b3d..fe82613 100644 --- a/src/Conflict/Resolver.php +++ b/src/Conflict/Resolver.php @@ -200,19 +200,7 @@ protected function resolve( Sub_Plugin $sub_plugin ): bool { return false; } - $this->deactivate( $sub_plugin ); - - // Asked again rather than assumed, because turning the standalone off is not the same - // event as the standalone being off. A site or mu-plugin filtering - // `option_active_plugins` puts it straight back, a host may have rebound - // `Plugin\Contracts\Deactivator_Interface` to something that does nothing, and a - // rebound `Plugin\Contracts\Checker_Interface` may mean by "active" something - // `deactivate_plugins()` never touches. Answering true on a standalone that is still - // running redirects to the screen the user asked for, where the next request detects - // the same conflict and redirects again -- until the browser gives up and the whole of - // wp-admin is out of reach, with the merge notice never drawn because every one of - // those requests exits before `all_admin_notices`. - return ! $this->detector->is_in_conflict( $sub_plugin ); + return $this->deactivate( $sub_plugin ); // NOTICE_ONLY, and anything is_valid() would accept that this switch has grown no // branch for. The default sits on the branch that only talks, never on the one that @@ -225,21 +213,46 @@ protected function resolve( Sub_Plugin $sub_plugin ): bool { } /** + * Turn the standalone off, and report the merge only if it really went off. + * + * The answer is also what decides the redirect. Sending the user back to the screen they asked for + * while the standalone is still running arrives at the same conflict, deactivates to no effect and + * redirects again -- until the browser gives up with the whole of wp-admin out of reach, and with + * every one of those requests exiting before `all_admin_notices` could draw anything. + * * @since 1.0.0 * * @param Sub_Plugin $sub_plugin Sub-plugin whose standalone is active. * * @throws Config_Exception When no hook prefix has been set, or a container binding is unusable. * - * @return void + * @return bool Whether the standalone is gone -- not whether deactivating it was attempted. */ - protected function deactivate( Sub_Plugin $sub_plugin ): void { + protected function deactivate( Sub_Plugin $sub_plugin ): bool { $this->plugin_deactivator->deactivate( $sub_plugin->get_standalone_plugin_basename() ); - // Queued as the deactivation is made rather than once at the end, so the explanation is - // durable whether or not the request goes on to redirect — and so a site with two - // standalones gets one notice per plugin it lost. + // Asked again rather than assumed, because turning the standalone off is not the same event as + // the standalone being off. A site or mu-plugin filtering `option_active_plugins` puts it + // straight back, a host may have rebound `Plugin\Contracts\Deactivator_Interface` to something + // that does nothing, and a rebound `Plugin\Contracts\Checker_Interface` may mean by "active" + // something `deactivate_plugins()` never touches. + if ( $this->detector->is_in_conflict( $sub_plugin ) ) { + // Nothing is said and nothing is reported. The merge notice would tell the owner a plugin + // they can watch still running had been deactivated, and would say it again on every admin + // GET for as long as the site kept putting it back; and this is the DEFER outcome reached + // by another route, so there is no failure to announce either -- the standalone is running, + // which means its own guard constant stands the bundled copy down and nothing re-declares. + // Every way of arriving here is a site's own configuration rather than a mistake in the + // host's code, and `Plugin\Contracts\Deactivator_Interface` invites one of them by name. + return false; + } + + // Queued as each deactivation is confirmed rather than once at the end, so the explanation is + // durable whether or not the request goes on to redirect — and so a site with two standalones + // gets one notice per plugin it lost. $this->notices->queue_merge_notice( $sub_plugin ); + + return true; } /** diff --git a/tests/unit/Conflict/ResolverTest.php b/tests/unit/Conflict/ResolverTest.php index b365fee..c9be210 100644 --- a/tests/unit/Conflict/ResolverTest.php +++ b/tests/unit/Conflict/ResolverTest.php @@ -511,8 +511,13 @@ public function test_it_deactivates_without_redirecting_once_the_headers_are_sen * Nothing exotic is required to get here: a site or mu-plugin filtering `option_active_plugins` * puts the standalone straight back into the active list, which is a pattern `learndash-core` * itself uses. + * + * And the merge notice is the other half. "We deactivated it for you" is a claim the owner can + * disprove by looking at the plugins list the notice is drawn above, and it is re-queued on every + * admin GET for as long as the standalone keeps coming back — so the one sentence this library + * says about a conflict has to wait until the re-check says there is a merge to report. */ - public function test_a_standalone_that_survives_deactivation_does_not_redirect(): void { + public function test_a_standalone_that_survives_deactivation_neither_redirects_nor_reports_a_merge(): void { $this->standalone_is( true ); $this->standalone_survives_deactivation( 'give-recurring/give-recurring.php' ); $this->register(); @@ -520,11 +525,47 @@ public function test_a_standalone_that_survives_deactivation_does_not_redirect() $this->resolve_all(); $this->assertCount( 1, $this->deactivations, 'The policy still runs: deactivation is asked for.' ); - $this->assertArrayHasKey( - 'give-recurring:merge', + $this->assertSame( + [], $this->queued_notices(), - 'The request goes on rendering, so the notice explaining the deactivation must survive to be drawn.' + 'A standalone the owner can watch still running must not be reported as deactivated.' + ); + } + + /** + * Silence, deliberately, and this is where it is pinned: a standalone that comes back is the + * outcome DEFER produces on purpose, reached by another route — the standalone is running, so its + * own guard constant stands the bundled copy down and nothing re-declares. Every way to get here + * is a site's own configuration rather than a mistake in the host's code, and the conflict is + * re-detected on every admin GET, so a report would be the same sentence on every screen of a + * debugging site for a site behaving as its owner set it up to. + * + * `_doing_it_wrong()` is covered without an assertion of its own: WPTestCase fails a test that + * receives one it never said to expect. + */ + public function test_a_standalone_that_survives_deactivation_is_not_reported_as_an_error(): void { + $announced = []; + + add_action( + 'give/plugin_absorber/error', + static function ( $message ) use ( &$announced ): void { + $announced[] = $message; + } ); + + $this->standalone_is( true ); + $this->standalone_survives_deactivation( 'give-recurring/give-recurring.php' ); + $this->register(); + + $this->resolve_all(); + + $this->assertSame( [], $announced, 'A supported configuration is not a failure to announce.' ); + + // The recorder is proved to work before its emptiness is believed: a listener that never + // attached would satisfy the assertion above for a reason no assertion names. + do_action( 'give/plugin_absorber/error', 'a listener that is really attached', null ); + + $this->assertSame( [ 'a listener that is really attached' ], $announced ); } /** @@ -572,7 +613,11 @@ static function () use ( $deactivator ): Deactivator_Interface { $this->deactivations, 'The bound deactivator is what deactivates, so nothing was ever taken out of the active list.' ); - $this->assertArrayHasKey( 'give-recurring:merge', $this->queued_notices() ); + $this->assertSame( + [], + $this->queued_notices(), + 'A host is invited to bind deactivation away; being told its plugin was deactivated anyway is the lie.' + ); } /** @@ -663,7 +708,11 @@ public function test_it_still_redirects_when_one_of_two_standalones_really_went_ ); $queued = $this->queued_notices(); - $this->assertArrayHasKey( 'give-recurring:merge', $queued ); + $this->assertArrayNotHasKey( + 'give-recurring:merge', + $queued, + 'The stubborn one is still running, whatever the request managed for the other.' + ); $this->assertArrayHasKey( 'give-fee-recovery:merge', $queued ); } @@ -740,8 +789,8 @@ public function test_it_really_deactivates_a_network_active_standalone(): void { } /** - * The notice is queued after the deactivation, so it must not depend on the plugin still - * being active — and it is the only record the site owner gets. + * The notice is queued after the deactivation and after the re-check that confirms it, so it must + * not depend on the plugin still being active — and it is the only record the site owner gets. */ public function test_the_merge_notice_is_queued_before_the_redirect_halts_the_request(): void { $this->standalone_is( true ); diff --git a/tests/unit/Scenario/ConflictTest.php b/tests/unit/Scenario/ConflictTest.php index 7fa337e..e3f0a13 100644 --- a/tests/unit/Scenario/ConflictTest.php +++ b/tests/unit/Scenario/ConflictTest.php @@ -245,6 +245,56 @@ public function test_the_request_after_a_deactivation_does_not_loop(): void { $this->assertTrue( defined( $constant ) ); } + /** + * The deactivation that does not take. A site whose mu-plugin filters `option_active_plugins` puts + * the standalone straight back — a pattern `learndash-core` itself uses — so core's own + * `deactivate_plugins()` really runs, really writes the option, and the plugin is active again by + * the time the resolver looks. + * + * Nothing may be claimed about that. A merge notice would report a deactivation the owner can + * disprove by looking at the list it is drawn above, and it would be re-queued on every admin GET + * for as long as the site keeps putting the plugin back. The redirect goes for the same reason: + * the request has nothing to shed, so the one it redirects to would arrive at the same conflict. + */ + public function test_a_standalone_the_site_puts_back_is_not_reported_as_merged(): void { + update_option( 'active_plugins', [ self::STANDALONE ] ); + + $this->add_tracked_filter( + 'option_active_plugins', + static function (): array { + return [ self::STANDALONE ]; + } + ); + + $this->register( + [ + 'standalone_plugin_basename' => self::STANDALONE, + 'conflict_policy' => Conflict_Policy::DEACTIVATE, + ] + ); + + $this->boot(); + + // run_request() fails the test on a redirect, which here is the loop itself rather than a + // symptom of one: every request after it would deactivate to no effect and redirect again. + $this->run_request(); + + $this->assertContains( + self::STANDALONE, + $this->active_plugins(), + 'The site put it back, and this library does not fight the site over it.' + ); + $this->assertSame( + [], + $this->queued_notices(), + 'A standalone the owner can watch still running must not be reported as deactivated.' + ); + + // The request really did reach this library, which is what makes the silence above mean + // something: a bootstrap that wired nothing at all would satisfy every assertion before it. + $this->assertSame( 1, $this->bundled_plugin_loads() ); + } + /** * DEFER hands the request to the standalone, and WordPress includes an active plugin from * wp-settings.php long before plugins_loaded — so by the time the resolver runs, the standalone From 595a05070779b8655a446eecb7ec7b0a39a875be Mon Sep 17 00:00:00 2001 From: Nikolay Strikhar Date: Mon, 24 Aug 2026 15:39:35 +0200 Subject: [PATCH 2/2] Promise only what the resolver can still fail on Container failure moved from read-time to build-time when the reader took its registrar as a constructor argument: a pass that exists has already resolved everything it needs. The hook prefix is what is left, and it is read when a notice is worded. --- src/Conflict/Resolver.php | 6 ++-- tests/unit/Conflict/ResolverTest.php | 43 ++++++---------------------- 2 files changed, 11 insertions(+), 38 deletions(-) diff --git a/src/Conflict/Resolver.php b/src/Conflict/Resolver.php index fe82613..ab9f404 100644 --- a/src/Conflict/Resolver.php +++ b/src/Conflict/Resolver.php @@ -99,7 +99,7 @@ public function __construct( /** * @since 1.0.0 * - * @throws Config_Exception When no hook prefix has been set, or a container binding is unusable. + * @throws Config_Exception When no hook prefix has been set. * * @return void */ @@ -168,7 +168,7 @@ public function resolve_all(): void { * * @param Sub_Plugin $sub_plugin Sub-plugin whose standalone is active. * - * @throws Config_Exception When no hook prefix has been set, or a container binding is unusable. + * @throws Config_Exception When no hook prefix has been set. * * @return bool Whether the standalone is gone -- not whether deactivating it was attempted. */ @@ -224,7 +224,7 @@ protected function resolve( Sub_Plugin $sub_plugin ): bool { * * @param Sub_Plugin $sub_plugin Sub-plugin whose standalone is active. * - * @throws Config_Exception When no hook prefix has been set, or a container binding is unusable. + * @throws Config_Exception When no hook prefix has been set. * * @return bool Whether the standalone is gone -- not whether deactivating it was attempted. */ diff --git a/tests/unit/Conflict/ResolverTest.php b/tests/unit/Conflict/ResolverTest.php index c9be210..f9ef081 100644 --- a/tests/unit/Conflict/ResolverTest.php +++ b/tests/unit/Conflict/ResolverTest.php @@ -516,6 +516,14 @@ public function test_it_deactivates_without_redirecting_once_the_headers_are_sen * disprove by looking at the plugins list the notice is drawn above, and it is re-queued on every * admin GET for as long as the standalone keeps coming back — so the one sentence this library * says about a conflict has to wait until the re-check says there is a merge to report. + * + * Nothing is reported to the developer either, and the silence is deliberate: the standalone is + * running, so its own guard constant stands the bundled copy down and nothing re-declares. Every + * way to get here is a site's own configuration rather than a mistake in the host's code, and the + * conflict is re-detected on every admin GET — a report would be the same sentence on every screen + * of a debugging site, for a site behaving as its owner set it up to. That is covered without an + * assertion of its own: WPTestCase fails a test that receives a `_doing_it_wrong()` it never said + * to expect. */ public function test_a_standalone_that_survives_deactivation_neither_redirects_nor_reports_a_merge(): void { $this->standalone_is( true ); @@ -532,41 +540,6 @@ public function test_a_standalone_that_survives_deactivation_neither_redirects_n ); } - /** - * Silence, deliberately, and this is where it is pinned: a standalone that comes back is the - * outcome DEFER produces on purpose, reached by another route — the standalone is running, so its - * own guard constant stands the bundled copy down and nothing re-declares. Every way to get here - * is a site's own configuration rather than a mistake in the host's code, and the conflict is - * re-detected on every admin GET, so a report would be the same sentence on every screen of a - * debugging site for a site behaving as its owner set it up to. - * - * `_doing_it_wrong()` is covered without an assertion of its own: WPTestCase fails a test that - * receives one it never said to expect. - */ - public function test_a_standalone_that_survives_deactivation_is_not_reported_as_an_error(): void { - $announced = []; - - add_action( - 'give/plugin_absorber/error', - static function ( $message ) use ( &$announced ): void { - $announced[] = $message; - } - ); - - $this->standalone_is( true ); - $this->standalone_survives_deactivation( 'give-recurring/give-recurring.php' ); - $this->register(); - - $this->resolve_all(); - - $this->assertSame( [], $announced, 'A supported configuration is not a failure to announce.' ); - - // The recorder is proved to work before its emptiness is believed: a listener that never - // attached would satisfy the assertion above for a reason no assertion names. - do_action( 'give/plugin_absorber/error', 'a listener that is really attached', null ); - - $this->assertSame( [ 'a listener that is really attached' ], $announced ); - } /** * The same failure through the seam a host owns rather than through the site's own filters: a