Updated sitemap to work with lazy routing - #29302
Merged
allouis merged 5 commits intoJul 15, 2026
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
🚧 Files skipped from review as they are similar to previous changes (7)
WalkthroughThe sitemap manager now builds lazy indexes from routable resources, coordinates concurrent reads and invalidation, and replays router entries during rebuilds. The URL service adds lazy resource enumeration, eager/lazy parity checks, and comparison opt-out support. Sitemap routes await asynchronous XML generation and forward failures through Express. Tests cover resource filtering, parity, rebuilding, routing, SQLite batching, and asynchronous failures. Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
| Command | Status | Duration | Result |
|---|---|---|---|
nx run ghost:test:ci:integration |
✅ Succeeded | 2m 46s | View ↗ |
nx run ghost:test:integration |
✅ Succeeded | 2m 52s | View ↗ |
nx run ghost:test:e2e |
✅ Succeeded | 2m 38s | View ↗ |
nx run ghost:test:legacy |
✅ Succeeded | 2m 3s | View ↗ |
nx run-many -t test:unit -p ghost |
✅ Succeeded | 29s | View ↗ |
nx run ghost-monorepo:lint:boundaries |
✅ Succeeded | 7s | View ↗ |
nx run @tryghost/admin:build |
✅ Succeeded | 7s | View ↗ |
nx run-many -t lint -p ghost,ghost-monorepo |
✅ Succeeded | 22s | View ↗ |
Additional runs (3) |
✅ Succeeded | ... | View ↗ |
💡 Verify your cache is correct by running tasks in a sandbox. Read docs ↗
☁️ Nx Cloud last updated this comment at 2026-07-15 10:15:27 UTC
allouis
force-pushed
the
fabien-hkg-1919-update-sitemap-to-work-with-lazy-routing
branch
3 times, most recently
from
July 14, 2026 04:47
91309b9 to
343fa2f
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ghost/core/core/frontend/services/sitemap/resource-fetcher.js`:
- Around line 10-13: Update the author fetch in the sitemap resource-fetcher
path to use an explicit projection or a tighter exclude list, removing email and
every other unused users column while retaining only fields required by sitemap
caching and URL computation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9ed954f8-109e-4de4-82fb-1f05b8818289
📒 Files selected for processing (15)
ghost/core/core/frontend/services/sitemap/base-site-map-generator.jsghost/core/core/frontend/services/sitemap/handler.jsghost/core/core/frontend/services/sitemap/parity-check.jsghost/core/core/frontend/services/sitemap/resource-fetcher.jsghost/core/core/frontend/services/sitemap/site-map-manager.jsghost/core/core/server/services/url/url-service-facade.tsghost/core/test/integration/sitemap-parity.test.jsghost/core/test/integration/url-service-parity.test.jsghost/core/test/unit/frontend/services/sitemap/generator.test.jsghost/core/test/unit/frontend/services/sitemap/handler.test.jsghost/core/test/unit/frontend/services/sitemap/manager.test.jsghost/core/test/unit/frontend/services/sitemap/parity-check.test.jsghost/core/test/unit/frontend/services/sitemap/resource-fetcher.test.jsghost/core/test/unit/server/services/url/url-service-facade.test.jsghost/core/test/utils/url-service-utils.js
allouis
force-pushed
the
fabien-hkg-1919-update-sitemap-to-work-with-lazy-routing
branch
2 times, most recently
from
July 14, 2026 06:34
a2c846d to
b37e364
Compare
allouis
force-pushed
the
fabien-hkg-1919-update-sitemap-to-work-with-lazy-routing
branch
2 times, most recently
from
July 14, 2026 07:04
a2d4ddc to
d5716d5
Compare
allouis
marked this pull request as ready for review
July 14, 2026 07:27
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
ghost/core/core/server/services/url/url-service-facade.ts (1)
138-199: 🚀 Performance & Scalability | 🔵 TrivialCompare-mode bulk fetch doubles DB load on every sitemap rebuild.
Every
getRoutableResources(type)call in compare mode kicks off a full (SQLite-batched) DB fetch via_compareRoutableResourcespurely for parity logging, in addition to the eager answer already served. Sinceindex.jswirescompare: truewheneverlazyRoutingis enabled, andsite-map-manager.js's_buildIndex()calls this for all 4 types on every rebuild, eachsite.changed/routers.resetinvalidation now costs 4 extra full-table (batched) reads — precisely during the heavy-editing windows the PR already flags as producingSITEMAP_BUILD_SUPERSEDED503s. Unlike the existing single-resource compare paths, this is bulk work with no throttling/dedup if invalidations outpace a compare cycle.Worth watching DB/connection-pool pressure during compare-mode soak testing on busy sites, or considering a cheaper comparison strategy (e.g., sampling, or a lower-frequency comparison cadence) for this particular check.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ghost/core/core/server/services/url/url-service-facade.ts` around lines 138 - 199, Throttle or deduplicate the bulk parity fetch triggered by getRoutableResources and _compareRoutableResources so repeated sitemap rebuilds do not start a full database read for every type on every call. Preserve the eager response and parity logging, while limiting comparisons to a lower-frequency or single in-flight check per type.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ghost/core/core/frontend/services/sitemap/site-map-manager.js`:
- Around line 222-227: Update _applyResource to stop filtering URLs via the
broad url.includes('/404/') check; use the URL service’s not-found sentinel or
an equivalent suffix-only check, while continuing to add valid canonical
URLs—including resources whose slug contains “404”—through this[type].addUrl.
---
Nitpick comments:
In `@ghost/core/core/server/services/url/url-service-facade.ts`:
- Around line 138-199: Throttle or deduplicate the bulk parity fetch triggered
by getRoutableResources and _compareRoutableResources so repeated sitemap
rebuilds do not start a full database read for every type on every call.
Preserve the eager response and parity logging, while limiting comparisons to a
lower-frequency or single in-flight check per type.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c05a8589-2b9c-4b38-9013-9095e539e8a2
📒 Files selected for processing (10)
ghost/core/core/frontend/services/sitemap/handler.jsghost/core/core/frontend/services/sitemap/site-map-manager.jsghost/core/core/server/services/url/index.jsghost/core/core/server/services/url/routable-resources.jsghost/core/core/server/services/url/url-service-facade.tsghost/core/test/integration/sitemap-parity.test.jsghost/core/test/unit/frontend/services/sitemap/handler.test.jsghost/core/test/unit/frontend/services/sitemap/manager.test.jsghost/core/test/unit/server/services/url/routable-resources.test.jsghost/core/test/unit/server/services/url/url-service-facade.test.js
🚧 Files skipped from review as they are similar to previous changes (2)
- ghost/core/test/unit/frontend/services/sitemap/handler.test.js
- ghost/core/test/integration/sitemap-parity.test.js
allouis
force-pushed
the
fabien-hkg-1919-update-sitemap-to-work-with-lazy-routing
branch
from
July 14, 2026 08:07
d5716d5 to
4f6711b
Compare
vershwal
approved these changes
Jul 14, 2026
vershwal
left a comment
vershwal
left a comment
Member
There was a problem hiding this comment.
Looks good overall, small nits. I also think some of these files have too many comments. We should clean them up either now or later when we remove comparison mode and tidy up this code.
allouis
force-pushed
the
fabien-hkg-1919-update-sitemap-to-work-with-lazy-routing
branch
from
July 15, 2026 03:10
4f6711b to
4bcec70
Compare
allouis
force-pushed
the
fabien-hkg-1919-sitemap-tidies
branch
from
July 15, 2026 05:55
ebd2f45 to
1176124
Compare
allouis
force-pushed
the
fabien-hkg-1919-update-sitemap-to-work-with-lazy-routing
branch
2 times, most recently
from
July 15, 2026 05:57
11051d1 to
e197bdb
Compare
ref https://linear.app/ghost/issue/HKG-1919 The sitemap needs the complete set of routable resources, and the lazy URL service deliberately holds no such set in memory. This fetches it on demand through the same raw-knex fast path and public-visibility gates the eager boot walk uses, batched in id order on SQLite to stay under the bound-variable limit. Callers name the extra columns they want back rather than maintaining exclude lists, and the columns and relations URL computation needs come from the lazy service itself, so rows can never be thin for whatever routing config is active.
allouis
force-pushed
the
fabien-hkg-1919-update-sitemap-to-work-with-lazy-routing
branch
from
July 15, 2026 06:22
e197bdb to
cfd35db
Compare
ref https://linear.app/ghost/issue/HKG-1919 Enumeration joins the facade so it answers like every other URL read: eager from its in-memory cache, lazy from the on-demand fetch, and compare returning the eager answer while diffing the lazy fetch id set in the background. The comparison is single-flighted per type and logs counts and id samples, never row bodies, because a large site has hundreds of thousands. Per-URL parity already comes from organic request traffic, so bulk callers pass skipComparison — teeing every row of a full build would capture a stack and queue a background computation per resource. Lazy mode without an injected fetcher throws so a miswired boot fails fast.
ref https://linear.app/ghost/issue/HKG-1919 Under lazy routing no per-URL events fire, so the manager builds its index itself: one fetch of every routable resource on first read, URLs computed through the URL service, fed to the untouched generators via addUrl. Wherever the eager service runs — flag off and compare mode — the per-URL events keep the index current from then on and nothing invalidates, which makes deploying this a serving no-op until the lazy flip. Under pure lazy the invalidation signals empty the index and the next read rebuilds; a build invalidated mid-flight fails the read with a 503 rather than ever serving pre-invalidation data, since the CDN would pin a stale 200 for the full cache maxAge, and there is deliberately no retry until the logs justify one. Static and collection route entries only arrive via router.created at boot and routes reload, so they are recorded and replayed into every rebuild, and the /404/ sentinel is matched exactly so a real resource slugged "404" stays in the sitemap.
ref https://linear.app/ghost/issue/HKG-1919 getIndexXml and getSiteMapXml are async now that the manager builds its index on first read, and awaiting inside the manager means no caller can forget to. Express 4 does not forward async handler rejections, so each handler body is fully wrapped: a failed build or render becomes an error response the next request can retry, instead of an unhandled rejection and a socket that never responds.
ref https://linear.app/ghost/issue/HKG-1919 The unit tests pin the enumeration against stubbed queries, which can never prove that the filters, has-posts gates and loaded relations agree with what the eager URL service actually caches. This runs both against a real database and asserts identical URL sets, across the default routing shape plus primary_tag and primary_author permalinks, the two URL shapes that silently degrade when a relation goes missing.
allouis
force-pushed
the
fabien-hkg-1919-update-sitemap-to-work-with-lazy-routing
branch
from
July 15, 2026 10:05
cfd35db to
a1dbe47
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #29302 +/- ##
==========================================
+ Coverage 74.00% 74.03% +0.03%
==========================================
Files 1577 1578 +1
Lines 136928 137430 +502
Branches 16577 16647 +70
==========================================
+ Hits 101328 101742 +414
- Misses 34560 34643 +83
- Partials 1040 1045 +5
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
allouis
deleted the
fabien-hkg-1919-update-sitemap-to-work-with-lazy-routing
branch
July 15, 2026 10:38
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
2 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.
ref https://linear.app/ghost/issue/HKG-1919
The sitemap generators have a single write path, addUrl, and today its only caller is the eager URL service's per-URL event stream. The lazy URL service emits no such events, so under lazy routing every sitemap would render empty. This PR adds a second path to addUrl without touching the generators or the XML rendering.
The URL service now exposes enumeration of routable resources. facade.getRoutableResources covers all three modes: eager reads the in-memory cache, lazy fetches on demand through the same raw-knex path and visibility gates the eager boot walk uses, and compare returns the eager result while diffing the lazy id set in the background, single-flighted per type and logging only counts and sample ids. Bulk callers pass skipComparison so a full build does not queue a background computation per resource.
SiteMapManager builds its index once on first read in every mode, computing URLs through the URL service and feeding them to the generators. Invalidation (site.changed, routers.reset, router.created) applies only under pure lazy. A build invalidated mid-flight fails the read with a 503 SITEMAP_BUILD_SUPERSEDED instead of serving pre-invalidation data the CDN would pin for the full maxAge, with deliberately no retry. Static and collection router entries are recorded and replayed into every rebuild, and the /404/ sentinel is matched exactly so a resource slugged "404" stays listed.
The key rollout property: wherever the eager service runs (flag off and compare mode) the per-URL events keep the index current after the first build and nothing invalidates, so deploying this is a serving no-op until the lazy flip.
Commits are reviewable independently and each is green on its own. An integration parity test pins enumeration against the eager cache on a real database across the default routing shape plus primary_tag and primary_author permalinks.