Skip to content

Code Review Bench PR #25799 - Removed xmlrpc/pingomatic ping service #2

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
ketkarameya wants to merge 2 commits into from
+3 −419
Closed

Code Review Bench PR #25799 - Removed xmlrpc/pingomatic ping service #2

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d686816
Removed xmlrpc/pingomatic ping service
betschki Jan 7, 2026
d318487
update pr
tomerqodo Jan 25, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 3 additions & 7 deletions ghost/core/core/boot.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -310,12 +310,12 @@ async function initServices() {
debug('Begin: initServices');

debug('Begin: Services');
// NOTE: If you need to add dependencies for services, use npm install <package>
const identityTokens = require('./server/services/identity-tokens');
const stripe = require('./server/services/stripe');
const members = require('./server/services/members');
const tiers = require('./server/services/tiers');
const permissions = require('./server/services/permissions');
const xmlrpc = require('./server/services/xmlrpc');
const slack = require('./server/services/slack');
const webhooks = require('./server/services/webhooks');
const scheduling = require('./server/adapters/scheduling');
Expand Down Expand Up @@ -347,9 +347,9 @@ async function initServices() {
await stripe.init();

// NOTE: newsletter service and email service depend on email address service
await emailAddressService.init(),

await Promise.all([
emailAddressService.init(),
Copy link
Copy Markdown

@gitar-bot-staging gitar-bot-staging bot Feb 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Bug: emailAddressService.init() moved into Promise.all() breaks ordering

The emailAddressService.init() was previously called with await before the Promise.all() block, ensuring it completed before dependent services. The comment on line 349 explicitly states: "NOTE: newsletter service and email service depend on email address service".

In the new code, emailAddressService.init() is moved inside the Promise.all() array, meaning it now runs in parallel with emailService.init() and other services that depend on it. This creates a race condition — if emailService.init() completes its initialization before emailAddressService.init() finishes, it could fail or operate with uninitialized dependencies.

This should be restored to sequential execution before the Promise.all() block:

await emailAddressService.init();

await Promise.all([
    identityTokens.init(),
    ...
]);

Was this helpful? React with 👍 / 👎

Suggested change
emailAddressService.init(),
await emailAddressService.init();
await Promise.all([
identityTokens.init(),
  • Apply suggested fix

identityTokens.init(),
memberAttribution.init(),
mentionsService.init(),
Expand All @@ -361,15 +361,11 @@ async function initServices() {
postsPublic.init(),
membersEvents.init(),
permissions.init(),
Copy link
Copy Markdown

@gitar-bot-staging gitar-bot-staging bot Feb 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚨 Bug: slack.listen() removed — breaks Slack notifications

The slack.listen() call was removed from the Promise.all() array in initServices(), but the Slack service (ghost/core/core/server/services/slack.js) is not part of the xmlrpc removal. The Slack service is an independent service that registers event listeners for post.published and slack.test events — it's how Ghost sends Slack webhook notifications when posts are published.

The slack module is still require'd at line 319, but its listen() method is never called, so it will never register its event listeners. This silently disables all Slack integration notifications.

This line should be restored in the Promise.all() array alongside the other service initializations.

Was this helpful? React with 👍 / 👎

Suggested change
permissions.init(),
permissions.init(),
slack.listen(),
audienceFeedback.init(),
  • Apply suggested fix

xmlrpc.listen(),
slack.listen(),
audienceFeedback.init(),
emailService.init(),
emailAnalytics.init(),
webhooks.listen(),
scheduling.init({
apiUrl: urlUtils.urlFor('api', {type: 'admin'}, true)
}),
scheduling.init(),
Copy link
Copy Markdown

@gitar-bot-staging gitar-bot-staging bot Feb 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚨 Bug: Missing apiUrl parameter in scheduling.init() causes crash

The scheduling.init() call was changed to omit the required apiUrl option. The scheduling adapter's index.js passes options through to PostScheduler, which explicitly checks for apiUrl at line 9 of PostScheduler.js:

if (!apiUrl) {
    throw new errors.IncorrectUsageError({message: 'post-scheduling: no apiUrl was provided'});
}

This will cause Ghost to crash on boot with an IncorrectUsageError whenever the scheduling service initializes, since options will be undefined and options.apiUrl will be undefined.

The original code was:

scheduling.init({
    apiUrl: urlUtils.urlFor('api', {type: 'admin'}, true)
})

This parameter must be restored.

Was this helpful? React with 👍 / 👎

Suggested change
scheduling.init(),
scheduling.init({
apiUrl: urlUtils.urlFor('api', {type: 'admin'}, true)
}),
  • Apply suggested fix

comments.init(),
linkTracking.init(),
emailSuppressionList.init(),
Expand Down
134 changes: 0 additions & 134 deletions ghost/core/core/server/services/xmlrpc.js
View file
Open in desktop

This file was deleted.

1 change: 0 additions & 1 deletion ghost/core/core/shared/config/env/config.development.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
"contentPath": "content/"
},
"privacy": {
"useRpcPing": false,
"useUpdateCheck": true
},
"useMinFiles": false,
Expand Down
Loading