From 95f34dbfb414b0dbbc274d8a27a0fc67ab26ce35 Mon Sep 17 00:00:00 2001 From: Michael Barrett Date: Thu, 30 Jul 2026 11:14:31 +0100 Subject: [PATCH 1/2] =?UTF-8?q?=F0=9F=90=9B=20Fixed=20500=20error=20when?= =?UTF-8?q?=20deleting=20posts=20with=20threaded=20comments?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ref https://linear.app/ghost/issue/ONC-1925 The `comments.in_reply_to_id` foreign key was created with `ON DELETE SET NULL`, while `comments.parent_id` uses `ON DELETE CASCADE`. When a post is deleted, InnoDB cascades into `comments` row by row and the `SET NULL` action issues an `UPDATE` on reply rows whose parent comment is delete-marked in the same cascade. That `UPDATE` re-validates the row's other foreign keys and fails with `ER_NO_REFERENCED_ROW_2` (`errno 1452`), making it impossible to delete any post that has a reply-to-reply comment Switching the action to `CASCADE` makes the post delete cascade deletes-only, which cannot fail this way. Nothing depends on the old `SET NULL` behaviour: it only fires when a comment row is hard-deleted, and the only hard delete in the product is the post delete cascade itself, where every comment is removed regardless. Individual comment deletion is always a soft delete (the `Comment` model overrides `destroy` to set `status='deleted'`), and the UI / serializers handle removed reply targets via that status, not via a nulled foreign key --- ...-comments-in-reply-to-deletion-strategy.js | 46 +++++++++++++++++++ ghost/core/core/server/data/schema/schema.js | 2 +- .../__snapshots__/posts-bulk.test.js.snap | 14 ++++++ .../admin/__snapshots__/posts.test.js.snap | 12 +++++ .../test/e2e-api/admin/posts-bulk.test.js | 41 +++++++++++++++++ ghost/core/test/e2e-api/admin/posts.test.js | 35 ++++++++++++++ .../unit/server/data/schema/integrity.test.js | 2 +- 7 files changed, 150 insertions(+), 2 deletions(-) create mode 100644 ghost/core/core/server/data/migrations/versions/6.55/2026-07-30-09-51-40-fix-comments-in-reply-to-deletion-strategy.js diff --git a/ghost/core/core/server/data/migrations/versions/6.55/2026-07-30-09-51-40-fix-comments-in-reply-to-deletion-strategy.js b/ghost/core/core/server/data/migrations/versions/6.55/2026-07-30-09-51-40-fix-comments-in-reply-to-deletion-strategy.js new file mode 100644 index 00000000000..688b58fd667 --- /dev/null +++ b/ghost/core/core/server/data/migrations/versions/6.55/2026-07-30-09-51-40-fix-comments-in-reply-to-deletion-strategy.js @@ -0,0 +1,46 @@ +const logging = require('@tryghost/logging'); +const {commands} = require('../../../schema'); +const {createTransactionalMigration} = require('../../utils'); + +module.exports = createTransactionalMigration( + async function up(knex) { + logging.info('Changing comments.in_reply_to_id foreign key to ON DELETE CASCADE'); + + await commands.dropForeign({ + fromTable: 'comments', + fromColumn: 'in_reply_to_id', + toTable: 'comments', + toColumn: 'id', + transaction: knex + }); + + await commands.addForeign({ + fromTable: 'comments', + fromColumn: 'in_reply_to_id', + toTable: 'comments', + toColumn: 'id', + cascadeDelete: true, + transaction: knex + }); + }, + async function down(knex) { + logging.info('Restoring comments.in_reply_to_id foreign key to ON DELETE SET NULL'); + + await commands.dropForeign({ + fromTable: 'comments', + fromColumn: 'in_reply_to_id', + toTable: 'comments', + toColumn: 'id', + transaction: knex + }); + + await commands.addForeign({ + fromTable: 'comments', + fromColumn: 'in_reply_to_id', + toTable: 'comments', + toColumn: 'id', + setNullDelete: true, + transaction: knex + }); + } +); diff --git a/ghost/core/core/server/data/schema/schema.js b/ghost/core/core/server/data/schema/schema.js index 6cb6ffd9b2a..9c05dd57af3 100644 --- a/ghost/core/core/server/data/schema/schema.js +++ b/ghost/core/core/server/data/schema/schema.js @@ -1035,7 +1035,7 @@ module.exports = { post_id: {type: 'string', maxlength: 24, nullable: false, unique: false, references: 'posts.id', cascadeDelete: true}, member_id: {type: 'string', maxlength: 24, nullable: true, unique: false, references: 'members.id', setNullDelete: true}, parent_id: {type: 'string', maxlength: 24, nullable: true, unique: false, references: 'comments.id', cascadeDelete: true}, - in_reply_to_id: {type: 'string', maxlength: 24, nullable: true, unique: false, references: 'comments.id', setNullDelete: true}, + in_reply_to_id: {type: 'string', maxlength: 24, nullable: true, unique: false, references: 'comments.id', cascadeDelete: true}, status: {type: 'string', maxlength: 50, nullable: false, defaultTo: 'published', validations: {isIn: [['published', 'hidden', 'deleted']]}}, html: {type: 'text', maxlength: 1000000000, fieldtype: 'long', nullable: true}, edited_at: {type: 'dateTime', nullable: true}, diff --git a/ghost/core/test/e2e-api/admin/__snapshots__/posts-bulk.test.js.snap b/ghost/core/test/e2e-api/admin/__snapshots__/posts-bulk.test.js.snap index dfbe695a723..4719f12701f 100644 --- a/ghost/core/test/e2e-api/admin/__snapshots__/posts-bulk.test.js.snap +++ b/ghost/core/test/e2e-api/admin/__snapshots__/posts-bulk.test.js.snap @@ -1,5 +1,19 @@ // Jest Snapshot v1, https://jestjs.io/docs/snapshot-testing +exports[`Posts Bulk API Delete Can delete a post with a threaded comment replying to another reply 1: [body] 1`] = ` +Object { + "bulk": Object { + "meta": Object { + "errors": Array [], + "stats": Object { + "successful": 1, + "unsuccessful": 0, + }, + }, + }, +} +`; + exports[`Posts Bulk API Delete Can delete all posts 1: [body] 1`] = ` Object { "bulk": Object { diff --git a/ghost/core/test/e2e-api/admin/__snapshots__/posts.test.js.snap b/ghost/core/test/e2e-api/admin/__snapshots__/posts.test.js.snap index aebb01f31cd..1c17f15cb01 100644 --- a/ghost/core/test/e2e-api/admin/__snapshots__/posts.test.js.snap +++ b/ghost/core/test/e2e-api/admin/__snapshots__/posts.test.js.snap @@ -2005,6 +2005,18 @@ Object { } `; +exports[`Posts API Delete Can destroy a post with a threaded comment replying to another reply 1: [headers] 1`] = ` +Object { + "access-control-allow-origin": "http://127.0.0.1:2369", + "cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0", + "content-version": StringMatching /v\\\\d\\+\\\\\\.\\\\d\\+/, + "etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/, + "vary": "Accept-Version, Origin", + "x-cache-invalidate": "/*", + "x-powered-by": "Express", +} +`; + exports[`Posts API Delete Cannot delete a non-existent posts 1: [body] 1`] = ` Object { "errors": Array [ diff --git a/ghost/core/test/e2e-api/admin/posts-bulk.test.js b/ghost/core/test/e2e-api/admin/posts-bulk.test.js index f25c6444654..c3b8ee0c204 100644 --- a/ghost/core/test/e2e-api/admin/posts-bulk.test.js +++ b/ghost/core/test/e2e-api/admin/posts-bulk.test.js @@ -324,6 +324,47 @@ describe('Posts Bulk API', function () { assert.equal(posts.meta.pagination.total, 0, `Expect all matching posts (${amount}) to be deleted`); }); + it('Can delete a post with a threaded comment replying to another reply', async function () { + const {body: {posts: [post]}} = await agent + .post('/posts/') + .body({posts: [{title: 'Post with a threaded comment', status: 'draft'}]}) + .expectStatus(201); + + const memberId = fixtureManager.get('members', 0).id; + const root = await models.Comment.add({ + post_id: post.id, + member_id: memberId, + html: '

Root comment

', + status: 'published' + }); + const reply = await models.Comment.add({ + post_id: post.id, + member_id: memberId, + parent_id: root.id, + html: '

Reply

', + status: 'published' + }); + await models.Comment.add({ + post_id: post.id, + member_id: memberId, + parent_id: root.id, + in_reply_to_id: reply.id, + html: '

Reply to the reply

', + status: 'published' + }); + + const filter = `id:['${post.id}']`; + const response = await agent + .delete('/posts/?filter=' + encodeURIComponent(filter)) + .expectStatus(200) + .matchBodySnapshot(); + + assert.equal(response.body.bulk.meta.stats.successful, 1, 'Expect the post with threaded comments to be deleted'); + + const comments = await models.Base.knex('comments').where('post_id', post.id); + assert.equal(comments.length, 0, 'Expected all comments on the post to be deleted with the post'); + }); + it('Can delete all posts', async function () { const filter = 'status:[published,draft,scheduled,sent]'; diff --git a/ghost/core/test/e2e-api/admin/posts.test.js b/ghost/core/test/e2e-api/admin/posts.test.js index 0fa8f0332c1..188d8304014 100644 --- a/ghost/core/test/e2e-api/admin/posts.test.js +++ b/ghost/core/test/e2e-api/admin/posts.test.js @@ -760,6 +760,41 @@ describe('Posts API', function () { }); }); + it('Can destroy a post with a threaded comment replying to another reply', async function () { + const post = fixtureManager.get('posts', 1); + + const root = await models.Comment.add({ + post_id: post.id, + html: '

Root comment

', + status: 'published' + }); + const reply = await models.Comment.add({ + post_id: post.id, + parent_id: root.id, + html: '

Reply

', + status: 'published' + }); + await models.Comment.add({ + post_id: post.id, + parent_id: root.id, + in_reply_to_id: reply.id, + html: '

Reply to the reply

', + status: 'published' + }); + + await agent + .delete(`posts/${post.id}/`) + .expectStatus(204) + .expectEmptyBody() + .matchHeaderSnapshot({ + 'content-version': anyContentVersion, + etag: anyEtag + }); + + const comments = await models.Base.knex('comments').where('post_id', post.id); + assert.equal(comments.length, 0, 'Expected all comments on the post to be deleted with the post'); + }); + it('Cannot delete a non-existent posts', async function () { // This error message from the API is not really what I would expect // Adding this as a guard to demonstrate how future refactoring improves the output diff --git a/ghost/core/test/unit/server/data/schema/integrity.test.js b/ghost/core/test/unit/server/data/schema/integrity.test.js index 21ef2befcfc..bc2d1728f2f 100644 --- a/ghost/core/test/unit/server/data/schema/integrity.test.js +++ b/ghost/core/test/unit/server/data/schema/integrity.test.js @@ -36,7 +36,7 @@ const parseYaml = require('../../../../../core/server/services/route-settings/ya */ describe('DB version integrity', function () { // Only these variables should need updating - const currentSchemaHash = 'c0fe7246714201a82b75f80308d5e300'; + const currentSchemaHash = '5355ba478bb4655ba6dcf1410ac451f2'; const currentFixturesHash = '065b413e1d1f4f95fa7cb7734c5e7934'; const currentSettingsHash = '8650db85b9a61afe4797ad6333066c62'; const currentRoutesHash = '3d180d52c663d173a6be791ef411ed01'; From 1f9a08d8bcab4d4cd56715b12d1287943fa8afb9 Mon Sep 17 00:00:00 2001 From: Michael Barrett Date: Mon, 3 Aug 2026 10:13:03 +0100 Subject: [PATCH 2/2] Changed comment cleanup on post deletion to run in the delete transaction ref https://linear.app/ghost/issue/ONC-1925 No foreign key delete rule works for `comments.in_reply_to_id`: `SET NULL` fails mid-cascade (`errno 1452`) for any post with a reply-to-reply comment, and `CASCADE` hits MySQL's 15-level cascade depth limit (`errno 3008`) on long back-and-forth reply chains, which form a linked list through `in_reply_to_id`. The previous approach of migrating the foreign key to `CASCADE` is therefore reverted - the fix is instead to clear `in_reply_to_id` for the post's comments inside the delete transaction (single and bulk destroy), so the existing `post_id` cascade can delete the comments regardless of thread shape --- ...-comments-in-reply-to-deletion-strategy.js | 46 ------------------- ghost/core/core/server/data/schema/schema.js | 2 +- ghost/core/core/server/models/post.js | 11 ++++- .../server/services/posts/posts-service.js | 11 +++++ .../test/e2e-api/admin/posts-bulk.test.js | 18 ++++++++ ghost/core/test/e2e-api/admin/posts.test.js | 17 +++++++ .../unit/server/data/schema/integrity.test.js | 2 +- 7 files changed, 58 insertions(+), 49 deletions(-) delete mode 100644 ghost/core/core/server/data/migrations/versions/6.55/2026-07-30-09-51-40-fix-comments-in-reply-to-deletion-strategy.js diff --git a/ghost/core/core/server/data/migrations/versions/6.55/2026-07-30-09-51-40-fix-comments-in-reply-to-deletion-strategy.js b/ghost/core/core/server/data/migrations/versions/6.55/2026-07-30-09-51-40-fix-comments-in-reply-to-deletion-strategy.js deleted file mode 100644 index 688b58fd667..00000000000 --- a/ghost/core/core/server/data/migrations/versions/6.55/2026-07-30-09-51-40-fix-comments-in-reply-to-deletion-strategy.js +++ /dev/null @@ -1,46 +0,0 @@ -const logging = require('@tryghost/logging'); -const {commands} = require('../../../schema'); -const {createTransactionalMigration} = require('../../utils'); - -module.exports = createTransactionalMigration( - async function up(knex) { - logging.info('Changing comments.in_reply_to_id foreign key to ON DELETE CASCADE'); - - await commands.dropForeign({ - fromTable: 'comments', - fromColumn: 'in_reply_to_id', - toTable: 'comments', - toColumn: 'id', - transaction: knex - }); - - await commands.addForeign({ - fromTable: 'comments', - fromColumn: 'in_reply_to_id', - toTable: 'comments', - toColumn: 'id', - cascadeDelete: true, - transaction: knex - }); - }, - async function down(knex) { - logging.info('Restoring comments.in_reply_to_id foreign key to ON DELETE SET NULL'); - - await commands.dropForeign({ - fromTable: 'comments', - fromColumn: 'in_reply_to_id', - toTable: 'comments', - toColumn: 'id', - transaction: knex - }); - - await commands.addForeign({ - fromTable: 'comments', - fromColumn: 'in_reply_to_id', - toTable: 'comments', - toColumn: 'id', - setNullDelete: true, - transaction: knex - }); - } -); diff --git a/ghost/core/core/server/data/schema/schema.js b/ghost/core/core/server/data/schema/schema.js index 9c05dd57af3..6cb6ffd9b2a 100644 --- a/ghost/core/core/server/data/schema/schema.js +++ b/ghost/core/core/server/data/schema/schema.js @@ -1035,7 +1035,7 @@ module.exports = { post_id: {type: 'string', maxlength: 24, nullable: false, unique: false, references: 'posts.id', cascadeDelete: true}, member_id: {type: 'string', maxlength: 24, nullable: true, unique: false, references: 'members.id', setNullDelete: true}, parent_id: {type: 'string', maxlength: 24, nullable: true, unique: false, references: 'comments.id', cascadeDelete: true}, - in_reply_to_id: {type: 'string', maxlength: 24, nullable: true, unique: false, references: 'comments.id', cascadeDelete: true}, + in_reply_to_id: {type: 'string', maxlength: 24, nullable: true, unique: false, references: 'comments.id', setNullDelete: true}, status: {type: 'string', maxlength: 50, nullable: false, defaultTo: 'published', validations: {isIn: [['published', 'hidden', 'deleted']]}}, html: {type: 'text', maxlength: 1000000000, fieldtype: 'long', nullable: true}, edited_at: {type: 'dateTime', nullable: true}, diff --git a/ghost/core/core/server/models/post.js b/ghost/core/core/server/models/post.js index e59db48fedc..6ca610ab96e 100644 --- a/ghost/core/core/server/models/post.js +++ b/ghost/core/core/server/models/post.js @@ -1346,7 +1346,16 @@ Post = ghostBookshelf.Model.extend({ destroy: function destroy(unfilteredOptions) { let options = this.filterOptions(unfilteredOptions, 'destroy', {extraAllowedProperties: ['id']}); - const destroyPost = () => { + const destroyPost = async () => { + // The `comments.in_reply_to_id` references form chains between a post's + // comments, which MySQL cannot resolve while cascade-deleting them + // alongside `comments.parent_id`. Clear the references first so the + // `comments.post_id` cascade delete can do its job + await ghostBookshelf.knex('comments') + .where('post_id', options.id) + .update('in_reply_to_id', null) + .transacting(options.transacting); + return ghostBookshelf.Model.destroy.call(this, options); }; diff --git a/ghost/core/core/server/services/posts/posts-service.js b/ghost/core/core/server/services/posts/posts-service.js index 6f6b9533039..a47117bcea4 100644 --- a/ghost/core/core/server/services/posts/posts-service.js +++ b/ghost/core/core/server/services/posts/posts-service.js @@ -312,6 +312,17 @@ class PostsService { }); } + // The `comments.in_reply_to_id` references form chains between a post's + // comments, which MySQL cannot resolve while cascade-deleting them + // alongside `comments.parent_id`. Clear the references first so the + // `comments.post_id` cascade delete can do its job + await this.models.Post.bulkEdit(deleteIds, 'comments', { + data: {in_reply_to_id: null}, + column: 'post_id', + transacting: options.transacting, + throwErrors: true + }); + // Posts and emails await this.models.Post.bulkDestroy(deleteEmailIds, 'emails', {transacting: options.transacting, throwErrors: true}); const result = await this.models.Post.bulkDestroy(deleteIds, 'posts', {...options, throwErrors: true}); diff --git a/ghost/core/test/e2e-api/admin/posts-bulk.test.js b/ghost/core/test/e2e-api/admin/posts-bulk.test.js index c3b8ee0c204..8689d057d67 100644 --- a/ghost/core/test/e2e-api/admin/posts-bulk.test.js +++ b/ghost/core/test/e2e-api/admin/posts-bulk.test.js @@ -353,6 +353,24 @@ describe('Posts Bulk API', function () { status: 'published' }); + // A long back-and-forth conversation, where each reply replies to the + // previous one, chains more levels than MySQL can cascade: InnoDB + // hard-limits nested foreign key cascades to 15 levels and fails the + // delete with error 3008 beyond that, so `in_reply_to_id` cannot use + // ON DELETE CASCADE and must be cleared in the delete transaction + // https://dev.mysql.com/doc/mysql-reslimits-excerpt/8.0/en/ansi-diff-foreign-keys.html + let previous = reply; + for (let i = 0; i < 20; i++) { + previous = await models.Comment.add({ + post_id: post.id, + member_id: memberId, + parent_id: root.id, + in_reply_to_id: previous.id, + html: `

Reply ${i} in a long conversation

`, + status: 'published' + }); + } + const filter = `id:['${post.id}']`; const response = await agent .delete('/posts/?filter=' + encodeURIComponent(filter)) diff --git a/ghost/core/test/e2e-api/admin/posts.test.js b/ghost/core/test/e2e-api/admin/posts.test.js index 188d8304014..ca333a4e4fb 100644 --- a/ghost/core/test/e2e-api/admin/posts.test.js +++ b/ghost/core/test/e2e-api/admin/posts.test.js @@ -782,6 +782,23 @@ describe('Posts API', function () { status: 'published' }); + // A long back-and-forth conversation, where each reply replies to the + // previous one, chains more levels than MySQL can cascade: InnoDB + // hard-limits nested foreign key cascades to 15 levels and fails the + // delete with error 3008 beyond that, so `in_reply_to_id` cannot use + // ON DELETE CASCADE and must be cleared in the delete transaction + // https://dev.mysql.com/doc/mysql-reslimits-excerpt/8.0/en/ansi-diff-foreign-keys.html + let previous = reply; + for (let i = 0; i < 20; i++) { + previous = await models.Comment.add({ + post_id: post.id, + parent_id: root.id, + in_reply_to_id: previous.id, + html: `

Reply ${i} in a long conversation

`, + status: 'published' + }); + } + await agent .delete(`posts/${post.id}/`) .expectStatus(204) diff --git a/ghost/core/test/unit/server/data/schema/integrity.test.js b/ghost/core/test/unit/server/data/schema/integrity.test.js index bc2d1728f2f..21ef2befcfc 100644 --- a/ghost/core/test/unit/server/data/schema/integrity.test.js +++ b/ghost/core/test/unit/server/data/schema/integrity.test.js @@ -36,7 +36,7 @@ const parseYaml = require('../../../../../core/server/services/route-settings/ya */ describe('DB version integrity', function () { // Only these variables should need updating - const currentSchemaHash = '5355ba478bb4655ba6dcf1410ac451f2'; + const currentSchemaHash = 'c0fe7246714201a82b75f80308d5e300'; const currentFixturesHash = '065b413e1d1f4f95fa7cb7734c5e7934'; const currentSettingsHash = '8650db85b9a61afe4797ad6333066c62'; const currentRoutesHash = '3d180d52c663d173a6be791ef411ed01';