Skip to content

Restored @slack/webhook dependency for the Ghost(Pro) migrate script#28498

Open
jonatansberg wants to merge 1 commit into
mainfrom
restore-slack-webhook-dep
Open

Restored @slack/webhook dependency for the Ghost(Pro) migrate script#28498
jonatansberg wants to merge 1 commit into
mainfrom
restore-slack-webhook-dep

Conversation

@jonatansberg

@jonatansberg jonatansberg commented Jun 10, 2026

Copy link
Copy Markdown
Member

Problem

Ghost(Pro) migrations are currently failing to alert. migrate.js (copied into the Ghost(Pro) image at release time) crashes with:

Error: Cannot find module '@slack/webhook'
    at Object.notifySlack (/home/ghost/migrate.js:101:31)

Because the require lives inside notifySlack, the MODULE_NOT_FOUND is thrown from inside the migration catch blocks before exitCode = 1 is set and before the platform is notified of failure — so the alert never sends, the platform is never told a major migration failed, and the script exits 0, letting the entrypoint attempt to boot Ghost instead of failing fast at the migrate step (Ghost then fails its own DB state check, so the failure surfaces as an unalerted boot crash-loop).

Background

  • @slack/webhook, superagent and superagent-throttle were deliberately added to ghost/core in 5832ab5 as a workaround for internal Pro adapter files that are copied into the image at release time. Nothing in this repo imports them, so knip/grep cannot see the consumers.
  • 8f2ec8d (Removed unused dependencies surfaced by knip #27720) removed all three in a knip cleanup pass.
  • bbc3981 restored the superagent packages after Pro crashed at boot (SchedulingPro requires them at module load), but @slack/webhook was missed — its lazy require meant nothing failed until a migration actually went wrong.

Changes

  • Re-added @slack/webhook@7.0.9 (the version removed in Removed unused dependencies surfaced by knip #27720) to ghost/core dependencies
  • Pinned all three packages in knip.json via a ghost/core workspace ignoreDependencies entry so the next cleanup pass does not remove them again (verified pnpm knip no longer flags any of them)

@coderabbitai

coderabbitai Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

This PR adds the @slack/webhook package (version 7.0.9) as a dependency to ghost/core and configures the knip dependency checker to ignore it along with its transitive dependencies (superagent and superagent-throttle) within the workspace. The changes consist of a single dependency declaration and corresponding knip workspace configuration to prevent dependency linting warnings.

Possibly related PRs

  • TryGhost/Ghost#27947: Modifies knip.json's ignoreDependencies configuration to manage ignored dependencies for the workspace.
🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately and concisely describes the main change: restoring the @slack/webhook dependency to fix Ghost(Pro) migrations.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description check ✅ Passed The pull request description clearly explains the problem (missing @slack/webhook dependency causing migration failures), provides relevant background context, and details the specific changes made.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch restore-slack-webhook-dep

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

no ref

- @slack/webhook, superagent and superagent-throttle were added to
  ghost/core as a workaround for internal Pro adapter files that are
  copied into the image at release time and resolve against core's
  node_modules
- 8f2ec8d removed all three because knip flagged them unused - the
  consumers live outside this repo so knip/grep cannot see them
- bbc3981 restored the superagent packages after Pro crashed at
  boot, but @slack/webhook was missed because migrate.js only requires
  it lazily inside notifySlack, so nothing failed until a migration
  actually went wrong - failed migrations then crashed with
  MODULE_NOT_FOUND, skipped failure alerting and exited 0
- pinned all three in knip.json ignoreDependencies so the next cleanup
  pass does not remove them again
@jonatansberg jonatansberg force-pushed the restore-slack-webhook-dep branch from 167efdd to b8aa5db Compare June 10, 2026 22:11
@jonatansberg jonatansberg marked this pull request as ready for review June 10, 2026 22:17
@codecov

codecov Bot commented Jun 10, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73.76%. Comparing base (ec13563) to head (b8aa5db).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main   #28498   +/-   ##
=======================================
  Coverage   73.76%   73.76%           
=======================================
  Files        1541     1541           
  Lines      132364   132364           
  Branches    15834    15833    -1     
=======================================
+ Hits        97637    97640    +3     
+ Misses      33760    33735   -25     
- Partials      967      989   +22     
Flag Coverage Δ
admin-tests 54.67% <ø> (ø)
e2e-tests 73.76% <ø> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant