Skip to content

12F: Wire the conflict step, and move the load pass behind it - #27

Open
nikolaystrikhar wants to merge 1 commit into
12E-conflict-resolverfrom
12F-sequence-wiring
Open

12F: Wire the conflict step, and move the load pass behind it#27
nikolaystrikhar wants to merge 1 commit into
12E-conflict-resolverfrom
12F-sequence-wiring

Conversation

@nikolaystrikhar

Copy link
Copy Markdown
Contributor

What: Boot\Scheduler wires conflict resolution at plugins_loaded priority 5 and moves the load pass to priority 6, with the too-late barrier now measuring against the lowest priority in the sequence.

Usage:

add_action( 'plugins_loaded', static function (): void {
    Config::set_container( give()->container );
    Absorber::boot();
}, 0 );

Why this way:

Both gates and the conflict probe live in the step, not the resolver, so a host binding its own Resolver_Interface cannot drop one by omission — and a request that fails any of them never builds a resolver.

The gates sit either side of the probe. current_user_can() resolves and caches the current user, so asking it on every admin GET would settle who is signed in ahead of an SSO or JWT plugin adding its determine_current_user filter from its own plugins_loaded callback. The detector reports and changes nothing, so the cheap question goes first.

The barrier measures against the first step, not the last. Measuring against the load would let a host wire the load and silently lose the conflict pass, which is the half a fatal depends on.

Resolution sits at 5, not 1, so the barrier leaves a host somewhere to stand. At 1 the only slot left was 0, and LearnDash and MemberDash both wire Harbor's set_container() at priority 1 — a host copying that habit landed exactly on the barrier and silently took the inline fallback.

Boot\Scheduler gains the conflict step: the request gate, the conflict probe and
the capability gate, then Resolver_Interface::resolve_all() behind all three. All
three live in the step rather than in the resolver, so a host binding its own
cannot drop one by omission, and a request that fails any of them never builds a
resolver.

The gates sit either side of Conflict\Detector::has_conflict() because
current_user_can() resolves and caches the current user: asking it on every admin
GET would settle who is signed in ahead of an SSO or JWT plugin adding its
determine_current_user filter from its own plugins_loaded callback. The detector
reports and changes nothing, so the cheap question goes first.

Resolution sits at 5 and the load moves to 6. The too-late barrier now measures
against the lowest priority in the sequence rather than a constant, so a host that
boots between the two steps is told. At 1 the only slot left was 0, which turned a
documented convention into a hard requirement -- and LearnDash and MemberDash both
wire Harbor's set_container() at priority 1, landing exactly on the barrier and
silently taking the inline fallback. The load stays one behind resolution, because
a standalone that survives the conflict defines its guard constant as it loads and
the load pass has to see that.
Comment thread src/Boot/Scheduler.php
Comment on lines +186 to +191
// The shape of the request first. It reads the request and nothing else, so cron, WP-CLI, a
// POST and every front-end view are turned away having resolved no user and built no
// resolver.
if ( ! $gatekeeper->request_may_resolve() ) {
return;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Realizing this now while reviewing this PR. Commenting here so I can revisit it later:

With the way this is currently written, a Standalone plugin is never deactivated until an Admin request is made.

This could result in an edge case where:

  1. Standalone plugin is active
  2. Sub Plugin for it is included in the Host Plugin in a new update
  3. That update is applied via SFTP/Composer/etc.
  4. An Admin request is never made

In this scenario, the Standalone plugin is always active and the Sub Plugin never loads. Effectively, the Deactivate Conflict Policy falls back to Defer.

While an edge case, if an Admin request is never made and the Host Plugin continues to get updates which update that Sub Plugin, it could result in "missing functionality".

That said, this implementation is safer because the way it is being handled in other repos could have resulted in weird edge cases where a redirect and exit could have broken cron or other similar tasks (temporarily) for that request.

Maybe there's a way to get a best-of-both-worlds. 🤔

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thought this over a bit and it'll be a larger/more complex change. It is better to merge this for now and revisit as its own PR for sure.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants