Repository navigation
Split server-entry.ts into domain-scoped bootstrap modules - #180
Merged
Merged
Conversation
server-entry.ts had grown to 1806 lines, owning ~110 /api route registrations, request-forwarding glue, raw DB queries, CSP/security header construction, and background-job aggregation for every feature area in the app. Move each concern into its own module under src/server/ (federation-middleware, security-headers, background-jobs, media-routes, content-negotiation-router) and split API routes into per-domain registrars under src/server/api/ (auth, user, group, event, discussion, event-notice, poll, place, admin/*, misc), mirroring the existing controllers/ domain layout. Also extract the three raw Drizzle queries that lived in the bootstrap file into new minimal repositories/actors.ts and repositories/polls.ts. This is a mechanical, behavior-preserving move: route paths, methods, forwarding logic, and app.use ordering are unchanged. server-entry.ts is now a ~35-line composition root.
📝 WalkthroughWalkthroughThe server entry is refactored into composed H3 middleware and route registrars. API domains, forwarding helpers, repositories, media handlers, security headers, background jobs, federation integration, and content negotiation are moved into dedicated modules. ChangesServer routing composition
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant H3App
participant ApiRouter
participant Controller
participant StartFetch
Client->>H3App: HTTP request
H3App->>ApiRouter: dispatch /api request
ApiRouter->>Controller: convert and forward request
Controller-->>ApiRouter: response
ApiRouter-->>H3App: API response
H3App->>StartFetch: fallback unmatched request
StartFetch-->>H3App: rendered response
H3App-->>Client: HTTP response
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
![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
🧹 Nitpick comments (2)
src/server/api/forwarding.ts (1)
39-62: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winStrip stale
content-lengthwhen rebuilding forwarded requests.Both
forwardJsonandforwardFormDatacopy headers from the original request but replace the body (mutated JSON, or FormData with a new multipart boundary). The copiedcontent-lengthno longer matches the new body. Downstream consumers that read the body stream are unaffected, but any that inspectcontent-lengthcould see a wrong value.♻️ Proposed fix
const headers = new Headers(request.headers); headers.set("content-type", "application/json"); + headers.delete("content-length"); return new Request(buildForwardUrl(request, pathname), {const headers = new Headers(request.headers); headers.delete("content-type"); + headers.delete("content-length"); return new Request(buildForwardUrl(request, pathname), {Please confirm whether your runtime (undici/Node fetch) honors an explicit
content-lengthheader over the actual body length when constructing aRequest.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/server/api/forwarding.ts` around lines 39 - 62, Update both forwardJson and forwardFormData to delete the inherited content-length header after copying request.headers and before constructing the replacement Request, alongside the existing content-type handling. Preserve all other forwarded headers and body behavior.src/server/api/group-routes.ts (1)
69-147: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract repeated group-lookup boilerplate.
findGroupHandleById(groupId)followed by a 404 check is duplicated verbatim across the group-update, avatar-upload, and create-note handlers (Lines 78-81, 105-108, 131-134).♻️ Proposed helper extraction
+async function requireGroupHandle( + groupId: string, +): Promise<{ handle: string } | { error: Response }> { + const handle = await findGroupHandleById(groupId); + if (!handle) { + return { error: Response.json({ error: "Group not found" }, { status: 404 }) }; + } + return { handle }; +}Then in each handler:
- const handle = await findGroupHandleById(groupId); - if (!handle) { - return Response.json({ error: "Group not found" }, { status: 404 }); - } + const result = await requireGroupHandle(groupId); + if ("error" in result) return result.error; + const { handle } = result;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/server/api/group-routes.ts` around lines 69 - 147, Extract the repeated findGroupHandleById lookup and “Group not found” 404 response into a shared helper near the route definitions. Update the PATCH group-update, avatar-upload, and group-post handlers to call this helper and reuse its result, while preserving the existing groupId validation and response behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/server/api/event-routes.ts`:
- Around line 59-286: Introduce a shared requireParam helper that validates
route parameters and returns the standardized 400 response, then replace the
listed inline guards in src/server/api/event-routes.ts (59-286),
src/server/api/poll-routes.ts (16-88), src/server/api/group-routes.ts (74-174),
src/server/api/discussion-routes.ts (78-109), and
src/server/api/event-notice-routes.ts (13-14). Use it consistently for each
required parameter while preserving existing handler behavior.
In `@src/server/repositories/actors.ts`:
- Around line 15-17: Update findGroupByHandle’s return type to use Drizzle’s
InferSelectModel derived from the actors table schema instead of a hand-written
record shape, preserving the existing undefined result for no match.
---
Nitpick comments:
In `@src/server/api/forwarding.ts`:
- Around line 39-62: Update both forwardJson and forwardFormData to delete the
inherited content-length header after copying request.headers and before
constructing the replacement Request, alongside the existing content-type
handling. Preserve all other forwarded headers and body behavior.
In `@src/server/api/group-routes.ts`:
- Around line 69-147: Extract the repeated findGroupHandleById lookup and “Group
not found” 404 response into a shared helper near the route definitions. Update
the PATCH group-update, avatar-upload, and group-post handlers to call this
helper and reuse its result, while preserving the existing groupId validation
and response behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c47b0898-ca9d-4aaf-b322-ecb8df71d7c2
📒 Files selected for processing (23)
src/server-entry.tssrc/server/api/admin/banner-routes.tssrc/server/api/admin/catalog-routes.tssrc/server/api/admin/directory-routes.tssrc/server/api/admin/router.tssrc/server/api/auth-routes.tssrc/server/api/discussion-routes.tssrc/server/api/event-notice-routes.tssrc/server/api/event-routes.tssrc/server/api/forwarding.tssrc/server/api/group-routes.tssrc/server/api/misc-routes.tssrc/server/api/place-routes.tssrc/server/api/poll-routes.tssrc/server/api/router.tssrc/server/api/user-routes.tssrc/server/background-jobs.tssrc/server/content-negotiation-router.tssrc/server/federation-middleware.tssrc/server/media-routes.tssrc/server/repositories/actors.tssrc/server/repositories/polls.tssrc/server/security-headers.ts
Comment on lines
+59
to
+286
| if (!eventId) | ||
| return Response.json({ error: "eventId is required" }, { status: 400 }); | ||
|
|
||
| return updateEvent({ | ||
| request: await forwardJson( | ||
| request, | ||
| `/api/events/${eventId}`, | ||
| "POST", | ||
| (body) => ({ | ||
| ...(body ?? {}), | ||
| eventId, | ||
| }), | ||
| ), | ||
| }); | ||
| }), | ||
| ); | ||
|
|
||
| router.get( | ||
| "/events/:eventId/rsvp", | ||
| defineEventHandler(async (event) => { | ||
| const request = toWebRequest(event); | ||
| const eventId = event.context.params?.eventId; | ||
| return rsvpStatus({ | ||
| request: forwardGet(request, `/api/events/${eventId}/rsvp`, { eventId }), | ||
| }); | ||
| }), | ||
| ); | ||
|
|
||
| router.put( | ||
| "/events/:eventId/rsvp", | ||
| defineEventHandler(async (event) => { | ||
| const request = toWebRequest(event); | ||
| const eventId = event.context.params?.eventId; | ||
| if (!eventId) | ||
| return Response.json({ error: "eventId is required" }, { status: 400 }); | ||
|
|
||
| return submitRsvp({ | ||
| request: await forwardJson( | ||
| request, | ||
| `/api/events/${eventId}/rsvp`, | ||
| "POST", | ||
| (body) => ({ | ||
| ...(body ?? {}), | ||
| eventId, | ||
| }), | ||
| ), | ||
| }); | ||
| }), | ||
| ); | ||
|
|
||
| router.put( | ||
| "/events/:eventId/rsvp/anonymous", | ||
| defineEventHandler(async (event) => { | ||
| const request = toWebRequest(event); | ||
| const eventId = event.context.params?.eventId; | ||
| if (!eventId) | ||
| return Response.json({ error: "eventId is required" }, { status: 400 }); | ||
|
|
||
| return submitAnonymousRsvp({ | ||
| request: await forwardJson( | ||
| request, | ||
| `/api/events/${eventId}/rsvp/anonymous`, | ||
| "POST", | ||
| (body) => ({ | ||
| ...(body ?? {}), | ||
| eventId, | ||
| }), | ||
| ), | ||
| }); | ||
| }), | ||
| ); | ||
|
|
||
| router.delete( | ||
| "/events/:eventId/rsvp/anonymous", | ||
| defineEventHandler(async (event) => { | ||
| const request = toWebRequest(event); | ||
| const eventId = event.context.params?.eventId; | ||
| if (!eventId) | ||
| return Response.json({ error: "eventId is required" }, { status: 400 }); | ||
|
|
||
| return cancelAnonymousRsvp({ request, eventId }); | ||
| }), | ||
| ); | ||
|
|
||
| router.patch( | ||
| "/events/:eventId/rsvps/:rsvpId", | ||
| defineEventHandler(async (event) => { | ||
| const request = toWebRequest(event); | ||
| const eventId = event.context.params?.eventId; | ||
| const rsvpId = event.context.params?.rsvpId; | ||
| if (!eventId || !rsvpId) | ||
| return Response.json( | ||
| { error: "eventId and rsvpId are required" }, | ||
| { status: 400 }, | ||
| ); | ||
|
|
||
| return manageRsvp({ | ||
| request: await forwardJson( | ||
| request, | ||
| `/api/events/${eventId}/rsvps/${rsvpId}`, | ||
| "PATCH", | ||
| (body) => body ?? {}, | ||
| ), | ||
| eventId, | ||
| rsvpId, | ||
| }); | ||
| }), | ||
| ); | ||
|
|
||
| router.get( | ||
| "/events/:eventId/favourite", | ||
| defineEventHandler(async (event) => { | ||
| const request = toWebRequest(event); | ||
| const eventId = event.context.params?.eventId; | ||
| return getFavouriteStatus({ | ||
| request: forwardGet(request, `/api/events/${eventId}/favourite`, { | ||
| eventId, | ||
| }), | ||
| }); | ||
| }), | ||
| ); | ||
|
|
||
| router.post( | ||
| "/events/:eventId/favourite", | ||
| defineEventHandler(async (event) => { | ||
| const request = toWebRequest(event); | ||
| const eventId = event.context.params?.eventId; | ||
| return toggleFavourite({ | ||
| request: await forwardJson( | ||
| request, | ||
| `/api/events/${eventId}/favourite`, | ||
| "POST", | ||
| (body) => ({ | ||
| ...body, | ||
| eventId, | ||
| }), | ||
| ), | ||
| }); | ||
| }), | ||
| ); | ||
|
|
||
| router.get( | ||
| "/events/:eventId/attendees", | ||
| defineEventHandler(async (event) => { | ||
| const request = toWebRequest(event); | ||
| const eventId = event.context.params?.eventId; | ||
| return eventAttendees({ | ||
| request: forwardGet(request, `/api/events/${eventId}/attendees`, { | ||
| eventId, | ||
| }), | ||
| }); | ||
| }), | ||
| ); | ||
|
|
||
| router.get( | ||
| "/events/:eventId/dashboard", | ||
| defineEventHandler(async (event) => { | ||
| const request = toWebRequest(event); | ||
| const eventId = event.context.params?.eventId; | ||
| return eventDashboard({ | ||
| request: forwardGet(request, `/api/events/${eventId}/dashboard`, { | ||
| eventId, | ||
| }), | ||
| }); | ||
| }), | ||
| ); | ||
|
|
||
| router.get( | ||
| "/events/:eventId/dashboard/activity", | ||
| defineEventHandler(async (event) => { | ||
| const request = toWebRequest(event); | ||
| const eventId = event.context.params?.eventId; | ||
| return eventDashboardActivity({ | ||
| request: forwardGet( | ||
| request, | ||
| `/api/events/${eventId}/dashboard/activity`, | ||
| { eventId }, | ||
| ), | ||
| }); | ||
| }), | ||
| ); | ||
|
|
||
| registerDiscussionRoutes(router); | ||
| registerEventNoticeRoutes(router); | ||
|
|
||
| router.post( | ||
| "/events/:eventId/publish", | ||
| defineEventHandler(async (event) => { | ||
| const request = toWebRequest(event); | ||
| const eventId = event.context.params?.eventId; | ||
| if (!eventId) | ||
| return Response.json({ error: "eventId is required" }, { status: 400 }); | ||
|
|
||
| return publishEvent({ | ||
| request: await forwardJson( | ||
| request, | ||
| `/api/events/${eventId}/publish`, | ||
| "POST", | ||
| (body) => ({ | ||
| ...(body ?? {}), | ||
| eventId, | ||
| }), | ||
| ), | ||
| }); | ||
| }), | ||
| ); | ||
|
|
||
| router.delete( | ||
| "/events/:eventId", | ||
| defineEventHandler(async (event) => { | ||
| const request = toWebRequest(event); | ||
| const eventId = event.context.params?.eventId; | ||
| if (!eventId) | ||
| return Response.json({ error: "eventId is required" }, { status: 400 }); | ||
|
|
||
| return deleteEvent({ | ||
| request: forwardGet(request, `/api/events/${eventId}`, { eventId }), | ||
| }); | ||
| }), | ||
| ); | ||
|
|
||
| router.post( | ||
| "/events/:eventId/header-image", | ||
| defineEventHandler(async (event) => { | ||
| const request = toWebRequest(event); | ||
| const eventId = event.context.params?.eventId; | ||
| if (!eventId) | ||
| return Response.json({ error: "eventId is required" }, { status: 400 }); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Extract a shared requireParam helper instead of repeating the same 400-guard ~19 times across 5 files.
Every occurrence re-implements the identical shape: read a route param, check for falsy, return a hand-written Response.json({ error: "... is required" }, { status: 400 }). This is copy/paste duplication across the whole router layer, and a shared helper would also make it trivial to close the inconsistency where sibling GET handlers on the same params (e.g. event-routes.ts lines 43-52, 76-85; poll-routes.ts lines 36-58; discussion-routes.ts lines 12-70) skip the check entirely.
src/server/api/event-routes.ts#L59-L286: replace the 8 inline guards (Lines 59-60, 92-93, 114-115, 136-137, 149-153, 249-250, 271-272, 285-286) with the shared helper.src/server/api/poll-routes.ts#L16-L88: replace the 3 inline guards (Lines 16-20, 65-66, 87-88) with the shared helper.src/server/api/group-routes.ts#L74-L174: replace the 5 inline guards (Lines 74-76, 102-103, 128-129, 154-155, 170-174) with the shared helper.src/server/api/discussion-routes.ts#L78-L109: replace the 2 inline guards (Lines 78-82, 105-109) with the shared helper.src/server/api/event-notice-routes.ts#L13-L14: replace the 1 inline guard with the shared helper.
♻️ Proposed helper (e.g. in forwarding.ts or a new params.ts)
+export function requireParam(
+ params: Record<string, string | undefined> | undefined,
+ name: string,
+): string | Response {
+ const value = params?.[name];
+ if (!value) {
+ return Response.json({ error: `${name} is required` }, { status: 400 });
+ }
+ return value;
+}Usage:
- const eventId = event.context.params?.eventId;
- if (!eventId)
- return Response.json({ error: "eventId is required" }, { status: 400 });
+ const eventId = requireParam(event.context.params, "eventId");
+ if (eventId instanceof Response) return eventId;📍 Affects 5 files
src/server/api/event-routes.ts#L59-L286(this comment)src/server/api/poll-routes.ts#L16-L88src/server/api/group-routes.ts#L74-L174src/server/api/discussion-routes.ts#L78-L109src/server/api/event-notice-routes.ts#L13-L14
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/server/api/event-routes.ts` around lines 59 - 286, Introduce a shared
requireParam helper that validates route parameters and returns the standardized
400 response, then replace the listed inline guards in
src/server/api/event-routes.ts (59-286), src/server/api/poll-routes.ts (16-88),
src/server/api/group-routes.ts (74-174), src/server/api/discussion-routes.ts
(78-109), and src/server/api/event-notice-routes.ts (13-14). Use it consistently
for each required parameter while preserving existing handler behavior.
Comment on lines
+15
to
+17
| export async function findGroupByHandle(handle: string): Promise< | ||
| { id: string; name: string | null; handle: string } | undefined | ||
| > { |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Derive the return record type from the schema.
findGroupByHandle returns a hand-written { id; name; handle } shape that can silently drift from the actors table definition. As per coding guidelines, "Repository types must use Drizzle's InferSelectModel / InferInsertModel for type definitions."
♻️ Proposed fix
+import type { InferSelectModel } from "drizzle-orm";
import { and, eq } from "drizzle-orm";
import { db } from "~/server/db/client";
import { actors } from "~/server/db/schema";
@@
-export async function findGroupByHandle(handle: string): Promise<
- { id: string; name: string | null; handle: string } | undefined
-> {
+export async function findGroupByHandle(
+ handle: string,
+): Promise<
+ Pick<InferSelectModel<typeof actors>, "id" | "name" | "handle"> | undefined
+> {📝 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.
Suggested change
| export async function findGroupByHandle(handle: string): Promise< | |
| { id: string; name: string | null; handle: string } | undefined | |
| > { | |
| import type { InferSelectModel } from "drizzle-orm"; | |
| import { and, eq } from "drizzle-orm"; | |
| import { db } from "~/server/db/client"; | |
| import { actors } from "~/server/db/schema"; | |
| export async function findGroupByHandle( | |
| handle: string, | |
| ): Promise< | |
| Pick<InferSelectModel<typeof actors>, "id" | "name" | "handle"> | undefined | |
| > { |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/server/repositories/actors.ts` around lines 15 - 17, Update
findGroupByHandle’s return type to use Drizzle’s InferSelectModel derived from
the actors table schema instead of a hand-written record shape, preserving the
existing undefined result for no match.
Source: Coding guidelines
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
src/server-entry.tshad grown to 1806 lines, owning ~110/api/*route registrations, request-forwarding glue, three raw Drizzle queries, inline CSP/security-header construction, and background-job aggregation for every feature area in the app.federation-middleware.ts,security-headers.ts,background-jobs.ts,media-routes.ts,content-negotiation-router.ts, and per-domain API route registrars undersrc/server/api/(auth, user, group, event, discussion, event-notice, poll, place,admin/{banner,directory,catalog}, misc), mirroring the existingcontrollers/domain layout.repositories/actors.tsandrepositories/polls.ts.server-entry.tsis now a ~35-line composition root.app.useordering changed.Addresses "Refactor Audit — Umbrella Issue 4: Server bootstrap and API registration cleanup".
Test plan
pnpm typecheckpasses clean/api/*routes, all 8 non-/apiapp.usepaths, and all 8 content-negotiation regex patterns match exactly between old and new code (scripted diff, zero missing/extra)pnpm devboots cleanly; background jobs confirmed starting via log output/api/*path falling through to TanStack's 404 rather than erroring) — all reached the correct controller with correct behaviorpnpm lint— pre-existing, unrelated issue: repo has noeslint.config.js, not something this PR introducesSummary by CodeRabbit
New Features
Security