Added support for improved Ghost development workflow #1487
Conversation
no ref We recently added a new development workflow for main Ghost repo (`yarn dev:forward`). These changes support this workflow by deferring proxying to the development gateway in the Ghost repo. Changes include: - adding a new command (`yarn dev:ghost`) that starts everything but `nginx` - exposing port `8080` so that the app can be accessed via `host.docker.internal` - adding a middleware to serve static files that have been uploaded (previously using nginx for this)
WalkthroughThis pull request adds development infrastructure for local storage and GCS emulator support. The changes include exposing port 8080 for the activitypub service in docker-compose.yml, adding a new npm script called "dev:ghost" to launch the activitypub container with Docker Compose, and implementing development-time static file serving in src/app.ts for local storage and GCS emulator proxying. All changes are development-environment specific with no production-facing modifications. Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Pre-merge checks and finishing touches✅ Passed checks (3 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 |
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)
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
docker-compose.yml(1 hunks)package.json(1 hunks)src/app.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.{ts,tsx}: Never use direct string comparisons for ActivityPub IDs; use SHA256 hash lookups instead - see ADR-0009
Always use the Result helper functions (isError,getError,getValue) with Result types instead of destructuring directly
Dependency injection parameter names must match registration names in Awilix CLASSIC injection mode
Routes should be defined using decorators (@APIRoute,@RequireRoles) rather than direct registration
Prefer class-based architecture with dependency injection over function factories for handlers
Repositories should not be used directly; they should be used through services
Controllers should be lean and delegate to services where appropriate
Views can talk directly to the database if necessary but should not be responsible for any business logic
Prefer immutable entities that generate domain events over mutable entities with dirty flags
Prefer error objects with context over string literal errors in Result types
Avoid mutable entities with dirty flags; use immutable entities with domain events instead
Files:
src/app.ts
🧠 Learnings (2)
📓 Common learnings
Learnt from: allouis Repo: TryGhost/ActivityPub PR: 1010 File: src/app.ts:744-744 Timestamp: 2025-07-09T06:47:50.064Z Learning: In the TryGhost/ActivityPub repository, allouis prefers to keep refactoring PRs focused on code restructuring only, without adding new functionality or changing behavior. When moving existing code that has issues, the issues should be preserved in the refactoring and addressed separately. 📚 Learning: 2025-09-25T12:39:45.755Z
Learnt from: mike182uk Repo: TryGhost/ActivityPub PR: 1329 File: .github/renovate.json5:31-34 Timestamp: 2025-09-25T12:39:45.755Z Learning: The jobs/update-external-accounts directory was removed from the TryGhost/ActivityPub repository and no longer exists, so Renovate configuration exclusions referencing this path are no longer needed. Applied to files:
package.json
⏰ 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). (1)
- GitHub Check: build / Build Docker Images (linux/amd64, amd64)
🔇 Additional comments (4)
docker-compose.yml (1)
5-5: LGTM!The port mapping correctly exposes container port 8080 to the host, enabling access via
host.docker.internalas described in the PR objectives. This aligns with the existingPORT=8080environment variable.package.json (1)
14-14: LGTM!The new
dev:ghostscript correctly starts only theactivitypubservice withoutnginx, enabling deferred proxying to the Ghost development gateway as described in the PR objectives.src/app.ts (2)
25-25: LGTM!The import of
serveStaticis correct and necessary for the development-time static file serving functionality.
534-544: Development-time static file serving looks good.The implementation correctly guards the functionality with a development environment check and uses the
serveStaticmiddleware with appropriate path rewriting. The middleware should handle path traversal security concerns.
| } | ||
| | ||
| if (process.env.GCP_STORAGE_EMULATOR_HOST) { | ||
| const gcsBucket = process.env.GCP_BUCKET_NAME!; |
There was a problem hiding this comment.
Add validation for required environment variable.
The non-null assertion operator assumes GCP_BUCKET_NAME is set, but there's no validation when GCP_STORAGE_EMULATOR_HOST is set. If GCP_BUCKET_NAME is undefined, it will cause a runtime error when constructing the GCS URL.
🔎 Apply this diff to add validation:
if (process.env.GCP_STORAGE_EMULATOR_HOST) { - const gcsBucket = process.env.GCP_BUCKET_NAME!; + const gcsBucket = process.env.GCP_BUCKET_NAME; + if (!gcsBucket) { + globalLogging.warn('GCP_BUCKET_NAME not set, skipping GCS emulator proxy setup'); + } else { + app.get('/.ghost/activitypub/gcs/*', async (ctx) => { + const gcsPath = ctx.req.path.replace( + '/.ghost/activitypub/gcs/', + '', + ); + // fake-gcs-server serves objects at /download/storage/v1/b/{bucket}/o/{object} + const gcsUrl = `${process.env.GCP_STORAGE_EMULATOR_HOST}/download/storage/v1/b/${gcsBucket}/o/${encodeURIComponent(gcsPath)}?alt=media`; - app.get('/.ghost/activitypub/gcs/*', async (ctx) => { - const gcsPath = ctx.req.path.replace( - '/.ghost/activitypub/gcs/', - '', - ); - // fake-gcs-server serves objects at /download/storage/v1/b/{bucket}/o/{object} - const gcsUrl = `${process.env.GCP_STORAGE_EMULATOR_HOST}/download/storage/v1/b/${gcsBucket}/o/${encodeURIComponent(gcsPath)}?alt=media`; - - const response = await fetch(gcsUrl); - - return new Response(response.body, { - status: response.status, - headers: response.headers, + const response = await fetch(gcsUrl); + + return new Response(response.body, { + status: response.status, + headers: response.headers, + }); }); - }); + } }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const gcsBucket = process.env.GCP_BUCKET_NAME!; | |
| if (process.env.GCP_STORAGE_EMULATOR_HOST) { | |
| const gcsBucket = process.env.GCP_BUCKET_NAME; | |
| if (!gcsBucket) { | |
| globalLogging.warn('GCP_BUCKET_NAME not set, skipping GCS emulator proxy setup'); | |
| } else { | |
| app.get('/.ghost/activitypub/gcs/*', async (ctx) => { | |
| const gcsPath = ctx.req.path.replace( | |
| '/.ghost/activitypub/gcs/', | |
| '', | |
| ); | |
| // fake-gcs-server serves objects at /download/storage/v1/b/{bucket}/o/{object} | |
| const gcsUrl = `${process.env.GCP_STORAGE_EMULATOR_HOST}/download/storage/v1/b/${gcsBucket}/o/${encodeURIComponent(gcsPath)}?alt=media`; | |
| const response = await fetch(gcsUrl); | |
| return new Response(response.body, { | |
| status: response.status, | |
| headers: response.headers, | |
| }); | |
| }); | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/app.ts around line 547, remove the non-null assertion on GCP_BUCKET_NAME and add validation: if process.env.GCP_STORAGE_EMULATOR_HOST is set but process.env.GCP_BUCKET_NAME is undefined or empty, log an explicit error (or throw) and exit/stop initialization with a clear message indicating GCP_BUCKET_NAME is required when using the emulator; otherwise read GCP_BUCKET_NAME into gcsBucket safely. | app.get('/.ghost/activitypub/gcs/*', async (ctx) => { | ||
| const gcsPath = ctx.req.path.replace( | ||
| '/.ghost/activitypub/gcs/', | ||
| '', | ||
| ); | ||
| // fake-gcs-server serves objects at /download/storage/v1/b/{bucket}/o/{object} | ||
| const gcsUrl = `${process.env.GCP_STORAGE_EMULATOR_HOST}/download/storage/v1/b/${gcsBucket}/o/${encodeURIComponent(gcsPath)}?alt=media`; | ||
| | ||
| const response = await fetch(gcsUrl); | ||
| | ||
| return new Response(response.body, { | ||
| status: response.status, | ||
| headers: response.headers, | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Add error handling for GCS emulator fetch.
The fetch call to the GCS emulator has no error handling. If the fetch fails (e.g., emulator is down, network error, invalid path), it will result in an unhandled promise rejection and crash the request handler.
🔎 Apply this diff to add error handling:
app.get('/.ghost/activitypub/gcs/*', async (ctx) => { const gcsPath = ctx.req.path.replace( '/.ghost/activitypub/gcs/', '', ); // fake-gcs-server serves objects at /download/storage/v1/b/{bucket}/o/{object} const gcsUrl = `${process.env.GCP_STORAGE_EMULATOR_HOST}/download/storage/v1/b/${gcsBucket}/o/${encodeURIComponent(gcsPath)}?alt=media`; - const response = await fetch(gcsUrl); - - return new Response(response.body, { - status: response.status, - headers: response.headers, - }); + try { + const response = await fetch(gcsUrl); + + return new Response(response.body, { + status: response.status, + headers: response.headers, + }); + } catch (error) { + ctx.get('logger').error('Failed to fetch from GCS emulator: {error}', { error }); + return new Response('Failed to fetch from GCS emulator', { + status: 500, + }); + } });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| app.get('/.ghost/activitypub/gcs/*', async (ctx) => { | |
| const gcsPath = ctx.req.path.replace( | |
| '/.ghost/activitypub/gcs/', | |
| '', | |
| ); | |
| // fake-gcs-server serves objects at /download/storage/v1/b/{bucket}/o/{object} | |
| const gcsUrl = `${process.env.GCP_STORAGE_EMULATOR_HOST}/download/storage/v1/b/${gcsBucket}/o/${encodeURIComponent(gcsPath)}?alt=media`; | |
| const response = await fetch(gcsUrl); | |
| return new Response(response.body, { | |
| status: response.status, | |
| headers: response.headers, | |
| }); | |
| }); | |
| app.get('/.ghost/activitypub/gcs/*', async (ctx) => { | |
| const gcsPath = ctx.req.path.replace( | |
| '/.ghost/activitypub/gcs/', | |
| '', | |
| ); | |
| // fake-gcs-server serves objects at /download/storage/v1/b/{bucket}/o/{object} | |
| const gcsUrl = `${process.env.GCP_STORAGE_EMULATOR_HOST}/download/storage/v1/b/${gcsBucket}/o/${encodeURIComponent(gcsPath)}?alt=media`; | |
| try { | |
| const response = await fetch(gcsUrl); | |
| return new Response(response.body, { | |
| status: response.status, | |
| headers: response.headers, | |
| }); | |
| } catch (error) { | |
| ctx.get('logger').error('Failed to fetch from GCS emulator: {error}', { error }); | |
| return new Response('Failed to fetch from GCS emulator', { | |
| status: 500, | |
| }); | |
| } | |
| }); |
🤖 Prompt for AI Agents
In src/app.ts around lines 549 to 563, the fetch to the GCS emulator lacks error handling; wrap the fetch in try/catch to catch network errors, log the error, and return an appropriate HTTP response (e.g., 502 Bad Gateway or 504) instead of letting the promise reject; after fetching, check response.ok and if false forward the response status and body (or translate to 404/502 as appropriate) so callers receive meaningful status codes; ensure you also handle and close response streams properly and avoid rethrowing unhandled errors. 
package.json Outdated
| "dev": "docker compose up activitypub nginx -d --build", | ||
| "dev:ghost": "docker compose up activitypub -d --build", |
There was a problem hiding this comment.
@sagzy I was thinking of changing this to:
dev-docker compose up activitypub -d --builddev:standalone-docker compose up activitypub nginx -d --build
but didn't want to break muscle memory 😅
There was a problem hiding this comment.
Let's do it! Think the yarn dev:forward experience generally is much nicer and we should use that as our new default :)
There was a problem hiding this comment.
Cool, i've added that here:
Docs are still focused on the old setup - we should keep this around until we've fully switched over
sagzy left a comment
sagzy left a comment There was a problem hiding this comment.
Works great for me locally
| One small nitpick: shall we update the readme to use https://github.com/tryghost/activitypub?tab=readme-ov-file#setup → step 5 |
no ref
We recently added a new development workflow for main Ghost repo (
yarn dev:forward). These changes support this workflow by deferring proxying to the development gateway in the Ghost repo. Changes include:yarn dev:ghost) that starts everything butnginx8080so that the app can be accessed viahost.docker.internal