7A. Stand the conflict pass down when the load gate says no

docs/conflict-handling.md

@@ -2,8 +2,10 @@
 
 ## Policies
 
-A policy is only reached for a sub-plugin that is enabled, names a `standalone_plugin_basename`, and
-whose standalone is active right now. Everything else is skipped before any policy is read.
+A policy is only reached for a sub-plugin that is enabled, names a `standalone_plugin_basename`,
+whose standalone is active right now, and that the [`should_load` filter](filters.md#the-load-gate)
+has not vetoed — a bundled copy nothing is going to load is worth no standalone's deactivation.
+Everything else is skipped before any policy is read.
 
 | Policy | Behavior |
 |---|---|

docs/filters.md

@@ -20,7 +20,7 @@ deactivate](conflict-handling.md#policies).
 
 | Filter | Arguments | Purpose |
 |---|---|---|
-| `{prefix}/plugin_absorber/should_load` | `bool $should_load`, `Sub_Plugin $sub_plugin` | Last word before `require_once`. |
+| `{prefix}/plugin_absorber/should_load` | `bool $should_load`, `Sub_Plugin $sub_plugin` | Last word before `require_once`, and before a conflict is resolved. |
 
 ```php
 add_filter( 'give/plugin_absorber/should_load', function ( $should_load, $sub_plugin ) {
@@ -33,6 +33,15 @@ the guard constant, the dependency check and the file check. Returning `true` ca
 past the guard constant; anything other than a truthy return skips the load, which is the safe
 direction.
 
+**Conflict handling reads it too**, at `plugins_loaded` priority 5, one ahead of the load. A
+sub-plugin you veto is invisible to it: no standalone is deactivated to make room for a bundled copy
+that is not going to load. So the filter may be asked more than once in a request, and it has to
+*decide* rather than do — no logging, no counters, no writes.
+
+A filter added later than priority 5 is too late for the conflict pass, which has already run. For a
+sub-plugin with a `standalone_plugin_basename`, put the toggle in the `enabled`
+[config key](configuration.md) instead: both passes read it, whenever it is set.
+
 **Watch the polarity when you wire an existing gate to this one.** `should_load` is true means *do
 load*. A host filter named for the opposite — LearnDash's `learndash_module_{x}_disabled`, where
 true means *do not load* — inverts the gate if passed through unnegated, and the failure is silent

src/Conflict/Detector.php

@@ -23,7 +23,7 @@
  * every host that binds its own resolver, and give the resolver two reasons to change: how a
  * conflict is found, and what to do about one.
  *
- * Both methods leave the request exactly as they found it. Nothing here resolves a user,
+ * Nothing here leaves a mark on the request. Nothing resolves a user,
  * deactivates a plugin or queues a notice — an answer is all a caller gets, and the acting is the
  * resolver's.
  *
@@ -65,7 +65,7 @@ public function __construct( Reader $registry, Checker_Interface $plugin_checker
 	 *
 	 * @since 1.0.0
 	 *
-	 * @throws Config_Exception When no container has been set, or a container binding is unusable.
+	 * @throws Config_Exception When no hook prefix has been set.
 	 *
 	 * @return bool
 	 */
@@ -88,18 +88,43 @@ public function has_conflict(): bool {
 	 * Policy is not consulted: a sub-plugin set to defer is still in conflict, and the resolver is
 	 * where the decision to leave it alone belongs.
 	 *
+	 * The `should_load` filter is, and it is the one piece of host code this class reads. It decides
+	 * whether the bundled copy will be in memory at all, one priority behind this — so a sub-plugin
+	 * the host vetoes there is in conflict with nothing: deactivating its standalone would take away
+	 * the only copy of that code the site has, and the merge notice would tell the owner the bundled
+	 * copy had taken over. Invisible here exactly as a disabled sub-plugin already is.
+	 *
+	 * Asked last, behind three checks that cost nothing. A filter is arbitrary host code and this
+	 * runs on every admin GET a site serves, so the sub-plugin has to be enabled, name a standalone,
+	 * and have that standalone actually running before any of it executes — which is also the order
+	 * the load pass asks it in, last of its own gates.
+	 *
+	 * Nothing here catches: `Boot\Scheduler` wraps the whole conflict step in `catch ( Throwable )`
+	 * and `Conflict\Resolver` catches per sub-plugin behind that, so a host filter that throws is
+	 * already reported and already survivable from both callers.
+	 *
 	 * @since 1.0.0
 	 *
 	 * @param Sub_Plugin $sub_plugin Sub-plugin to test.
 	 *
+	 * @throws Config_Exception When no hook prefix has been set.
+	 *
 	 * @return bool
 	 */
 	public function is_in_conflict( Sub_Plugin $sub_plugin ): bool {
 		if ( ! $sub_plugin->is_enabled() || ! $sub_plugin->has_standalone_plugin() ) {
 			return false;
 		}
 
-		return $this->plugin_checker->is_active( $sub_plugin->get_standalone_plugin_basename() );
+		if ( ! $this->plugin_checker->is_active( $sub_plugin->get_standalone_plugin_basename() ) ) {
+			return false;
+		}
+
+		// The same two arguments the load pass passes, so a host wires one filter and sees one
+		// signature wherever it is asked from.
+		$should_load = apply_filters( Config::get_hook_name( 'should_load' ), true, $sub_plugin );
+
+		return (bool) $should_load;
 	}
 
 	/**

tests/unit/Conflict/DetectorTest.php

@@ -34,7 +34,9 @@
  * anything it changed would be a change made on behalf of an anonymous visitor, and anything
  * expensive it read would be read on every admin GET a site ever serves. Policy is the expensive
  * thing it must not read — a host's policy callable and the filter behind it can do anything at all,
- * and neither says whether a standalone is running.
+ * and neither says whether a standalone is running. The `should_load` filter is the one piece of host
+ * code it does read, because that one decides whether the bundled copy will be there at all — and it
+ * is read last, of a sub-plugin every cheap check has already agreed is in conflict.
  *
  * `has_conflict()` reads the registry through the reader it was built with, so most of the tests for
  * it register through the facade the default reader is behind; `is_in_conflict()` is handed the
@@ -62,6 +64,16 @@ class DetectorTest extends WPTestCase {
 	 */
 	private $asked = [];
 
+	/**
+	 * The `should_load` filter a test installed, taken back off in tearDown.
+	 *
+	 * By identity rather than with `remove_all_filters()`, which would strip the hook bare for the
+	 * rest of the process — including whatever the library itself has on it.
+	 *
+	 * @var callable|null
+	 */
+	private $load_gate = null;
+
 	/**
 	 * Every deactivate_plugins() call, as the arguments it was made with.
 	 *
@@ -106,6 +118,9 @@ static function ( $plugins, $silent = false, $network_wide = null ) use ( &$deac
 	}
 
 	public function tearDown(): void {
+		// Before Config_State::reset(), which takes away the prefix the hook name is built from.
+		$this->remove_the_load_gate();
+
 		$this->stop_expecting_incorrect_usage();
 		$this->clear_notices();
 		Absorber_State::reset();
@@ -179,6 +194,23 @@ public function test_it_ignores_a_sub_plugin_with_no_standalone(): void {
 		$this->assertFalse( $this->detector()->has_conflict() );
 	}
 
+	/**
+	 * The gate the load pass reads one priority later, read here as well. A host that vetoes a
+	 * sub-plugin through `should_load` is getting no bundled copy at priority 6, so a probe that
+	 * answered yes at priority 5 would send the step on to deactivate the only copy of that code the
+	 * site has — and the merge notice would tell the owner the bundled one had taken over.
+	 */
+	public function test_it_ignores_a_sub_plugin_the_load_gate_vetoes(): void {
+		$this->standalone_is( true );
+		$this->register();
+		$this->gate_the_load_with( static fn( $should_load ) => false );
+
+		$this->assertFalse(
+			$this->detector()->has_conflict(),
+			'A sub-plugin that is not going to load is no more in conflict than a disabled one.'
+		);
+	}
+
 	public function test_it_is_false_with_nothing_registered(): void {
 		$this->standalone_is( true );
 
@@ -414,6 +446,92 @@ public function test_is_in_conflict_asks_about_the_configured_basename(): void {
 		$this->assertSame( [ 'give-fee-recovery/give-fee-recovery.php' ], $this->asked );
 	}
 
+	public function test_is_in_conflict_is_false_when_the_load_gate_vetoes_the_sub_plugin(): void {
+		$detector = new Detector( new Stub_Registry_Reader(), $this->recording_checker( true ) );
+
+		$this->gate_the_load_with( static fn( $should_load ) => false );
+
+		$this->assertFalse(
+			$detector->is_in_conflict(
+				$this->make_sub_plugin( [ 'standalone_plugin_basename' => 'give-recurring/give-recurring.php' ] )
+			),
+			'The standalone is the only copy of that code the site is going to run.'
+		);
+	}
+
+	/**
+	 * The same two arguments the load pass passes, in the same order. A host wires one filter and it
+	 * is asked from two places now, so a signature that differed between them would make the second
+	 * call the one that broke it.
+	 */
+	public function test_is_in_conflict_asks_the_load_gate_the_way_the_load_pass_does(): void {
+		$seen       = [];
+		$sub_plugin = $this->make_sub_plugin(
+			[ 'standalone_plugin_basename' => 'give-recurring/give-recurring.php' ]
+		);
+
+		$this->gate_the_load_with(
+			static function ( $should_load, $filtered ) use ( &$seen ) {
+				$seen[] = [ $should_load, $filtered ];
+
+				return $should_load;
+			}
+		);
+
+		$detector = new Detector( new Stub_Registry_Reader(), $this->recording_checker( true ) );
+
+		$this->assertTrue( $detector->is_in_conflict( $sub_plugin ) );
+		$this->assertSame(
+			[ [ true, $sub_plugin ] ],
+			$seen,
+			'The default is true and the sub-plugin is the one being asked about, exactly as at the load.'
+		);
+	}
+
+	/**
+	 * Last of the four, behind every check that costs nothing. The gate is host code that may do
+	 * anything at all, and this runs on every admin GET a site serves — so a sub-plugin that is
+	 * disabled, has no standalone, or has no standalone *running* is turned away before any of it.
+	 */
+	public function test_is_in_conflict_asks_the_load_gate_last(): void {
+		$asked = [];
+
+		$this->gate_the_load_with(
+			static function ( $should_load, $filtered ) use ( &$asked ) {
+				$asked[] = $filtered instanceof Sub_Plugin ? $filtered->get_slug() : '';
+
+				return $should_load;
+			}
+		);
+
+		$standalone = new Detector( new Stub_Registry_Reader(), $this->recording_checker( false ) );
+
+		$standalone->is_in_conflict(
+			$this->make_sub_plugin(
+				[
+					'enabled'                    => false,
+					'standalone_plugin_basename' => 'give-recurring/give-recurring.php',
+				]
+			)
+		);
+		$standalone->is_in_conflict( $this->make_sub_plugin() );
+		$standalone->is_in_conflict(
+			$this->make_sub_plugin( [ 'standalone_plugin_basename' => 'give-recurring/give-recurring.php' ] )
+		);
+
+		$this->assertSame( [], $asked, 'None of those three is a conflict, so none of them may run host code.' );
+
+		// A filter that failed to install leaves the same empty log, so it is shown working once the
+		// cheap checks have all passed.
+		$conflicted = new Detector( new Stub_Registry_Reader(), $this->recording_checker( true ) );
+
+		$conflicted->is_in_conflict(
+			$this->make_sub_plugin( [ 'standalone_plugin_basename' => 'give-recurring/give-recurring.php' ] )
+		);
+
+		$this->assertSame( [ 'give-recurring' ], $asked, 'The gate really is on the hook and really is read.' );
+	}
+
 	/**
 	 * A checker that was never reached and a checker whose recorder was never installed leave the
 	 * same empty log, so every test asserting the checker was *not* asked shows it working once
@@ -748,4 +866,34 @@ private function register_fee_recovery( array $overrides = [] ): void {
 	private function standalone_is( bool $active ): void {
 		$this->setFunctionReturn( 'is_plugin_active', $active );
 	}
+
+	/**
+	 * Put a host's `should_load` filter on the hook, as a host would.
+	 *
+	 * The real hook and the real name `Config` builds, not a double: what these tests are about is
+	 * that the detector reads the same gate the load pass reads, and a filter installed under any
+	 * other name would prove the opposite.
+	 *
+	 * @param callable $callback Filter callback.
+	 *
+	 * @return void
+	 */
+	private function gate_the_load_with( callable $callback ): void {
+		$this->load_gate = $callback;
+
+		add_filter( Config::get_hook_name( 'should_load' ), $callback, 10, 2 );
+	}
+
+	/**
+	 * @return void
+	 */
+	private function remove_the_load_gate(): void {
+		if ( $this->load_gate === null ) {
+			return;
+		}
+
+		remove_filter( Config::get_hook_name( 'should_load' ), $this->load_gate );
+
+		$this->load_gate = null;
+	}
 }

tests/unit/Scenario/ConflictTest.php

@@ -11,6 +11,7 @@
 
 use Nexcess\PluginAbsorber\Config;
 use Nexcess\PluginAbsorber\Conflict_Policy;
+use Nexcess\PluginAbsorber\Sub_Plugin;
 
 /**
  * Every policy branch, driven end to end against a real WordPress.
@@ -190,6 +191,58 @@ public function test_a_network_active_standalone_is_deactivated_when_the_host_is
 		$this->assertSame( admin_url( 'plugins.php' ), $location );
 	}
 
+	/**
+	 * The combination that costs a site its feature outright if the two passes disagree: a standalone
+	 * still active, the default policy, and a host vetoing the bundled copy through `should_load`.
+	 * Deactivating here would take away the only copy of that code the request is going to have —
+	 * priority 6 is not going to require anything — and the merge notice would tell the owner the
+	 * opposite, that the bundled copy is now loaded automatically.
+	 *
+	 * The second half is what makes the first mean anything: lift the veto and the same site, on the
+	 * next page view, deactivates and merges. So the veto is what stood the conflict pass down, not a
+	 * gate somewhere else that would have refused this request anyway.
+	 */
+	public function test_a_vetoed_sub_plugin_leaves_its_standalone_alone(): void {
+		update_option( 'active_plugins', [ self::STANDALONE ] );
+
+		$constant = $this->register(
+			[
+				'standalone_plugin_basename' => self::STANDALONE,
+				'conflict_policy'            => Conflict_Policy::DEACTIVATE,
+			]
+		);
+
+		$veto = static function ( $should_load, $sub_plugin ) {
+			return $sub_plugin instanceof Sub_Plugin && $sub_plugin->get_slug() === self::SLUG
+				? false
+				: $should_load;
+		};
+
+		$this->add_tracked_filter( Config::get_hook_name( 'should_load' ), $veto, 10, 2 );
+
+		$this->boot();
+
+		// run_request() fails the test if anything redirects, which is the third of the three things
+		// a resolution would have done.
+		$this->run_request();
+
+		$this->assertContains(
+			self::STANDALONE,
+			$this->active_plugins(),
+			'Nothing may deactivate the standalone while the bundled copy is vetoed.'
+		);
+		$this->assertSame( [], $this->queued_notices(), 'And nothing may claim a merge that did not happen.' );
+		$this->assertSame( 0, $this->bundled_plugin_loads() );
+		$this->assertFalse( defined( $constant ) );
+
+		remove_filter( Config::get_hook_name( 'should_load' ), $veto, 10 );
+
+		$this->run_halted_request();
+
+		$this->assertNotContains( self::STANDALONE, $this->active_plugins() );
+		$this->assertArrayHasKey( self::SLUG . ':merge', $this->queued_notices() );
+	}
+
 	/**
 	 * All the way to the screen. The merge notice is the one this library raises exactly once and
 	 * never re-queues, so the admin page load after the deactivation has to draw it — and consume it,