✨ Added IndexNow integration for faster search engine indexing #25773
Conversation
WalkthroughThis PR introduces IndexNow integration to Ghost, a protocol enabling sites to notify search engines about newly published or updated content. The implementation includes a new IndexNow service that monitors post publishing and editing events, detects SEO-relevant field changes (title, slug, meta description, canonical URL, HTML, status), and pings the IndexNow API endpoint accordingly. The service includes API key management through settings, configurable via a labs feature flag, with a dedicated middleware serving the API key verification file. Configuration adds an indexnow_api_key setting field, the service integrates into Ghost's boot sequence, and the API key is excluded from data exports for security. The changes encompass schema updates, middleware registration, event handling, and comprehensive unit tests validating service behavior across various scenarios. Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
ghost/core/core/frontend/web/middleware/serve-indexnow-key.js (1)
31-31: Consider extracting the regex pattern as a constant.For better maintainability and testability, consider extracting the regex pattern to a named constant:
+const INDEXNOW_KEY_PATTERN = /^\/[a-f0-9]{32}\.txt$/; + function serveIndexNowKey(req, res, next) { // Only handle requests for .txt files at the root - if (!req.path.match(/^\/[a-f0-9]{32}\.txt$/)) { + if (!req.path.match(INDEXNOW_KEY_PATTERN)) { return next(); }This makes the pattern reusable and self-documenting.
ghost/core/core/server/services/indexnow.js (1)
136-173: Refactor error handling for clarity.The current error handling relies on the
@tryghost/requestlibrary throwing errors withstatusCodeproperties for non-2xx responses. This implicit behavior makes the code harder to understand. Consider explicitly checkingresponse.statusCodein the try block before throwing.Additionally, line 165 has a redundant
err: err(should be justerr).🔎 Proposed refactoring for clearer error handling
try { const response = await request(indexNowUrl.toString(), options); - if (response.statusCode !== 200 && response.statusCode !== 202) { - throw new errors.InternalServerError({ - message: `IndexNow returned unexpected status: ${response.statusCode}` - }); + // Handle non-success responses explicitly + if (response.statusCode === 429) { + throw new errors.TooManyRequestsError({ + message: 'IndexNow rate limit exceeded', + context: tpl(messages.requestFailedError, {service: 'IndexNow'}), + help: tpl(messages.requestFailedHelp, {url: 'https://ghost.org/docs/'}) + }); + } else if (response.statusCode === 422) { + throw new errors.ValidationError({ + message: 'IndexNow key validation failed', + context: tpl(messages.requestFailedError, {service: 'IndexNow'}), + help: 'Ensure your IndexNow API key file is accessible at the correct URL' + }); + } else if (response.statusCode !== 200 && response.statusCode !== 202) { + throw new errors.InternalServerError({ + message: `IndexNow returned unexpected status: ${response.statusCode}` + }); } logging.info(`IndexNow: Successfully pinged ${url}`); } catch (err) { - let error; - if (err.statusCode === 429) { - error = new errors.TooManyRequestsError({ - err, - message: err.message, - context: tpl(messages.requestFailedError, {service: 'IndexNow'}), - help: tpl(messages.requestFailedHelp, {url: 'https://ghost.org/docs/'}) - }); - } else if (err.statusCode === 422) { - // 422 means the URL is invalid or key doesn't match - error = new errors.ValidationError({ - err, - message: 'IndexNow key validation failed', - context: tpl(messages.requestFailedError, {service: 'IndexNow'}), - help: 'Ensure your IndexNow API key file is accessible at the correct URL' - }); - } else { - error = new errors.InternalServerError({ - err: err, - message: err.message, - context: tpl(messages.requestFailedError, {service: 'IndexNow'}), - help: tpl(messages.requestFailedHelp, {url: 'https://ghost.org/docs/'}) - }); + // Handle network errors, timeouts, or rethrow Ghost errors + if (err.errorType) { + // Already a Ghost error from above, just log it + logging.error(err); + } else { + // Network error, timeout, or other unexpected error + const error = new errors.InternalServerError({ + err, + message: err.message, + context: tpl(messages.requestFailedError, {service: 'IndexNow'}), + help: tpl(messages.requestFailedHelp, {url: 'https://ghost.org/docs/'}) + }); + logging.error(error); } - logging.error(error); } }
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
apps/admin-x-settings/src/components/settings/advanced/labs/private-features.tsxghost/core/core/boot.jsghost/core/core/frontend/web/middleware/index.jsghost/core/core/frontend/web/middleware/serve-indexnow-key.jsghost/core/core/frontend/web/site.jsghost/core/core/server/api/endpoints/utils/serializers/input/settings.jsghost/core/core/server/data/schema/default-settings/default-settings.jsonghost/core/core/server/services/indexnow.jsghost/core/core/shared/labs.jsghost/core/test/unit/server/services/indexnow.test.js
🧰 Additional context used
🧠 Learnings (8)
📚 Learning: 2025-04-23T15:44:52.549Z
Learnt from: 9larsons
Repo: TryGhost/Ghost PR: 21866
File: ghost/core/core/server/web/admin/middleware/serve-auth-frame-file.js:10-19
Timestamp: 2025-04-23T15:44:52.549Z
Learning: The existing implementation in `ghost/core/core/server/web/admin/middleware/serve-auth-frame-file.js` using `path.parse(req.url).base` is secure against path traversal attacks as it properly extracts only the filename component without any directory parts.
Applied to files:
ghost/core/core/frontend/web/middleware/serve-indexnow-key.jsghost/core/core/frontend/web/site.js
📚 Learning: 2025-06-10T11:07:10.800Z
Learnt from: kevinansfield
Repo: TryGhost/Ghost PR: 23770
File: apps/admin-x-settings/src/components/settings/email/newsletters/NewsletterDetailModalLabs.tsx:435-450
Timestamp: 2025-06-10T11:07:10.800Z
Learning: In Ghost's newsletter customization features, when promoting a feature from alpha to beta status, the feature flag guards are updated to make the feature available under both the `emailCustomization` (beta) and `emailCustomizationAlpha` (alpha) flags. This is done by either removing conditional guards entirely when the component is already behind both flags, or by updating conditionals to check for either flag.
Applied to files:
apps/admin-x-settings/src/components/settings/advanced/labs/private-features.tsxghost/core/core/shared/labs.js
📚 Learning: 2025-10-09T15:31:06.587Z
Learnt from: kevinansfield
Repo: TryGhost/Ghost PR: 25118
File: apps/portal/src/actions.js:160-173
Timestamp: 2025-10-09T15:31:06.587Z
Learning: When reviewing PRs that introduce feature-flagged changes (e.g., `labs?.membersSigninOTCAlpha`), avoid suggesting modifications to non-flagged code paths unless they're directly related to the PR's objectives. Keep the scope focused on the feature-flag-specific changes only.
Applied to files:
apps/admin-x-settings/src/components/settings/advanced/labs/private-features.tsx
📚 Learning: 2025-11-25T14:28:50.351Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Ghost admin serves admin-x apps from `/ghost/assets/{app-name}/{app-name}.js` URLs
Applied to files:
ghost/core/core/frontend/web/middleware/index.js
📚 Learning: 2025-11-25T14:28:50.351Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Admin-x React apps build to `apps/*/dist` using Vite, which are then copied by `ghost/admin/lib/asset-delivery` to `ghost/core/core/built/admin/assets/*`
Applied to files:
ghost/core/core/frontend/web/middleware/index.js
📚 Learning: 2025-03-13T09:00:20.205Z
Learnt from: mike182uk
Repo: TryGhost/Ghost PR: 22471
File: apps/admin-x-activitypub/src/utils/pending-activity.ts:13-71
Timestamp: 2025-03-13T09:00:20.205Z
Learning: The pending activity utilities in the Ghost ActivityPub module are covered by tests in the file `apps/admin-x-activitypub/test/unit/utils/pending-activity.ts`.
Applied to files:
ghost/core/test/unit/server/services/indexnow.test.js
📚 Learning: 2025-03-13T09:00:20.205Z
Learnt from: mike182uk
Repo: TryGhost/Ghost PR: 22471
File: apps/admin-x-activitypub/src/utils/pending-activity.ts:13-71
Timestamp: 2025-03-13T09:00:20.205Z
Learning: The pending activity utilities in Ghost's ActivityPub module are thoroughly tested in `apps/admin-x-activitypub/test/unit/utils/pending-activity.ts`, which covers `generatePendingActivityId`, `isPendingActivity`, and `generatePendingActivity` functions.
Applied to files:
ghost/core/test/unit/server/services/indexnow.test.js
📚 Learning: 2025-11-24T11:12:15.712Z
Learnt from: sagzy
Repo: TryGhost/Ghost PR: 24673
File: ghost/i18n/lib/i18n.js:34-35
Timestamp: 2025-11-24T11:12:15.712Z
Learning: In the Ghost i18n package (ghost/i18n/lib/i18n.js), changing existing locale codes requires backwards compatibility handling for users who have already configured those locales. Such changes should be done in a separate PR with migration logic rather than included in feature PRs.
Applied to files:
ghost/core/core/shared/labs.js
🧬 Code graph analysis (2)
apps/admin-x-settings/src/components/settings/advanced/labs/private-features.tsx (1)
ghost/admin/app/services/feature.js (1)
feature(9-39)
ghost/core/core/server/services/indexnow.js (2)
ghost/core/core/shared/labs.js (3)
require(135-135)settingsCache(13-13)labs(67-67)ghost/core/core/frontend/web/middleware/serve-indexnow-key.js (2)
settingsCache(23-23)labs(24-24)
🪛 Gitleaks (8.30.0)
ghost/core/test/unit/server/services/indexnow.test.js
[high] 418-418: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: Legacy tests (Node 22.18.0, sqlite3)
- GitHub Check: Legacy tests (Node 22.18.0, mysql8)
- GitHub Check: Ghost-CLI tests
- GitHub Check: Acceptance tests (Node 22.18.0, mysql8)
- GitHub Check: Admin-X Settings tests
- GitHub Check: Unit tests (Node 22.18.0)
- GitHub Check: Lint
- GitHub Check: Cursor Bugbot
- GitHub Check: Build & Push Docker Image
🔇 Additional comments (21)
ghost/core/core/server/api/endpoints/utils/serializers/input/settings.js (1)
81-82: LGTM! Settings list properly extended.The addition of
indexnow_api_keyto the editable settings list correctly aligns with the new IndexNow integration. The setting is defined in the schema and properly restricted to the editable allowlist.ghost/core/core/shared/labs.js (1)
59-60: LGTM! Feature flag properly added.The
indexnowfeature flag is correctly added toPRIVATE_FEATURES, following the established pattern for developer experiment features. This enables the IndexNow integration to be gated behind the labs flag.apps/admin-x-settings/src/components/settings/advanced/labs/private-features.tsx (2)
70-74: LGTM! IndexNow feature properly added to UI.The feature entry is well-defined with a clear description that explains the benefit to users. The flag name matches the backend configuration.
107-107: LGTM! Key prop correctly added.Adding the
keyprop usingfeature.flagfollows React best practices for list rendering and ensures proper component reconciliation.ghost/core/core/frontend/web/middleware/index.js (1)
8-8: LGTM! Middleware export properly added.The
serveIndexNowKeymiddleware is correctly exported following the existing module pattern.ghost/core/core/server/data/schema/default-settings/default-settings.json (1)
641-645: LGTM! IndexNow setting properly defined.The
indexnow_api_keysetting is correctly added to the schema with appropriate type and default value. The structure follows the established pattern for API key settings.ghost/core/core/boot.js (2)
319-319: LGTM! Service correctly required.The IndexNow service is properly imported following the established pattern for other services.
366-366: LGTM! Service initialization properly integrated.The
indexnow.listen()call is correctly placed in thePromise.allblock alongside similar services (xmlrpc, slack), ensuring concurrent initialization during Ghost boot.ghost/core/core/frontend/web/middleware/serve-indexnow-key.js (1)
29-62: LGTM! Middleware logic is secure and correct.The implementation is well-designed:
- Regex pattern is appropriately restrictive (32-char hex only)
- Feature flag gating prevents unintended exposure
- Key matching is secure (exact match required)
- Path extraction logic is correct
- Response headers are appropriate (24-hour cache for a static resource)
- Proper delegation to next() for all non-matching cases
ghost/core/test/unit/server/services/indexnow.test.js (5)
1-35: Test setup is well-structured.The test file properly initializes stubs for events, settings, labs, and models, with sensible defaults. The use of
rewireto access private functions is appropriate for unit testing internal logic.
37-44: LGTM!The test correctly verifies that
listen()registers handlers for bothpost.publishedandpost.published.editedevents.
46-251: Comprehensive coverage of SEO change detection.The tests effectively validate the
indexnowListenerlogic:
- Correctly pings when SEO-relevant fields change
- Skips pinging during imports
- Skips pinging when no SEO fields have changed
- Tests individual field changes (title, slug, meta_description)
253-414: Thorough validation of ping() behavior.The tests cover all critical paths:
- Success responses (200, 202)
- Error handling (400, 422, 429)
- Conditional skipping (pages, default posts, labs flag, private sites)
- API key auto-generation
The retry pattern with
setTimeoutis acceptable for async test verification.
416-432: LGTM!The tests correctly verify
getApiKey()returns the stored key or null when absent.Note: The static analysis warning about line 418 containing a generic API key is a false positive—this is test data.
ghost/core/core/server/services/indexnow.js (7)
1-38: Excellent documentation.The header comment provides clear context about the IndexNow protocol, how it works, and references the middleware that serves the key file. Error message templates follow Ghost conventions.
40-51: LGTM!The default post slugs correctly identify Ghost's sample content, and the IndexNow endpoint is accurate.
53-89: Secure API key generation.The use of
crypto.randomBytes(16)provides 128 bits of entropy, which is appropriate for an API key. The key management logic correctly handles creation, retrieval, and persistence.
175-194: Well-designed change detection.The
hasSeoRelevantChangesfunction correctly identifies fields that impact search engine visibility and efficiently checks for changes usingArray.some().
196-214: LGTM!The listener correctly filters out import operations and non-SEO changes before pinging, preventing unnecessary API calls.
216-229: Proper event listener registration.The
removeListener().on()pattern prevents duplicate listeners iflisten()is called multiple times. Both publish and edit events are correctly handled.
231-234: Clean public API.The module correctly exports only the necessary public functions (
listenandgetApiKey), keeping internal implementation details private.
ce19ef8 to
528e2df
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
ghost/core/core/frontend/web/site.js (1)
117-118: Middleware ordering contradicts documented behavior.There is an inconsistency between the middleware placement and its documented behavior. The comment in
serve-indexnow-key.js(lines 17-18) states the middleware "runs after theme static assets, so legitimate .txt files in themes take precedence," but here the middleware is registered beforemw.staticTheme()at line 121.In Express, middleware executes in registration order, so the current placement gives IndexNow key files precedence over theme .txt files—opposite to what the documentation claims.
Recommendation: Either move
siteApp.use(mw.serveIndexNowKey)to after line 121 (aftermw.staticTheme()) to match the documented behavior, or update the comment inserve-indexnow-key.jsto reflect that IndexNow keys take precedence.Based on learnings, this issue was already flagged in a previous review.
🧹 Nitpick comments (1)
ghost/core/core/server/services/indexnow.js (1)
114-114: Consider using.includes()for better readability.The condition
defaultPostSlugs.indexOf(post.slug) > -1works correctly but could be modernized todefaultPostSlugs.includes(post.slug)for improved readability.🔎 Proposed refactor
- if (defaultPostSlugs.indexOf(post.slug) > -1) { + if (defaultPostSlugs.includes(post.slug)) { return; }
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
apps/admin-x-settings/src/components/settings/advanced/labs/private-features.tsxghost/core/core/boot.jsghost/core/core/frontend/web/middleware/index.jsghost/core/core/frontend/web/middleware/serve-indexnow-key.jsghost/core/core/frontend/web/site.jsghost/core/core/server/api/endpoints/utils/serializers/input/settings.jsghost/core/core/server/data/migrations/versions/6.11/2025-12-27-11-17-40-add-indexnow-api-key-setting.jsghost/core/core/server/data/schema/default-settings/default-settings.jsonghost/core/core/server/services/indexnow.jsghost/core/core/shared/labs.jsghost/core/test/unit/server/data/schema/integrity.test.jsghost/core/test/unit/server/services/indexnow.test.js
🚧 Files skipped from review as they are similar to previous changes (5)
- ghost/core/core/frontend/web/middleware/index.js
- ghost/core/test/unit/server/services/indexnow.test.js
- ghost/core/core/frontend/web/middleware/serve-indexnow-key.js
- ghost/core/core/shared/labs.js
- ghost/core/core/server/api/endpoints/utils/serializers/input/settings.js
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-04-23T15:44:52.549Z
Learnt from: 9larsons
Repo: TryGhost/Ghost PR: 21866
File: ghost/core/core/server/web/admin/middleware/serve-auth-frame-file.js:10-19
Timestamp: 2025-04-23T15:44:52.549Z
Learning: The existing implementation in `ghost/core/core/server/web/admin/middleware/serve-auth-frame-file.js` using `path.parse(req.url).base` is secure against path traversal attacks as it properly extracts only the filename component without any directory parts.
Applied to files:
ghost/core/core/frontend/web/site.js
📚 Learning: 2025-05-29T08:34:28.248Z
Learnt from: ErisDS
Repo: TryGhost/Ghost PR: 0
File: :0-0
Timestamp: 2025-05-29T08:34:28.248Z
Learning: When examining corrected code after a user fixes indentation issues, the proper approach is 4-space indentation for conditional statements and consistent alignment of nested code blocks within the Ghost codebase middleware files.
Applied to files:
ghost/core/core/frontend/web/site.js
📚 Learning: 2025-10-09T15:31:06.587Z
Learnt from: kevinansfield
Repo: TryGhost/Ghost PR: 25118
File: apps/portal/src/actions.js:160-173
Timestamp: 2025-10-09T15:31:06.587Z
Learning: When reviewing PRs that introduce feature-flagged changes (e.g., `labs?.membersSigninOTCAlpha`), avoid suggesting modifications to non-flagged code paths unless they're directly related to the PR's objectives. Keep the scope focused on the feature-flag-specific changes only.
Applied to files:
apps/admin-x-settings/src/components/settings/advanced/labs/private-features.tsx
📚 Learning: 2025-06-10T11:07:10.800Z
Learnt from: kevinansfield
Repo: TryGhost/Ghost PR: 23770
File: apps/admin-x-settings/src/components/settings/email/newsletters/NewsletterDetailModalLabs.tsx:435-450
Timestamp: 2025-06-10T11:07:10.800Z
Learning: In Ghost's newsletter customization features, when promoting a feature from alpha to beta status, the feature flag guards are updated to make the feature available under both the `emailCustomization` (beta) and `emailCustomizationAlpha` (alpha) flags. This is done by either removing conditional guards entirely when the component is already behind both flags, or by updating conditionals to check for either flag.
Applied to files:
apps/admin-x-settings/src/components/settings/advanced/labs/private-features.tsx
🧬 Code graph analysis (3)
ghost/core/core/server/services/indexnow.js (1)
ghost/core/core/frontend/web/middleware/serve-indexnow-key.js (2)
settingsCache(23-23)labs(24-24)
apps/admin-x-settings/src/components/settings/advanced/labs/private-features.tsx (1)
ghost/admin/app/services/feature.js (1)
feature(9-39)
ghost/core/core/boot.js (2)
ghost/core/test/unit/server/services/indexnow.test.js (1)
indexnow(6-6)ghost/core/core/server/data/migrations/versions/6.11/2025-12-27-11-17-40-add-indexnow-api-key-setting.js (1)
require(1-1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Admin-X Settings tests
- GitHub Check: Ghost-CLI tests
- GitHub Check: Legacy tests (Node 22.18.0, mysql8)
- GitHub Check: Legacy tests (Node 22.18.0, sqlite3)
- GitHub Check: Unit tests (Node 22.18.0)
- GitHub Check: Build & Push Docker Image
- GitHub Check: Cursor Bugbot
🔇 Additional comments (16)
apps/admin-x-settings/src/components/settings/advanced/labs/private-features.tsx (2)
70-74: LGTM! IndexNow feature entry is well-defined.The new feature entry follows the established pattern and provides a clear description of the IndexNow functionality. The flag name 'indexnow' aligns with the backend implementation.
107-107: Good addition of React key prop.Adding
key={feature.flag}to the mapped LabItem components follows React best practices for list rendering and ensures proper component reconciliation.ghost/core/core/server/data/migrations/versions/6.11/2025-12-27-11-17-40-add-indexnow-api-key-setting.js (1)
1-8: LGTM! Migration is correctly structured.The migration properly uses the
addSettingutility to add theindexnow_api_keysetting with appropriate defaults. Thenullinitial value aligns with the service's lazy generation pattern.ghost/core/test/unit/server/data/schema/integrity.test.js (1)
40-40: LGTM! Hash updated correctly for settings changes.The settings hash update reflects the addition of the
indexnow_api_keysetting indefault-settings.json. This is a standard maintenance update for the integrity test.ghost/core/core/server/data/schema/default-settings/default-settings.json (1)
640-646: LGTM! IndexNow settings schema is correctly defined.The new
indexnowsection follows the established schema patterns. ThedefaultValue: nullis appropriate since the API key is generated lazily on first use. Thetype: "string"matches the 32-character hex key format.ghost/core/core/boot.js (2)
319-319: LGTM! Service loading follows established patterns.The
indexnowservice is loaded consistently with other notification services in the boot sequence.
366-366: LGTM! Service initialization follows the established pattern.The
indexnow.listen()call is correctly placed in thePromise.allblock alongside similar services likexmlrpc.listen()andslack.listen(), ensuring the IndexNow event listeners are registered during boot.ghost/core/core/server/services/indexnow.js (9)
1-51: Well-documented service with clear architecture.The service documentation clearly explains the IndexNow protocol and integration flow. The default post slug filtering is a sensible optimization to avoid pinging for Ghost's example content.
57-59: LGTM! Cryptographically secure key generation.The
generateApiKeyfunction usescrypto.randomBytes(16).toString('hex')to generate a 32-character hexadecimal key, which is cryptographically secure and meets IndexNow requirements.
65-67: LGTM! Simple and correct settings retrieval.The function cleanly retrieves the API key from settings cache with appropriate null fallback.
74-89: LGTM! Proper lazy initialization with settings persistence.The function correctly implements lazy key generation with appropriate internal context and helpful logging. The use of
models.Settings.editwith{context: {internal: true}}is the right approach for system-generated settings.
95-173: Robust ping implementation with appropriate guards and error handling.The function includes comprehensive validation checks and properly handles different HTTP response codes according to IndexNow specifications. The 5-second timeout and error categorization (429, 422, others) are well-considered.
181-194: SEO field detection is well-designed.The function correctly identifies content and metadata changes that affect search engine indexing. The field selection is thoughtful and avoids unnecessary pings for non-SEO changes.
201-214: LGTM! Event listener with appropriate filtering.The listener correctly filters out import operations and non-SEO changes, preventing unnecessary IndexNow pings during bulk operations and minor edits.
219-229: LGTM! Proper event listener registration.The function correctly registers listeners for both new publications and edits to published posts. The defensive
removeListenercalls prevent duplicate listeners, which is important for service restarts or hot reloads.
231-234: LGTM! Clean public API surface.The exported functions
listenandgetApiKeyprovide the necessary public interface while keeping internal implementation details private.
There was a problem hiding this comment.
This PR is being reviewed by Cursor Bugbot
Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
528e2df to
3cbb0b8
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
ghost/core/core/server/services/indexnow.js (1)
74-89: Race condition in concurrent API key generation already flagged.The check-then-act pattern in
getOrCreateApiKey()can lead to multiple keys being generated concurrently, with the last write winning and earlier pings using invalid keyLocation URLs.
🧹 Nitpick comments (1)
ghost/core/core/server/services/indexnow.js (1)
114-116: Consider usingArray.includes()for clarity.The check
defaultPostSlugs.indexOf(post.slug) > -1works correctly but could be more readable asdefaultPostSlugs.includes(post.slug).🔎 Suggested refactor
- if (defaultPostSlugs.indexOf(post.slug) > -1) { + if (defaultPostSlugs.includes(post.slug)) { return; }
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
apps/admin-x-settings/src/components/settings/advanced/labs/private-features.tsxghost/core/core/boot.jsghost/core/core/frontend/web/middleware/index.jsghost/core/core/frontend/web/middleware/serve-indexnow-key.jsghost/core/core/frontend/web/site.jsghost/core/core/server/api/endpoints/utils/serializers/input/settings.jsghost/core/core/server/data/migrations/versions/6.10/2025-12-27-11-17-40-add-indexnow-api-key-setting.jsghost/core/core/server/data/schema/default-settings/default-settings.jsonghost/core/core/server/services/indexnow.jsghost/core/core/shared/labs.jsghost/core/test/unit/server/data/schema/integrity.test.jsghost/core/test/unit/server/services/indexnow.test.js
🚧 Files skipped from review as they are similar to previous changes (8)
- ghost/core/core/frontend/web/middleware/index.js
- ghost/core/core/boot.js
- ghost/core/core/frontend/web/middleware/serve-indexnow-key.js
- ghost/core/core/server/data/schema/default-settings/default-settings.json
- apps/admin-x-settings/src/components/settings/advanced/labs/private-features.tsx
- ghost/core/core/shared/labs.js
- ghost/core/core/frontend/web/site.js
- ghost/core/core/server/api/endpoints/utils/serializers/input/settings.js
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-03-13T09:00:20.205Z
Learnt from: mike182uk
Repo: TryGhost/Ghost PR: 22471
File: apps/admin-x-activitypub/src/utils/pending-activity.ts:13-71
Timestamp: 2025-03-13T09:00:20.205Z
Learning: The pending activity utilities in the Ghost ActivityPub module are covered by tests in the file `apps/admin-x-activitypub/test/unit/utils/pending-activity.ts`.
Applied to files:
ghost/core/test/unit/server/services/indexnow.test.js
📚 Learning: 2025-03-13T09:00:20.205Z
Learnt from: mike182uk
Repo: TryGhost/Ghost PR: 22471
File: apps/admin-x-activitypub/src/utils/pending-activity.ts:13-71
Timestamp: 2025-03-13T09:00:20.205Z
Learning: The pending activity utilities in Ghost's ActivityPub module are thoroughly tested in `apps/admin-x-activitypub/test/unit/utils/pending-activity.ts`, which covers `generatePendingActivityId`, `isPendingActivity`, and `generatePendingActivity` functions.
Applied to files:
ghost/core/test/unit/server/services/indexnow.test.js
🧬 Code graph analysis (2)
ghost/core/test/unit/server/services/indexnow.test.js (1)
ghost/core/core/server/services/indexnow.js (7)
events(32-32)settingsCache(30-30)labs(31-31)logging(28-28)models(33-33)key(75-75)key(119-119)
ghost/core/core/server/services/indexnow.js (2)
ghost/core/test/unit/server/services/indexnow.test.js (5)
settingsCache(8-8)labs(9-9)key(421-421)key(428-428)ping(254-254)ghost/core/core/frontend/web/middleware/serve-indexnow-key.js (2)
settingsCache(23-23)labs(24-24)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: Legacy tests (Node 22.18.0, sqlite3)
- GitHub Check: Admin-X Settings tests
- GitHub Check: Legacy tests (Node 22.18.0, mysql8)
- GitHub Check: Acceptance tests (Node 22.18.0, mysql8)
- GitHub Check: Ghost-CLI tests
- GitHub Check: Unit tests (Node 22.18.0)
- GitHub Check: Lint
- GitHub Check: Cursor Bugbot
- GitHub Check: Build & Push Docker Image
🔇 Additional comments (12)
ghost/core/test/unit/server/data/schema/integrity.test.js (1)
40-40: LGTM - Hash update reflects new default setting.The updated hash correctly reflects the addition of the
indexnow_api_keysetting to default settings, as expected by this integrity test.ghost/core/core/server/data/migrations/versions/6.10/2025-12-27-11-17-40-add-indexnow-api-key-setting.js (1)
1-8: LGTM - Migration follows standard pattern.The migration correctly adds the
indexnow_api_keysetting with appropriate parameters (null initial value, string type, indexnow group). This aligns with the default settings definition and the lazy key generation approach in the service.ghost/core/test/unit/server/services/indexnow.test.js (4)
1-35: LGTM - Test setup is well-structured.The test suite properly stubs dependencies (events, settings, labs, models, logging) and provides sensible defaults in
beforeEach, with proper cleanup inafterEach.
37-44: LGTM - Event registration test is correct.The test properly verifies that
listen()registers handlers for bothpost.publishedandpost.published.editedevents.
46-251: LGTM - Comprehensive listener behavior tests.The test suite thoroughly covers the listener's decision logic: calling
ping()when SEO-relevant fields change, skipping during imports, and avoiding unnecessary pings when only non-SEO fields change. The mock model structure appropriately simulates the Bookshelf model interface.
253-431: LGTM - Thorough coverage of ping behavior and error handling.The tests comprehensively cover:
- Successful pings (200/202 responses)
- Validation logic (pages, default posts, privacy, feature flags)
- Auto-generation and persistence of API key
- Error scenarios (400, 422, 429) with appropriate logging
The retry pattern using
setTimeoutin the async tests (lines 347-354, 366-373, etc.) is unconventional but functional for verifying async operations complete.ghost/core/core/server/services/indexnow.js (6)
1-52: LGTM - Well-documented service structure.The header documentation clearly explains the IndexNow protocol and integration approach. Imports are appropriate, error messages are templated, and the default post slugs list covers standard Ghost posts.
57-67: LGTM - API key generation and retrieval are secure and correct.
generateApiKey()correctly usescrypto.randomBytes(16)to produce a cryptographically secure 32-character hex string.getApiKey()appropriately returns null when the setting is not configured.
118-145: LGTM - Request construction and success handling are correct.The IndexNow request is properly constructed with required parameters (
url,key,keyLocation), uses an appropriate 5-second timeout, and correctly handles success responses (200 and 202 per the IndexNow specification).
146-172: LGTM - Error handling appropriately categorizes failures.The error handling correctly maps HTTP status codes to Ghost error types (429 → TooManyRequestsError, 422 → ValidationError), provides helpful context, and logs errors without re-throwing (fire-and-forget pattern is appropriate for this notification service).
201-214: LGTM - Listener appropriately filters events and fires ping asynchronously.The listener correctly:
- Skips import operations to avoid spam
- Checks for SEO-relevant changes to prevent unnecessary pings
- Calls
ping()without awaiting (fire-and-forget pattern is appropriate for this background notification service)
219-234: LGTM - Event registration and exports are correct.The
listen()function properly removes existing listeners before registering new ones to prevent duplicates. The module exports are minimal and appropriate (listenfor initialization,getApiKeyfor middleware use).
3cbb0b8 to
46f3db3
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
ghost/core/core/frontend/web/site.js (1)
117-118: Middleware ordering inconsistency with documentation.This middleware is mounted before
mw.staticTheme()(line 121), giving IndexNow key files precedence over theme.txtfiles. However, the comment inserve-indexnow-key.js(lines 17-18) incorrectly states that "theme static assets take precedence."Either update the comment in
serve-indexnow-key.jsto accurately reflect that IndexNow keys take precedence, or move this middleware aftermw.staticTheme()if theme files should take precedence.ghost/core/core/frontend/web/middleware/serve-indexnow-key.js (1)
15-18: Documentation states incorrect middleware ordering.Lines 17-18 claim "The middleware runs after theme static assets, so legitimate .txt files in themes take precedence," but in
site.js(line 118) this middleware is registered beforemw.staticTheme()(line 121). This means IndexNow keys actually take precedence over theme files.Update the comment to reflect the actual behavior, or coordinate with the change in
site.jsto reorder the middleware if theme precedence is desired.
🧹 Nitpick comments (1)
ghost/core/test/unit/server/services/indexnow.test.js (1)
375-392: Consider using async/await instead of polling retry pattern.The polling pattern with
setTimeoutand recursiveretry()can introduce flakiness and slow down tests unnecessarily. Sinceping()is async, consider usingawaitdirectly:🔎 Suggested improvement
- it('should handle 202 response as success', function (done) { + it('should handle 202 response as success', async function () { loggingStub = sinon.stub(logging, 'info'); const pingRequest = nock('https://api.indexnow.org') .get(/\/indexnow/) .reply(202); const testPost = _.clone(testUtils.DataGenerator.Content.posts[2]); - ping(testPost); + await ping(testPost); - (function retry() { - if (pingRequest.isDone()) { - loggingStub.calledOnce.should.be.true(); - return done(); - } - - setTimeout(retry, 100); - }()); + pingRequest.isDone().should.be.true(); + loggingStub.calledOnce.should.be.true(); });The same pattern applies to the 400, 422, and 429 response tests (lines 394-450).
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
apps/admin-x-settings/src/components/settings/advanced/labs/private-features.tsxghost/core/core/boot.jsghost/core/core/frontend/web/middleware/index.jsghost/core/core/frontend/web/middleware/serve-indexnow-key.jsghost/core/core/frontend/web/site.jsghost/core/core/server/api/endpoints/utils/serializers/input/settings.jsghost/core/core/server/data/exporter/table-lists.jsghost/core/core/server/data/migrations/versions/6.10/2025-12-27-11-17-40-add-indexnow-api-key-setting.jsghost/core/core/server/data/schema/default-settings/default-settings.jsonghost/core/core/server/services/indexnow.jsghost/core/core/shared/labs.jsghost/core/test/unit/server/data/schema/integrity.test.jsghost/core/test/unit/server/services/indexnow.test.js
🚧 Files skipped from review as they are similar to previous changes (6)
- apps/admin-x-settings/src/components/settings/advanced/labs/private-features.tsx
- ghost/core/core/server/data/schema/default-settings/default-settings.json
- ghost/core/core/server/services/indexnow.js
- ghost/core/core/server/api/endpoints/utils/serializers/input/settings.js
- ghost/core/core/server/data/migrations/versions/6.10/2025-12-27-11-17-40-add-indexnow-api-key-setting.js
- ghost/core/core/boot.js
🧰 Additional context used
🧠 Learnings (8)
📚 Learning: 2025-11-25T14:28:50.351Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Ghost admin serves admin-x apps from `/ghost/assets/{app-name}/{app-name}.js` URLs
Applied to files:
ghost/core/core/frontend/web/middleware/index.js
📚 Learning: 2025-11-25T14:28:50.351Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Admin-x React apps build to `apps/*/dist` using Vite, which are then copied by `ghost/admin/lib/asset-delivery` to `ghost/core/core/built/admin/assets/*`
Applied to files:
ghost/core/core/frontend/web/middleware/index.js
📚 Learning: 2025-03-13T09:00:20.205Z
Learnt from: mike182uk
Repo: TryGhost/Ghost PR: 22471
File: apps/admin-x-activitypub/src/utils/pending-activity.ts:13-71
Timestamp: 2025-03-13T09:00:20.205Z
Learning: The pending activity utilities in the Ghost ActivityPub module are covered by tests in the file `apps/admin-x-activitypub/test/unit/utils/pending-activity.ts`.
Applied to files:
ghost/core/test/unit/server/services/indexnow.test.js
📚 Learning: 2025-03-13T09:00:20.205Z
Learnt from: mike182uk
Repo: TryGhost/Ghost PR: 22471
File: apps/admin-x-activitypub/src/utils/pending-activity.ts:13-71
Timestamp: 2025-03-13T09:00:20.205Z
Learning: The pending activity utilities in Ghost's ActivityPub module are thoroughly tested in `apps/admin-x-activitypub/test/unit/utils/pending-activity.ts`, which covers `generatePendingActivityId`, `isPendingActivity`, and `generatePendingActivity` functions.
Applied to files:
ghost/core/test/unit/server/services/indexnow.test.js
📚 Learning: 2025-11-25T14:28:50.351Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Run `yarn test:integration` for integration tests in ghost/core
Applied to files:
ghost/core/test/unit/server/services/indexnow.test.js
📚 Learning: 2025-05-29T07:45:35.714Z
Learnt from: ErisDS
Repo: TryGhost/Ghost PR: 23582
File: ghost/core/.c8rc.json:24-24
Timestamp: 2025-05-29T07:45:35.714Z
Learning: In Ghost project, app.js files under core/server/web are intentionally excluded from unit test coverage because they are not easily unit-testable due to being entry points with initialization code and side effects.
Applied to files:
ghost/core/test/unit/server/services/indexnow.test.js
📚 Learning: 2025-04-23T15:44:52.549Z
Learnt from: 9larsons
Repo: TryGhost/Ghost PR: 21866
File: ghost/core/core/server/web/admin/middleware/serve-auth-frame-file.js:10-19
Timestamp: 2025-04-23T15:44:52.549Z
Learning: The existing implementation in `ghost/core/core/server/web/admin/middleware/serve-auth-frame-file.js` using `path.parse(req.url).base` is secure against path traversal attacks as it properly extracts only the filename component without any directory parts.
Applied to files:
ghost/core/core/frontend/web/middleware/serve-indexnow-key.jsghost/core/core/frontend/web/site.js
📚 Learning: 2025-05-29T08:34:28.248Z
Learnt from: ErisDS
Repo: TryGhost/Ghost PR: 0
File: :0-0
Timestamp: 2025-05-29T08:34:28.248Z
Learning: When examining corrected code after a user fixes indentation issues, the proper approach is 4-space indentation for conditional statements and consistent alignment of nested code blocks within the Ghost codebase middleware files.
Applied to files:
ghost/core/core/frontend/web/site.js
🧬 Code graph analysis (2)
ghost/core/test/unit/server/services/indexnow.test.js (1)
ghost/core/core/server/services/indexnow.js (7)
events(32-32)settingsCache(30-30)labs(31-31)logging(28-28)models(33-33)key(75-75)key(132-132)
ghost/core/core/frontend/web/middleware/serve-indexnow-key.js (1)
ghost/core/core/server/services/indexnow.js (2)
settingsCache(30-30)labs(31-31)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: Admin-X Settings tests
- GitHub Check: Ghost-CLI tests
- GitHub Check: Legacy tests (Node 22.18.0, mysql8)
- GitHub Check: Acceptance tests (Node 22.18.0, mysql8)
- GitHub Check: Lint
- GitHub Check: Legacy tests (Node 22.18.0, sqlite3)
- GitHub Check: Unit tests (Node 22.18.0)
- GitHub Check: Cursor Bugbot
- GitHub Check: Build & Push Docker Image
🔇 Additional comments (6)
ghost/core/test/unit/server/data/schema/integrity.test.js (1)
40-40: Hash update correctly reflects new IndexNow setting.The updated
currentSettingsHashproperly accounts for the addition ofindexnow_api_keyto default settings. This is the expected maintenance step when modifyingdefault-settings.json.ghost/core/core/server/data/exporter/table-lists.js (1)
96-97: Correct addition of IndexNow API key to export blocklist.Adding
indexnow_api_keytoSETTING_KEYS_BLOCKLISTfollows the established pattern for sensitive, site-specific credentials that should not be included in export files. This is consistent with how other API keys (Stripe, etc.) are handled.ghost/core/core/shared/labs.js (1)
59-60: Feature flag correctly added to private features.Adding
indexnowtoPRIVATE_FEATURESis appropriate since it's an experimental feature gated behind developer experiments. This follows the established pattern for features under development.ghost/core/test/unit/server/services/indexnow.test.js (1)
1-50: Good test structure and comprehensive coverage.The test suite properly covers:
- Event listener initialization
- SEO-relevant field change detection
- Import skip behavior
- API key generation and persistence flows
- Various HTTP response codes
The use of
rewirefor accessing internal functions andnockfor HTTP mocking follows good testing practices.ghost/core/core/frontend/web/middleware/index.js (1)
8-8: Clean middleware export addition.The new export follows the established pattern and maintains consistent ordering with other middleware exports.
ghost/core/core/frontend/web/middleware/serve-indexnow-key.js (1)
29-62: Secure and well-implemented middleware.The implementation correctly:
- Validates the request path matches the exact 32-character hex pattern
- Checks the labs flag before processing
- Only serves the key when it exactly matches the configured value
- Uses appropriate cache headers for a verification file
The path extraction logic (
req.path.slice(1, -4)) correctly transforms/key.txttokey.
46f3db3 to
e68a467
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
ghost/core/core/server/services/indexnow.js (4)
108-109: Verify handling of 404 fallback URL from URL service.The
urlService.getUrlByResourceIdmethod may return a URL containing/404/when the post is not yet registered in the URL service. Other services in Ghost (e.g.,AudienceFeedbackService.buildLink) check for this pattern before using the URL.Run the following script to check how other services handle URLs from
getUrlByResourceId:#!/bin/bash # Search for patterns where getUrlByResourceId is followed by /404/ checks rg -nP -A5 'getUrlByResourceId' --type=js | rg -P '(\/404\/|404)' -A2 -B2
74-102: Critical: Race condition in API key generation.The
getOrCreateApiKey()function has a check-then-act race condition. When multiple posts are published concurrently before a key exists, each concurrent call checksgetApiKey()at line 75, finds null, generates different keys, and attempts to save them. The last write wins, causing earlier pings to usekeyLocationURLs with keys that no longer match the stored key.When IndexNow attempts to verify those keys, the middleware returns 404 because the keys don't match, resulting in silent ping failures during first-time concurrent publishes.
🔎 Suggested fix using a simple in-memory lock
+// Simple in-memory lock for key generation +let keyGenerationPromise = null; + async function getOrCreateApiKey() { let key = getApiKey(); if (!key) { + // If another call is already generating a key, wait for it + if (keyGenerationPromise) { + return await keyGenerationPromise; + } + + // Create a promise that other concurrent calls can wait on + keyGenerationPromise = (async () => { + try { + // Double-check after acquiring the lock + key = getApiKey(); + if (key) { + return key; + } + key = generateApiKey(); logging.info('IndexNow: Generated new API key'); const existingSetting = await models.Settings.findOne({key: 'indexnow_api_key'}); if (existingSetting) { await models.Settings.edit([{ key: 'indexnow_api_key', value: key }], {context: {internal: true}}); } else { await models.Settings.forge({ key: 'indexnow_api_key', value: key, type: 'string', group: 'indexnow' }).save(null, {context: {internal: true}}); } + return key; + } finally { + keyGenerationPromise = null; + } + })(); + + return await keyGenerationPromise; - key = generateApiKey(); - logging.info('IndexNow: Generated new API key'); - - const existingSetting = await models.Settings.findOne({key: 'indexnow_api_key'}); - - if (existingSetting) { - await models.Settings.edit([{ - key: 'indexnow_api_key', - value: key - }], {context: {internal: true}}); - } else { - await models.Settings.forge({ - key: 'indexnow_api_key', - value: key, - type: 'string', - group: 'indexnow' - }).save(null, {context: {internal: true}}); - } } return key; }
132-186: Critical: Unhandled promise rejection in async key creation.The
getOrCreateApiKey()call at line 132 is outside the try-catch block (lines 149-185), andindexnowListenercallsping()without await at line 226. IfgetOrCreateApiKey()throws an error during database operations, it will result in an unhandled promise rejection.🔎 Proposed fix to wrap key creation in try-catch
// Don't ping for the default posts if (defaultPostSlugs.indexOf(post.slug) > -1) { return; } + try { // Get or create the API key (auto-generates if not set) const key = await getOrCreateApiKey(); // Get the site URL for the keyLocation parameter const siteUrl = urlUtils.urlFor('home', true); // Build the IndexNow request URL const indexNowUrl = new URL(INDEXNOW_ENDPOINT); indexNowUrl.searchParams.set('url', url); indexNowUrl.searchParams.set('key', key); indexNowUrl.searchParams.set('keyLocation', `${siteUrl}${key}.txt`); const options = { timeout: { request: 5 * 1000 // 5 second timeout } }; - try { const response = await request(indexNowUrl.toString(), options); if (response.statusCode !== 200 && response.statusCode !== 202) { throw new errors.InternalServerError({ message: `IndexNow returned unexpected status: ${response.statusCode}` }); } logging.info(`IndexNow: Successfully pinged ${url}`); } catch (err) { let error; if (err.statusCode === 429) { error = new errors.TooManyRequestsError({ err, message: err.message, context: tpl(messages.requestFailedError, {service: 'IndexNow'}), help: tpl(messages.requestFailedHelp, {url: 'https://ghost.org/docs/'}) }); } else if (err.statusCode === 422) { // 422 means the URL is invalid or key doesn't match error = new errors.ValidationError({ err, message: 'IndexNow key validation failed', context: tpl(messages.requestFailedError, {service: 'IndexNow'}), help: 'Ensure your IndexNow API key file is accessible at the correct URL' }); } else { error = new errors.InternalServerError({ err: err, message: err.message, context: tpl(messages.requestFailedError, {service: 'IndexNow'}), help: tpl(messages.requestFailedHelp, {url: 'https://ghost.org/docs/'}) }); } logging.error(error); } }
135-141: Critical: URL construction may produce malformed keyLocation URL.The
keyLocationURL is constructed using direct string concatenation${siteUrl}${key}.txtwherekeydoesn't start with/. IfurlUtils.urlFor('home', true)returns a URL without a trailing slash (e.g.,https://example.com), the resulting URL would be malformed likehttps://example.comabc123...txtinstead ofhttps://example.com/abc123...txt.This would cause IndexNow key verification to fail completely, breaking the feature.
🔎 Suggested fix using urlJoin or proper path concatenation
// Get or create the API key (auto-generates if not set) const key = await getOrCreateApiKey(); // Get the site URL for the keyLocation parameter const siteUrl = urlUtils.urlFor('home', true); + // Ensure the keyLocation URL is properly formed + const keyLocation = urlUtils.urlJoin(siteUrl, `${key}.txt`); // Build the IndexNow request URL const indexNowUrl = new URL(INDEXNOW_ENDPOINT); indexNowUrl.searchParams.set('url', url); indexNowUrl.searchParams.set('key', key); - indexNowUrl.searchParams.set('keyLocation', `${siteUrl}${key}.txt`); + indexNowUrl.searchParams.set('keyLocation', keyLocation);
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
ghost/core/test/e2e-api/admin/__snapshots__/config.test.js.snapis excluded by!**/*.snap
📒 Files selected for processing (13)
apps/admin-x-settings/src/components/settings/advanced/labs/private-features.tsxghost/core/core/boot.jsghost/core/core/frontend/web/middleware/index.jsghost/core/core/frontend/web/middleware/serve-indexnow-key.jsghost/core/core/frontend/web/site.jsghost/core/core/server/api/endpoints/utils/serializers/input/settings.jsghost/core/core/server/data/exporter/table-lists.jsghost/core/core/server/data/migrations/versions/6.10/2025-12-27-11-17-40-add-indexnow-api-key-setting.jsghost/core/core/server/data/schema/default-settings/default-settings.jsonghost/core/core/server/services/indexnow.jsghost/core/core/shared/labs.jsghost/core/test/unit/server/data/schema/integrity.test.jsghost/core/test/unit/server/services/indexnow.test.js
🚧 Files skipped from review as they are similar to previous changes (7)
- ghost/core/core/boot.js
- ghost/core/core/server/data/exporter/table-lists.js
- ghost/core/core/server/data/schema/default-settings/default-settings.json
- ghost/core/core/frontend/web/site.js
- ghost/core/core/frontend/web/middleware/index.js
- ghost/core/core/frontend/web/middleware/serve-indexnow-key.js
- ghost/core/test/unit/server/data/schema/integrity.test.js
🧰 Additional context used
🧠 Learnings (7)
📚 Learning: 2025-06-10T11:07:10.800Z
Learnt from: kevinansfield
Repo: TryGhost/Ghost PR: 23770
File: apps/admin-x-settings/src/components/settings/email/newsletters/NewsletterDetailModalLabs.tsx:435-450
Timestamp: 2025-06-10T11:07:10.800Z
Learning: In Ghost's newsletter customization features, when promoting a feature from alpha to beta status, the feature flag guards are updated to make the feature available under both the `emailCustomization` (beta) and `emailCustomizationAlpha` (alpha) flags. This is done by either removing conditional guards entirely when the component is already behind both flags, or by updating conditionals to check for either flag.
Applied to files:
apps/admin-x-settings/src/components/settings/advanced/labs/private-features.tsx
📚 Learning: 2025-10-09T15:31:06.587Z
Learnt from: kevinansfield
Repo: TryGhost/Ghost PR: 25118
File: apps/portal/src/actions.js:160-173
Timestamp: 2025-10-09T15:31:06.587Z
Learning: When reviewing PRs that introduce feature-flagged changes (e.g., `labs?.membersSigninOTCAlpha`), avoid suggesting modifications to non-flagged code paths unless they're directly related to the PR's objectives. Keep the scope focused on the feature-flag-specific changes only.
Applied to files:
apps/admin-x-settings/src/components/settings/advanced/labs/private-features.tsx
📚 Learning: 2025-08-11T19:39:00.428Z
Learnt from: kevinansfield
Repo: TryGhost/Ghost PR: 24651
File: ghost/core/test/utils/urlUtils.js:53-57
Timestamp: 2025-08-11T19:39:00.428Z
Learning: In Ghost's test utilities, when fixing specific issues like async behavior, it's preferred to maintain existing error handling patterns (even if suboptimal) to keep PRs focused on their primary objective. Error handling improvements can be addressed in separate, dedicated PRs.
Applied to files:
ghost/core/core/server/services/indexnow.js
📚 Learning: 2025-03-13T09:00:20.205Z
Learnt from: mike182uk
Repo: TryGhost/Ghost PR: 22471
File: apps/admin-x-activitypub/src/utils/pending-activity.ts:13-71
Timestamp: 2025-03-13T09:00:20.205Z
Learning: The pending activity utilities in the Ghost ActivityPub module are covered by tests in the file `apps/admin-x-activitypub/test/unit/utils/pending-activity.ts`.
Applied to files:
ghost/core/test/unit/server/services/indexnow.test.js
📚 Learning: 2025-03-13T09:00:20.205Z
Learnt from: mike182uk
Repo: TryGhost/Ghost PR: 22471
File: apps/admin-x-activitypub/src/utils/pending-activity.ts:13-71
Timestamp: 2025-03-13T09:00:20.205Z
Learning: The pending activity utilities in Ghost's ActivityPub module are thoroughly tested in `apps/admin-x-activitypub/test/unit/utils/pending-activity.ts`, which covers `generatePendingActivityId`, `isPendingActivity`, and `generatePendingActivity` functions.
Applied to files:
ghost/core/test/unit/server/services/indexnow.test.js
📚 Learning: 2025-11-25T14:28:50.351Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Run `yarn test:integration` for integration tests in ghost/core
Applied to files:
ghost/core/test/unit/server/services/indexnow.test.js
📚 Learning: 2025-05-29T07:45:35.714Z
Learnt from: ErisDS
Repo: TryGhost/Ghost PR: 23582
File: ghost/core/.c8rc.json:24-24
Timestamp: 2025-05-29T07:45:35.714Z
Learning: In Ghost project, app.js files under core/server/web are intentionally excluded from unit test coverage because they are not easily unit-testable due to being entry points with initialization code and side effects.
Applied to files:
ghost/core/test/unit/server/services/indexnow.test.js
🧬 Code graph analysis (3)
apps/admin-x-settings/src/components/settings/advanced/labs/private-features.tsx (1)
ghost/admin/app/services/feature.js (1)
feature(9-39)
ghost/core/core/server/services/indexnow.js (2)
ghost/core/test/unit/server/services/indexnow.test.js (7)
logging(10-10)settingsCache(8-8)labs(9-9)events(7-7)models(11-11)key(458-458)key(465-465)ghost/core/core/frontend/web/middleware/serve-indexnow-key.js (2)
settingsCache(23-23)labs(24-24)
ghost/core/test/unit/server/services/indexnow.test.js (1)
ghost/core/core/server/services/indexnow.js (7)
events(32-32)settingsCache(30-30)labs(31-31)logging(28-28)models(33-33)key(75-75)key(132-132)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Build & Push Docker Image
- GitHub Check: Setup
- GitHub Check: Cursor Bugbot
🔇 Additional comments (6)
apps/admin-x-settings/src/components/settings/advanced/labs/private-features.tsx (2)
70-74: LGTM! IndexNow feature entry is well-structured.The new feature entry follows the established pattern and provides a clear description of the functionality.
107-107: Good fix for React list key requirement.Adding the
keyprop usingfeature.flagensures each LabItem has a stable, unique identifier, which is essential for React's reconciliation algorithm.ghost/core/core/server/api/endpoints/utils/serializers/input/settings.js (1)
81-82: LGTM! Setting properly added to editable list.The
indexnow_api_keyis correctly added toEDITABLE_SETTINGS, enabling it to be managed through the admin API.ghost/core/core/shared/labs.js (1)
59-60: LGTM! Feature flag properly registered.The
indexnowflag is correctly added toPRIVATE_FEATURES, making it available for toggling in the admin interface.ghost/core/core/server/data/migrations/versions/6.10/2025-12-27-11-17-40-add-indexnow-api-key-setting.js (1)
1-8: LGTM! Migration correctly adds the IndexNow API key setting.The migration follows the standard pattern and appropriately initializes the setting with a null value, allowing the service to generate the key on first use.
ghost/core/test/unit/server/services/indexnow.test.js (1)
1-469: LGTM! Comprehensive test coverage for the IndexNow service.The test suite thoroughly validates:
- Event registration and listener behavior
- SEO-relevant change detection logic
- Ping behavior with various scenarios (success, errors, rate limiting)
- API key generation and persistence flows
- Edge case handling (private sites, pages, default posts, labs flag)
The tests provide strong confidence in the service implementation.
e68a467 to
c7836a0
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (4)
ghost/core/core/frontend/web/site.js (1)
117-118: Middleware ordering gives IndexNow precedence over theme files.This middleware is mounted before
mw.staticTheme()(line 121), so IndexNow key files at/{key}.txtwill take precedence over any.txtfiles in themes. This is likely the correct behavior for the feature to work, but there's a documentation mismatch inserve-indexnow-key.jsthat claims theme files take precedence.Ensure the comment in
serve-indexnow-key.jsis updated to reflect the actual behavior, or reorder if theme files should genuinely take precedence.ghost/core/core/server/services/indexnow.js (3)
74-102: Race condition remains in API key generation.Concurrent post publishes before a key exists can each generate a different key, with the last write winning. Earlier pings would reference keys that no longer match the stored value, causing verification failures.
Consider using a mutex/lock pattern or moving key generation to boot time when IndexNow is enabled.
138-141: URL construction may produce malformed keyLocation.If
urlUtils.urlFor('home', true)returns a URL without a trailing slash, the keyLocation would be malformed (e.g.,https://example.comabc123.txt). Consider usingurlUtils.urlJoin()for safe concatenation:🔎 Suggested fix
- indexNowUrl.searchParams.set('keyLocation', `${siteUrl}${key}.txt`); + indexNowUrl.searchParams.set('keyLocation', urlUtils.urlJoin(siteUrl, `${key}.txt`));
108-114: Missing check for 404 fallback URL.
urlService.getUrlByResourceIdreturns a URL containing/404/when the post isn't yet registered. Similar toAudienceFeedbackService.buildLink, consider checking for this pattern before pinging:🔎 Suggested fix
async function ping(post) { const url = urlService.getUrlByResourceId(post.id, {absolute: true}); + // Skip if URL service hasn't registered this post yet + if (!url || url.includes('/404/')) { + return; + } + // Skip pages - only ping for posts if (post.type === 'page') { return;
🧹 Nitpick comments (1)
ghost/core/test/unit/server/services/indexnow.test.js (1)
375-392: Consider using async/await instead of polling retry pattern.The retry loop pattern is fragile and can lead to flaky tests. Since
ping()is async, await it directly and assert afterward:🔎 Suggested refactor
- it('should handle 202 response as success', function (done) { + it('should handle 202 response as success', async function () { loggingStub = sinon.stub(logging, 'info'); const pingRequest = nock('https://api.indexnow.org') .get(/\/indexnow/) .reply(202); const testPost = _.clone(testUtils.DataGenerator.Content.posts[2]); - ping(testPost); - - (function retry() { - if (pingRequest.isDone()) { - loggingStub.calledOnce.should.be.true(); - return done(); - } - - setTimeout(retry, 100); - }()); + await ping(testPost); + + pingRequest.isDone().should.be.true(); + loggingStub.calledOnce.should.be.true(); });The same pattern applies to the other polling tests (lines 394-411, 413-431, 433-450).
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
ghost/core/test/e2e-api/admin/__snapshots__/config.test.js.snapis excluded by!**/*.snap
📒 Files selected for processing (13)
apps/admin-x-settings/src/components/settings/advanced/labs/private-features.tsxghost/core/core/boot.jsghost/core/core/frontend/web/middleware/index.jsghost/core/core/frontend/web/middleware/serve-indexnow-key.jsghost/core/core/frontend/web/site.jsghost/core/core/server/api/endpoints/utils/serializers/input/settings.jsghost/core/core/server/data/exporter/table-lists.jsghost/core/core/server/data/migrations/versions/6.10/2025-12-27-11-17-40-add-indexnow-api-key-setting.jsghost/core/core/server/data/schema/default-settings/default-settings.jsonghost/core/core/server/services/indexnow.jsghost/core/core/shared/labs.jsghost/core/test/unit/server/data/schema/integrity.test.jsghost/core/test/unit/server/services/indexnow.test.js
🚧 Files skipped from review as they are similar to previous changes (8)
- ghost/core/test/unit/server/data/schema/integrity.test.js
- ghost/core/core/frontend/web/middleware/index.js
- ghost/core/core/server/data/migrations/versions/6.10/2025-12-27-11-17-40-add-indexnow-api-key-setting.js
- ghost/core/core/server/data/exporter/table-lists.js
- ghost/core/core/frontend/web/middleware/serve-indexnow-key.js
- ghost/core/core/shared/labs.js
- apps/admin-x-settings/src/components/settings/advanced/labs/private-features.tsx
- ghost/core/core/server/data/schema/default-settings/default-settings.json
🧰 Additional context used
🧠 Learnings (6)
📚 Learning: 2025-05-29T08:34:28.248Z
Learnt from: ErisDS
Repo: TryGhost/Ghost PR: 0
File: :0-0
Timestamp: 2025-05-29T08:34:28.248Z
Learning: When examining corrected code after a user fixes indentation issues, the proper approach is 4-space indentation for conditional statements and consistent alignment of nested code blocks within the Ghost codebase middleware files.
Applied to files:
ghost/core/core/frontend/web/site.js
📚 Learning: 2025-08-11T19:39:00.428Z
Learnt from: kevinansfield
Repo: TryGhost/Ghost PR: 24651
File: ghost/core/test/utils/urlUtils.js:53-57
Timestamp: 2025-08-11T19:39:00.428Z
Learning: In Ghost's test utilities, when fixing specific issues like async behavior, it's preferred to maintain existing error handling patterns (even if suboptimal) to keep PRs focused on their primary objective. Error handling improvements can be addressed in separate, dedicated PRs.
Applied to files:
ghost/core/core/server/services/indexnow.js
📚 Learning: 2025-03-13T09:00:20.205Z
Learnt from: mike182uk
Repo: TryGhost/Ghost PR: 22471
File: apps/admin-x-activitypub/src/utils/pending-activity.ts:13-71
Timestamp: 2025-03-13T09:00:20.205Z
Learning: The pending activity utilities in the Ghost ActivityPub module are covered by tests in the file `apps/admin-x-activitypub/test/unit/utils/pending-activity.ts`.
Applied to files:
ghost/core/test/unit/server/services/indexnow.test.js
📚 Learning: 2025-03-13T09:00:20.205Z
Learnt from: mike182uk
Repo: TryGhost/Ghost PR: 22471
File: apps/admin-x-activitypub/src/utils/pending-activity.ts:13-71
Timestamp: 2025-03-13T09:00:20.205Z
Learning: The pending activity utilities in Ghost's ActivityPub module are thoroughly tested in `apps/admin-x-activitypub/test/unit/utils/pending-activity.ts`, which covers `generatePendingActivityId`, `isPendingActivity`, and `generatePendingActivity` functions.
Applied to files:
ghost/core/test/unit/server/services/indexnow.test.js
📚 Learning: 2025-11-25T14:28:50.351Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Run `yarn test:integration` for integration tests in ghost/core
Applied to files:
ghost/core/test/unit/server/services/indexnow.test.js
📚 Learning: 2025-05-29T07:45:35.714Z
Learnt from: ErisDS
Repo: TryGhost/Ghost PR: 23582
File: ghost/core/.c8rc.json:24-24
Timestamp: 2025-05-29T07:45:35.714Z
Learning: In Ghost project, app.js files under core/server/web are intentionally excluded from unit test coverage because they are not easily unit-testable due to being entry points with initialization code and side effects.
Applied to files:
ghost/core/test/unit/server/services/indexnow.test.js
🧬 Code graph analysis (2)
ghost/core/test/unit/server/services/indexnow.test.js (1)
ghost/core/core/server/services/indexnow.js (7)
events(32-32)settingsCache(30-30)labs(31-31)logging(28-28)models(33-33)key(75-75)key(132-132)
ghost/core/core/boot.js (1)
ghost/core/test/unit/server/services/indexnow.test.js (1)
indexnow(6-6)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: Legacy tests (Node 22.18.0, sqlite3)
- GitHub Check: Ghost-CLI tests
- GitHub Check: Legacy tests (Node 22.18.0, mysql8)
- GitHub Check: Acceptance tests (Node 22.18.0, mysql8)
- GitHub Check: Acceptance tests (Node 22.18.0, sqlite3)
- GitHub Check: Admin-X Settings tests
- GitHub Check: i18n
- GitHub Check: Unit tests (Node 22.18.0)
- GitHub Check: Lint
- GitHub Check: Build & Push Docker Image
- GitHub Check: Cursor Bugbot
🔇 Additional comments (5)
ghost/core/core/boot.js (1)
319-319: LGTM!The IndexNow service integration follows the established pattern for event-based services in the boot sequence - requiring the module alongside similar services (xmlrpc, slack) and calling
listen()within thePromise.allblock. This is consistent with how other notification services are initialized.Also applies to: 366-366
ghost/core/test/unit/server/services/indexnow.test.js (1)
43-49: LGTM - good event listener verification.The test correctly verifies that
listen()registers handlers for bothpost.publishedandpost.published.editedevents. The assertion pattern is appropriate for verifying event wiring.ghost/core/core/server/services/indexnow.js (2)
226-228: Good error handling for fire-and-forget ping.The
.catch()handler properly logs errors from the async ping, preventing unhandled promise rejections. This addresses the fire-and-forget concern.
234-244: LGTM - clean event listener registration.Removing existing listeners before adding new ones prevents duplicate handlers on hot-reload scenarios. This is a good defensive pattern.
ghost/core/core/server/api/endpoints/utils/serializers/input/settings.js (1)
81-82: Theindexnow_api_keyaddition follows the same pattern as other API keys already inEDITABLE_SETTINGS(e.g.,stripe_secret_key,mailgun_api_key). The key can be either auto-generated or user-provided (default is null), making it consistent with credentials like Stripe and Mailgun that users manage directly. No additional concerns compared to existing API key fields.
c7836a0 to
2d54444
Compare
c65ab3d to
463c6bb
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
ghost/core/test/unit/server/services/indexnow.test.js (1)
375-392: Consider using async/await for consistency.These tests use a callback-based polling pattern while other tests in the file use async/await. This works but creates inconsistency. Consider awaiting the promise directly for cleaner code.
♻️ Optional refactor example
- it('should handle 202 response as success', function (done) { + it('should handle 202 response as success', async function () { loggingStub = sinon.stub(logging, 'info'); const pingRequest = nock('https://api.indexnow.org') .get(/\/indexnow/) .reply(202); const testPost = _.clone(testUtils.DataGenerator.Content.posts[2]); - ping(testPost); + await ping(testPost); - (function retry() { - if (pingRequest.isDone()) { - loggingStub.calledOnce.should.be.true(); - return done(); - } - - setTimeout(retry, 100); - }()); + pingRequest.isDone().should.be.true(); + loggingStub.calledOnce.should.be.true(); });
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
ghost/core/test/e2e-api/admin/__snapshots__/config.test.js.snapis excluded by!**/*.snap
📒 Files selected for processing (13)
apps/admin-x-settings/src/components/settings/advanced/labs/private-features.tsxghost/core/core/boot.jsghost/core/core/frontend/web/middleware/index.jsghost/core/core/frontend/web/middleware/serve-indexnow-key.jsghost/core/core/frontend/web/site.jsghost/core/core/server/api/endpoints/utils/serializers/input/settings.jsghost/core/core/server/data/exporter/table-lists.jsghost/core/core/server/data/migrations/versions/6.10/2025-12-27-11-17-40-add-indexnow-api-key-setting.jsghost/core/core/server/data/schema/default-settings/default-settings.jsonghost/core/core/server/services/indexnow.jsghost/core/core/shared/labs.jsghost/core/test/unit/server/data/schema/integrity.test.jsghost/core/test/unit/server/services/indexnow.test.js
🚧 Files skipped from review as they are similar to previous changes (9)
- apps/admin-x-settings/src/components/settings/advanced/labs/private-features.tsx
- ghost/core/core/frontend/web/middleware/serve-indexnow-key.js
- ghost/core/core/server/api/endpoints/utils/serializers/input/settings.js
- ghost/core/core/shared/labs.js
- ghost/core/test/unit/server/data/schema/integrity.test.js
- ghost/core/core/server/data/exporter/table-lists.js
- ghost/core/core/frontend/web/site.js
- ghost/core/core/frontend/web/middleware/index.js
- ghost/core/core/boot.js
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2025-03-13T09:00:20.205Z
Learnt from: mike182uk
Repo: TryGhost/Ghost PR: 22471
File: apps/admin-x-activitypub/src/utils/pending-activity.ts:13-71
Timestamp: 2025-03-13T09:00:20.205Z
Learning: The pending activity utilities in the Ghost ActivityPub module are covered by tests in the file `apps/admin-x-activitypub/test/unit/utils/pending-activity.ts`.
Applied to files:
ghost/core/test/unit/server/services/indexnow.test.js
📚 Learning: 2025-11-25T14:28:50.351Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Run `yarn test:integration` for integration tests in ghost/core
Applied to files:
ghost/core/test/unit/server/services/indexnow.test.js
📚 Learning: 2025-03-13T09:00:20.205Z
Learnt from: mike182uk
Repo: TryGhost/Ghost PR: 22471
File: apps/admin-x-activitypub/src/utils/pending-activity.ts:13-71
Timestamp: 2025-03-13T09:00:20.205Z
Learning: The pending activity utilities in Ghost's ActivityPub module are thoroughly tested in `apps/admin-x-activitypub/test/unit/utils/pending-activity.ts`, which covers `generatePendingActivityId`, `isPendingActivity`, and `generatePendingActivity` functions.
Applied to files:
ghost/core/test/unit/server/services/indexnow.test.js
📚 Learning: 2025-05-29T07:45:35.714Z
Learnt from: ErisDS
Repo: TryGhost/Ghost PR: 23582
File: ghost/core/.c8rc.json:24-24
Timestamp: 2025-05-29T07:45:35.714Z
Learning: In Ghost project, app.js files under core/server/web are intentionally excluded from unit test coverage because they are not easily unit-testable due to being entry points with initialization code and side effects.
Applied to files:
ghost/core/test/unit/server/services/indexnow.test.js
📚 Learning: 2025-08-11T19:39:00.428Z
Learnt from: kevinansfield
Repo: TryGhost/Ghost PR: 24651
File: ghost/core/test/utils/urlUtils.js:53-57
Timestamp: 2025-08-11T19:39:00.428Z
Learning: In Ghost's test utilities, when fixing specific issues like async behavior, it's preferred to maintain existing error handling patterns (even if suboptimal) to keep PRs focused on their primary objective. Error handling improvements can be addressed in separate, dedicated PRs.
Applied to files:
ghost/core/core/server/services/indexnow.js
🧬 Code graph analysis (2)
ghost/core/test/unit/server/services/indexnow.test.js (1)
ghost/core/core/server/services/indexnow.js (7)
events(32-32)settingsCache(30-30)labs(31-31)logging(28-28)models(33-33)key(75-75)key(133-133)
ghost/core/core/server/services/indexnow.js (2)
ghost/core/core/shared/labs.js (3)
require(135-135)settingsCache(13-13)labs(67-67)ghost/core/core/frontend/web/middleware/serve-indexnow-key.js (2)
settingsCache(25-25)labs(26-26)
🪛 Gitleaks (8.30.0)
ghost/core/test/unit/server/services/indexnow.test.js
[high] 455-455: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: Acceptance tests (Node 22.18.0, mysql8)
- GitHub Check: Admin-X Settings tests
- GitHub Check: Acceptance tests (Node 22.18.0, sqlite3)
- GitHub Check: Legacy tests (Node 22.18.0, sqlite3)
- GitHub Check: Unit tests (Node 22.18.0)
- GitHub Check: Ghost-CLI tests
- GitHub Check: Legacy tests (Node 22.18.0, mysql8)
- GitHub Check: Lint
- GitHub Check: Cursor Bugbot
- GitHub Check: Build & Push Docker Image
🔇 Additional comments (10)
ghost/core/core/server/data/schema/default-settings/default-settings.json (1)
640-646: LGTM!The new
indexnowsection follows the established pattern for settings in this file. Usingnullas the default is appropriate since the API key is auto-generated on first use by the IndexNow service.ghost/core/core/server/data/migrations/versions/6.10/2025-12-27-11-17-40-add-indexnow-api-key-setting.js (1)
1-8: LGTM!Clean migration following Ghost's standard pattern for adding new settings. The
addSettingutility handles the migration lifecycle properly.ghost/core/test/unit/server/services/indexnow.test.js (2)
1-11: LGTM!Comprehensive test setup with appropriate use of
rewirefor accessing private functions andnockfor HTTP mocking.
454-460: Static analysis false positive — test data, not a real key.The Gitleaks warning about "generic-api-key" is a false positive. The value
test-api-key-12345is clearly a test placeholder used to verify thegetApiKey()function returns the expected value from settings.ghost/core/core/server/services/indexnow.js (6)
1-22: Excellent documentation!The header comment provides clear context about the IndexNow protocol, how the integration works, and where to find related code. This will help future maintainers understand the feature quickly.
53-67: LGTM!Clean implementation using
crypto.randomBytesfor secure key generation. The 32-character hex string matches the IndexNow specification requirements.
69-102: LGTM!The key creation logic properly handles both cases: updating an existing null-valued setting (from migration) and creating a new setting if it doesn't exist. The author's clarification that key generation occurs at feature activation rather than during concurrent publishes addresses the earlier race condition concern.
159-186: LGTM!Robust error handling that gracefully degrades without disrupting the publishing flow. The specific handling for 422 (key validation) and 429 (rate limiting) provides useful context for debugging.
189-208: LGTM!Smart optimization to avoid spamming IndexNow with non-SEO-relevant edits. The field list covers all aspects that affect search engine results.
210-251: LGTM!The
.catch()handler on line 227 addresses the previous fire-and-forget concern. TheremoveListenercalls beforeonprevent duplicate handlers during development/hot-reload scenarios.
463c6bb to
010f24b
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @ghost/core/test/unit/server/services/indexnow.test.js:
- Around line 375-450: The tests using polling should be converted to
async/await: mark each spec as async, replace the setTimeout retry loop with a
simple await ping(testPost) call, then assert loggingStub.calledOnce (and
additional expectations like loggingStub.args[0][0].message.contains('key
validation failed') for the 422 case) and optionally check pingRequest.isDone()
for extra certainty; update the four tests that call ping(testPost) without
awaiting (the ones creating pingRequest and loggingStub) to use this pattern and
remove the retry closure.
🧹 Nitpick comments (1)
ghost/core/test/unit/server/services/indexnow.test.js (1)
116-160: Consider adding explicit tests formeta_titleandcanonical_urlchanges.The test at lines 116-160 includes
meta_titleandcanonical_urlin the list of SEO-relevant fields, but unliketitle,slug, andmeta_description(which have dedicated tests at lines 162-256), there are no explicit tests verifying that changes tometa_titleorcanonical_urlalone triggerping().📝 Suggested additional test cases
Add test cases similar to the existing ones for title/slug/meta_description:
it('calls ping() when meta_title changes', function () { const testPost = _.clone(testUtils.DataGenerator.Content.posts[2]); const testModel = { toJSON: function () { return testPost; }, get: function (key) { if (key === 'meta_title') { return 'New Meta Title'; } return 'same-value'; }, previous: function (key) { if (key === 'meta_title') { return 'Old Meta Title'; } return 'same-value'; } }; const pingStub = sinon.stub().resolves(); const resetIndexNow = indexnow.__set__('ping', pingStub); const listener = indexnow.__get__('indexnowListener'); listener(testModel); pingStub.calledOnce.should.be.true(); resetIndexNow(); }); it('calls ping() when canonical_url changes', function () { const testPost = _.clone(testUtils.DataGenerator.Content.posts[2]); const testModel = { toJSON: function () { return testPost; }, get: function (key) { if (key === 'canonical_url') { return 'https://example.com/new-url'; } return 'same-value'; }, previous: function (key) { if (key === 'canonical_url') { return 'https://example.com/old-url'; } return 'same-value'; } }; const pingStub = sinon.stub().resolves(); const resetIndexNow = indexnow.__set__('ping', pingStub); const listener = indexnow.__get__('indexnowListener'); listener(testModel); pingStub.calledOnce.should.be.true(); resetIndexNow(); });
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
ghost/core/test/e2e-api/admin/__snapshots__/config.test.js.snapis excluded by!**/*.snap
📒 Files selected for processing (13)
apps/admin-x-settings/src/components/settings/advanced/labs/private-features.tsxghost/core/core/boot.jsghost/core/core/frontend/web/middleware/index.jsghost/core/core/frontend/web/middleware/serve-indexnow-key.jsghost/core/core/frontend/web/site.jsghost/core/core/server/api/endpoints/utils/serializers/input/settings.jsghost/core/core/server/data/exporter/table-lists.jsghost/core/core/server/data/migrations/versions/6.10/2025-12-27-11-17-40-add-indexnow-api-key-setting.jsghost/core/core/server/data/schema/default-settings/default-settings.jsonghost/core/core/server/services/indexnow.jsghost/core/core/shared/labs.jsghost/core/test/unit/server/data/schema/integrity.test.jsghost/core/test/unit/server/services/indexnow.test.js
🚧 Files skipped from review as they are similar to previous changes (7)
- ghost/core/core/server/api/endpoints/utils/serializers/input/settings.js
- ghost/core/core/shared/labs.js
- ghost/core/core/server/data/exporter/table-lists.js
- ghost/core/core/frontend/web/middleware/serve-indexnow-key.js
- ghost/core/core/server/services/indexnow.js
- ghost/core/core/server/data/migrations/versions/6.10/2025-12-27-11-17-40-add-indexnow-api-key-setting.js
- ghost/core/core/frontend/web/middleware/index.js
🧰 Additional context used
🧠 Learnings (7)
📚 Learning: 2025-05-29T08:34:28.248Z
Learnt from: ErisDS
Repo: TryGhost/Ghost PR: 0
File: :0-0
Timestamp: 2025-05-29T08:34:28.248Z
Learning: When examining corrected code after a user fixes indentation issues, the proper approach is 4-space indentation for conditional statements and consistent alignment of nested code blocks within the Ghost codebase middleware files.
Applied to files:
ghost/core/core/frontend/web/site.js
📚 Learning: 2025-03-13T09:00:20.205Z
Learnt from: mike182uk
Repo: TryGhost/Ghost PR: 22471
File: apps/admin-x-activitypub/src/utils/pending-activity.ts:13-71
Timestamp: 2025-03-13T09:00:20.205Z
Learning: The pending activity utilities in the Ghost ActivityPub module are covered by tests in the file `apps/admin-x-activitypub/test/unit/utils/pending-activity.ts`.
Applied to files:
ghost/core/test/unit/server/services/indexnow.test.js
📚 Learning: 2025-11-25T14:28:50.351Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Run `yarn test:integration` for integration tests in ghost/core
Applied to files:
ghost/core/test/unit/server/services/indexnow.test.js
📚 Learning: 2025-03-13T09:00:20.205Z
Learnt from: mike182uk
Repo: TryGhost/Ghost PR: 22471
File: apps/admin-x-activitypub/src/utils/pending-activity.ts:13-71
Timestamp: 2025-03-13T09:00:20.205Z
Learning: The pending activity utilities in Ghost's ActivityPub module are thoroughly tested in `apps/admin-x-activitypub/test/unit/utils/pending-activity.ts`, which covers `generatePendingActivityId`, `isPendingActivity`, and `generatePendingActivity` functions.
Applied to files:
ghost/core/test/unit/server/services/indexnow.test.js
📚 Learning: 2025-05-29T07:45:35.714Z
Learnt from: ErisDS
Repo: TryGhost/Ghost PR: 23582
File: ghost/core/.c8rc.json:24-24
Timestamp: 2025-05-29T07:45:35.714Z
Learning: In Ghost project, app.js files under core/server/web are intentionally excluded from unit test coverage because they are not easily unit-testable due to being entry points with initialization code and side effects.
Applied to files:
ghost/core/test/unit/server/services/indexnow.test.js
📚 Learning: 2025-06-10T11:07:10.800Z
Learnt from: kevinansfield
Repo: TryGhost/Ghost PR: 23770
File: apps/admin-x-settings/src/components/settings/email/newsletters/NewsletterDetailModalLabs.tsx:435-450
Timestamp: 2025-06-10T11:07:10.800Z
Learning: In Ghost's newsletter customization features, when promoting a feature from alpha to beta status, the feature flag guards are updated to make the feature available under both the `emailCustomization` (beta) and `emailCustomizationAlpha` (alpha) flags. This is done by either removing conditional guards entirely when the component is already behind both flags, or by updating conditionals to check for either flag.
Applied to files:
apps/admin-x-settings/src/components/settings/advanced/labs/private-features.tsx
📚 Learning: 2025-10-09T15:31:06.587Z
Learnt from: kevinansfield
Repo: TryGhost/Ghost PR: 25118
File: apps/portal/src/actions.js:160-173
Timestamp: 2025-10-09T15:31:06.587Z
Learning: When reviewing PRs that introduce feature-flagged changes (e.g., `labs?.membersSigninOTCAlpha`), avoid suggesting modifications to non-flagged code paths unless they're directly related to the PR's objectives. Keep the scope focused on the feature-flag-specific changes only.
Applied to files:
apps/admin-x-settings/src/components/settings/advanced/labs/private-features.tsx
🧬 Code graph analysis (1)
apps/admin-x-settings/src/components/settings/advanced/labs/private-features.tsx (1)
ghost/admin/app/services/feature.js (1)
feature(9-39)
🪛 Gitleaks (8.30.0)
ghost/core/test/unit/server/services/indexnow.test.js
[high] 455-455: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: Ghost-CLI tests
- GitHub Check: Acceptance tests (Node 22.18.0, sqlite3)
- GitHub Check: Legacy tests (Node 22.18.0, mysql8)
- GitHub Check: Legacy tests (Node 22.18.0, sqlite3)
- GitHub Check: Acceptance tests (Node 22.18.0, mysql8)
- GitHub Check: Admin-X Settings tests
- GitHub Check: Lint
- GitHub Check: Unit tests (Node 22.18.0)
- GitHub Check: Cursor Bugbot
- GitHub Check: Build & Push Docker Image
🔇 Additional comments (10)
apps/admin-x-settings/src/components/settings/advanced/labs/private-features.tsx (2)
70-74: LGTM! Clear feature description for the IndexNow integration.The feature object follows the established pattern and provides a clear description of what IndexNow does. The description correctly highlights that it's for notifying search engines when content is published or updated.
107-107: Good fix: Adding the requiredkeyprop to list items.This resolves a React warning about missing keys in lists. The
key={feature.flag}is an appropriate choice since flag values are unique identifiers for each feature.ghost/core/core/server/data/schema/default-settings/default-settings.json (1)
641-645: LGTM! IndexNow API key setting structure is appropriate.The setting follows the established pattern for API keys in Ghost:
defaultValue: nullis correct since the key is auto-generated on first use (as mentioned in the PR summary)type: "string"is appropriate- No
PUBLICflag, which is correct for sensitive API keys- The setting is in its own
indexnowgroup, maintaining good organizationghost/core/test/unit/server/data/schema/integrity.test.js (1)
40-40: LGTM! Expected hash update for the new IndexNow setting.The
currentSettingsHashupdate is necessary and correct due to the addition of theindexnow_api_keysetting indefault-settings.json. This ensures the integrity test passes with the new default settings structure.ghost/core/core/boot.js (1)
318-318: No action required — error handling is already properly implemented.The
indexnow.listen()method is synchronous and only registers event listeners; it does not perform I/O or throw errors during initialization. All actual work happens asynchronously in event handlers, where errors are already properly caught and logged (lines 159-186 in the ping function and line 227 in the listener). This pattern is consistent with other similar services likeslack.listen()andwebhooks.listen().Likely an incorrect or invalid review comment.
ghost/core/core/frontend/web/site.js (1)
97-98: The middleware already validates the IndexNow labs flag. Lines 37-40 checkif (!labs.isSet('indexnow'))and callnext()to skip serving the key file when the feature is disabled. No action needed.Likely an incorrect or invalid review comment.
ghost/core/test/unit/server/services/indexnow.test.js (4)
1-41: LGTM!The test setup is well-structured with appropriate stubs for dependencies and sensible defaults. The use of rewire to access private functions for unit testing is appropriate here.
43-50: LGTM!The test correctly verifies that
listen()initializes the event handlers for both post publication events.
453-468: LGTM!The
getApiKey()tests provide good coverage of both the success and null cases.Note: The Gitleaks warning on line 455 is a false positive—
'test-api-key-12345'is clearly test fixture data.
324-373: Excellent coverage of API key generation scenarios.These tests thoughtfully cover both paths:
- Lines 324-348: Setting doesn't exist → uses
forge()to create a new record- Lines 350-373: Setting exists but is null → uses
edit()to updateThe regex validation at lines 343 and 366 (
/^[a-f0-9]{32}$/) ensures the generated key has the correct format. Good defensive testing.
010f24b to
96089f3
Compare
| value: null, | ||
| type: 'string', | ||
| group: 'indexnow' | ||
| }); |
There was a problem hiding this comment.
As database migrations have caused pain in the past, we generally prefer to create a separate PR/commit for database migrations, that contains minimal changes (example: https://github.com/TryGhost/Ghost/pull/15705/changes)
Would you mind moving the database changes to a separate PR, so that we can merge the database changes separately from the code changes?
There was a problem hiding this comment.
Makes perfect sense! Created #25808 for that
ref TryGhost#25773 This setting stores the IndexNow API key for search engine notification
ref TryGhost#25773 This setting stores the IndexNow API key for search engine notification
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @ghost/core/core/server/services/indexnow.js:
- Line 87: The call to urlService.getUrlByResourceId(post.id, {absolute: true})
may return null/undefined; update the logic around the const url =
urlService.getUrlByResourceId(...) assignment to guard against a falsy url
before passing it to the IndexNow submission code (e.g., skip processing this
post, log a warning, or return early). Ensure you reference
urlService.getUrlByResourceId and the subsequent IndexNow submission path so the
function safely handles missing URLs and never passes a null/undefined value to
the IndexNow call.
🧹 Nitpick comments (2)
ghost/core/core/frontend/web/middleware/serve-indexnow-key.js (1)
31-64: Consider using constant-time comparison for the API key.The direct string comparison at line 53 could be vulnerable to timing attacks, where an attacker measures response times to guess the API key character by character. While the risk is low given the 32-character hex key space (2^128 possibilities), using a constant-time comparison would eliminate this theoretical attack vector.
🔒 Proposed fix using crypto.timingSafeEqual
+const crypto = require('crypto'); + /** * Middleware to serve the IndexNow API key verification file */ function serveIndexNowKey(req, res, next) { // Only handle requests for .txt files at the root if (!req.path.match(/^\/[a-f0-9]{32}\.txt$/)) { return next(); } // Check if IndexNow is enabled if (!labs.isSet('indexnow')) { return next(); } const apiKey = settingsCache.get('indexnow_api_key'); // No key configured if (!apiKey) { return next(); } // Extract the requested key from the path (remove leading / and trailing .txt) const requestedKey = req.path.slice(1, -4); - // Only serve if the requested key matches the configured key - if (requestedKey !== apiKey) { - return next(); - } + // Only serve if the requested key matches the configured key (timing-safe comparison) + try { + const requestedKeyBuffer = Buffer.from(requestedKey); + const apiKeyBuffer = Buffer.from(apiKey); + + if (requestedKeyBuffer.length !== apiKeyBuffer.length || + !crypto.timingSafeEqual(requestedKeyBuffer, apiKeyBuffer)) { + return next(); + } + } catch (err) { + return next(); + } // Serve the key as plain text res.set({ 'Content-Type': 'text/plain', 'Cache-Control': 'public, max-age=86400' // Cache for 24 hours }); return res.send(apiKey); }ghost/core/core/server/services/indexnow.js (1)
82-84: Optional: Use.includes()for better readability.The
indexOf > -1pattern can be replaced with the more modern and readable.includes()method.♻️ Proposed refactor
- if (defaultPostSlugs.indexOf(post.slug) > -1) { + if (defaultPostSlugs.includes(post.slug)) { return; }
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
ghost/core/test/e2e-api/admin/__snapshots__/config.test.js.snapis excluded by!**/*.snap
📒 Files selected for processing (13)
apps/admin-x-settings/src/components/settings/advanced/labs/private-features.tsxghost/core/core/boot.jsghost/core/core/frontend/web/middleware/index.jsghost/core/core/frontend/web/middleware/serve-indexnow-key.jsghost/core/core/frontend/web/site.jsghost/core/core/server/api/endpoints/utils/serializers/input/settings.jsghost/core/core/server/data/exporter/table-lists.jsghost/core/core/server/data/schema/default-settings/default-settings.jsonghost/core/core/server/models/settings.jsghost/core/core/server/services/indexnow.jsghost/core/core/shared/labs.jsghost/core/test/unit/server/data/schema/integrity.test.jsghost/core/test/unit/server/services/indexnow.test.js
🚧 Files skipped from review as they are similar to previous changes (4)
- ghost/core/test/unit/server/services/indexnow.test.js
- ghost/core/core/frontend/web/site.js
- apps/admin-x-settings/src/components/settings/advanced/labs/private-features.tsx
- ghost/core/core/server/data/exporter/table-lists.js
🧰 Additional context used
🧠 Learnings (9)
📚 Learning: 2026-01-08T10:26:38.700Z
Learnt from: rob-ghost
Repo: TryGhost/Ghost PR: 25791
File: ghost/core/core/server/api/endpoints/member-comment-ban.js:64-68
Timestamp: 2026-01-08T10:26:38.700Z
Learning: In the Ghost API, endpoints rely on the serialization layer to prepare frame.data[docName] as a non-empty array before query() executes. Endpoints access frame.data[docName][0] directly (e.g., frame.data.comment_bans[0], frame.data.members[0], frame.data.posts[0]) without per-endpoint validation. This pattern is common across API endpoints. When maintaining or creating endpoints, avoid duplicating validation for frame.data[docName] and ensure the serializer guarantees the shape and non-emptiness. If you add a new endpoint that uses this frame.data[docName], follow the same assumption and avoid redundant checks unless there's a documented exception.
Applied to files:
ghost/core/core/boot.jsghost/core/core/server/api/endpoints/utils/serializers/input/settings.jsghost/core/test/unit/server/data/schema/integrity.test.jsghost/core/core/server/services/indexnow.jsghost/core/core/server/models/settings.jsghost/core/core/frontend/web/middleware/index.jsghost/core/core/frontend/web/middleware/serve-indexnow-key.jsghost/core/core/shared/labs.js
📚 Learning: 2025-10-07T12:19:15.174Z
Learnt from: kevinansfield
Repo: TryGhost/Ghost PR: 25031
File: ghost/core/test/utils/fixtures/config/defaults.json:68-80
Timestamp: 2025-10-07T12:19:15.174Z
Learning: In Ghost's configuration system (ghost/core/core/shared/config/), environment-specific config files (e.g., config.development.json, config.production.json) automatically fall back to values defined in defaults.json. It's only necessary to specify changed overrides on a per-env basis. Missing keys in env configs are not an issue—they're intentional and will use the default values.
Applied to files:
ghost/core/core/server/data/schema/default-settings/default-settings.json
📚 Learning: 2025-08-11T19:39:00.428Z
Learnt from: kevinansfield
Repo: TryGhost/Ghost PR: 24651
File: ghost/core/test/utils/urlUtils.js:53-57
Timestamp: 2025-08-11T19:39:00.428Z
Learning: In Ghost's test utilities, when fixing specific issues like async behavior, it's preferred to maintain existing error handling patterns (even if suboptimal) to keep PRs focused on their primary objective. Error handling improvements can be addressed in separate, dedicated PRs.
Applied to files:
ghost/core/core/server/services/indexnow.js
📚 Learning: 2025-10-09T08:10:18.956Z
Learnt from: rob-ghost
Repo: TryGhost/Ghost PR: 25075
File: ghost/admin/app/services/whats-new.js:77-83
Timestamp: 2025-10-09T08:10:18.956Z
Learning: In ghost/admin/app/services/whats-new.js, reusing `this._whatsNewSettings` (initialized once in `init()`) when persisting defaults for new users without `lastSeenDate` is acceptable. The team is aware that this means sequential logins by different admins in the same tab could share the service initialization timestamp, but this tradeoff is not a concern for their use case.
Applied to files:
ghost/core/core/server/services/indexnow.js
📚 Learning: 2025-11-25T14:28:50.351Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Ghost admin serves admin-x apps from `/ghost/assets/{app-name}/{app-name}.js` URLs
Applied to files:
ghost/core/core/frontend/web/middleware/index.js
📚 Learning: 2025-11-25T14:28:50.351Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Frontend dev servers and foundation libraries run on host machine during `yarn dev:forward`, while Ghost Core backend, MySQL, Redis, Mailpit, and Caddy run in Docker
Applied to files:
ghost/core/core/frontend/web/middleware/index.js
📚 Learning: 2025-11-25T14:28:50.351Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Admin-x React apps build to `apps/*/dist` using Vite, which are then copied by `ghost/admin/lib/asset-delivery` to `ghost/core/core/built/admin/assets/*`
Applied to files:
ghost/core/core/frontend/web/middleware/index.js
📚 Learning: 2025-05-29T08:34:28.248Z
Learnt from: ErisDS
Repo: TryGhost/Ghost PR: 0
File: :0-0
Timestamp: 2025-05-29T08:34:28.248Z
Learning: When examining corrected code after a user fixes indentation issues, the proper approach is 4-space indentation for conditional statements and consistent alignment of nested code blocks within the Ghost codebase middleware files.
Applied to files:
ghost/core/core/frontend/web/middleware/serve-indexnow-key.js
📚 Learning: 2025-04-23T15:44:52.549Z
Learnt from: 9larsons
Repo: TryGhost/Ghost PR: 21866
File: ghost/core/core/server/web/admin/middleware/serve-auth-frame-file.js:10-19
Timestamp: 2025-04-23T15:44:52.549Z
Learning: The existing implementation in `ghost/core/core/server/web/admin/middleware/serve-auth-frame-file.js` using `path.parse(req.url).base` is secure against path traversal attacks as it properly extracts only the filename component without any directory parts.
Applied to files:
ghost/core/core/frontend/web/middleware/serve-indexnow-key.js
🧬 Code graph analysis (3)
ghost/core/core/server/services/indexnow.js (1)
ghost/core/core/frontend/web/middleware/serve-indexnow-key.js (2)
settingsCache(25-25)labs(26-26)
ghost/core/core/server/models/settings.js (5)
ghost/core/test/unit/server/data/schema/integrity.test.js (1)
crypto(4-4)ghost/core/test/utils/fixtures/data-generator.js (1)
crypto(2-2)ghost/core/core/server/data/migrations/versions/6.3/2025-10-02-15-13-31-add-members-otc-secret-setting.js (1)
crypto(2-2)ghost/core/core/server/models/post.js (1)
crypto(3-3)ghost/core/core/server/services/settings/settings-utils.js (1)
crypto(1-1)
ghost/core/core/frontend/web/middleware/serve-indexnow-key.js (2)
ghost/core/core/server/services/indexnow.js (2)
settingsCache(29-29)labs(30-30)ghost/core/core/shared/labs.js (3)
settingsCache(13-13)require(135-135)labs(67-67)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cursor Bugbot
🔇 Additional comments (17)
ghost/core/core/server/api/endpoints/utils/serializers/input/settings.js (1)
81-82: LGTM!The addition of
explore_ping_growthandindexnow_api_keyto the EDITABLE_SETTINGS list follows the existing pattern and integrates properly with the new IndexNow feature.ghost/core/core/frontend/web/middleware/index.js (1)
7-7: LGTM!The middleware export follows the existing pattern and is properly positioned alphabetically.
ghost/core/core/server/data/schema/default-settings/default-settings.json (1)
641-646: LGTM!The IndexNow API key setting is properly configured with a null default value, which will be replaced by the dynamic default generator in the settings model. This pattern is consistent with other auto-generated secrets in the system.
ghost/core/test/unit/server/data/schema/integrity.test.js (1)
40-40: LGTM!The settings hash update correctly reflects the addition of the IndexNow configuration to the default settings schema.
ghost/core/core/server/models/settings.js (1)
63-64: The 32-character hexadecimal key generated bycrypto.randomBytes(16).toString('hex')complies with IndexNow specification requirements (8–128 characters, containing letters, digits, and hyphens). No changes needed.ghost/core/core/shared/labs.js (1)
59-60: LGTM!The feature flag additions follow the existing pattern and correctly extend the private features list for the IndexNow integration.
ghost/core/core/boot.js (2)
318-318: LGTM!The service require follows the established pattern and is correctly placed with other service dependencies.
364-364: LGTM!The
indexnow.listen()call follows the established pattern for event-driven services like Slack and webhooks, and is correctly placed in the concurrent initialization array.ghost/core/core/frontend/web/middleware/serve-indexnow-key.js (2)
1-23: LGTM!Excellent documentation that clearly explains the middleware's purpose, security considerations, and route collision handling.
25-26: LGTM!The required modules are appropriate for the middleware's needs and paths are correct.
ghost/core/core/server/services/indexnow.js (7)
1-50: LGTM!The documentation is comprehensive and clearly explains the IndexNow protocol. The constants and default post slugs are well-defined.
57-59: LGTM!Simple and correct implementation for retrieving the API key from settings.
156-169: LGTM!The SEO-relevant field detection is comprehensive and correctly uses the Bookshelf model API to detect changes. This provides good content-based deduplication to avoid spamming IndexNow.
176-192: LGTM!The listener correctly handles import scenarios and uses the SEO-relevant change detection to avoid unnecessary pings. Error handling is appropriate.
197-207: LGTM!The event listener registration correctly prevents duplicate listeners and covers both publish and edit scenarios.
209-212: LGTM!The module exports are correct and provide the necessary public API for boot-time wiring and key retrieval.
113-117: Status code handling is correct.The implementation properly accepts both 200 (OK) and 202 (Accepted) responses, which are documented as valid success statuses in the IndexNow API. The 202 status is typically returned on initial submissions during ownership validation.
| } | ||
|
|
||
| try { | ||
| const url = urlService.getUrlByResourceId(post.id, {absolute: true}); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add null check for URL retrieval.
The urlService.getUrlByResourceId() call could potentially return null or undefined if the URL service hasn't finished initializing or if the resource isn't found. This would result in passing an invalid URL to IndexNow.
🛡️ Proposed fix
try {
const url = urlService.getUrlByResourceId(post.id, {absolute: true});
+
+ if (!url) {
+ logging.warn(`IndexNow: URL not available for post ${post.id}`);
+ return;
+ }
// Get the API key (auto-generated on boot by settings service)
const key = getApiKey();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const url = urlService.getUrlByResourceId(post.id, {absolute: true}); | |
| const url = urlService.getUrlByResourceId(post.id, {absolute: true}); | |
| if (!url) { | |
| logging.warn(`IndexNow: URL not available for post ${post.id}`); | |
| return; | |
| } | |
| // Get the API key (auto-generated on boot by settings service) | |
| const key = getApiKey(); |
🤖 Prompt for AI Agents
In @ghost/core/core/server/services/indexnow.js at line 87, The call to
urlService.getUrlByResourceId(post.id, {absolute: true}) may return
null/undefined; update the logic around the const url =
urlService.getUrlByResourceId(...) assignment to guard against a falsy url
before passing it to the IndexNow submission code (e.g., skip processing this
post, log a warning, or return early). Ensure you reference
urlService.getUrlByResourceId and the subsequent IndexNow submission path so the
function safely handles missing URLs and never passes a null/undefined value to
the IndexNow call.
0b6264a to
7ae301c
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
ghost/core/core/server/services/indexnow.js (1)
82-84: Consider modernizing array check.Using
.includes()is the modern alternative toindexOf() > -1:♻️ Proposed refactor
- if (defaultPostSlugs.indexOf(post.slug) > -1) { + if (defaultPostSlugs.includes(post.slug)) { return; }
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
ghost/core/test/e2e-api/admin/__snapshots__/config.test.js.snapis excluded by!**/*.snap
📒 Files selected for processing (13)
apps/admin-x-settings/src/components/settings/advanced/labs/private-features.tsxghost/core/core/boot.jsghost/core/core/frontend/web/middleware/index.jsghost/core/core/frontend/web/middleware/serve-indexnow-key.jsghost/core/core/frontend/web/site.jsghost/core/core/server/api/endpoints/utils/serializers/input/settings.jsghost/core/core/server/data/exporter/table-lists.jsghost/core/core/server/data/schema/default-settings/default-settings.jsonghost/core/core/server/models/settings.jsghost/core/core/server/services/indexnow.jsghost/core/core/shared/labs.jsghost/core/test/unit/server/data/schema/integrity.test.jsghost/core/test/unit/server/services/indexnow.test.js
🚧 Files skipped from review as they are similar to previous changes (6)
- ghost/core/core/server/data/exporter/table-lists.js
- ghost/core/test/unit/server/services/indexnow.test.js
- ghost/core/core/server/api/endpoints/utils/serializers/input/settings.js
- ghost/core/core/frontend/web/middleware/index.js
- ghost/core/core/frontend/web/site.js
- ghost/core/test/unit/server/data/schema/integrity.test.js
🧰 Additional context used
🧠 Learnings (9)
📚 Learning: 2026-01-08T10:26:38.700Z
Learnt from: rob-ghost
Repo: TryGhost/Ghost PR: 25791
File: ghost/core/core/server/api/endpoints/member-comment-ban.js:64-68
Timestamp: 2026-01-08T10:26:38.700Z
Learning: In the Ghost API, endpoints rely on the serialization layer to prepare frame.data[docName] as a non-empty array before query() executes. Endpoints access frame.data[docName][0] directly (e.g., frame.data.comment_bans[0], frame.data.members[0], frame.data.posts[0]) without per-endpoint validation. This pattern is common across API endpoints. When maintaining or creating endpoints, avoid duplicating validation for frame.data[docName] and ensure the serializer guarantees the shape and non-emptiness. If you add a new endpoint that uses this frame.data[docName], follow the same assumption and avoid redundant checks unless there's a documented exception.
Applied to files:
ghost/core/core/boot.jsghost/core/core/frontend/web/middleware/serve-indexnow-key.jsghost/core/core/server/models/settings.jsghost/core/core/shared/labs.jsghost/core/core/server/services/indexnow.js
📚 Learning: 2025-05-29T08:34:28.248Z
Learnt from: ErisDS
Repo: TryGhost/Ghost PR: 0
File: :0-0
Timestamp: 2025-05-29T08:34:28.248Z
Learning: When examining corrected code after a user fixes indentation issues, the proper approach is 4-space indentation for conditional statements and consistent alignment of nested code blocks within the Ghost codebase middleware files.
Applied to files:
ghost/core/core/frontend/web/middleware/serve-indexnow-key.js
📚 Learning: 2025-04-23T15:44:52.549Z
Learnt from: 9larsons
Repo: TryGhost/Ghost PR: 21866
File: ghost/core/core/server/web/admin/middleware/serve-auth-frame-file.js:10-19
Timestamp: 2025-04-23T15:44:52.549Z
Learning: The existing implementation in `ghost/core/core/server/web/admin/middleware/serve-auth-frame-file.js` using `path.parse(req.url).base` is secure against path traversal attacks as it properly extracts only the filename component without any directory parts.
Applied to files:
ghost/core/core/frontend/web/middleware/serve-indexnow-key.js
📚 Learning: 2025-10-09T08:10:18.956Z
Learnt from: rob-ghost
Repo: TryGhost/Ghost PR: 25075
File: ghost/admin/app/services/whats-new.js:77-83
Timestamp: 2025-10-09T08:10:18.956Z
Learning: In ghost/admin/app/services/whats-new.js, reusing `this._whatsNewSettings` (initialized once in `init()`) when persisting defaults for new users without `lastSeenDate` is acceptable. The team is aware that this means sequential logins by different admins in the same tab could share the service initialization timestamp, but this tradeoff is not a concern for their use case.
Applied to files:
ghost/core/core/server/models/settings.jsghost/core/core/server/services/indexnow.js
📚 Learning: 2025-06-10T11:07:10.800Z
Learnt from: kevinansfield
Repo: TryGhost/Ghost PR: 23770
File: apps/admin-x-settings/src/components/settings/email/newsletters/NewsletterDetailModalLabs.tsx:435-450
Timestamp: 2025-06-10T11:07:10.800Z
Learning: In Ghost's newsletter customization features, when promoting a feature from alpha to beta status, the feature flag guards are updated to make the feature available under both the `emailCustomization` (beta) and `emailCustomizationAlpha` (alpha) flags. This is done by either removing conditional guards entirely when the component is already behind both flags, or by updating conditionals to check for either flag.
Applied to files:
ghost/core/core/shared/labs.jsapps/admin-x-settings/src/components/settings/advanced/labs/private-features.tsx
📚 Learning: 2025-08-11T19:39:00.428Z
Learnt from: kevinansfield
Repo: TryGhost/Ghost PR: 24651
File: ghost/core/test/utils/urlUtils.js:53-57
Timestamp: 2025-08-11T19:39:00.428Z
Learning: In Ghost's test utilities, when fixing specific issues like async behavior, it's preferred to maintain existing error handling patterns (even if suboptimal) to keep PRs focused on their primary objective. Error handling improvements can be addressed in separate, dedicated PRs.
Applied to files:
ghost/core/core/server/services/indexnow.js
📚 Learning: 2025-10-30T17:13:26.190Z
Learnt from: sam-lord
Repo: TryGhost/Ghost PR: 25303
File: ghost/core/core/server/services/email-service/BatchSendingService.js:19-19
Timestamp: 2025-10-30T17:13:26.190Z
Learning: In ghost/core/core/server/services/email-service/BatchSendingService.js and similar files in the Ghost codebase, prefer using `{...options}` spread syntax without explicit guards like `...(options || {})` when spreading potentially undefined objects, as the maintainer prefers cleaner syntax over defensive patterns when the behavior is safe.
Applied to files:
ghost/core/core/server/services/indexnow.js
📚 Learning: 2025-10-09T15:31:06.587Z
Learnt from: kevinansfield
Repo: TryGhost/Ghost PR: 25118
File: apps/portal/src/actions.js:160-173
Timestamp: 2025-10-09T15:31:06.587Z
Learning: When reviewing PRs that introduce feature-flagged changes (e.g., `labs?.membersSigninOTCAlpha`), avoid suggesting modifications to non-flagged code paths unless they're directly related to the PR's objectives. Keep the scope focused on the feature-flag-specific changes only.
Applied to files:
apps/admin-x-settings/src/components/settings/advanced/labs/private-features.tsx
📚 Learning: 2025-11-25T14:28:50.351Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Applies to ghost/i18n/locales/en/ghost.json : Add UI translations to `ghost/i18n/locales/en/ghost.json` for Admin UI features
Applied to files:
ghost/core/core/server/data/schema/default-settings/default-settings.json
🧬 Code graph analysis (5)
ghost/core/core/boot.js (1)
ghost/core/test/unit/server/services/indexnow.test.js (1)
indexnow(6-6)
ghost/core/core/frontend/web/middleware/serve-indexnow-key.js (2)
ghost/core/core/server/services/indexnow.js (2)
settingsCache(29-29)labs(30-30)ghost/core/core/shared/labs.js (3)
settingsCache(13-13)require(135-135)labs(67-67)
ghost/core/core/server/models/settings.js (5)
ghost/core/test/unit/server/data/schema/integrity.test.js (1)
crypto(4-4)ghost/core/test/utils/fixtures/data-generator.js (1)
crypto(2-2)ghost/core/core/server/data/migrations/versions/6.3/2025-10-02-15-13-31-add-members-otc-secret-setting.js (1)
crypto(2-2)ghost/core/core/server/models/post.js (1)
crypto(3-3)ghost/core/core/server/services/settings/settings-utils.js (1)
crypto(1-1)
ghost/core/core/server/services/indexnow.js (2)
ghost/core/core/frontend/web/site.js (2)
require(5-5)urlUtils(10-10)ghost/core/core/frontend/web/middleware/serve-indexnow-key.js (1)
settingsCache(25-25)
apps/admin-x-settings/src/components/settings/advanced/labs/private-features.tsx (1)
ghost/admin/app/services/feature.js (1)
feature(9-39)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: Legacy tests (Node 22.18.0, sqlite3)
- GitHub Check: Acceptance tests (Node 22.18.0, sqlite3)
- GitHub Check: Legacy tests (Node 22.18.0, mysql8)
- GitHub Check: Ghost-CLI tests
- GitHub Check: Admin-X Settings tests
- GitHub Check: Unit tests (Node 22.18.0)
- GitHub Check: Acceptance tests (Node 22.18.0, mysql8)
- GitHub Check: Lint
- GitHub Check: Cursor Bugbot
- GitHub Check: Build & Push Docker Image
🔇 Additional comments (15)
apps/admin-x-settings/src/components/settings/advanced/labs/private-features.tsx (2)
66-70: LGTM! IndexNow feature addition is well-structured.The feature definition follows the established pattern and correctly integrates with the labs feature flag system.
103-103: Good addition of React key prop.Using
feature.flagas the key is appropriate since feature flags are unique identifiers. This resolves the React list rendering requirement.ghost/core/core/server/data/schema/default-settings/default-settings.json (1)
640-646: LGTM! IndexNow settings section follows established patterns.The setting structure is consistent with other API key settings in the schema. The
defaultValue: nullis appropriate since the dynamic default is generated insettings.js.ghost/core/core/shared/labs.js (1)
58-60: LGTM! Private feature flags correctly registered.The
indexnowflag is appropriately added toPRIVATE_FEATURES, making it available for labs configuration and contributing to theWRITABLE_KEYS_ALLOWLIST.ghost/core/core/boot.js (2)
318-318: Service dependency properly loaded.The IndexNow service is correctly required alongside other Ghost services.
364-364: No changes required.The
listen()function is synchronous and only registers event listeners—it does not perform async operations or throw errors. Error handling is already implemented in theping()function (with comprehensive try-catch) and in the event listener's.catch()handler to prevent unhandled rejections. The function is safe to call withinPromise.all().Likely an incorrect or invalid review comment.
ghost/core/core/server/models/settings.js (1)
63-64: IndexNow API key format is compliant.The generated key using
crypto.randomBytes(16).toString('hex')produces 32 hexadecimal characters, which satisfies IndexNow's requirement of 8–128 characters using only lowercase a–z, uppercase A–Z, digits 0–9, and dashes.ghost/core/core/frontend/web/middleware/serve-indexnow-key.js (2)
31-56: LGTM! Secure key validation logic.The middleware correctly validates the request pattern, checks feature enablement, and performs exact key matching. The use of
req.pathwith regex validation prevents path traversal, and falling through tonext()for non-matches is the correct behavior.
57-64: LGTM! Appropriate response headers and caching.The 24-hour cache duration is sensible for a static verification file, and the plain text response format matches IndexNow protocol requirements.
ghost/core/core/server/services/indexnow.js (6)
100-117: LGTM! Correct URL construction and request handling.The use of
URL.searchParams.set()properly encodes parameters, the 5-second timeout is appropriate for a non-critical external service, and accepting both 200 and 202 status codes aligns with IndexNow protocol specifications.
120-147: LGTM! Comprehensive and appropriate error handling.The error handling correctly maps specific HTTP status codes to appropriate error types (429 → TooManyRequestsError, 422 → ValidationError), logs warnings without disrupting the publish flow, and provides helpful context messages.
156-169: LGTM! Appropriate SEO field detection.The selected fields (html, title, slug, meta_title, meta_description, canonical_url, status) comprehensively cover SEO-relevant changes that warrant notifying search engines.
176-192: LGTM! Proper event filtering and error handling.The listener correctly skips import events and non-SEO changes, preventing unnecessary IndexNow pings. The catch block appropriately handles already-logged errors without causing unhandled rejection warnings.
197-207: LGTM! Correct event listener registration.Removing listeners before adding them prevents duplicate registrations during hot-reload scenarios, and the chained
.removeListener().on()pattern is consistent with Ghost's event handling conventions.
86-94: Handle/404/URLs returned when post is unavailable in URL service.
urlService.getUrlByResourceIdnever returnsnullorundefined—it always returns a string. However, when a post is not yet available in the URL service (e.g., during rapid publish events or service initialization), it returns/404/. The current code doesn't check for this case before sending the URL to IndexNow, which would result in invalid requests. Add a check similar to the API key validation:const url = urlService.getUrlByResourceId(post.id, {absolute: true}); // Handle post not yet available in URL service if (url === '/404/') { logging.warn('IndexNow: Post URL not available'); return; }
There was a problem hiding this comment.
This PR is being reviewed by Cursor Bugbot
Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
83cc694 to
fecdfcd
Compare
IndexNow is a protocol that allows websites to notify search engines
(Bing, Yandex, Seznam.cz, Naver) about content changes for faster
indexing. This integration automatically pings IndexNow when posts are
published or updated, reducing the time for content to appear in search
results from days to minutes.
The feature is behind the 'indexnow' labs flag in developer experiments.
API keys are auto-generated on first use and the verification file is
served dynamically at /{key}.txt.
fecdfcd to
8d553bb
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
ghost/core/core/server/data/schema/default-settings/default-settings.json (1)
641-645: LGTM! Settings schema follows established patterns.The new
indexnow_api_keysetting definition is consistent with other settings in the schema. ThedefaultValue: nullis intentional since the PR description indicates the key is auto-generated on first use via dynamic defaults in the settings model.Optional: Consider adding validation constraints
If the API key format is strictly 32-character hex, you could add validation to enforce this:
"indexnow_api_key": { "defaultValue": null, + "validations": { + "isLength": { + "min": 32, + "max": 32 + } + }, "type": "string" }However, this may be unnecessary if the key is only generated internally and never edited by users.
ghost/core/test/unit/server/services/indexnow.test.js (1)
1-403: Excellent test coverage for the IndexNow integration!The test suite is comprehensive and well-structured, covering:
- Event registration via
listen()- SEO-relevant change detection in
listener()- Various gating conditions (labs flag, private sites, pages, default posts)
- HTTP response handling (200, 202, 400, 422, 429)
- API key retrieval and null handling
The use of stubs, nock, and rewire is appropriate for unit testing this service.
Note on static analysis warning: The Gitleaks warning on line 389 is a false positive—it's just a test string, not an actual API key.
Optional: Extract test model factory to reduce repetition
The test model setup with
get()andprevious()functions is repeated across multiple tests (lines 47-67, 84-94, 110-140, etc.). While the tests are clear as-is, you could extract a helper factory if you prefer DRY:function createTestModel(testPost, getCurrentValue, getPreviousValue) { return { toJSON: () => testPost, get: (key) => getCurrentValue(key), previous: (key) => getPreviousValue(key) }; }However, this is purely optional—explicit test setup often improves readability.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
ghost/core/test/e2e-api/admin/__snapshots__/config.test.js.snapis excluded by!**/*.snap
📒 Files selected for processing (13)
apps/admin-x-settings/src/components/settings/advanced/labs/private-features.tsxghost/core/core/boot.jsghost/core/core/frontend/web/middleware/index.jsghost/core/core/frontend/web/middleware/serve-indexnow-key.jsghost/core/core/frontend/web/site.jsghost/core/core/server/api/endpoints/utils/serializers/input/settings.jsghost/core/core/server/data/exporter/table-lists.jsghost/core/core/server/data/schema/default-settings/default-settings.jsonghost/core/core/server/models/settings.jsghost/core/core/server/services/indexnow.jsghost/core/core/shared/labs.jsghost/core/test/unit/server/data/schema/integrity.test.jsghost/core/test/unit/server/services/indexnow.test.js
🚧 Files skipped from review as they are similar to previous changes (9)
- ghost/core/core/server/services/indexnow.js
- ghost/core/core/server/data/exporter/table-lists.js
- ghost/core/test/unit/server/data/schema/integrity.test.js
- ghost/core/core/frontend/web/middleware/index.js
- ghost/core/core/server/models/settings.js
- ghost/core/core/server/api/endpoints/utils/serializers/input/settings.js
- apps/admin-x-settings/src/components/settings/advanced/labs/private-features.tsx
- ghost/core/core/frontend/web/middleware/serve-indexnow-key.js
- ghost/core/core/shared/labs.js
🧰 Additional context used
🧠 Learnings (12)
📓 Common learnings
Learnt from: kevinansfield
Repo: TryGhost/Ghost PR: 23770
File: apps/admin-x-settings/src/components/settings/email/newsletters/NewsletterDetailModalLabs.tsx:435-450
Timestamp: 2025-06-10T11:07:10.800Z
Learning: In Ghost's newsletter customization features, when promoting a feature from alpha to beta status, the feature flag guards are updated to make the feature available under both the `emailCustomization` (beta) and `emailCustomizationAlpha` (alpha) flags. This is done by either removing conditional guards entirely when the component is already behind both flags, or by updating conditionals to check for either flag.
📚 Learning: 2025-05-29T08:34:28.248Z
Learnt from: ErisDS
Repo: TryGhost/Ghost PR: 0
File: :0-0
Timestamp: 2025-05-29T08:34:28.248Z
Learning: When examining corrected code after a user fixes indentation issues, the proper approach is 4-space indentation for conditional statements and consistent alignment of nested code blocks within the Ghost codebase middleware files.
Applied to files:
ghost/core/core/frontend/web/site.js
📚 Learning: 2026-01-08T10:26:38.700Z
Learnt from: rob-ghost
Repo: TryGhost/Ghost PR: 25791
File: ghost/core/core/server/api/endpoints/member-comment-ban.js:64-68
Timestamp: 2026-01-08T10:26:38.700Z
Learning: In the Ghost API, endpoints rely on the serialization layer to prepare frame.data[docName] as a non-empty array before query() executes. Endpoints access frame.data[docName][0] directly (e.g., frame.data.comment_bans[0], frame.data.members[0], frame.data.posts[0]) without per-endpoint validation. This pattern is common across API endpoints. When maintaining or creating endpoints, avoid duplicating validation for frame.data[docName] and ensure the serializer guarantees the shape and non-emptiness. If you add a new endpoint that uses this frame.data[docName], follow the same assumption and avoid redundant checks unless there's a documented exception.
Applied to files:
ghost/core/core/frontend/web/site.jsghost/core/core/boot.jsghost/core/test/unit/server/services/indexnow.test.js
📚 Learning: 2025-11-25T14:28:50.351Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Applies to ghost/i18n/locales/en/ghost.json : Add UI translations to `ghost/i18n/locales/en/ghost.json` for Admin UI features
Applied to files:
ghost/core/core/server/data/schema/default-settings/default-settings.json
📚 Learning: 2025-03-13T09:00:20.205Z
Learnt from: mike182uk
Repo: TryGhost/Ghost PR: 22471
File: apps/admin-x-activitypub/src/utils/pending-activity.ts:13-71
Timestamp: 2025-03-13T09:00:20.205Z
Learning: The pending activity utilities in the Ghost ActivityPub module are covered by tests in the file `apps/admin-x-activitypub/test/unit/utils/pending-activity.ts`.
Applied to files:
ghost/core/test/unit/server/services/indexnow.test.js
📚 Learning: 2025-11-25T14:28:50.351Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Run `yarn test:integration` for integration tests in ghost/core
Applied to files:
ghost/core/test/unit/server/services/indexnow.test.js
📚 Learning: 2025-03-13T09:00:20.205Z
Learnt from: mike182uk
Repo: TryGhost/Ghost PR: 22471
File: apps/admin-x-activitypub/src/utils/pending-activity.ts:13-71
Timestamp: 2025-03-13T09:00:20.205Z
Learning: The pending activity utilities in Ghost's ActivityPub module are thoroughly tested in `apps/admin-x-activitypub/test/unit/utils/pending-activity.ts`, which covers `generatePendingActivityId`, `isPendingActivity`, and `generatePendingActivity` functions.
Applied to files:
ghost/core/test/unit/server/services/indexnow.test.js
📚 Learning: 2025-05-29T07:45:35.714Z
Learnt from: ErisDS
Repo: TryGhost/Ghost PR: 23582
File: ghost/core/.c8rc.json:24-24
Timestamp: 2025-05-29T07:45:35.714Z
Learning: In Ghost project, app.js files under core/server/web are intentionally excluded from unit test coverage because they are not easily unit-testable due to being entry points with initialization code and side effects.
Applied to files:
ghost/core/test/unit/server/services/indexnow.test.js
📚 Learning: 2025-08-11T19:39:00.428Z
Learnt from: kevinansfield
Repo: TryGhost/Ghost PR: 24651
File: ghost/core/test/utils/urlUtils.js:53-57
Timestamp: 2025-08-11T19:39:00.428Z
Learning: In Ghost's test utilities, when fixing specific issues like async behavior, it's preferred to maintain existing error handling patterns (even if suboptimal) to keep PRs focused on their primary objective. Error handling improvements can be addressed in separate, dedicated PRs.
Applied to files:
ghost/core/test/unit/server/services/indexnow.test.js
📚 Learning: 2025-11-24T17:29:43.865Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: e2e/AGENTS.md:0-0
Timestamp: 2025-11-24T17:29:43.865Z
Learning: Applies to e2e/**/*.test.ts : Use Playwright's auto-waiting capabilities and run tests multiple times to ensure stability
Applied to files:
ghost/core/test/unit/server/services/indexnow.test.js
📚 Learning: 2025-11-24T17:29:43.865Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: e2e/AGENTS.md:0-0
Timestamp: 2025-11-24T17:29:43.865Z
Learning: Applies to e2e/**/*.test.ts : Never use hard-coded waits like `waitForTimeout()`
Applied to files:
ghost/core/test/unit/server/services/indexnow.test.js
📚 Learning: 2025-11-10T11:30:41.316Z
Learnt from: rob-ghost
Repo: TryGhost/Ghost PR: 25357
File: apps/admin/test-utils/test-helpers.ts:6-12
Timestamp: 2025-11-10T11:30:41.316Z
Learning: In apps/admin/test-utils/test-helpers.ts, the waitForQuerySettled helper is intentionally designed to timeout for idle/disabled queries. It should only treat queries as settled when they reach a terminal state (isSuccess or isError) and are not fetching. This ensures tests properly wait for active queries to complete.
Applied to files:
ghost/core/test/unit/server/services/indexnow.test.js
🧬 Code graph analysis (1)
ghost/core/test/unit/server/services/indexnow.test.js (1)
ghost/core/core/server/services/indexnow.js (4)
settingsCache(29-29)labs(30-30)logging(27-27)key(90-90)
🪛 Gitleaks (8.30.0)
ghost/core/test/unit/server/services/indexnow.test.js
[high] 389-389: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: Admin-X Settings tests
- GitHub Check: Acceptance tests (Node 22.18.0, sqlite3)
- GitHub Check: Ghost-CLI tests
- GitHub Check: Legacy tests (Node 22.18.0, sqlite3)
- GitHub Check: Lint
- GitHub Check: Unit tests (Node 22.18.0)
- GitHub Check: Legacy tests (Node 22.18.0, mysql8)
- GitHub Check: i18n
- GitHub Check: Acceptance tests (Node 22.18.0, mysql8)
- GitHub Check: Build & Push Docker Image
- GitHub Check: Cursor Bugbot
🔇 Additional comments (2)
ghost/core/core/boot.js (1)
318-318: LGTM! IndexNow service properly integrated into boot sequence.The service import and initialization follow the established pattern for other services like Slack and webhooks. Adding
indexnow.listen()to the concurrent initialization array is appropriate for an event-driven service.Also applies to: 364-364
ghost/core/core/frontend/web/site.js (1)
97-98: LGTM! Middleware mounted in the correct position.The IndexNow key verification middleware is correctly positioned after internal apps and before theme static assets. This ensures the dynamic
/{key}.txtendpoint takes precedence over any theme files.
- Added indexnow_api_key to dynamicDefault in settings model - Removed getOrCreateApiKey logic from indexnow service - Added 'coming-soon' to default post slugs to skip - Separated database migration to PR TryGhost#25804
8d553bb to
bac2db7
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
ghost/core/test/unit/server/services/indexnow.test.js (1)
43-247: Comprehensive listener() test coverage.The tests thoroughly verify the listener behavior:
- Pings on SEO-relevant changes ✓
- Skips during imports ✓
- Deduplicates when no SEO fields changed ✓
- Individual field tests for title, slug, meta_description ✓
The test coverage effectively validates the deduplication logic and guard conditions.
💡 Optional: Consider additional field coverage
While the current tests cover the core fields (title, slug, meta_description), you could add similar tests for the remaining SEO-relevant fields:
meta_title,canonical_url, andstatustransitions. This would provide complete coverage of all fields checked inhasSeoRelevantChanges().ghost/core/core/server/services/indexnow.js (1)
96-148: Well-implemented HTTP ping with appropriate error handling.The function correctly:
- Constructs the IndexNow URL with required parameters (url, key, keyLocation) ✓
- Sets a reasonable 5-second timeout ✓
- Accepts both 200 and 202 as success (per IndexNow spec) ✓
- Handles specific error codes with appropriate error types ✓
- Logs errors as warnings without disrupting publishing ✓
♻️ Optional: ES6 shorthand syntax
Line 140 can use ES6 shorthand property notation:
} else { error = new errors.InternalServerError({ - err: err, + err, message: err.message, context: tpl(messages.requestFailedError, {service: 'IndexNow'}), help: tpl(messages.requestFailedHelp, {url: 'https://ghost.org/docs/'}) });
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
ghost/core/core/server/services/indexnow.jsghost/core/core/shared/labs.jsghost/core/test/unit/server/services/indexnow.test.js
🧰 Additional context used
🧠 Learnings (14)
📚 Learning: 2025-03-13T09:00:20.205Z
Learnt from: mike182uk
Repo: TryGhost/Ghost PR: 22471
File: apps/admin-x-activitypub/src/utils/pending-activity.ts:13-71
Timestamp: 2025-03-13T09:00:20.205Z
Learning: The pending activity utilities in the Ghost ActivityPub module are covered by tests in the file `apps/admin-x-activitypub/test/unit/utils/pending-activity.ts`.
Applied to files:
ghost/core/test/unit/server/services/indexnow.test.js
📚 Learning: 2025-11-25T14:28:50.351Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Run `yarn test:integration` for integration tests in ghost/core
Applied to files:
ghost/core/test/unit/server/services/indexnow.test.js
📚 Learning: 2025-03-13T09:00:20.205Z
Learnt from: mike182uk
Repo: TryGhost/Ghost PR: 22471
File: apps/admin-x-activitypub/src/utils/pending-activity.ts:13-71
Timestamp: 2025-03-13T09:00:20.205Z
Learning: The pending activity utilities in Ghost's ActivityPub module are thoroughly tested in `apps/admin-x-activitypub/test/unit/utils/pending-activity.ts`, which covers `generatePendingActivityId`, `isPendingActivity`, and `generatePendingActivity` functions.
Applied to files:
ghost/core/test/unit/server/services/indexnow.test.js
📚 Learning: 2025-05-29T07:45:35.714Z
Learnt from: ErisDS
Repo: TryGhost/Ghost PR: 23582
File: ghost/core/.c8rc.json:24-24
Timestamp: 2025-05-29T07:45:35.714Z
Learning: In Ghost project, app.js files under core/server/web are intentionally excluded from unit test coverage because they are not easily unit-testable due to being entry points with initialization code and side effects.
Applied to files:
ghost/core/test/unit/server/services/indexnow.test.js
📚 Learning: 2025-08-11T19:39:00.428Z
Learnt from: kevinansfield
Repo: TryGhost/Ghost PR: 24651
File: ghost/core/test/utils/urlUtils.js:53-57
Timestamp: 2025-08-11T19:39:00.428Z
Learning: In Ghost's test utilities, when fixing specific issues like async behavior, it's preferred to maintain existing error handling patterns (even if suboptimal) to keep PRs focused on their primary objective. Error handling improvements can be addressed in separate, dedicated PRs.
Applied to files:
ghost/core/test/unit/server/services/indexnow.test.jsghost/core/core/server/services/indexnow.js
📚 Learning: 2025-11-24T17:29:43.865Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: e2e/AGENTS.md:0-0
Timestamp: 2025-11-24T17:29:43.865Z
Learning: Applies to e2e/**/*.test.ts : Use Playwright's auto-waiting capabilities and run tests multiple times to ensure stability
Applied to files:
ghost/core/test/unit/server/services/indexnow.test.js
📚 Learning: 2025-11-24T17:29:43.865Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: e2e/AGENTS.md:0-0
Timestamp: 2025-11-24T17:29:43.865Z
Learning: Applies to e2e/**/*.test.ts : Never use hard-coded waits like `waitForTimeout()`
Applied to files:
ghost/core/test/unit/server/services/indexnow.test.js
📚 Learning: 2025-11-10T11:30:41.316Z
Learnt from: rob-ghost
Repo: TryGhost/Ghost PR: 25357
File: apps/admin/test-utils/test-helpers.ts:6-12
Timestamp: 2025-11-10T11:30:41.316Z
Learning: In apps/admin/test-utils/test-helpers.ts, the waitForQuerySettled helper is intentionally designed to timeout for idle/disabled queries. It should only treat queries as settled when they reach a terminal state (isSuccess or isError) and are not fetching. This ensures tests properly wait for active queries to complete.
Applied to files:
ghost/core/test/unit/server/services/indexnow.test.js
📚 Learning: 2026-01-08T10:26:38.700Z
Learnt from: rob-ghost
Repo: TryGhost/Ghost PR: 25791
File: ghost/core/core/server/api/endpoints/member-comment-ban.js:64-68
Timestamp: 2026-01-08T10:26:38.700Z
Learning: In the Ghost API, endpoints rely on the serialization layer to prepare frame.data[docName] as a non-empty array before query() executes. Endpoints access frame.data[docName][0] directly (e.g., frame.data.comment_bans[0], frame.data.members[0], frame.data.posts[0]) without per-endpoint validation. This pattern is common across API endpoints. When maintaining or creating endpoints, avoid duplicating validation for frame.data[docName] and ensure the serializer guarantees the shape and non-emptiness. If you add a new endpoint that uses this frame.data[docName], follow the same assumption and avoid redundant checks unless there's a documented exception.
Applied to files:
ghost/core/test/unit/server/services/indexnow.test.jsghost/core/core/shared/labs.jsghost/core/core/server/services/indexnow.js
📚 Learning: 2025-06-10T11:07:10.800Z
Learnt from: kevinansfield
Repo: TryGhost/Ghost PR: 23770
File: apps/admin-x-settings/src/components/settings/email/newsletters/NewsletterDetailModalLabs.tsx:435-450
Timestamp: 2025-06-10T11:07:10.800Z
Learning: In Ghost's newsletter customization features, when promoting a feature from alpha to beta status, the feature flag guards are updated to make the feature available under both the `emailCustomization` (beta) and `emailCustomizationAlpha` (alpha) flags. This is done by either removing conditional guards entirely when the component is already behind both flags, or by updating conditionals to check for either flag.
Applied to files:
ghost/core/core/shared/labs.js
📚 Learning: 2025-10-09T15:31:06.587Z
Learnt from: kevinansfield
Repo: TryGhost/Ghost PR: 25118
File: apps/portal/src/actions.js:160-173
Timestamp: 2025-10-09T15:31:06.587Z
Learning: When reviewing PRs that introduce feature-flagged changes (e.g., `labs?.membersSigninOTCAlpha`), avoid suggesting modifications to non-flagged code paths unless they're directly related to the PR's objectives. Keep the scope focused on the feature-flag-specific changes only.
Applied to files:
ghost/core/core/shared/labs.js
📚 Learning: 2025-06-13T11:57:58.226Z
Learnt from: kevinansfield
Repo: TryGhost/Ghost PR: 23824
File: ghost/core/core/server/services/email-service/email-templates/partials/styles.hbs:919-926
Timestamp: 2025-06-13T11:57:58.226Z
Learning: In `ghost/core/core/server/services/email-service/email-templates/partials/styles.hbs`, some style blocks (e.g., `.latest-post p` and `.latest-post p a`) still use the legacy colour `#73818c` on purpose; they are later overridden by `emailCustomization` feature rules, as noted by inline TODO comments. These occurrences should not be flagged as inconsistencies.
Applied to files:
ghost/core/core/shared/labs.js
📚 Learning: 2025-10-09T08:10:18.956Z
Learnt from: rob-ghost
Repo: TryGhost/Ghost PR: 25075
File: ghost/admin/app/services/whats-new.js:77-83
Timestamp: 2025-10-09T08:10:18.956Z
Learning: In ghost/admin/app/services/whats-new.js, reusing `this._whatsNewSettings` (initialized once in `init()`) when persisting defaults for new users without `lastSeenDate` is acceptable. The team is aware that this means sequential logins by different admins in the same tab could share the service initialization timestamp, but this tradeoff is not a concern for their use case.
Applied to files:
ghost/core/core/server/services/indexnow.js
📚 Learning: 2025-10-30T17:13:26.190Z
Learnt from: sam-lord
Repo: TryGhost/Ghost PR: 25303
File: ghost/core/core/server/services/email-service/BatchSendingService.js:19-19
Timestamp: 2025-10-30T17:13:26.190Z
Learning: In ghost/core/core/server/services/email-service/BatchSendingService.js and similar files in the Ghost codebase, prefer using `{...options}` spread syntax without explicit guards like `...(options || {})` when spreading potentially undefined objects, as the maintainer prefers cleaner syntax over defensive patterns when the behavior is safe.
Applied to files:
ghost/core/core/server/services/indexnow.js
🧬 Code graph analysis (2)
ghost/core/test/unit/server/services/indexnow.test.js (1)
ghost/core/core/server/services/indexnow.js (5)
events(31-31)settingsCache(29-29)labs(30-30)logging(27-27)key(90-90)
ghost/core/core/server/services/indexnow.js (1)
ghost/core/core/frontend/web/middleware/serve-indexnow-key.js (2)
settingsCache(25-25)labs(26-26)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: Admin-X Settings tests
- GitHub Check: Ghost-CLI tests
- GitHub Check: Acceptance tests (Node 22.18.0, sqlite3)
- GitHub Check: Unit tests (Node 22.18.0)
- GitHub Check: Legacy tests (Node 22.18.0, sqlite3)
- GitHub Check: Acceptance tests (Node 22.18.0, mysql8)
- GitHub Check: Lint
- GitHub Check: Legacy tests (Node 22.18.0, mysql8)
- GitHub Check: Cursor Bugbot
- GitHub Check: Build & Push Docker Image
🔇 Additional comments (10)
ghost/core/core/shared/labs.js (1)
58-59: LGTM! Feature flag properly registered.The 'indexnow' feature flag is correctly added to PRIVATE_FEATURES, which gates the new IndexNow integration behind the developer experiments flag. This follows the established pattern for private lab features.
ghost/core/test/unit/server/services/indexnow.test.js (4)
1-33: Solid test setup and cleanup.The test suite uses appropriate mocking tools (sinon, nock, rewire) and establishes sensible defaults in
beforeEach(IndexNow enabled, non-private site, API key present). Cleanup is properly handled inafterEach.
34-41: Good coverage of event registration.The test verifies that
listen()correctly registers handlers for bothpost.publishedandpost.published.editedevents, which aligns with the implementation's dual event listening strategy.
250-385: Excellent ping() test coverage.The test suite comprehensively validates:
- All guard conditions (pages, default posts, private sites, labs flag, missing API key) ✓
- Success scenarios (200, 202 responses) ✓
- Error handling (400, 422, 429) with appropriate logging ✓
- Specific error messages (e.g., "key validation failed" for 422) ✓
The tests effectively verify both the happy path and error scenarios, with proper use of nock to mock HTTP interactions.
387-402: Complete getApiKey() coverage.The tests properly verify both cases: returning the API key from settings and returning null when no key is configured. This adequately covers the simple getter function.
ghost/core/core/server/services/indexnow.js (5)
1-50: Excellent documentation and well-defined constants.The comprehensive header documentation clearly explains the IndexNow protocol, implementation flow, and cross-references the middleware. The default post slugs list appropriately excludes Ghost's demo content from indexing notifications.
57-59: Clean API key retrieval.The function properly retrieves the API key from settings cache with a null fallback, providing a clean public interface for key access.
65-94: Robust guard conditions and security checks.The function properly validates eligibility before pinging:
- Content type filtering (posts only) ✓
- Privacy enforcement (skips private sites) ✓
- Feature gating (labs flag check) ✓
- Demo content exclusion (default posts) ✓
- API key validation with helpful warning ✓
These guards ensure IndexNow is only triggered for appropriate, public content.
156-169: Smart deduplication based on SEO-relevant fields.The function effectively prevents unnecessary IndexNow pings by focusing on fields that impact search engine visibility: content (html), metadata (title, slug, meta_title, meta_description, canonical_url), and publication status. The implementation efficiently uses
Array.some()for change detection.
176-207: Clean event listener implementation with proper guards.The listener function appropriately:
- Filters out imports to avoid pinging during data migrations ✓
- Applies SEO-relevant change detection for deduplication ✓
- Catches ping errors to prevent unhandled rejections (errors already logged) ✓
The
listen()function follows Ghost's pattern of removing existing listeners before registration to prevent duplicates.
| const indexNowUrl = new URL(INDEXNOW_ENDPOINT); | ||
| indexNowUrl.searchParams.set('url', url); | ||
| indexNowUrl.searchParams.set('key', key); | ||
| indexNowUrl.searchParams.set('keyLocation', urlUtils.urlJoin(siteUrl, `${key}.txt`)); |
There was a problem hiding this comment.
Is there a requirement for the key to be in the URL? e.g. would it make sense to use .well-known/index_now_key.txt, or make it part of the API? I think this would also simplify the router
e: Since we have to support sub-directories, we're using option 2 (docs), which requires the API key to be hosted on the (sub-dir) root. I think we can use a static path for the key location, but it can't be nested.
There was a problem hiding this comment.
In case of serving Ghost from sub-directory, urlUtils.urlJoin(siteUrl, `${key}.txt`) will result a txt file url under that sub-directory as well, right?
There was a problem hiding this comment.
Yes, but we can use a static route rather than the key (e.g indexnow.txt)
There was a problem hiding this comment.
Ah ok. You mean simplifying the route definition. Good point.
IndexNow is a protocol that allows websites to notify search engines (Bing, Yandex, Seznam.cz, Naver) about content changes for faster indexing. This integration automatically pings IndexNow when posts are published or updated, reducing the time for content to appear in search results from days to minutes.
Ghost already integrates Ping-O-Matic, though the actual benefit of that service is rather limited. While IndexNow is not supported by Google, it supports a wider user base than Ping-O-Matic
The feature is behind the 'indexnow' labs flag in developer experiments. API keys are auto-generated on first use and the verification file is served dynamically at /{key}.txt.
Got some code for us? Awesome 🎊!
Please take a minute to explain the change you're making:
Please check your PR against these items:
We appreciate your contribution! 🙏
Note
Introduces IndexNow for faster indexing, gated by
labs.indexnow.core/server/services/indexnowlistens topost.publishedandpost.published.edited, pingshttps://api.indexnow.org/indexnowwhen SEO-relevant fields change; initialized inboot.jsserve-indexnow-keyserves the verification file at/{key}.txt; mounted insite.jsindexnow_api_key(in default settings), allowed via settings input serializer and excluded from exportsindexnowto labs flags and Admin Labs UI (with keyed list items)Written by Cursor Bugbot for commit 8650feb. This will update automatically on new commits. Configure here.