Changed urlService require() on app to boot faster

[rmgpinto]

ref https://linear.app/ghost/issue/BER-3132

• The urlService require() adds 70-100ms on boot time, this is due to the package's own initialization. Since it's not needed at boot, it can be moved inside the isMaintenanceModeEnabled function.

[coderabbitai]

Walkthrough

The top-level import of urlService in ghost/core/core/app.js was removed. isMaintenanceModeEnabled now performs a lazy require('./server/services/url') on first invocation, storing it in a module-scoped _urlService. The maintenance-check logic and return behavior are unchanged; only the timing and location of the urlService initialization were altered.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

• Verify isMaintenanceModeEnabled still returns the same values in all call-site scenarios.
• Confirm the lazy require does not introduce startup timing issues when called early.
• Check for any remaining or changed circular dependency behavior that motivated the change.
• Ensure the require succeeds at invocation and that deferring initialization has no unintended side effects.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically describes the main change: moving urlService require() to improve boot time performance.
Description check	✅ Passed	The description is directly related to the changeset, explaining the motivation and implementation details of the performance optimization.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

• 📝 Generate docstrings

���� Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch shift-require-url-service

---

📜 Recent review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between cceb482 and accdd96.

📒 Files selected for processing (1)

• ghost/core/core/app.js (1 hunks)

🧰 Additional context used

🧠 Learnings (4)

📚 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/core/app.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/app.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/app.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/app.js

⏰ 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). (8)

• 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: Lint
• GitHub Check: Acceptance tests (Node 22.18.0, sqlite3)
• GitHub Check: Acceptance tests (Node 22.18.0, mysql8)
• GitHub Check: Unit tests (Node 22.18.0)
• GitHub Check: Build & Push Docker Image

🔇 Additional comments (2)

ghost/core/core/app.js (2)

9-10: Module-scoped caching pattern looks good.

The _urlService cache variable correctly implements the memoization pattern suggested in previous review, avoiding repeated require() lookups on subsequent requests.

---

12-12: Verify lazy loading doesn't trigger premature 503 responses on initial requests.

The maintenance mode check on line 13 calls urlService.hasFinished() immediately after lazy-loading on first request. Since urlService.init() is initiated without await during boot (core/boot.js:121) and returns false while processing (confirmed in UrlService tests), early requests arriving during initialization will receive 503 responses despite not being in actual maintenance mode.

Verify with integration tests:

1. First request after boot returns 200 (not 503) when not in actual maintenance
2. Understand the initialization window and whether it's acceptable

If premature 503s are occurring, consider:

• Awaiting urlService.init() completion before enabling request handling
• Adjusting the maintenance check to distinguish "initialization in progress" from "actual maintenance mode"

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 2

📜 Review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 774b07f and 1e82669.

📒 Files selected for processing (1)

• ghost/core/core/app.js (1 hunks)

⏰ 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). (8)

• GitHub Check: Legacy tests (Node 22.18.0, sqlite3)
• GitHub Check: Acceptance tests (Node 22.18.0, sqlite3)
• GitHub Check: Acceptance tests (Node 22.18.0, mysql8)
• GitHub Check: Legacy tests (Node 22.18.0, mysql8)
• GitHub Check: Ghost-CLI tests
• GitHub Check: Lint
• GitHub Check: Unit tests (Node 22.18.0)
• GitHub Check: Build & Push Docker Image

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Search for urlService references in the codebase
echo "=== Searching for urlService imports and usage ==="
rg -n --type=js 'require.*url.*service|urlService' -g '!node_modules' -C 3

echo ""
echo "=== Searching for hasFinished and related methods ==="
rg -n --type=js 'hasFinished|isFinished|ready.*url|\.finished' -g '!node_modules' -C 2

echo ""
echo "=== Looking for initialization patterns at boot ==="
rg -n --type=js 'require.*services|init|boot' -g '!node_modules' | head -50

Repository: TryGhost/Ghost

Length of output: 50371

---

🏁 Script executed:

#!/bin/bash
# Get app.js context around lines 10-11
echo "=== app.js context (lines 1-30) ==="
head -30 ghost/core/core/app.js | cat -n

echo ""
echo "=== Check if there's any documentation or changes related to this ==="
rg -n 'maintenance.*urlService|urlService.*maintenance' -g '!node_modules' -C 2

Repository: TryGhost/Ghost

Length of output: 1718

---

Deferred urlService initialization will cause initial requests to return 503 Maintenance Mode.

The require() is now deferred to first request execution (line 10), meaning urlService is required inside the per-request middleware. When the first request arrives, isMaintenanceModeEnabled() will require urlService and immediately check hasFinished(). Since initialization has not yet started or completed, this check will return true, causing the middleware to respond with 503 and serve maintenance.html. This blocks legitimate traffic until urlService finishes initializing on a subsequent request.

Initialization should not be deferred to request handling. Keep the require() at module load time so urlService begins initializing during boot, and hasFinished() will be true by the time the first request arrives.

🤖 Prompt for AI Agents

In ghost/core/core/app.js around lines 10-11, the urlService is required inside
the per-request check which defers initialization and causes the first request
to see hasFinished() === false and return 503; move the const urlService =
require('./server/services/url'); out of the request handler to module load/top
of the file so urlService begins initializing during boot and hasFinished() will
be true by first request, leaving the rest of the middleware logic unchanged.