Unify site provisioning + capability detection into one per-site source#22944
Draft
jkmassel wants to merge 12 commits into
Draft
Unify site provisioning + capability detection into one per-site source#22944jkmassel wants to merge 12 commits into
jkmassel wants to merge 12 commits into
Conversation
Collaborator
Generated by 🚫 Danger |
Contributor
|
|
Contributor
|
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## trunk #22944 +/- ##
==========================================
+ Coverage 37.28% 37.32% +0.03%
==========================================
Files 2330 2331 +1
Lines 125701 125760 +59
Branches 17122 17143 +21
==========================================
+ Hits 46872 46936 +64
+ Misses 75032 75018 -14
- Partials 3797 3806 +9 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
jkmassel
force-pushed
the
jkmassel/editorcapabilitystate-visibility
branch
from
79c6afa to
50b2b88
Compare
jkmassel
changed the title
Contributor
🤖 Build Failure AnalysisThis build has failures. Claude has analyzed them - check the build annotations for details. |
jkmassel
force-pushed
the
jkmassel/editorcapabilitystate-visibility
branch
from
2ad8edf to
85497da
Compare
This was referenced Jun 5, 2026
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.
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.
On a first provision of an Atomic site, the My Site screen showed a false "Unable to connect to your site" banner. `ensureAuth` minted an application password, but `detectCapabilities` re-read the `SiteModel` fresh and a concurrent whole-row site write (`insertOrUpdateSite` uses `UpdateAllExceptId`) had clobbered the just-encrypted credential columns before that read (#22905). With no credentials present, the authenticated direct-host probe was skipped, the unauthenticated discovery failed against the private host, and the probe reported the site unreachable. `ensureAuth` now returns the credentials it obtained in an internal `AuthResult`, and `detectCapabilities` overlays that immutable value onto its own coroutine-local `SiteModel` copy. The stages still read fresh and write only their own column — no `SiteModel` is shared or mutated across the parallel detect/`recoverXmlRpc` branches, so there's no race. Verified on-device against a private Atomic site: the authenticated direct-host probe runs and the banner is gone.
discoverAndVerifyXmlRpcUrl caught only DiscoveryException, so a RuntimeException from the discovery/verify path would escape the async, cancel the whole provisioning coroutine, and reach appScope (no SupervisorJob/handler). Catch CancellationException + generic Exception like the sibling SiteApiRestUrlRecoverer, so any failure degrades to the XML-RPC-disabled card and retries next run.
…riter #22947 made the app-password columns single-writer — excluded from the generic full-row update, written only by updateApplicationPasswordCredentials — so a fresh getSiteByLocalId after a mint reliably carries the credentials and a concurrent site write can't clobber them (#22905). The pipeline no longer needs to thread them as a value: remove ProvisionedCredentials, AuthResult.credentials, and the detectCapabilities overlay. ensureAuth returns a bare SiteAuthState; detectCapabilities and recoverXmlRpcIfNeeded read the credentials off the fresh read.
jkmassel
force-pushed
the
jkmassel/editorcapabilitystate-visibility
branch
from
26b93d5 to
3f20dc4
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.
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+ agetSelectedSite()re-read tried to referee), and mutating its ownSiteModel. Two structural problems get fixed:wpApiRestUrldoesn't survive across launches on Atomic sites #22905) — flows held aSiteModel, mutated a field, and wrote the whole row back, clobbering concurrent changes. Now no model is held across stages; each stage reads fresh and writes only its own column, and Make wpApiRestUrl, xmlRpcUrl, and app-password creds single-writer #22947 (landed) excludes those columns from the generic full-row update so nothing else can clobber them either.SiteProvisioningSourceA
@Singletonexposing a per-siteStateFlow<SiteReadiness>; single-flight perSiteModel.id(which also subsumes the card's old 409-safety guard — two concurrent mints destroy the winner's creds). Each stage takes asiteLocalId, reads theSiteModelfresh viagetSiteByLocalId, and writes back only the column it changed: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. (
detectCapabilitiesitself 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
SiteReadinessNeedsAuth(Provisioning)NeedsAuth(Unprovisionable)UnreachableReady¹ 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)
xmlRpcUrlfresh.ApplicationPasswordViewModelSliceis now a pure renderer — validate/mint and XML-RPC rediscovery moved into the pipeline; its dependency list drops to four.CredentialsChangedNotifier+ its wiring (the event bus andgetSelectedSite()staleness race, Unify editor capability detection into a single per-site state #22942's defect Implementing WordPress.com Notifications Activities #2).source.clear().XML-RPC recovery
SiteXmlRpcUrlRecoverer(new, mirroringSiteApiRestUrlRecoverer) discovers + authenticates a self-hosted site's XML-RPC endpoint and persists it via #22947'sSiteSqlUtils.updateXmlRpcUrl— no FluxC changes here; that writer landed with #22947. Recovery is best-effort: a discovery/verify failure is contained and surfaces as the XML-RPC-disabled card (retried next run), never escaping to cancel the pipeline.Out of scope (follow-up)
A
RELEASE-NOTES.txtentry (26.9) notes the reliability rework.Testing instructions
Unit tests:
./gradlew :WordPress:testJetpackDebugUnitTestacrossSiteProvisioningSourceTest,SiteXmlRpcUrlRecovererTest,ApplicationPasswordViewModelSliceTest,SiteConnectivityBannerViewModelSliceTest,GutenbergEditorPreloaderTest,ApplicationPasswordLoginHelperTest,EditorSettingsRepositoryTest.Private Atomic site (first login — the banner race):
Application-password card (the mint move):
Regression checks:
Sign-out: