-
-
Notifications
You must be signed in to change notification settings - Fork 11.2k
Added support for activitypub with improved dev workflow #25759
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.
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
base: main
Are you sure you want to change the base?
Conversation
ref TryGhost/ActivityPub#1487 Added ActivityPub routing and HMR support for external domains - Added `dev-gateway` routing for ActivityPub API (`/.ghost/activitypub/*`) and federation endpoints (`/.well-known/webfinger`, `/.well-known/nodeinfo`) - Fixed HMR WebSocket connections through reverse proxies by updating `vite-deep-links.ts` to skip WebSocket upgrades and root path requests - Changed `allowedHosts` to `true` to support external domains (Tailscale, etc.) - Added `/ghost/.well-known/*` route to Ghost backend for JWKS endpoint - Simplified admin reverse proxy config (Caddy handles WebSocket automatically)
WalkthroughThis pull request introduces ActivityPub integration to the development gateway infrastructure. Changes include modifying the Vite admin server configuration to use permissive host allowlisting, adding ActivityPub proxy routing rules to the Caddyfile for federation endpoints (/.ghost/activitypub/*, /.well-known/webfinger, /.well-known/nodeinfo), configuring the ActivityPub proxy target in the Dockerfile, and updating documentation with the new routing rules and environment variable requirements. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches�� Generate unit tests (beta)
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 |
jonatansberg
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall, this looks really good. I have a couple of questions that would be relevant to go over before merging.
| "::1", | ||
| "host.docker.internal", | ||
| ] | ||
| allowedHosts: true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any reason for changing this? I went for the more restrictive list based on the recommendations in the Vite docs. In practice, this should probably be fine as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was to allow support for external domains (i.e when funneling via tailscale)
apps/admin/vite-deep-links.ts
Outdated
| // Skip WebSocket upgrade requests (used for HMR) | ||
| if (req.headers.upgrade?.toLowerCase() === 'websocket') { | ||
| next(); | ||
| return; | ||
| } | ||
|
|
||
| // Skip root path with query string only (e.g. /ghost/?token=xxx for Vite HMR) | ||
| const urlPath = req.originalUrl?.split('?')[0]; | ||
| if (urlPath === `${base}/` || urlPath === base) { | ||
| next(); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We might need to handle the token case in the gateway rather than this middleware? I think the same goes for the WebSocket thing. If that has made it all the way here, then something's off, no? 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Heh this ain't needed at all 😅
I think I added it because I couldn't get the Caddy changes working at one point, but works fine without this so all good
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
docker/dev-gateway/Caddyfile (1)
172-180: JWKS endpoint routing is correctly positioned.The handler is properly placed before the general
/ghost*catch-all to ensure it's matched correctly. The configuration follows the same pattern as other Ghost backend routes.Optional: Consider clarifying the comment to reflect the wildcard pattern
The comment mentions "JWKS endpoint" (singular) but the path pattern
/ghost/.well-known/*suggests it may serve multiple well-known endpoints. Consider updating the comment if multiple endpoints are expected:- # JWKS endpoint - must go to Ghost backend for JWT verification + # Well-known endpoints (e.g., JWKS) - must go to Ghost backend for JWT verification
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docker/dev-gateway/Caddyfile(3 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
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`.
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.
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Use React with Vite for Admin app development (admin-x-settings, admin-x-activitypub, posts, stats)
📚 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:
docker/dev-gateway/Caddyfile
📚 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:
docker/dev-gateway/Caddyfile
📚 Learning: 2025-05-27T18:08:00.458Z
Learnt from: cmraible
Repo: TryGhost/Ghost PR: 23546
File: compose.yml:58-59
Timestamp: 2025-05-27T18:08:00.458Z
Learning: The Ghost Docker Compose setup has two independent profiles: `ghost` profile (v0, runs all apps in a single container) and `split` profile (work in progress, runs Ghost server, admin, and frontend apps in separate services). The `split` profile will eventually replace `ghost` as the default.
Applied to files:
docker/dev-gateway/Caddyfile
⏰ 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: E2E Tests (Ember 2/4)
- GitHub Check: [Optional] E2E Tests (React 2/4)
- GitHub Check: E2E Tests (Ember 4/4)
- GitHub Check: [Optional] E2E Tests (React 1/4)
- GitHub Check: E2E Tests (Ember 1/4)
- GitHub Check: [Optional] E2E Tests (React 4/4)
- GitHub Check: E2E Tests (Ember 3/4)
- GitHub Check: [Optional] E2E Tests (React 3/4)
🔇 Additional comments (2)
docker/dev-gateway/Caddyfile (2)
184-189: Admin handler simplification aligns with PR objectives.The path change from
/ghost/*to/ghost*now matches requests to/ghostwithout a trailing slash, and the simplified header configuration relies on Caddy's built-in WebSocket support for HMR.These changes align with the PR description: "Simplified admin reverse proxy configuration, relying on Caddy to handle WebSocket automatically" and "Fixed HMR WebSocket connections through reverse proxies."
49-78: ActivityPub routing is correctly configured.The three ActivityPub endpoints (API, WebFinger, NodeInfo) are properly set up with appropriate headers for federation. The
ACTIVITYPUB_PROXY_TARGETenvironment variable is already defined in the Dockerfile with a sensible default (host.docker.internal:8080), and positioning of these handlers ensures correct route matching before the general/ghost*handler.
ref TryGhost/ActivityPub#1487
Added ActivityPub routing and HMR support for external domains
dev-gatewayrouting for ActivityPub API (/.ghost/activitypub/*) and federation endpoints (/.well-known/webfinger,/.well-known/nodeinfo)vite-deep-links.tsto skip WebSocket upgrades and root path requestsallowedHoststotrueto support external domains (Tailscale, etc.)/ghost/.well-known/*route to Ghost backend for JWKS endpoint