Conversation
github-actions
Bot
added
the
[Type] Breaking Change
XCFramework BuildThis PR's XCFramework is available for testing. Add the following to your .package(url: "https://github.com/wordpress-mobile/GutenbergKit", branch: "pr-build/747")Built from 1ffc94a |
dcalhoun
reviewed
Sep 30, 2026
dcalhoun
left a comment
dcalhoun
left a comment
Member
There was a problem hiding this comment.
Capturing findings from review with Claude. I haven't validated each closely with this quick review, but wanted to note the details here for future review.
The most notable are the two related to the stand-in file and the query encoding. One of the stand-in file findings breaks VideoPress uploads entirely. I reproduced it with Simple and WoW sites where Jetpack extends the core/video block with VideoPress-powered uploads.
| const files = await Promise.all( | ||
| items.map( async ( media ) => { | ||
| try { | ||
| if ( media.nativeUpload ) { |
Member
There was a problem hiding this comment.
Finding from Claude:
Stand-ins can reach plugin files transforms that never call POST /wp/v2/media. With Jetpack's resumable VideoPress uploads enabled, Jetpack replaces core/video's transform (priority 9) with one that puts the File in fileForImmediateUpload and uploads it over tus, so VideoPress receives the stand-in.
Reproduced on a WordPress.com site with VideoPress by inserting a 30 MB video through the native inserter: the tus upload was 566,969 bytes (preview + marker), the block reported "Upload Complete!", and WP Admin shows a 554 KB video/mp4 with "Conversion failed." The real file never leaves the device. Trunk fetched the real bytes here, so this is a regression.
Screenshots
| Missing poster | Failed upload |
|---|---|
 |
 |
Fix options:
- For stand-ins, skip plugin
filestransforms: create the core block and callmediaUploaddirectly, so the stand-in always reaches the middleware. Jetpack's own non-resumable branch does exactly this on VideoPress sites (extensions/blocks/videopress/editor.jsx), and its edit component reads the VideoPress guid from the attachment afterward — worth confirming on a site. Costs Jetpack's title/poster uploader UI for native-inserter picks. - Skip stand-ins for video when
window.videoPressResumableEnabledis set, fetching the real file as before. Narrow, and keeps the old memory ceiling for large videos on those sites. - Build real
Files whenever the winning transform isn't core's. Most general, but hard to detect: Jetpack replaces core/video's transform under the same block name.
Option 1 seems the most robust.
Comment on lines
+46
to
+51
| // `query` is the raw request query in wire form (leading "?"). Set it via | ||
| // `percentEncodedQuery` so a value that isn't URL-safe can't make | ||
| // `URL(string:)` return nil and silently drop the query. | ||
| var components = URLComponents(url: base, resolvingAgainstBaseURL: false) | ||
| components?.percentEncodedQuery = String(query.dropFirst()) | ||
| return components?.url ?? base |
Member
There was a problem hiding this comment.
Finding from Claude:
The percentEncodedQuery setter traps on invalid characters rather than returning nil ("Attempting to set percentEncodedQuery with invalid characters"). query arrives raw from the page (requestQuery in api-fetch.js takes a substring of the apiFetch path), so apiFetch( { path: '/wp/v2/media/5?title=café', method: 'DELETE' } ) crashes the app, as do spaces, #, ", and <>. Core's own queries are clean, so it takes plugin code.
URLComponents(string:) percent-encodes invalid characters instead and keeps valid escapes (verified with café, a b, a[]=1, a%zz, a%20b):
Suggested change
| // `query` is the raw request query in wire form (leading "?"). Set it via | |
| // `percentEncodedQuery` so a value that isn't URL-safe can't make | |
| // `URL(string:)` return nil and silently drop the query. | |
| var components = URLComponents(url: base, resolvingAgainstBaseURL: false) | |
| components?.percentEncodedQuery = String(query.dropFirst()) | |
| return components?.url ?? base | |
| // `query` is the raw request query in wire form (leading "?"). `URLComponents(string:)` | |
| // percent-encodes characters the `percentEncodedQuery` setter would trap on. | |
| guard var components = URLComponents(url: base, resolvingAgainstBaseURL: false), | |
| let parsed = URLComponents(string: query) else { return base } | |
| components.percentEncodedQuery = parsed.percentEncodedQuery | |
| return components.url ?? base |
Comment on lines
+41
to
+43
| const parts = preview ? [ preview ] : []; | ||
| parts.push( `${ MARKER }${ sessionId }` ); | ||
| return new File( parts, filename, { type } ); |
Member
There was a problem hiding this comment.
Finding from Claude:
For video, the stand-in is JPEG bytes typed video/*, so anything that plays the File fails to decode while the upload runs. On device, Jetpack's poster picker shows "No Poster Selected"; in a WebKit probe, core/video's <video src=blob:…> fails with MEDIA_ERR_SRC_NOT_SUPPORTED, leaving a blank player under the spinner. The native preview frame is never shown. Could it serve as a temporary poster, or is the trade-off worth documenting?
| data = nil | ||
| } | ||
| guard let data else { return nil } | ||
| let previewURL = fileURL.deletingLastPathComponent().appending(component: ".preview.jpg") |
Member
There was a problem hiding this comment.
Finding from Claude:
adopt() gives each file its own directory, but writeData (the data fallback in import) still writes flat into Uploads/. Two fallback items in one selection then share Uploads/.preview.jpg, so both blocks show the last one while uploading. That file is also never cleaned up, since cleanupOldFiles skips hidden files. Naming the preview after its file avoids both:
Suggested change
| let previewURL = fileURL.deletingLastPathComponent().appending(component: ".preview.jpg") | |
| let previewName = fileURL.deletingPathExtension().lastPathComponent + ".preview.jpg" | |
| let previewURL = fileURL.deletingLastPathComponent().appending(component: previewName) |
| if expired { | ||
| released.signal() | ||
| } else { | ||
| released.wait() |
Member
There was a problem hiding this comment.
Finding from Claude:
Each finish parks a thread here for its whole upload. In a simulator probe, ~70 concurrent activities stalled DispatchQueue.global() work, and activities past that never started, so those uploads got no background time. Core starts a batch's uploads at once, so a large multi-select can get there. Could one shared activity, held while any upload is in flight, replace the per-upload one?
Comment on lines
+131
to
+132
| let id = try await store.begin(filename: body.filename, mimeType: body.mimeType, expectedSize: body.size) | ||
| return .json(201, ["id": id]) |
Member
There was a problem hiding this comment.
Finding from Claude:
disable() runs removeAll() in an unstructured Task, so a begin already waiting on the store can land after it (reproduced in a probe). The session and an empty staging file come back, and every later request 503s at the service guard, so nothing discards them until the handler is freed. Re-checking after the awaits closes it:
Suggested change
| let id = try await store.begin(filename: body.filename, mimeType: body.mimeType, expectedSize: body.size) | |
| return .json(201, ["id": id]) | |
| let id = try await store.begin(filename: body.filename, mimeType: body.mimeType, expectedSize: body.size) | |
| // `disable()` may have run while this waited on the store. | |
| guard isEnabled else { | |
| await store.discard(id) | |
| return .error(503, code: "native_upload_unavailable", message: "Native media uploads are not available in this editor.") | |
| } | |
| return .json(201, ["id": id]) |
| * | ||
| * @return {Object} Request details object. | ||
| */ | ||
| /** |
Member
There was a problem hiding this comment.
Finding from Claude:
isNetworkURL landed between extractRequestDetails's JSDoc and the function, so isNetworkURL has two doc blocks and extractRequestDetails has none. It's also defined before extractRequestDetails but called after it (AGENTS.md orders helpers by first call). Moving isNetworkURL and its JSDoc below extractRequestDetails fixes both.
Comment on lines
+75
to
+76
| - **Disabled.** After `stopMediaHandling()` every request gets a `503`, which the page | ||
| takes as the cue to fall back. |
Member
There was a problem hiding this comment.
Finding from Claude:
Only uploads that haven't reached finish fall back. A finish (including every native inserter upload) or a delete fails instead.
Suggested change
| - **Disabled.** After `stopMediaHandling()` every request gets a `503`, which the page | |
| takes as the cue to fall back. | |
| - **Disabled.** After `stopMediaHandling()` every request gets a `503`. An upload that | |
| hasn't reached `finish` falls back through the web view; a `finish` or delete fails. |
An error occurred while trying to automatically change base from
jkmassel/dependency-fetch-cancelled
to
test/media-mock-cleanup
October 1, 2026 22:43
This was referenced Oct 2, 2026
…701) * fix(ios): free editors mid-fetch, and share site requests in flight Follow-ups to keeping the fetch running, from reviewing #651: - The async dependency fetch no longer holds its editor. - A cancelled asset bundle build is never published. - Every cache for a site shares one SQLite store. - Identical requests and bundle builds in flight are shared. The fetch held its editor for as long as it ran: `await self?.prepareEditor()` optional-chains a weak `self` into an async call, which holds a strong `self` across every suspension inside it. A host that released the editor mid-fetch didn't free it until the fetch ended, and in between the full load tail — bundle provider, upload server bind, `loadFileURL` — still ran on a controller nobody held. The fetch now belongs to an `EditorDependencyLoader`, and the editor never awaits it. The editor owns the loader; the loader reaches back only through a `weak let delegate` whose requirements are all synchronous, so nothing it calls can suspend while holding the editor. A released editor is freed at once and nothing runs on it, while the fetch, still never cancelled, runs on and warms the cache for the next editor. The task starts from `fetch(from:)` rather than `init`, where a bare `delegate` would resolve to the strong parameter instead of the weak property. `prepareEditor()` goes away: the async flow is now "fetch, then the fast path", through `startLoadingEditor(dependencies:)`, which also takes over the #357 note about cancelling mid-`startUploadServer()`. The progress view now fades out as the load starts, rather than after `loadEditor` returns. `EditorAssetLibrary.buildBundle` published bundles from a cancelled build. Its task group swallows every per-asset failure, cancellation included, so a cancelled build reached `bundle.copy(to:)` with assets missing — and `readAssetBundles()` reads only the manifest, so every later launch served the gap. It now checks for cancellation before publishing. WordPress-iOS's `EditorDependencyManager._invalidate` can reach this today: it cancels an in-flight prefetch and purges without waiting for the task to finish. Every `EditorService` builds its own `EditorURLCache`, and each opened its own `SQLiteKVCache` on the site's `editorurlcache.sqlite` — which the store documents as undefined behavior, and measured, it is worse than contention. `connection()` opens lazily and caches the result, failure included, for the life of the instance, and nothing set a busy timeout. Two caches making their first read at the same moment left at least one of them broken in 50 runs out of 50, every later read and store throwing `databaseUnavailable`. Opened one after the other and then written concurrently, 189 of 400 writes still failed; through one instance, none did. That is the shape of WordPress-iOS's launch — `warmUpEditor(for:)` starts the warmup editor's fetch and the prefetch together, each with its own service — and a broken cache fails `prepare()` outright, since a read error is not a network error. Not reproduced in WordPress-iOS itself. `SQLiteKVCache.shared(handle:directory:diskCapacity:)` now hands every caller the live instance for its file, held weakly so a file no one is using is closed as before, and asserts that callers sharing a file ask for the same capacity. Being weak, it can hand out a fresh instance while the last one's `deinit` is still checkpointing the WAL, so the store now also sets a 5s busy timeout: reopening in that window failed 200 times in 200 without it, and never with it. The timeout doesn't replace `shared`. With it set, two instances opening at once still break one, because the switch to WAL returns `SQLITE_BUSY` without waiting on it. Nothing site-level was shared while in flight, so an editor opened mid-prefetch repeated the prefetch's requests and its bundle build, splitting the bandwidth the prefetch needed. Sharing now happens at the level of what goes over the wire and what lands on disk, which needs no analysis of the editor configuration: - `EditorHTTPClient.perform(_:)` joins an identical request already in flight. The key is the request as configured — URL, method, and headers, auth included — plus the session, and the timeout and network service type, which `URLRequest`'s own `==` ignores (measured). Only safe requests without a body are shared, and only from clients no delegate is watching. The table is process-wide, and since `EditorHTTPClient` is public, that includes a host's own GETs. - `EditorAssetLibrary.buildBundle(for:)` joins a build in flight for the same directory: storage root and manifest checksum. That holds whatever client either library has. The build downloads over the client of the library that started it, but the bundle is shared by site once it's on disk anyway, and builds kept apart by client race into the same directory through `copy(to:)`, where one can fail. Both go through `InFlightTasks`: cancelling a caller ends only that caller's wait, and shared work stops once no caller is left waiting on it. A caller that joins raises the work to its own priority, since waiting on a continuation doesn't escalate it the way awaiting `task.value` would; that needs iOS 26 or macOS 26. An editor opened mid-prefetch now joins the settings, theme, site settings, post types, and bundle build already in flight. It still fetches its own post, and the `editor-assets` manifest: that isn't cached, and the prefetch's request for it has usually finished by the time its build is running. The shared store is what makes this safe: a shared response reaches every waiter at the same instant, and each writes it through its own `EditorURLCache`. A shared task reports progress to its waiters one at a time, and a waiter can leave while an earlier one's callback is suspended. So `InFlightTasks` checks each waiter is still waiting just before its turn, and `EditorService.incrementProgress` drops progress that arrives after its `prepare()` has cleared it, rather than trapping on a precondition. Neither is enough alone: a call already under way when its caller leaves can't be recalled. With only the old precondition, a shared build reporting to a service whose `prepare()` had given up trapped, reproduced with two services sharing a build. The guard also fixes an older trap: two overlapping `prepare()` calls on one service, where the first to finish clears progress the second is still reporting. `theInFlightFetchKeepsTheEditorAlive` flips to `releasingTheEditorMidFetchFreesIt`: against the previous commit the editor is still alive after 2s; it now passes in 0.36s. Each of these fails against the code it pins: `buildBundlePublishesNothingWhenCancelled` with the cancelled bundle on disk, both new `EditorURLCacheTests` with one store per cache, `sharedReopensAFileWhileItCloses` without the busy timeout, `aCallerThatHasLeftHearsNoMoreProgress` without the re-check, `overlappingPrepareCallsDontTrap` without the guard, and `aHigherPriorityCallerRaisesTheTask` without the escalation. Mutation-tested too: a loader holding its delegate across the `await`, never sharing requests, and keying builds per library rather than per directory are each caught. `ParkedURLSession` moves to `Helpers/` so these suites can share it. * fix(ios): don't share a failed cache store, or the request for a post Follow-ups from review of the sharing this branch introduces. `SQLiteKVCache.shared` went on handing out an instance that could no longer work, to every caller for as long as anything held it: - One whose open failed. An instance keeps that failure for life, so a passing fault — a full disk, an I/O error — failed every later `prepare()` for the site, where each service used to get its own attempt. A failed instance now takes itself out of the registry. It has closed its handle, so the next caller's instance has the file to itself. `shared` doesn't ask the instance whether it failed: that would wait on `openLock`, held for the whole open and so for as long as the busy timeout, on the main thread where an editor builds its service. - One whose file `EditorViewController.deleteAllData()` had deleted. Both `get` and `put` through it throw "disk I/O error (code 10)", so the next editor failed to load rather than starting from an empty cache. `deleteAllData()` now goes through `EditorURLCache.deleteAll(in:)`, which stops sharing every store under the directory it removes. The stale instance closing later leaves the new file alone: 20 entries of 20 written beside it survived. `RESTAPIRepository.fetchPost` went through the shared `perform(_:)`, so an editor reopened on a post joined the GET an editor since closed still had in flight for it — a response that can predate an edit made in between. The post is deliberately never cached, and is now never shared either: `perform(_:)` sends a request alone when its cache policy asks to skip the cache, and the post request asks. A host's own requests through `EditorHTTPClient` can opt out the same way. `buildBundlePublishesNothingWhenCancelled` checked the disk as soon as its caller's wait ended, and `InFlightTasks` ends that wait before it cancels the build. With `try Task.checkCancellation()` removed the test still passed in 100 runs of 100 run one at a time, the bundle landing on disk moments later. It now waits for the abandoned build through a `task(for:)` test hook, and fails against that mutant in 20 runs of 20. `deliversTheError` gains the `defer` that `ParkedURLSession.release()` asks of every test. Each new test fails without its fix: a failed instance left in the registry, `deleteAll` not forgetting its stores, the forgotten prefix matching a sibling directory, the client ignoring the cache policy, and the post request keeping the default one.
`EditorService`'s cache policy was documented to cover asset manifests, but `prepareAssetBundle` returned the newest bundle on disk before the policy was ever consulted. `.maxAge` and `.ignore` refreshed API responses and never assets, so the only way to pick up a plugin or theme change was `purge()`, which forces a cold load. The policy now decides when to check the site's manifest again. An unchanged manifest keeps the bundle already on disk and resets its age rather than downloading every asset again: asset URLs carry their version, so the same manifest means the same assets. A changed manifest builds the new bundle beside the old one, which `cleanup()` removes later. `.always`, the default and what WordPress-iOS uses, behaves as before. The check for an existing bundle runs inside the shared build for its directory, so marking a bundle current doesn't race a build of the same bundle replacing it. `fetchManifest()` no longer consults the policy: once the manifest is fetched, a bundle with the same checksum was built from exactly that manifest, so reusing its parsed copy is always right. `downloadAssetBundle(cachePolicy:progress:)` never used `cachePolicy`. It's now a deprecated overload of `downloadAssetBundle(progress:)`.
jkmassel
changed the base branch from
jkmassel/dependency-fetch-cancelled
to
jkmassel/asset-bundle-cache-policy
October 2, 2026 03:29
jkmassel
force-pushed
the
jkmassel/scheme-upload-transport
branch
from
October 2, 2026 03:29
211b9b6 to
1f732ac
Compare
`JSON.description` handed the enum itself to `JSONSerialization`, which takes Foundation objects and raises an Objective-C exception for anything else. Swift can't catch one, so describing a `JSON` — or anything holding one, like an `EditorSettings` or `EditorDependencies` — ended the process. Nothing in the library describes one, but a log message, a `po` in the debugger, or a failing test expectation does. It now encodes with `JSONEncoder`, which the type already supports, and writes a number JSON can't represent as a string rather than failing.
A refresh could return a bundle that was no longer on disk. The check for an existing bundle and the write marking it current were separated by a progress report, and a `cleanup()` or `purge()` landing in between left `markCurrent` recreating the directory with only `manifest.json`. The editor then opened without plugin or theme assets, and said nothing. The bundle is now looked for again before it's marked, under a lock shared with `cleanup()` and `purge()`, and built again if it's gone. `cleanup()` keeps every bundle handed out since launch. A refresh makes a second bundle on disk routine, and dependencies a host prepared earlier, or an open editor, may still be reading the first. Those go in a cleanup after the next launch. A check now stamps `lastCheckedDate` in the bundle's manifest rather than resetting `downloadDate`, and bundle equality ignores it, so a check leaves a bundle equal to the copy a host already holds. Bundles are ordered, and aged for the cache policy, by when the manifest last matched them: the last check, or the download for a bundle never checked. `readAssetBundles()` also built each bundle's path from the directory listing, which resolves symlinks. On macOS, where the tests run, it returned a bundle under `/private/var` where a build returned the same bundle under `/var`, and the two compared unequal. It now builds the path from the storage root, as a build does. With automatic network fallback, a `.maxAge` or `.ignore` service that can't reach the site returns what's on disk, however old, before it falls back to empty dependencies. The documented refresh recipe hands its result to the next editor, which would otherwise have opened with nothing while everything it needed was on disk. Also: - A check downloads the assets an earlier build of the bundle failed to, rather than marking a bundle with gaps as fresh. - The manifest request for a check asks for a fresh answer, so neither a stored response nor a request in flight can stand in for it. `.always` still shares a request in flight. - `readLatestAssetBundle()` is public, since it's what applies the library's cache policy. - Automatic cleanup takes its daily turn per site. One key covered every site, so whichever was prepared first each day used it. - Progress no longer passes its total. The bundle download reports a running total, which was added in full on every report. Each fix has a test that fails without it.
`uploadClient()` dropped the REST timeout but left uploads on `URLRequest`'s 60s default. That timer is an inactivity timer, and an upload goes silent after its body is sent while WordPress generates image sub-sizes. Against a WordPress whose response was held back 90s, the upload failed with `NSURLErrorTimedOut` while the attachment it had created stayed on the site: an orphan, and a duplicate once the user retries. The upload client now raises every request's timeout to at least `uploadInactivityTimeout` (600s) and keeps a longer one a request already asks for.
iOS routed the page's media uploads through a loopback HTTP server. iOS reclaims a suspended app's sockets once the device can idle-sleep, and the listener keeps reporting `.ready` on a port nothing answers, so every upload after an ordinary screen lock failed until the editor was reopened. The page now uploads over `gbk-upload:`, a `WKURLSchemeHandler` in the editor's own web view. It has no socket to reclaim, no port or token to advertise, and no connection limit. WebKit hands a scheme handler only bodies it has buffered, and it drops a `Blob` or `FormData` body without an error. So the page sends the file in 4 MB `ArrayBuffer` chunks to a session, then asks native code to finish it with the upload's fields and query. Native code reassembles the chunks on disk and delivers the file through the host's processor and uploader, or `InternalMediaClient`. WordPress's response comes back verbatim, including `x-wp-upload-attachment-id`, which every response exposes under CORS so core's post-process retry still works. - A failure before `finish` falls back to the web view's own upload: WordPress never saw the file, so nothing can be duplicated. - Once `stopMediaHandling()` runs, the handler answers 503 and the page uploads directly. - A task WebKit stops is never answered (answering one raises), and its upload to WordPress is cancelled. - A background-task assertion covers each upload's trip to WordPress. Android keeps its loopback server. The middleware picks a transport from what `GBKit` advertises: `nativeUploadScheme` on iOS, `nativeUploadPort`/`nativeUploadToken` on Android. The processor, uploader and client routing moves out of the deleted `MediaUploadServer` into `MediaUploadService`, which the scheme handler and the native inserter share. `InternalMediaClient` now takes form fields instead of multipart parts, and its passthrough path is gone: the page no longer sends a multipart body to pass through.
With uploads on `gbk-upload:`, nothing in GutenbergKit uses `GutenbergKitHTTP` any more. It was a hand-written HTTP/1.1 and multipart parser listening on a port every process on the device could reach, so it is removed rather than kept around unused: the library and its public product, its tests, `GutenbergKitDebugServer`, and the demo app's Media Proxy Server screen, which only exercised it. The shared `test-fixtures/http` corpus stays; Android's parser tests still read it.
…task `MediaFileSchemeHandler` answered each request with `Data(contentsOf:)`, sent in one `didReceive`. For a 1.1 GB video that put the whole file in app memory twice (1.1 GB after the read, 2.2 GB after `didReceive`), and WebKit's page process was killed at its 2,048 MB limit before `blob()` resolved. The handler also ignored `webView(_:stop:)`, and answering a task WebKit has stopped raises `NSInternalInconsistencyException`. It now streams the file in 1 MB chunks read off the main actor. It holds every request it is serving strongly, and delivers nothing to one WebKit has stopped. Responses carry the file's `Content-Type` and `Content-Length`. A URL whose path resolves outside the media directory is refused rather than read.
… the page The native inserter loaded each picked photo or video into memory (`loadTransferable(type: Data.self)`), wrote it to disk, served it back to the page as one response, and let the page upload it: three full copies in memory. A 1.1 GB video killed WebKit's page process at its 2,048 MB limit before the upload could start. Now: - Photos hands the item over as a file, which is copied into the uploads directory. On APFS that is a clone, so an import of any size costs neither memory nor disk. The file keeps its own name, so the attachment WordPress creates is named after it. Camera videos are copied the same way, and get a real MIME type instead of `video/MOV`. - When the editor can upload natively, it registers the file with the upload scheme and gives the page a small JPEG preview and the session ID instead of the file. - The page builds a stand-in `File`: the preview plus a marker naming the session, in the bytes because core re-creates the `File` when it builds `FormData`. The block uploads through Gutenberg's own pipeline (placeholder, save lock, notices, post-process recovery), and the middleware hands the stand-in's upload to native code, which uploads the real file. A stand-in is never uploaded through the web view. - The page checks the real size against the site's upload limit itself, with core's message: the stand-in is small, so core's check would pass it. Without native media handling, inserter media still reaches the page as before and the page uploads it.
With network logging on, the fetch interceptor reported every request, including those to `gbk-upload:` and `gbk-media-file:`. Native code serves those, not the network, and a native upload sends a request per 4 MB chunk — hundreds for a large video — while its real request to WordPress is already logged natively. The interceptor now passes non-HTTP(S) requests straight through.
Adds `docs/code/media-uploads.md`: the two upload sources, the iOS `gbk-upload:` scheme and why it chunks (with the body types WebKit drops, as measured), the native inserter's stand-ins, recovery and fallback rules, background and timeout limits, and where the tests live. `integration.md` gains a section on how uploads reach native code and what survives the app backgrounding. It also scopes the loopback server and its cleartext requirement to Android, and drops the remaining references to an iOS upload server from doc comments.
`EditorAssetLibrary` built the manifest URL with `siteApiRoot.appending(path: "/wpcom/v2/editor-assets")`. When the API root ends in a slash, Foundation on iOS 17 and 18 keeps both slashes (`…/wp-json//wpcom/v2/editor-assets`), and WordPress answers that with `404 rest_no_route`, so the editor fails to load on any site with plugin assets enabled. Newer Foundation collapses the slash, which is why this only shows on older iOS. Measured on the iOS 17.5 simulator and an iPhone 14 Pro on iOS 18.6.2. The manifest URL now goes through `appending(rawPath:)`, the helper every other REST URL already uses. The new test asserts the requested URL for an API root with and without a trailing slash. It can only fail on an iOS 17 or 18 runtime: the host's Foundation never produced the double slash.
`transformOEmbedApiResponse` caught every failed oEmbed request and resolved with a made-up response holding a link to the URL. Core then stored that as a finished preview. A block that polls for its preview never saw the failure. After a VideoPress upload, the first `/oembed/1.0/proxy` request returns `404` until WordPress.com can embed the new video. The block took the link for a preview, stopped asking, and rendered it in its player sandbox: an empty box, with the link saved into the block's `cacheHtml`. The middleware now leaves a failed request to reject, so core stores `false`, as it does everywhere else. The core Embed block is unaffected: it treats `false` and a link fallback the same way and shows "could not be embedded" for both. A link fallback that WordPress itself returns still passes through, and the wrapper stripping for YouTube, Vimeo, Dailymotion and TED embeds is unchanged.
The native inserter gave the page a stand-in for each picked photo or video — a JPEG preview with a marker — and finished the upload natively when core's `mediaUpload` sent it. A block that uploads on its own never goes through `mediaUpload`. VideoPress took the stand-in for the video and uploaded it: 537,895 bytes of preview, stored as a zero-length video. The page now gets a real `File`. It clicks a hidden file input (`requestNativeFiles`), and `NativeFileInput` answers the open panel WebKit would otherwise show with the files the inserter imported. That is what the system picker gives the page: a `File` WebKit reads from disk as it is sliced. An inserter pick is then an Upload-button pick, for core blocks and for any block's own uploader. Measured on an iPhone 14 Pro (iOS 18.6.2, Lockdown off): the page read a 1,131,169,717-byte video through 4 MB slices in 1.05 s with matching SHA-256s, and the VideoPress block uploaded a 70 MB video handed over this way. - WebKit asks its UI delegate for the panel from iOS 18.4. Before that the inserter hides the photo library and the camera, and media is added from a block's own upload button. - A UI delegate that implements the panel answers every file input, so `NativeFileInput` is the web view's UI delegate only for the insertion, and puts the host's delegate back. - The click needs the user activation the native script call carries, so the page asks for the files before its first `await`, and native code now waits for `insertMedia` to finish. - If the files don't arrive, the page fetches them from `gbk-media-file:` as it did before. - WebKit copies every file a file input receives into `tmp/WKFileUploadPanel-…` and never deletes it. `MediaFileManager` now removes the ones older than two days.
Nothing creates a stand-in now that the native inserter hands the page real files, so the machinery behind it goes: - `native-upload-reference.js`: the stand-in and its marker. - The middleware's branch that finished a session a stand-in named. - `MediaUploadSchemeHandler.register(_:)` and the store's registered sessions. Every session is a staging copy the store wrote, so `Finished.isStagingCopy` and `Failure.notReceiving` go too. - `MediaPreview`, which made the stand-in's JPEG. A block previews the file itself from a blob URL.
Under Lockdown Mode the editor's `file://` page loses its CORS exemption, and a site that doesn't answer its `Origin: file://` with CORS headers fails every request with `TypeError: Load failed`. Seen on an iPhone 15 Pro (iOS 27, Lockdown on) against a self-hosted Jetpack site: eleven requests failed in the editor's first ten seconds. The page's `fetch` is now wrapped (`fetch-relay.js`) so a request for the site's REST API goes to `gbk-rest://relay/proxy/<path>`. `RestRelaySchemeHandler` serves that scheme in the editor's own web view, and `RestRelay` sends the request to the site with the configured credential and answers with CORS headers the web view accepts. Every request takes this path, Lockdown Mode or not, so there is one path to keep working. This is #611's relay on a scheme handler in place of the loopback server, which this branch removed. Its rules carry over, with their tests: the page supplies a path that native code resolves against the API root, dot segments are refused, redirects out of the root are refused, the site credential is injected natively, and only the configured site is relayed. What the scheme changes: - No port and no token: only this web view can load the scheme. - WebKit drops a `Blob` body, and a `FormData` holding one, on the way to a scheme handler, without an error. The wrapper reads those into an `ArrayBuffer` first. That is how a block's own uploader reaches the site: VideoPress sends 5 MB `Blob` chunks. - A scheme request's body is in memory, so the redirect guard has no stream to reopen. - `Content-Length` is stripped from the response along with `Content-Encoding`: the upstream length is the compressed one. Not carried over: #611's wp-env integration tests and its rename of the network-logging wrapper. The relay is installed ahead of the existing logger, which wraps it and records the request the editor made.
… adds The base now lints with ESLint 10 and `@wordpress/eslint-plugin` 27, which reject dependency-group comment blocks, a blank line between import groups, and the JSDoc types `Function`, `*` and `any`. The code this branch adds was written before that, and failed `make lint-web` with 12 errors and 7 warnings. Nothing changes at runtime: the comment blocks are removed, two imports in `api-fetch-upload-scheme.test.js` move above the mocks, and seven JSDoc types are named.
jkmassel
force-pushed
the
jkmassel/scheme-upload-transport
branch
from
October 2, 2026 04:28
1f732ac to
36d68ef
Compare
…longer The iOS Simulator job failed a few tests on timeouts and then made no progress for 46 minutes. A sample of the test process on the CI machine showed the main thread inside `MediaFileSchemeHandlerTests.streamsFiles`, computing a collection difference: when an `==` between two collections fails, Swift Testing works out the difference between them to describe it. The handler had delivered one of the file's two 1 MiB chunks when the test's five-second wait ran out, so the difference was between 1 MiB and 2 MiB of bytes, on the main actor, with every other main-actor test waiting behind it. Payload checks now go through `Data.hasSameBytes(as:)`, which compares without leaving Swift Testing two collections to difference. The check in `EditorViewControllerMediaTeardownTests` compared 9 MiB the same way. The waits were also too short for the machine. A run's first results take half a minute to arrive there, against deadlines of five and ten seconds. `waitUntil`, `waitUntilStarted` and `waitUntilAnswered` now wait up to a minute. They return as soon as what they wait for happens, so only a wait that is going to fail takes longer. `loadBlankPage` gets the same deadline, and now stops its test when the page never loads. It recorded an issue and carried on, so each test went on to fail a second time with a JavaScript `NotReadableError` from a page that wasn't there.
3 tasks done
On the CI machine a web view takes about a minute to load its first page, however small: in three runs it began answering 60 to 66 seconds in. That sits on top of the one-minute wait `loadBlankPage` was given, so the four tests that load a page passed one run and failed the next.
This was referenced Oct 2, 2026
jkmassel
force-pushed
the
jkmassel/asset-bundle-cache-policy
branch
from
October 2, 2026 15:14
378d85d to
1163f84
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.
Moves iOS native media uploads off the loopback HTTP server and onto a
gbk-upload:URL scheme handler, and uploads media picked in the native inserter straight from disk. Stacked on #742. Replaces the loopback-recovery approach in #669 and #733.What?
gbk-upload:, aWKURLSchemeHandlerin the editor's own web view, instead ofhttp://localhost:<port>.MediaUploadServer,GutenbergKitHTTP(library, public product, and tests),GutenbergKitDebugServer, and the demo app's Media Proxy Server screen are deleted — about 10,300 lines.Android is unchanged: it keeps its loopback server, and the middleware picks a transport from what
GBKitadvertises (nativeUploadSchemeon iOS,nativeUploadPort/nativeUploadTokenon Android).Why?
iOS reclaims a suspended app's sockets once the device becomes idle-sleep eligible — RunningBoard's
_systemPreventIdleSleepStateDidChangecallspid_shutdown_socketsfor every suspended process. The upload server'sNWListenerkeeps reporting.readyon a port nothing answers, so after an ordinary screen lock on an unplugged phone every upload failed until the editor was reopened. #669 detected that and restarted the server; its foreground probe could also misread a busy server as dead and restart it mid-upload. A scheme handler has no socket to reclaim, no port or token to advertise, and no connection limit.Measured along the way, and fixed here:
loadTransferable(type: Data.self), thenData(contentsOf:)inMediaFileSchemeHandler, thenblob()in the page). A 1.1 GB video took the app to 2,181 MB and WebKit's page process was killed at its 2,048 MB limit before the upload started.uploadClient()keptURLRequest's 60s inactivity timeout. Against a WordPress that took 90s to generate sub-sizes, the upload failed withNSURLErrorTimedOutwhile the attachment it created stayed on the site — an orphan, and a duplicate once the user retries.MediaFileSchemeHandlerignoredwebView(_:stop:). Answering a stopped task raisesNSInternalInconsistencyException.How?
Files the page holds (Upload button, drag-and-drop, paste)
WebKit hands a scheme handler only bodies it has buffered, and drops some without an error. On iOS 27 under Lockdown Mode:
fetchbodyArrayBufferhttpBodyBlob/File, orFormDataholding onefetchstill resolves200FileinFormDatahttpBodyStreamFileinFormDataSo
nativeMediaUploadMiddlewarenever relies on the body type: it sends the file as 4 MBArrayBufferchunks to a session (POST …/sessions,…/chunks?offset=N), then asks native code to…/finishit with the upload's fields and query.MediaUploadSessionStorewrites each chunk straight to a staging file and refuses one at the wrong offset.MediaUploadServicethen runs the host's processor and delivers through its uploader orInternalMediaClient.x-wp-upload-attachment-idunder CORS, so core'spost-processretry still works.finishmeans WordPress never saw the file, so the page uploads through the web view instead. Fromfinishon, nothing is retried.stopMediaHandling()every request gets a503, which the page takes as the cue to fall back.Media from the native inserter
The inserter asks Photos for the item as a file and copies it into the uploads directory — an APFS clone, so an import of any size costs neither memory nor disk. The file keeps its own name. When the editor can upload natively, it registers the file with the scheme handler, and the page gets a stand-in
File: a JPEG preview followed by a marker naming the session. The marker travels in the bytes because core re-creates theFilewhen it buildsFormData. The block uploads through Gutenberg's own pipeline (placeholder, save lock, notices,post-processrecovery), and the middleware finishes the session natively instead of sending the stand-in. A stand-in is never uploaded through the web view.Also
EditorHTTPClient.uploadInactivityTimeout).MediaFileSchemeHandlerstreams files in 1 MB chunks, honoursstop, and refuses paths outside the media directory.video/MOV.gbk-upload:/gbk-media-file:requests out of network logging: a large upload is hundreds of chunks.docs/code/media-uploads.md;integration.mdscopes the loopback server to Android.Breaking: the
GutenbergKitHTTPproduct is removed (no consumers found outside this repo), andGBKitGlobal.inittakesnativeUploadScheme:instead ofnativeUploadPort:/nativeUploadToken:.Not in this PR
URLSessionsurvives suspension, but when WordPress was slow to answer,nsurlsessiondre-sent the whole upload about every 100s — five attachments for one upload — so it needs dedupe first.onUploadErroronly shows a snackbar. That's upstream.PhotosFileProvideris killed at its 20 MB limit converting it to JPEG. That's pre-existing and outside GutenbergKit; the native inserter doesn't hit it.Testing Instructions
make test-web-unit— 363 tests, including the scheme transport and core'spost-processrecovery over itswift test— 682 tests — and theGutenbergKit-Packagetests in the iOS Simulator (iPhone 18 Pro, iOS 27.0) — 701 tests, including chunked uploads through the handler in the editor's ownWKWebViewOn a device (iPhone 15 Pro, iOS 27, Lockdown Mode on), with the demo app's native media upload enabled against a local WordPress:
recovermode (make wp-env-media-failure MODE=recoveron wp-env): the upload fails, and core'spost-processretry completes it.alwaysmode: fivepost-processattempts, then the orphan'sDELETEgoes through native code and the snackbar appears.