From 420210ffa080d8ebb46d29837a1c9d413c92b276 Mon Sep 17 00:00:00 2001 From: openhands Date: Fri, 25 Sep 2026 19:47:10 +0200 Subject: [PATCH] fix(build): make the production build pass, and stop it eating 20GB MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `next build` had never completed on this host, so three real defects were sitting in the tree untested. All three are now fixed and the build is green. - The build was not memory-bound the way it looked. Turbopack's builder reached 20.5GB RSS and died, and raising `--max-old-space-size` could never have helped: that flag caps the V8 heap, while the 20GB sat in Turbopack's own Rust allocator. The first symptom was misleading because the process doing the allocating is a grandchild of `npx`, so watching the direct child shows a 95MB shim the whole time. Building with `--webpack` puts the build back under the JS heap, where the flag actually applies: peak 5.9GB, 150s, exit 0. - withAdmin's second parameter was typed `{ params?: ... }` and given a `= {}` default, which made it optional and `RouteContext | undefined`. Next's generated route types assert that argument against `ParamCheck` and reject it, across 113 route files. `tsc --noEmit` cannot see this, because Next only adds `.next/types` to the project during a production build — so the type check that everyone runs locally was structurally incapable of catching the only type error that blocks a deploy. `params` is now required, which is also what the code already assumed: it is awaited with no guard. The 35 test call sites that invoked a handler with one argument now pass a real context, and the await got a guard so a direct internal call cannot turn a missing context into a 500. - `src/app/api/admin/import/furni/route.ts` re-exported `ensureDirectories` and `importSingleFurni` for "backward compatibility" that nothing used; the batch route imports from `@/lib/services/furni-import` directly. Next rejects any value export from a route module that is not an HTTP verb or config, so this had been breaking the build for as long as it existed. Removed. - `isomorphic-dompurify` builds its server-side DOM through jsdom. Bundled, that pulls jsdom's `browser/default-stylesheet.css` into the server chunk, where the path no longer resolves, and page-data collection dies with ENOENT on every page that sanitizes HTML. Marked external so Node resolves it from node_modules and the standalone tracer includes it. The remaining build warning is a pre-existing circular dependency between chunks that share the webpack runtime. It costs hash reuse, not correctness, and is left alone rather than churned here. Verified: build exit 0, 276 static pages generated, 3223 tests pass, tsc and biome clean. --- next.config.ts | 13 +++++++- .../admin/catalog-export/config/route.test.ts | 11 ++++--- .../api/admin/catalog/search/route.test.ts | 3 ++ src/app/api/admin/csrf/route.test.ts | 3 ++ src/app/api/admin/import/furni/route.ts | 9 +++--- src/app/api/admin/search/route.test.ts | 2 ++ .../api/admin/studio/inspect/route.test.ts | 17 +++++++---- .../admin/studio/nitro-quality/route.test.ts | 17 ++++++++--- .../admin/studio/nitro-scale32/route.test.ts | 30 ++++++++++++++----- .../admin/studio/source-assets/route.test.ts | 13 ++++++-- src/lib/api-handler-correlation.test.ts | 16 ++++++---- src/lib/api-handler.ts | 23 ++++++++++++-- src/test/route-context.ts | 15 ++++++++++ 13 files changed, 135 insertions(+), 37 deletions(-) create mode 100644 src/test/route-context.ts diff --git a/next.config.ts b/next.config.ts index 201d4d3d..109f7556 100644 --- a/next.config.ts +++ b/next.config.ts @@ -36,7 +36,18 @@ const nextConfig: NextConfig = { reactStrictMode: true, compress: true, productionBrowserSourceMaps: false, - serverExternalPackages: ["lzma-wasm", "sharp", "pino", "pino-pretty"], + // `isomorphic-dompurify` builds a DOM through jsdom on the server. Bundled, + // it drags jsdom's `browser/default-stylesheet.css` into the server chunk, + // where the path no longer exists and page-data collection dies with ENOENT + // on any page that sanitizes HTML. Kept external, Node resolves it from + // node_modules at runtime and the standalone output traces it in. + serverExternalPackages: [ + "lzma-wasm", + "sharp", + "pino", + "pino-pretty", + "isomorphic-dompurify", + ], async redirects() { return [ diff --git a/src/app/api/admin/catalog-export/config/route.test.ts b/src/app/api/admin/catalog-export/config/route.test.ts index 2fae15a7..f7940e72 100644 --- a/src/app/api/admin/catalog-export/config/route.test.ts +++ b/src/app/api/admin/catalog-export/config/route.test.ts @@ -17,6 +17,7 @@ vi.mock("@/lib/services/catalog-git-managed", () => ({ })); import { PERMS } from "@/lib/permission-slugs"; +import { emptyRouteContext } from "@/test/route-context"; import { GET, POST } from "./route"; let root: string; @@ -48,7 +49,7 @@ describe("Gitea configuration endpoint", () => { expect(mocks.guard).toHaveBeenCalledWith({ permission: PERMS.SETTINGS_EDIT, }); - const response = await POST(request()); + const response = await POST(request(), emptyRouteContext()); expect(response.status).toBe(200); const data = await response.json(); expect(data.hasToken).toBe(true); @@ -61,27 +62,29 @@ describe("Gitea configuration endpoint", () => { await ( await GET( new NextRequest("http://localhost/api/admin/catalog-export/config"), + emptyRouteContext(), ) ).text(), ).not.toContain(input.token); }); it("does not save unverified enabled configurations", async () => { mocks.connection.mockRejectedValue(new Error("Cannot connect")); - expect((await POST(request())).status).toBe(400); + expect((await POST(request(), emptyRouteContext())).status).toBe(400); expect(await fs.readdir(root)).toEqual([]); }); it("rejects configuration changes during an export", async () => { await fs.writeFile(path.join(root, "worker.lock"), "active"); - expect((await POST(request())).status).toBe(409); + expect((await POST(request(), emptyRouteContext())).status).toBe(409); expect(mocks.connection).not.toHaveBeenCalled(); }); it("prevents moving queued changes to a different repository", async () => { - await POST(request()); + await POST(request(), emptyRouteContext()); await fs.writeFile(path.join(root, "change.pending"), "pending"); expect( ( await POST( request({ ...input, remote: "https://other.example/user/repo" }), + emptyRouteContext(), ) ).status, ).toBe(409); diff --git a/src/app/api/admin/catalog/search/route.test.ts b/src/app/api/admin/catalog/search/route.test.ts index 1472c82a..dc1cc4e5 100644 --- a/src/app/api/admin/catalog/search/route.test.ts +++ b/src/app/api/admin/catalog/search/route.test.ts @@ -19,6 +19,7 @@ vi.mock("@/lib/api-handler", () => ({ })); import { PERMS } from "@/lib/permissions"; +import { emptyRouteContext } from "@/test/route-context"; import { GET } from "./route"; beforeEach(() => { @@ -34,6 +35,7 @@ describe("catalog search route", () => { new NextRequest( "https://cms.test/api/admin/catalog/search?catalog=bc&q=chair", ), + emptyRouteContext(), ); expect(search).toHaveBeenCalledWith("bc", "chair"); expect(await response.json()).toEqual({ ok: true, results: [] }); @@ -42,6 +44,7 @@ describe("catalog search route", () => { search.mockRejectedValue(Error("Invalid search")); const response = await GET( new NextRequest("https://cms.test/api/admin/catalog/search?q=a"), + emptyRouteContext(), ); expect(response.status).toBe(400); expect(await response.json()).toEqual({ error: "Invalid search" }); diff --git a/src/app/api/admin/csrf/route.test.ts b/src/app/api/admin/csrf/route.test.ts index c2717c27..99a6f568 100644 --- a/src/app/api/admin/csrf/route.test.ts +++ b/src/app/api/admin/csrf/route.test.ts @@ -16,6 +16,7 @@ vi.mock("@/lib/foundation/security", () => ({ setCsrfCookie: mocks.setCsrfCookie, })); +import { emptyRouteContext } from "@/test/route-context"; import { GET } from "./route"; describe("admin CSRF bootstrap route", () => { @@ -29,6 +30,7 @@ describe("admin CSRF bootstrap route", () => { const response = await GET( new NextRequest("http://localhost/api/admin/csrf"), + emptyRouteContext(), ); expect(response.status).toBe(200); @@ -41,6 +43,7 @@ describe("admin CSRF bootstrap route", () => { const response = await GET( new NextRequest("http://localhost/api/admin/csrf"), + emptyRouteContext(), ); expect(response.status).toBe(500); diff --git a/src/app/api/admin/import/furni/route.ts b/src/app/api/admin/import/furni/route.ts index e8446253..618f880b 100644 --- a/src/app/api/admin/import/furni/route.ts +++ b/src/app/api/admin/import/furni/route.ts @@ -47,11 +47,10 @@ import { streamImportResponse } from "@/lib/services/import/core/single-response import { rcon } from "@/lib/services/rcon"; import { convertSwfToNitro } from "@/lib/services/swf-to-nitro"; -// Re-export for batch route backward compatibility -export { - ensureDirectories, - importSingleFurni, -} from "@/lib/services/furni-import"; +// No value re-exports here. A route module may only export HTTP verbs and +// config, and Next's generated types reject anything else — `tsc --noEmit` does +// not see it, because the check lives in `.next/types`. Both of these are +// imported from `@/lib/services/furni-import` where they are actually needed. export type { ImportSingleResult } from "@/types/furni"; import type { ImportSingleResult } from "@/types/furni"; diff --git a/src/app/api/admin/search/route.test.ts b/src/app/api/admin/search/route.test.ts index e10bc90b..e897fa93 100644 --- a/src/app/api/admin/search/route.test.ts +++ b/src/app/api/admin/search/route.test.ts @@ -46,6 +46,7 @@ import { WebsiteTicket, } from "@/lib/db"; import { PERMS } from "@/lib/permission-slugs"; +import { emptyRouteContext } from "@/test/route-context"; import { GET } from "./route"; const calls: { @@ -97,6 +98,7 @@ const search = (q: string) => new NextRequest( `https://cms.test/api/admin/search?q=${encodeURIComponent(q)}`, ), + emptyRouteContext(), ); describe("unified admin search", () => { diff --git a/src/app/api/admin/studio/inspect/route.test.ts b/src/app/api/admin/studio/inspect/route.test.ts index b3aa6e55..9d1348e0 100644 --- a/src/app/api/admin/studio/inspect/route.test.ts +++ b/src/app/api/admin/studio/inspect/route.test.ts @@ -26,6 +26,7 @@ vi.mock("@/lib/services/furni-asset-dirs", () => ({ })); vi.mock("node:fs", () => ({ promises: { stat: mocks.stat } })); +import { emptyRouteContext } from "@/test/route-context"; import { POST } from "./route"; const request = (classnames: unknown) => @@ -48,7 +49,9 @@ describe("furniture inspection", () => { }); }); it("rejects path traversal before reading data", async () => { - expect((await POST(request(["../secret"]))).status).toBe(400); + expect( + (await POST(request(["../secret"]), emptyRouteContext())).status, + ).toBe(400); expect(mocks.execute).not.toHaveBeenCalled(); expect(mocks.stat).not.toHaveBeenCalled(); }); @@ -70,7 +73,7 @@ describe("furniture inspection", () => { [{ classname: "chair*2", id: 5, pageId: 7, credits: 3, points: 0 }], [], ]); - const response = await POST(request(["chair*2"])); + const response = await POST(request(["chair*2"]), emptyRouteContext()); const body = await response.json(); expect(body.items[0].sql[0].id).toBe(90); expect(body.items[0].catalog[0].pageId).toBe(7); @@ -83,7 +86,9 @@ describe("furniture inspection", () => { it("distinguishes unreadable data and denied assets from missing files", async () => { mocks.read.mockRejectedValue(new Error("unreadable")); mocks.stat.mockRejectedValue({ code: "EACCES" }); - const body = await (await POST(request(["chair"]))).json(); + const body = await ( + await POST(request(["chair"]), emptyRouteContext()) + ).json(); expect(body.items[0].furnidataReadable).toBe(false); expect(body.items[0].icon.exists).toBeNull(); expect(body.items[0].nitro.exists).toBeNull(); @@ -93,7 +98,7 @@ describe("furniture inspection", () => { roomitemtypes: { furnitype: {} }, wallitemtypes: { furnitype: [null] }, }); - const response = await POST(request(["chair"])); + const response = await POST(request(["chair"]), emptyRouteContext()); expect(response.status).toBe(200); expect((await response.json()).items[0].furnidataReadable).toBe(false); }); @@ -136,7 +141,9 @@ describe("furniture inspection", () => { return { size: 256, isFile: () => true }; throw { code: "ENOENT" }; }); - const result = await (await POST(request(["recycler_kintsugiB"]))).json(); + const result = await ( + await POST(request(["recycler_kintsugiB"]), emptyRouteContext()) + ).json(); expect(result.items[0].sql[0].id).toBe(2000037258); expect(result.items[0].catalog).toHaveLength(1); expect(result.items[0].furnidata).toHaveLength(1); diff --git a/src/app/api/admin/studio/nitro-quality/route.test.ts b/src/app/api/admin/studio/nitro-quality/route.test.ts index b19835a9..68ff881a 100644 --- a/src/app/api/admin/studio/nitro-quality/route.test.ts +++ b/src/app/api/admin/studio/nitro-quality/route.test.ts @@ -16,6 +16,7 @@ vi.mock("@/lib/services/furni-asset-dirs", () => ({ getFurniAssetDirs: async () => ({ nitroDir: "/nitro" }), })); +import { emptyRouteContext } from "@/test/route-context"; import { GET } from "./route"; const request = (query: string) => @@ -40,11 +41,13 @@ it("requires import permission", () => permission: PERMS.ASSETS_IMPORT, })); it("rejects path traversal before reading disk", async () => { - expect((await GET(request("classname=../secret"))).status).toBe(400); + expect( + (await GET(request("classname=../secret"), emptyRouteContext())).status, + ).toBe(400); expect(mocks.read).not.toHaveBeenCalled(); }); it("reports both scales from the actual bundle", async () => { - const response = await GET(request("classname=chair")); + const response = await GET(request("classname=chair"), emptyRouteContext()); expect( (await response.json()).report.scales.map( (s: { state: string }) => s.state, @@ -52,7 +55,10 @@ it("reports both scales from the actual bundle", async () => { ).toEqual(["present", "missing"]); }); it("extracts the actual sprite dimensions without resizing", async () => { - const response = await GET(request("classname=chair&asset=chair_32_a_0_0")); + const response = await GET( + request("classname=chair&asset=chair_32_a_0_0"), + emptyRouteContext(), + ); expect(response.status).toBe(200); const meta = await sharp( Buffer.from(await response.arrayBuffer()), @@ -60,5 +66,8 @@ it("extracts the actual sprite dimensions without resizing", async () => { expect([meta.width, meta.height]).toEqual([2, 3]); }); it("rejects an unknown sprite instead of returning a different one", async () => { - expect((await GET(request("classname=chair&asset=other"))).status).toBe(404); + expect( + (await GET(request("classname=chair&asset=other"), emptyRouteContext())) + .status, + ).toBe(404); }); diff --git a/src/app/api/admin/studio/nitro-scale32/route.test.ts b/src/app/api/admin/studio/nitro-scale32/route.test.ts index 21348a96..11270549 100644 --- a/src/app/api/admin/studio/nitro-scale32/route.test.ts +++ b/src/app/api/admin/studio/nitro-scale32/route.test.ts @@ -38,6 +38,7 @@ vi.mock("@/lib/services/swf/nitro-builder", () => ({ parseNitroBundle: () => ({ json: { assets: {} } }), })); +import { emptyRouteContext } from "@/test/route-context"; import { POST } from "./route"; const original = Buffer.from("original"), @@ -59,21 +60,31 @@ beforeEach(() => { }); }); it("preview does not write anything", async () => { - expect((await POST(request({ action: "preview" }))).status).toBe(200); + expect( + (await POST(request({ action: "preview" }), emptyRouteContext())).status, + ).toBe(200); expect(mocks.write).not.toHaveBeenCalled(); }); it("rejects stale source before generation or writes", async () => { expect( - (await POST(request({ action: "apply", expectedHash: hash(generated) }))) - .status, + ( + await POST( + request({ action: "apply", expectedHash: hash(generated) }), + emptyRouteContext(), + ) + ).status, ).toBe(409); expect(mocks.generate).not.toHaveBeenCalled(); expect(mocks.write).not.toHaveBeenCalled(); }); it("backs up the original before replacing the bundle", async () => { expect( - (await POST(request({ action: "apply", expectedHash: hash(original) }))) - .status, + ( + await POST( + request({ action: "apply", expectedHash: hash(original) }), + emptyRouteContext(), + ) + ).status, ).toBe(200); expect(mocks.write.mock.calls[0][1]).toEqual(original); expect(mocks.write.mock.calls[1][1]).toEqual(generated); @@ -89,6 +100,7 @@ it("restores the exact backup after checking the current hash", async () => { expectedHash: hash(generated), backupHash: hash(original), }), + emptyRouteContext(), ) ).status, ).toBe(200); @@ -97,8 +109,12 @@ it("restores the exact backup after checking the current hash", async () => { it("does not replace the live file when backup writing fails", async () => { mocks.write.mockRejectedValueOnce(Error("disk full")); expect( - (await POST(request({ action: "apply", expectedHash: hash(original) }))) - .status, + ( + await POST( + request({ action: "apply", expectedHash: hash(original) }), + emptyRouteContext(), + ) + ).status, ).toBe(422); expect(mocks.rename).not.toHaveBeenCalled(); }); diff --git a/src/app/api/admin/studio/source-assets/route.test.ts b/src/app/api/admin/studio/source-assets/route.test.ts index 9948ffc6..32bc9735 100644 --- a/src/app/api/admin/studio/source-assets/route.test.ts +++ b/src/app/api/admin/studio/source-assets/route.test.ts @@ -22,6 +22,7 @@ vi.mock("@/lib/services/furniture-source-assets", () => ({ inspectSourceAssets: mocks.inspect, })); +import { emptyRouteContext } from "@/test/route-context"; import { POST } from "./route"; const item = { @@ -59,12 +60,19 @@ it.each([ [{ ...item, revision: -1 }], [null], ])("rejects invalid items before network checks: %j", async (items) => { - expect((await POST(request({ items }))).status).toBe(400); + expect((await POST(request({ items }), emptyRouteContext())).status).toBe( + 400, + ); expect(mocks.inspect).not.toHaveBeenCalled(); }); it("rejects unknown configured sources", async () => { expect( - (await POST(request({ items: [item], sourceId: "missing" }))).status, + ( + await POST( + request({ items: [item], sourceId: "missing" }), + emptyRouteContext(), + ) + ).status, ).toBe(404); expect(mocks.inspect).not.toHaveBeenCalled(); }); @@ -77,6 +85,7 @@ it("uses server source configuration instead of client URLs", async () => { sourceId: "custom", nitroBaseUrl: "https://untrusted.example", }), + emptyRouteContext(), ); expect(response.status).toBe(200); expect(mocks.inspect).toHaveBeenCalledWith(item, [], source); diff --git a/src/lib/api-handler-correlation.test.ts b/src/lib/api-handler-correlation.test.ts index ea96c1fe..57b9b341 100644 --- a/src/lib/api-handler-correlation.test.ts +++ b/src/lib/api-handler-correlation.test.ts @@ -32,6 +32,7 @@ vi.mock("@/lib/services/catalog-git-queue", () => ({ beginCatalogExport: vi.fn(), })); +import { emptyRouteContext } from "@/test/route-context"; import { withAdmin } from "./api-handler"; const request = () => @@ -50,7 +51,10 @@ describe("admin API correlation", () => { await Promise.resolve(); return NextResponse.json(getOperationContext()); }); - const [a, b] = await Promise.all([handler(request()), handler(request())]); + const [a, b] = await Promise.all([ + handler(request(), emptyRouteContext()), + handler(request(), emptyRouteContext()), + ]); expect(a.headers.get("x-operation-id")).not.toBe("forged-client-id"); expect(a.headers.get("x-operation-id")).not.toBe( b.headers.get("x-operation-id"), @@ -71,7 +75,7 @@ describe("admin API correlation", () => { response.cookies.set("second", "two", { httpOnly: true }); return response; }); - const response = await handler(request()); + const response = await handler(request(), emptyRouteContext()); expect(response.status).toBe(307); expect(response.headers.get("location")).toBe( "https://hotel.test/admin/users", @@ -104,7 +108,7 @@ describe("admin API correlation", () => { }, ), ); - const response = await handler(request()); + const response = await handler(request(), emptyRouteContext()); expect(response.headers.get("content-type")).toBe("text/event-stream"); expect(response.headers.get("x-custom")).toBe("retained"); const body = response.text(); @@ -121,17 +125,17 @@ describe("admin API correlation", () => { throw new Error("private-database-message"); }); state.authorized = false; - const unauthorized = await handler(request()); + const unauthorized = await handler(request(), emptyRouteContext()); expect(unauthorized.status).toBe(401); expect(state.record).not.toHaveBeenCalled(); expect(unauthorized.headers.get("x-operation-id")).toBeTruthy(); state.authorized = true; state.permitted = false; - const forbidden = await handler(request()); + const forbidden = await handler(request(), emptyRouteContext()); expect(forbidden.status).toBe(403); expect(forbidden.headers.get("x-operation-id")).toBeTruthy(); state.permitted = true; - const failed = await handler(request()); + const failed = await handler(request(), emptyRouteContext()); expect(failed.status).toBe(500); expect(failed.headers.get("x-operation-id")).toBeTruthy(); expect(await failed.json()).toEqual({ diff --git a/src/lib/api-handler.ts b/src/lib/api-handler.ts index 3fe03c6f..a247fc65 100644 --- a/src/lib/api-handler.ts +++ b/src/lib/api-handler.ts @@ -25,7 +25,11 @@ const MUTATING_METHODS = new Set(["POST", "PUT", "PATCH", "DELETE"]); const MAX_BODY_BYTES = 10 * 1024 * 1024; // 10 MB type AdminContext = NonNullable>>; -type RouteContext = { params?: Promise> }; +// Next 15+ passes `params` as a promise and its generated route types require it +// to be present, so this is deliberately not optional. `routeContext.params` is +// awaited below without a guard, so an optional type would have been a lie +// anyway. A route with no dynamic segments still receives `Promise<{}>`. +type RouteContext = { params: Promise> }; type AdminHandler = ( request: NextRequest, context: AdminContext, @@ -40,7 +44,16 @@ export function withAdmin( }, handler: AdminHandler, ) { - return async (request: NextRequest, routeContext: RouteContext = {}) => { + // No default value here on purpose. `RouteContext` already has an optional + // `params`, so `{}` was always assignable and the default only widened the + // parameter to `RouteContext | undefined`. Next's generated route types assert + // the second argument against `ParamCheck`, and the `undefined` + // fails that check. `tsc --noEmit` does not see it, because Next only adds + // `.next/types` to the project during a production build. + // Deliberately no default. A parameter with a default is optional in the + // signature, which makes its type `RouteContext | undefined` and fails Next's + // `ParamCheck`. Tests that call a handler directly pass `emptyRouteContext()`. + return async (request: NextRequest, routeContext: RouteContext) => { const store = createStore("unknown" as IpAddress); const { value: response, metrics } = await collectPerformance(() => runWithStore(store, async () => { @@ -153,7 +166,11 @@ export function withAdmin( operationId: store.requestId, route: routeTemplate( request.nextUrl.pathname, - await routeContext.params, + // Next always passes a context, and the type requires it, so + // this is only a guard against a direct internal call. The + // worst case is a slightly less specific metric label, which + // is not worth a 500. + await routeContext?.params, ), method: request.method, status: response.status, diff --git a/src/test/route-context.ts b/src/test/route-context.ts new file mode 100644 index 00000000..fe8c109e --- /dev/null +++ b/src/test/route-context.ts @@ -0,0 +1,15 @@ +/** + * The route context Next hands to a route handler, for tests that call one + * directly. `params` has been a promise since Next 15, and a route with no + * dynamic segments still receives an empty one, so this is the real call + * contract rather than a convenience shim. + * + * Deliberately self-contained: it does not import from `@/lib/api-handler`, + * because route tests routinely `vi.mock` that module, and pulling a value out + * of a mock would fail at runtime. + */ +export function emptyRouteContext(): { + params: Promise>; +} { + return { params: Promise.resolve({}) }; +}