Repository navigation
refactor: migrate holyship to bootPlatformServer - #259
Conversation
Replace manual platform boot sequence with bootPlatformServer() from platform-core. Keeps all product-specific code (engine, flows, github, worker pool, ship-it) as route plugins on platform.app. - Delete app.ts (Hono app, CORS, health now handled by boot) - Use createTRPCContext from core instead of local copy - Engine DB created from container.pool - Engine migrations run after platform boot - Bump platform-core to 1.71.0 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughThe changes consolidate application boot logic by replacing manual sequential setup with a single Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Coverage Report
File Coverage
|
||||||||||||||||||||||||||||||||||||||
Greptile SummaryThis PR replaces holyship's manual platform boot sequence (DB, migrations, auth, credits, org, gateway) with a single Confidence Score: 3/5Hold until the crypto webhook regression is confirmed resolved and CI passes. The refactor is clean and well-structured, but the silent removal of the crypto payment webhook route is a potential data-loss regression — incoming payment confirmations could 404 undetected. The fix is small (confirm platform-core handles it or re-mount the route), but it needs explicit verification. The productConfig null-safety issue is also a regression, though caught by try/catch. CI is not yet green per the PR's own checklist. src/index.ts — verify the crypto webhook route is handled by bootPlatformServer() or re-add the explicit mount; fix productConfig optional chaining.
|
| Filename | Overview |
|---|---|
| src/index.ts | Major boot refactor to bootPlatformServer(); crypto webhook route silently dropped, productConfig null-safety regression, and as-never type cast introduced. |
| src/app.ts | Deleted — Hono app, CORS, secure headers, and BetterAuth handler moved into bootPlatformServer() and index.ts respectively. |
| package.json | platform-core bumped from ^1.67.1 to ^1.71.0; lock file updated with new transitive deps (@aws-sdk/client-ses, platform-crypto-server, viem, ox). |
| pnpm-lock.yaml | Lock file updated consistently with package.json bump; platform-crypto-server 1.1.1 introduces a second drizzle-orm version (0.44.7) as a transitive dep alongside the app's 0.45.1. |
Sequence Diagram
sequenceDiagram
participant Main as main()
participant Boot as bootPlatformServer()
participant Platform as platform container
participant App as Hono app
Main->>Boot: { slug, databaseUrl, host, port, features }
Boot-->>Main: { app, container, start(), stop() }
Main->>Platform: container.db (platformDb)
Main->>Main: run local drizzle/ migrations
Main->>Platform: container.pool → drizzle(engineDb)
Main->>App: mount /api/auth/* (BetterAuth)
Main->>Platform: container.orgMemberRepo → setTrpcOrgMemberRepo()
Main->>App: mount /v1 gateway (if OPENROUTER_API_KEY)
Main->>App: mount /trpc/* (appRouter)
Main->>Main: init Engine + WorkerPool
Main->>App: mount /api engine routes
Main->>App: mount /api/ship-it routes
Main->>App: mount /api/github/* routes
Main->>App: mount /api flow-editor + interrogation routes
Main->>Boot: platform.start() → listen on HOST:PORT
note over Main,App: ⚠️ /api/webhooks/crypto NOT remounted
Prompt To Fix All With AI
This is a comment left during a code review.
Path: src/index.ts
Line: 670-714
Comment:
**Crypto webhook route silently dropped**
The old code (step 13 in the previous boot sequence) wired and mounted the crypto payment webhook route:
```ts
setCryptoWebhookDeps({ chargeStore: cryptoChargeRepo, creditLedger, replayGuard }, config.CRYPTO_WEBHOOK_SECRET);
app.route("/api/webhooks/crypto", cryptoWebhookRoutes);
```
This block is entirely absent from the new code. `features.crypto` is passed to `bootPlatformServer()`, but `CRYPTO_SERVICE_URL` and `CRYPTO_WEBHOOK_SECRET` are **not** — meaning platform-core cannot reconstruct the route with the correct webhook secret. If `bootPlatformServer()` does not internally mount `/api/webhooks/crypto`, any incoming crypto payment confirmations will receive a 404 and users will never be credited. This is a silent data-loss regression when `CRYPTO_SERVICE_URL` and `CRYPTO_WEBHOOK_SECRET` are set.
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: src/index.ts
Line: 684-688
Comment:
**`container.productConfig` null-safety regression**
The old code accessed `_productConfig?.product.fromEmail` — safe because `_productConfig` could be `null` after a failed product config boot. The new code:
```ts
const productConfig = container.productConfig;
const emailClient = getEmailClient({
from: productConfig.product?.fromEmail || undefined,
```
If `container.productConfig` is `undefined` or `null`, `productConfig.product` throws a `TypeError` at runtime. The entire notification pipeline is wrapped in a `try/catch`, so this would be swallowed silently, but it represents a regression in null-safety compared to the previous pattern. Consider using:
```suggestion
const productConfig = container.productConfig;
const emailClient = getEmailClient({
from: productConfig?.product?.fromEmail || undefined,
replyTo: productConfig?.product?.emailSupport || undefined,
```
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: src/index.ts
Line: 104
Comment:
**`as never` type cast bypasses type safety**
`platformDb as never` is an overly aggressive cast that completely erases the type and suppresses any future type-checker complaints. The `migrate` function expects `NodePgDatabase<any>`, which `container.db` almost certainly satisfies. A safer cast would be:
```suggestion
await migrate(platformDb as import("drizzle-orm/node-postgres").NodePgDatabase<Record<string, never>>, { migrationsFolder: localMigrations });
```
Or at minimum `as any` with a `biome-ignore` comment and reason, which is already the established pattern in this file.
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "refactor: migrate holyship to bootPlatfo..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| @@ -715,9 +681,10 @@ async function main() { | |||
|
|
|||
| // biome-ignore lint/suspicious/noExplicitAny: PgDatabase generic | |||
| const pgDb = platformDb as any; | |||
| const productConfig = container.productConfig; | |||
| const emailClient = getEmailClient({ | |||
| from: _productConfig?.product.fromEmail || undefined, | |||
| replyTo: _productConfig?.product.emailSupport || undefined, | |||
| from: productConfig.product?.fromEmail || undefined, | |||
| replyTo: productConfig.product?.emailSupport || undefined, | |||
| }); | |||
| const queueStore = new DrizzleNotificationQueueStore(platformDb); | |||
| const prefsStore = new DrizzleNotificationPreferencesStore(platformDb); | |||
| @@ -746,24 +713,22 @@ async function main() { | |||
| } | |||
| } | |||
There was a problem hiding this comment.
The old code (step 13 in the previous boot sequence) wired and mounted the crypto payment webhook route:
setCryptoWebhookDeps({ chargeStore: cryptoChargeRepo, creditLedger, replayGuard }, config.CRYPTO_WEBHOOK_SECRET);
app.route("/api/webhooks/crypto", cryptoWebhookRoutes);This block is entirely absent from the new code. features.crypto is passed to bootPlatformServer(), but CRYPTO_SERVICE_URL and CRYPTO_WEBHOOK_SECRET are not — meaning platform-core cannot reconstruct the route with the correct webhook secret. If bootPlatformServer() does not internally mount /api/webhooks/crypto, any incoming crypto payment confirmations will receive a 404 and users will never be credited. This is a silent data-loss regression when CRYPTO_SERVICE_URL and CRYPTO_WEBHOOK_SECRET are set.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/index.ts
Line: 670-714
Comment:
**Crypto webhook route silently dropped**
The old code (step 13 in the previous boot sequence) wired and mounted the crypto payment webhook route:
```ts
setCryptoWebhookDeps({ chargeStore: cryptoChargeRepo, creditLedger, replayGuard }, config.CRYPTO_WEBHOOK_SECRET);
app.route("/api/webhooks/crypto", cryptoWebhookRoutes);
```
This block is entirely absent from the new code. `features.crypto` is passed to `bootPlatformServer()`, but `CRYPTO_SERVICE_URL` and `CRYPTO_WEBHOOK_SECRET` are **not** — meaning platform-core cannot reconstruct the route with the correct webhook secret. If `bootPlatformServer()` does not internally mount `/api/webhooks/crypto`, any incoming crypto payment confirmations will receive a 404 and users will never be credited. This is a silent data-loss regression when `CRYPTO_SERVICE_URL` and `CRYPTO_WEBHOOK_SECRET` are set.
How can I resolve this? If you propose a fix, please make it concise.| const productConfig = container.productConfig; | ||
| const emailClient = getEmailClient({ | ||
| from: _productConfig?.product.fromEmail || undefined, | ||
| replyTo: _productConfig?.product.emailSupport || undefined, | ||
| from: productConfig.product?.fromEmail || undefined, | ||
| replyTo: productConfig.product?.emailSupport || undefined, | ||
| }); |
There was a problem hiding this comment.
container.productConfig null-safety regression
The old code accessed _productConfig?.product.fromEmail — safe because _productConfig could be null after a failed product config boot. The new code:
const productConfig = container.productConfig;
const emailClient = getEmailClient({
from: productConfig.product?.fromEmail || undefined,If container.productConfig is undefined or null, productConfig.product throws a TypeError at runtime. The entire notification pipeline is wrapped in a try/catch, so this would be swallowed silently, but it represents a regression in null-safety compared to the previous pattern. Consider using:
| const productConfig = container.productConfig; | |
| const emailClient = getEmailClient({ | |
| from: _productConfig?.product.fromEmail || undefined, | |
| replyTo: _productConfig?.product.emailSupport || undefined, | |
| from: productConfig.product?.fromEmail || undefined, | |
| replyTo: productConfig.product?.emailSupport || undefined, | |
| }); | |
| const productConfig = container.productConfig; | |
| const emailClient = getEmailClient({ | |
| from: productConfig?.product?.fromEmail || undefined, | |
| replyTo: productConfig?.product?.emailSupport || undefined, |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/index.ts
Line: 684-688
Comment:
**`container.productConfig` null-safety regression**
The old code accessed `_productConfig?.product.fromEmail` — safe because `_productConfig` could be `null` after a failed product config boot. The new code:
```ts
const productConfig = container.productConfig;
const emailClient = getEmailClient({
from: productConfig.product?.fromEmail || undefined,
```
If `container.productConfig` is `undefined` or `null`, `productConfig.product` throws a `TypeError` at runtime. The entire notification pipeline is wrapped in a `try/catch`, so this would be swallowed silently, but it represents a regression in null-safety compared to the previous pattern. Consider using:
```suggestion
const productConfig = container.productConfig;
const emailClient = getEmailClient({
from: productConfig?.product?.fromEmail || undefined,
replyTo: productConfig?.product?.emailSupport || undefined,
```
How can I resolve this? If you propose a fix, please make it concise.| const { migrate } = await import("drizzle-orm/node-postgres/migrator"); | ||
| const localMigrations = path.resolve(process.cwd(), "drizzle"); | ||
| if (existsSync(localMigrations)) { | ||
| await migrate(platformDb as never, { migrationsFolder: localMigrations }); |
There was a problem hiding this comment.
as never type cast bypasses type safety
platformDb as never is an overly aggressive cast that completely erases the type and suppresses any future type-checker complaints. The migrate function expects NodePgDatabase<any>, which container.db almost certainly satisfies. A safer cast would be:
| await migrate(platformDb as never, { migrationsFolder: localMigrations }); | |
| await migrate(platformDb as import("drizzle-orm/node-postgres").NodePgDatabase<Record<string, never>>, { migrationsFolder: localMigrations }); |
Or at minimum as any with a biome-ignore comment and reason, which is already the established pattern in this file.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/index.ts
Line: 104
Comment:
**`as never` type cast bypasses type safety**
`platformDb as never` is an overly aggressive cast that completely erases the type and suppresses any future type-checker complaints. The `migrate` function expects `NodePgDatabase<any>`, which `container.db` almost certainly satisfies. A safer cast would be:
```suggestion
await migrate(platformDb as import("drizzle-orm/node-postgres").NodePgDatabase<Record<string, never>>, { migrationsFolder: localMigrations });
```
Or at minimum `as any` with a `biome-ignore` comment and reason, which is already the established pattern in this file.
How can I resolve this? If you propose a fix, please make it concise.
|
🎉 This PR is included in version 1.47.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Summary
bootPlatformServer()from platform-coreapp.ts— Hono app, CORS, health endpoint now handled by bootcreateTRPCContextfrom core instead of local copycontainer.poolinstead of old singletonTest plan
npm run checkpasses (biome + tsc)🤖 Generated with Claude Code
Note
Migrate holyship server boot to
bootPlatformServerfrom platform-core@hono/node-serverusage and src/app.ts) withbootPlatformServerfrom@wopr-network/platform-core, which provides the Hono app instance and a shared service container.createTRPCContextis now imported fromplatform-core/trpcinstead of defined locally, which may change howuserandtenantIdare populated in tRPC procedures./api/webhooks/crypto) and the explicit/healthendpoint are removed and will no longer be served by this module.Macroscope summarized 3acd750.
Summary by CodeRabbit