[pull] main from TryGhost:main - #1500
Merged
Merged
Conversation
…fter leaving it ref https://linear.app/ghost/issue/BER-3958 The members list keeps its filters in the URL, and when the URL changed from outside, a link or a saved view, it wrote its previous filters back over the new URL before settling on the new ones. A navigation that landed during that write-back was lost, so leaving a saved view and clicking straight back opened the list with no filter. The browser test for reopening a country filter view hit this every time on this branch. The list now skips that one stale write, keeping the guard that protects its own pending edits.
ref https://linear.app/ghost/issue/BER-3958 A member's avatar is worked out from their email rather than stored, and only the member model knew how, so anything reading members without the model had no avatar to show. The rule now lives in one helper the model calls, which lets a query that selects only the member columns it needs still show the same avatar.
ref https://linear.app/ghost/issue/BER-3958 A member updating their own fields in Portal leaves no trace on their activity feed, so a publisher looking at a value has no way to tell who changed it or when. This is the storage for that entry, on its own so the schema change can be reviewed apart from the code that writes and reads it. It records which fields changed rather than their values, so an address a member has since replaced does not live on in a history table.
ref https://linear.app/ghost/issue/BER-3958 Once members can edit their own fields, a value has more than one author, and a publisher looking at one needs to see who changed it, where and when. Every write to a member's custom fields now leaves an entry on their activity feed naming the fields it touched, whether it came from staff in Admin, an integration, an import, checkout or the member in Portal. The entry is written by the value write itself, in the same transaction, so the feed cannot disagree with what is stored, and each write has to name who made it and where, with pairs that cannot happen refused by the compiler. The entries belong to the metafields domain, which reads them with knex for the members events endpoint rather than through a Bookshelf model, and each field's name is copied so an entry still reads after the field is renamed or deleted.
ref https://linear.app/ghost/issue/BER-3958 The members events endpoint now returns an entry whenever a member's custom fields change, and without this Admin would render it as an unlabelled row. The row names the changed fields and where the change was made, and the activity page can filter on it where the site has custom fields. The preview on a member's page now leaves out the same events as their full activity page, so following "View all" never shows a different set.
ref https://linear.app/ghost/issue/BER-3958 The members activity page is still Ember unless the React version's labs flag is on, and Ember had no case for the custom field change event, so each entry rendered as a row with no text. Ember now describes the entry the same way React Admin does, offers it as a filter, and hides it where custom fields are not available, so both versions of the page show the same thing.
ref https://linear.app/ghost/issue/HKG-1980 Newsletter sends ran as closures on the legacy job manager's inline queue. A closure only exists in the process that created it, so a restart mid-dispatch lost the send and we relied on the boot resume scan to recover it. Sends are now serialisable jobs, which is what lets us move them to a durable backend. With one in place, a send survives a restart without further changes to the email path. We also give them their own lane to run in, so that they're not blocked by other jobs like imports and email analytics fetching
no ref
- the sitemap generators hold one entry per routable resource for the lifetime of the index, and each entry was a nested element tree for the xml package plus a Moment in a parallel map. Measured against a heap snapshot from the Pro performance benchmark seeded with 10,000 posts, that came to 9.99 MB retained — 8.1% of the process heap, 159,568 objects, 9,381 of them Moments that nothing ever read as Moments: the sort coerces them with a unary minus
and the lastmod wants an ISO string.
Each entry is now a flat {loc, ts, imageLoc} record and the timestamp is epoch milliseconds. moment still does the parsing, because it is more forgiving than `new Date` about the shapes the database returns; only the stored value changed.
Rendering builds the document by joining an array instead of handing the tree to the xml package. That drops the largest allocation in the path — the package concatenates its output, and a rope of several megabytes costs multiples of its length until something flattens it — and lets the two transform-ready urls be made absolute as they are added, rather than by a regex across the finished document. The element shape is fixed and the five characters the package escaped are escaped the same way, so the bytes are unchanged: a differential harness rendered ~2,500 resources through both implementations, including xml special characters in slugs and image names, transform-ready and external canonical urls, missing dates and the author generator's stricter image validation, and every document matched.
The index generator now caches its xml. It is the only sitemap a crawler fetches repeatedly, and rendering it counts every resource of every type — at 10,000 posts that `Object.keys(...).length` was 240 ms of the benchmark's profile, a third of all sitemap CPU, recomputed per request. The cache is valid exactly while the manager's index is built, so it is dropped with it.
At 50,000 posts the generators hold 14.6 MB rather than 51.1 MB, a render peaks 30 MB above steady state rather than 162 MB, and takes 176 ms rather than 1,796 ms.
no ref - every publish, update or delete empties the sitemap index, and the next sitemap read rebuilt it in one synchronous loop over every routable resource. The loop had to be synchronous because it reset and refilled the live generators, so a yield would have let a request render a half-built index. With the real url service and 300,000 posts that loop blocked the event loop for 1.9 s on a fast laptop, stalling every other request on the instance, and longer on production CPUs. A rebuild now fills fresh generators and swaps them in once complete. Nothing reads the new set before the swap, so the build yields every 10 ms. The build checks for invalidation after every yield, not just after the fetch. That matters because a RouteRegistered can now arrive mid-build. An invalidated build is abandoned without swapping. The index generator is recreated at the swap because it holds references to the generators it counts. Route entries are replayed into each new pages generator instead of being added to the live one. The longest block during a rebuild went from 72 ms to 10 ms at 10k posts, 337 ms to 12 ms at 50k, and 1,947 ms to 14 ms at 300k. Total rebuild time rose about 10% at 300k. The slice is a time budget rather than a row count, so it holds on slower CPUs; 5 to 50 ms slices gave the same total, and the longest block tracked the slice plus a few ms of GC. Reads still wait for the rebuild rather than being served the previous index. site.changed is emitted by the same response that purges the CDN, and builds start on read, so the first read after a purge is the one the CDN stores for the full cache maxAge. Serving the previous index there would pin a sitemap missing the change on every publish. After this change the wait only affects sitemap readers, not the rest of the site. Longer builds make it more likely a change lands mid-build, so a read now retries up to three builds before failing with the existing SITEMAP_BUILD_SUPERSEDED 503. Both generator sets are in memory at the swap. To keep that from raising the peak, each fetched row is released once applied. Live heap at 300k peaks at 308 MB either way (old generators plus fetched rows); at the swap it is 273 MB, where holding the rows would have made it 395 MB.
no ref Updated the sidebar handling to use a portion of the editor state instead of the full object.
…ages (#30759) no ref - Centralizes the shared tier and newsletter query parameters - Makes the author picker fetch every staff page - Makes email preview fetch every newsletter page - Reuses the publish flow’s newsletter cache, then filters active newsletters client-side - Keeps archived newsletters selectable when already attached to a post - Adds tests covering pagination and shared request URLs
ref [GVA-988](https://linear.app/ghost/issue/GVA-988) Give publishers advance warning of hosting payment failures and the suspension deadline, with payment and export actions for owners and guidance for staff. Drive warning severity from host configuration behind a flag. Clear warnings after successful payment, preserve legacy behavior when configuration is unavailable, and let open dialogs close before showing the takeover.
…ng helpers ref https://linear.app/ghost/issue/ONC-1983 The automations service was the only consumer of the scheduler's idempotency key support, so the helper that builds the key lived in its folder and hard-coded the `ghost-automations-` prefix. Post scheduling is about to send keys too, and it needs the same recipe with a different namespace. The helper now lives in `adapters/scheduling`, beside `build-signed-job` which is the other half of assembling a job, and takes the namespace as an argument. The automations call site passes `automations`, so the keys it produces are byte-for-byte the same as before and jobs already queued under the old keys still dedupe correctly. This change should not change behaviour.
ref https://linear.app/ghost/issue/ONC-1983 Ghost(Pro)'s scheduler keeps its job queue in a database, so every boot rebuild has to replace the job it already holds for each scheduled post: delete by URL, then create. Those two calls are not ordered on the wire. When the create lands first, the delete removes both the old job and the new one, and the post silently never publishes. That is what happened to Hyperallergic on 1 September. The scheduler can already recognise a re-registration if the job carries an idempotency key, which is how the automations poll avoids duplicates. Post and page jobs now send one too, so the boot rebuild can stop deleting in a follow-up once every queued job has a key. The key is a hash of the fire time and the final callback URL, namespaced `post-scheduling`. The URL is the right identity because it already carries the resource ID and a token signed for that fire time under the current signing key: registering the same job again yields the same key, while a reschedule or a key rotation yields a new one. A key based on the post ID alone would dedupe a rescheduled job against the old one that the paired delete is about to remove, which recreates the same race in a new place. This change should not change behaviour: the delete-then-create on boot is untouched, and a persistent queue that already holds a key-less job for a post simply gains a keyed replacement, exactly as it gained a key-less replacement before.
ref https://linear.app/ghost/issue/ONC-1983 The schedules endpoint read the post and then edited it with no transaction and no row lock. A scheduler with a persistent queue can hold two jobs for one post and fire both in the same tick, so the two callbacks overlap: both see the post as scheduled, both flip it to published, and both try to create the newsletter email. Today only the unique index on `emails.post_id` stops a second send, and it does so by failing the second request. In production this pattern shows up a couple of times a day. The read and the edit now run inside one transaction with the row locked for update. The second delivery waits for the first to commit, then finds the post is no longer scheduled and takes the existing no-op path: an empty 2xx the scheduler treats as done. The post is published once, the email is created once, and neither request fails. This is needed before the boot rebuild stops deleting existing jobs, because sites that sleep through that release will briefly hold a key-less job and a keyed twin for each pending post.
…30680) ref https://linear.app/ghost/issue/ONC-1983 The previous commit ran the scheduled publish read and edit inside one transaction with the post row locked for update, so overlapping deliveries would serialise on the database. That lock is applied to every eagerly loaded relation of the post as well, which on MySQL means gap locks on the `emails`, `posts_authors` and similar indexes. Publishing a post with a newsletter then creates the email outside that transaction and hands it to the batch sender straight away, so the insert waits on the locked post until the InnoDB lock wait timeout and the publish fails with the transaction rolled back. Any unrelated post insert in the meantime waits on the same gap locks, which is how the email preview acceptance test timed out in CI once a file earlier in the same worker had left a listening server for the scheduler to ping. Deliveries for one resource are now chained in process instead: the second runs after the first has finished, reads the post, finds it is no longer scheduled and takes the existing no-op path. This covers the production case of two jobs for one post firing in the same tick, and removes the row lock from the publish path entirely. The legacy overlapping-deliveries test now publishes a post with a newsletter and asserts the email is created once, which fails against the row lock.
Fixes https://linear.app/ghost/issue/DES-1535/paper-cuts-from-new-pill-styles Corrects the pill-style inconsistencies found during Admin 7 testing, while preserving the existing appearance when `admin7Pill` is disabled. - Keep form selects and input groups rounded, with pill-shaped header controls and embedded buttons. Preview/Copy actions are 24px tall. - Order Members header actions as Search → More → Filter → New member, apply the shared primary-action spacing on member details, and fix header dropdown icon sizing and spacing. - Use consistent icon-sized controls for the Automation back button and staff profile menu, including the standard outline treatment. Round the staff selector in History. - Add a shared `destructive-ghost` button variant with translucent red hover/pressed backgrounds and replace repeated local destructive-button styling. - Update the PageHeader contract and stories, including a generic “No primary action” example. Remove one CSS-class-only CopyField test; retain clipboard behavior tests. Validation: - Shade: 278 existing tests passed; lint and build passed. - Admin: type checking and lint on changed files passed; MembersActions' 7 existing tests passed. - Visually checked the affected controls in Admin and Storybook, including the flag-off header/button examples. - Repository-wide `pnpm check`: formatting and lint passed. The test phase encountered Core failures (a cron day-of-week assertion and timeouts) and two Admin country-filter timeouts. The Admin file passes independently (11 tests). Stopped the remaining unrelated Koenig run after these failures; the full check is not green. Reported screenshots and reproduction details are in DES-1535. These are visual fixes; no new style-assertion tests were added.
ref https://linear.app/ghost/issue/HKG-1973 The wrapper's startFetch loop is about to change transport, so lock down the behaviour the migration must preserve first: overlapping fetches are skipped while a sibling wrapper keeps going, fetch order and budgets, a failed fetch does not block the next tick, and a restarted fetch continues detached from the invocation that started it.
ref https://linear.app/ghost/issue/HKG-1973 The newsletter jobs migration needs concrete executors at registration. Retained separate wrapper instances and cursor namespaces, with guarded accessors and legacy subscriptions owned by analytics initialization.
ref https://linear.app/ghost/issue/HKG-1973 Since construction replaced init(), the wrapper's service, config and metrics are always assigned, yet they stayed optional behind three 'not initialized' error branches no call site can reach. The scheduler likewise guarded a GiftDelivery model that the models index always exports. Making the fields required removes the dead branches and the errors import that only served them.
ref https://linear.app/ghost/issue/HKG-1973 The wrapper mapped its log name to the background job name through a switch, so the name lived in the switch and again wherever the job was registered. Passing the job type in makes the wrapper's log lines follow whichever job registers it, which the jobs migration relies on.
ref https://linear.app/ghost/issue/HKG-1973 Each pipeline's completed run was only a free-text line, so nothing could wait on or chart a run finishing. Attach a <job_type>.completed system event with the event count and duration, matching the other in-process jobs, so the migration's integration tests and dashboards have a signal. The message itself is unchanged so existing log queries keep matching.
…eduler ref https://linear.app/ghost/issue/HKG-1973 The three pipelines were copies of the same block: flag, config check, thirty-day probe, random cron, log, register, flag. One private schedule step keyed on the job name keeps that contract in a single place, so the jobs migration can swap the registration call once instead of three times.
…ess (#30857) no ref Comments have always been readable on posts a visitor can't access, and some sites rely on that to show discussion as a preview of paid content. #30402 put every comment read behind the post's access check, which broke those sites. The gated content that was actually exposed came from the post excerpt, the first 500 characters of the post's plaintext, which the comment serializer added for Admin's thread sidebar and also returned through the members API with `include=post`. The members API now never returns that excerpt, and comment reads go back to not checking post access. Editing, deleting and removing a vote stay open to members who have lost access, so someone who cancels can still take down what they wrote, while liking and disliking keep requiring access like commenting and replying do.
…30850) no ref The 499 early returns added to the renderer in #30298 skip res.end(). express-queue only releases an active slot when res.end() is called, so every client that disconnected mid-render leaked a slot permanently. Once concurrencyLimit slots had leaked, the queue stopped serving dynamic requests entirely: each queued request waited until an upstream timeout closed it, and only a restart recovered the site. Ending the response is a no-op on the already-destroyed socket but emits the event the queue waits for. The new test runs the renderer behind the real queue middleware and fails with the old code.
no ref express-queue only releases a slot when res.end() is called, so any handler that never ends its response holds a slot forever; once concurrencyLimit slots leak, dynamic requests stop being served until a restart. The package is unmaintained (last release 2022) and has no fix. The replacement releases a slot on res.end() or connection close, whichever comes first, and drops queued requests whose client disconnects. It is also a base for smarter load shedding than a fixed depth limit. Request logs now record client disconnects as 499 instead of 200. Requests cancelled while queued were previously logged as 200 before express-queue set its 204, so disconnects could not be told apart from normal responses. Queue wait time is logged as req.extra.queueWaitMs, separating it from service time in res.responseTime.
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
8 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.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )