Skip to content

Unify site provisioning + capability detection into one per-site source#22944

Draft
jkmassel wants to merge 9 commits into
trunkfrom
jkmassel/editorcapabilitystate-visibility
Draft

Unify site provisioning + capability detection into one per-site source#22944
jkmassel wants to merge 9 commits into
trunkfrom
jkmassel/editorcapabilitystate-visibility

Conversation

@jkmassel

@jkmassel jkmassel commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Description

Refactors getting-a-site-ready into a single source of truth: one per-site, single-flight pipeline that provisions application-password credentials, recovers the REST root, recovers the XML-RPC endpoint, and detects editor capabilities. This is PR 1 of #22942.

ℹ️ #22943 (the release/26.8trunk backmerge that brought #22926's fix) has landed, so this is now based on trunk directly. The core REST fix is already in trunk; this PR's diff is the new architecture plus the removal of #22926's interim coordination layer (CredentialsChangedNotifier).

Provisioning used to be spread across the connectivity banner, the editor preloader, and the application-password card — each triggering its slice of the work, racing the others (a process-global CredentialsChangedNotifier + a getSelectedSite() re-read tried to referee), and mutating its own SiteModel. Two structural problems get fixed:

  1. The first-login race — the capability probe could run before the credential was minted. Now auth is awaited first, and the probe is a downstream stage, so the race is impossible, not mitigated.
  2. Stale-model writes (Recovered wpApiRestUrl doesn't survive across launches on Atomic sites #22905) — flows held a SiteModel, mutated a field, and wrote the whole row back, clobbering concurrent changes. Now no model is held across stages.

SiteProvisioningSource

A @Singleton exposing a per-site StateFlow<SiteReadiness>; single-flight per SiteModel.id (which also subsumes the card's old 409-safety guard — two concurrent mints destroy the winner's creds). Each stage takes a siteLocalId, reads the SiteModel fresh via getSiteByLocalId, and writes back only the column it changed:

ensureAuth(id)                       -> SiteAuthState        (validate / mint; persists creds)
  +- if Provisioned, in parallel:
       |- recoverRestUrl(id) -> detectCapabilities(id)      -> SiteReadiness   (persistApiRootUrl)
       +- recoverXmlRpc(id)                                  (self-hosted)     (persistXmlRpcUrl)

The REST-capability chain and XML-RPC recovery are independent post-auth probes, so they run in parallel — and because each reads fresh and writes only its own column, the two branches can't clobber each other. (detectCapabilities itself still fans out into the route ∥ theme probes.)

Entry points: stateFor (reactive), await (one-shot, preloader), invalidate (PTR / retry), clear (sign-out).

Consumers render slices of one state

SiteReadiness Connectivity banner Application-password card Preloader
NeedsAuth(Provisioning) hidden hidden wait
NeedsAuth(Unprovisionable) hidden auth prompt wait
Unreachable show hidden¹ proceed
Ready hidden hidden¹ use caps

¹ a true self-hosted site whose XML-RPC endpoint the pipeline couldn't recover still gets the XML-RPC-disabled card — the card now just reads the (pipeline-recovered) xmlRpcUrl fresh.

FluxC

Adds SiteSqlUtils.updateXmlRpcUrl(localId, url) — a one-column targeted write mirroring the existing updateWpApiRestUrl (the #22905 pattern) — and a SiteXmlRpcUrlRecoverer mirroring SiteApiRestUrlRecoverer.

Out of scope (follow-up)

A RELEASE-NOTES.txt entry (26.9) notes the reliability rework.

Testing instructions

Unit tests:

  • ./gradlew :WordPress:testJetpackDebugUnitTest + :libs:fluxc:testDebugUnitTest across SiteProvisioningSourceTest, SiteXmlRpcUrlRecovererTest, SiteSqlUtilsTest, ApplicationPasswordViewModelSliceTest, SiteConnectivityBannerViewModelSliceTest, GutenbergEditorPreloaderTest, ApplicationPasswordLoginHelperTest.

Private Atomic site (first login — the banner race):

  1. Sign out, sign into a WP.com account with a private Atomic site that has an application password.
  2. Open My Site, let it settle — do not pull to refresh.
  • The "Unable to connect to your site" banner does not flash; capabilities settle on their own.
  1. Pull to refresh; create a new post.
  • No banner; the editor opens correctly.

Application-password card (the mint move):

  1. A self-hosted site that supports application passwords but has none stored.
  • The "authenticate" card appears; tapping it starts the authorization flow.
  1. A site whose stored application password was revoked server-side.
  • The re-authentication card appears.
  1. A self-hosted site with XML-RPC genuinely disabled.
  • The XML-RPC disabled card appears (and stays hidden where XML-RPC is reachable — the pipeline recovers it).

Regression checks:

  1. Public Atomic site, then a WP.com Simple site — open My Site, pull to refresh.
  • No banner; capability detection still completes.

Sign-out:

  1. Sign out, then sign back in.
  • No crash; provisioning + detection run fresh.

@dangermattic

Copy link
Copy Markdown
Collaborator
1 Warning
⚠️ This PR is larger than 300 lines of changes. Please consider splitting it into smaller PRs for easier and faster reviews.
1 Message
📖 This PR is still a Draft: some checks will be skipped.

Generated by 🚫 Danger

@wpmobilebot

wpmobilebot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

App Icon📲 You can test the changes from this Pull Request in WordPress Android by scanning the QR code below to install the corresponding build.

App NameWordPress Android
Build TypeDebug
Versionpr22944-26b93d5
Build Number1493
Application IDorg.wordpress.android.prealpha
Commit26b93d5
Installation URL0gn9ngqc5ctug
Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

@wpmobilebot

wpmobilebot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

App Icon📲 You can test the changes from this Pull Request in Jetpack Android by scanning the QR code below to install the corresponding build.

App NameJetpack Android
Build TypeDebug
Versionpr22944-26b93d5
Build Number1493
Application IDcom.jetpack.android.prealpha
Commit26b93d5
Installation URL6okk6il2dv0po
Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

@codecov

codecov Bot commented Jun 4, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 82.60870% with 28 lines in your changes missing coverage. Please review.
✅ Project coverage is 37.36%. Comparing base (d973e15) to head (26b93d5).

Files with missing lines Patch % Lines
...ess/android/repositories/SiteProvisioningSource.kt 87.35% 0 Missing and 11 partials ⚠️
...ationpassword/ApplicationPasswordViewModelSlice.kt 54.54% 3 Missing and 7 partials ⚠️
.../main/java/org/wordpress/android/AppInitializer.kt 0.00% 2 Missing ⚠️
...nnectivity/SiteConnectivityBannerViewModelSlice.kt 80.00% 0 Missing and 2 partials ⚠️
...press/android/ui/posts/GutenbergEditorPreloader.kt 71.42% 1 Missing and 1 partial ⚠️
...s/android/repositories/EditorSettingsRepository.kt 0.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##            trunk   #22944      +/-   ##
==========================================
+ Coverage   37.33%   37.36%   +0.03%     
==========================================
  Files        2321     2322       +1     
  Lines      124757   124814      +57     
  Branches    16961    16982      +21     
==========================================
+ Hits        46576    46638      +62     
+ Misses      74417    74403      -14     
- Partials     3764     3773       +9     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@jkmassel jkmassel force-pushed the jkmassel/editorcapabilitystate-visibility branch from 79c6afa to 50b2b88 Compare June 4, 2026 18:21
@jkmassel jkmassel changed the base branch from trunk to merge/release-26.8-into-trunk June 4, 2026 18:21
@jkmassel jkmassel changed the title Unify editor capability detection into a per-site detector Unify site provisioning + capability detection into one per-site source Jun 4, 2026
@wpmobilebot

wpmobilebot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

🤖 Build Failure Analysis

This build has failures. Claude has analyzed them - check the build annotations for details.

Base automatically changed from merge/release-26.8-into-trunk to trunk June 4, 2026 20:22
jkmassel added 6 commits June 4, 2026 14:29
Introduces EditorCapabilityDetector — one app-scoped owner of editor REST
capability state, exposed as a per-site StateFlow — so the connectivity
banner and editor preloader share a single, deduplicated probe instead of
each re-deriving the state and racing the same async preconditions. Folds
in the authenticated direct-host probe fallback so private Atomic sites
detect correctly on trunk. Part of #22942.
#22926's first-login hardening signalled credential establishment through a
process-global CredentialsChangedNotifier that the banner collected against a
re-read of getSelectedSite() — the staleness race called out in #22942.
EditorCapabilityDetector.refresh(storedSite), called from the mint path on the
exact mutated SiteModel, replaces that coordination, so drop the notifier and
its wiring in ApplicationPasswordLoginHelper.
…Source

Promotes the EditorCapabilityDetector into SiteProvisioningSource: one
per-site, single-flight pipeline that ensures application-password
credentials, recovers the REST root, and detects editor capabilities —
each stage awaited before the next. Because the capability probe is now
structurally downstream of credential provisioning, it can never run
before the mint, so the first-login race is gone by construction rather
than mitigated. The application-password card, connectivity banner, and
editor preloader all render slices of the one SiteReadiness state.

The mint/validate mechanics move out of ApplicationPasswordViewModelSlice
(now a renderer) into the source's ensureAuth stage; the per-site
single-flight subsumes the card's old 409-safety guard. The duplicate
wpApiRestUrl heal collapses into the source's recoverRestUrl stage.
Removes the threaded, mutated SiteModel from the provisioning pipeline: each
stage now reads the site fresh by local id and writes back only the column it
changed (persistApiRootUrl / a new persistXmlRpcUrl), so the parallel branches
can't clobber one another and there's no stale-model write (#22905).

With writes targeted, XML-RPC endpoint recovery moves out of the
application-password card into the pipeline as a parallel branch off ensureAuth
-- independent of the REST capability probe, since it needs only the
credentials. The card becomes a pure renderer that reads the recovered
xmlRpcUrl fresh. Adds SiteSqlUtils.updateXmlRpcUrl and a SiteXmlRpcUrlRecoverer
mirroring SiteApiRestUrlRecoverer.
- @Suppress("ReturnCount") on ensureAuth — its five returns are each a distinct
  auth outcome; a single-return rewrite would read worse.
- Drop a stray blank line before a brace left from removing a test region.
@jkmassel jkmassel force-pushed the jkmassel/editorcapabilitystate-visibility branch from 2ad8edf to 85497da Compare June 4, 2026 20:31
jkmassel added 3 commits June 4, 2026 14:41
The IfNeeded suffix makes the short-circuit (skip when the field is already
present / not applicable) clear at the call site.
WP.com Simple sites are proxy-served and OAuth-authed; the application-password
mint returns NotSupported for them, so the pipeline was returning Unprovisionable
and never reaching capability detection (which works fine through the proxy) -- a
regression vs. the old ungated probe. ensureAuth now short-circuits them to a new
SiteAuthState.NotApplicable (treated like Provisioned), and recoverRestUrlIfNeeded
skips them too.
The route-support probe only sent Atomic sites to the direct host; Jetpack
WPCom-REST sites fell through to the WP.com proxy. Since the proxy and the
direct host advertise different route lists (the #22879 premise), Jetpack sites
got the wrong answer. Broaden the predicate to isUsingWpComRestApi &&
!isWPComSimpleSite (Atomic + Jetpack); the proxy is only for minting the
application password.
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.

3 participants