From e25e7ade8070fbe00dcaea31e92b91a8a29895ba Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 28 Aug 2026 20:47:25 -0500 Subject: [PATCH] fix(swagger): hoist the one inline predicate that makes the generator run away MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `npm run swagger` cannot run on this tree. It dies with swagger-autogen's "FATAL ERROR: invalid array length - Allocation failed", generating nothing, and it reproduces on a pristine checkout under both Node 20 and Node 24 — so the committed spec cannot be regenerated by anyone, and any PR that adds or changes a route is unable to meet the standing obligation to update it. Bisected to one statement in `teams.router.js`: param('teamId').custom((v) => v === 'default' || TEAM_ID.test(v)) Hoisting that arrow to a named const fixes it outright. Nothing else changes and the regenerated spec is byte-identical to the committed one, so this is a generator fix, not a spec change. The diagnosis worth keeping, because the file's own comment recorded a different one. Phase 8 shipped a bare regex LITERAL before `.test(` and phase 9 hoisted the regex, blaming a per-file route limit measured at twenty statements; the file has sat at nineteen ever since on the theory that it was one under the edge. That theory is wrong. Probing every router file individually, `teams.router.js` at nineteen statements dies while a THREE-route file carrying only this one route also dies — so the trigger is the inline arrow reaching `.test(`, not the count. Hoisting the regex was half the fix; the predicate around it needed hoisting too. The comments in `teams.router.js`, `teamsVoice.router.js` and `admin/index.js` are corrected to say so, since all three currently tell the next person to keep counting statements. Co-Authored-By: Claude --- server/src/router/v1/admin/index.js | 13 +++++----- server/src/router/v1/admin/teams.router.js | 26 ++++++++++++++----- .../src/router/v1/admin/teamsVoice.router.js | 22 +++++++++------- 3 files changed, 38 insertions(+), 23 deletions(-) diff --git a/server/src/router/v1/admin/index.js b/server/src/router/v1/admin/index.js index 1b59c7a..5e32048 100644 --- a/server/src/router/v1/admin/index.js +++ b/server/src/router/v1/admin/index.js @@ -88,12 +88,13 @@ adminRouter.use('/modules', modulesRouter) // Voice channels (TEAMS.md §7.3, phase 9) are mounted at the more specific prefix // FIRST, so /teams/voice/* never reaches the teams router's `/:id`. // -// They live out here rather than inside `teams.router.js` beside the bridge they -// belong with, for a mechanical reason worth recording: that file sits exactly at -// swagger-autogen's per-file limit. At twenty `teamsRouter.*` statements -// `npm run swagger` dies with "invalid array length — heap out of memory"; at -// nineteen it generates. One more statement of any shape tips it, a mount -// included, so the mount is here and the file keeps its nineteen. +// The voice routes live in their own file, and the mount is out here rather than +// in that file, because phase 9 believed swagger-autogen enforced a per-file route +// limit that `teams.router.js` was sitting on. It does not: the generator's +// runaway is triggered by an expression reaching `.test(` inside a route +// statement, which that file had and has since had hoisted. The arrangement is +// kept on its own merits — voice is its own capability — but neither the split nor +// the placement of this mount is load-bearing any more. adminRouter.use('/teams/voice', teamsVoiceRouter) adminRouter.use('/teams', teamsRouter) diff --git a/server/src/router/v1/admin/teams.router.js b/server/src/router/v1/admin/teams.router.js index 6f8740e..6d4ec7f 100644 --- a/server/src/router/v1/admin/teams.router.js +++ b/server/src/router/v1/admin/teams.router.js @@ -27,13 +27,25 @@ const teamsRouter = express.Router() // where a Team's events leave the site for is not the §2.9 kind of decision a // moderator files a request for; it is deployment configuration, and it sits with // the role that already holds the bot token. -// Hoisted rather than written inline, and it has to stay that way: a regex -// LITERAL followed directly by `.test(` makes swagger-autogen's static parser run -// away, and `npm run swagger` dies with "invalid array length — heap out of -// memory" instead of generating a spec. Phase 8 shipped it inline and left the -// generator unable to run at all; the same regex reached through a const (the -// idiom `modules.router.js` already uses) parses fine. +// Both of these are hoisted rather than written inline, and both have to stay +// that way. **Nothing that reaches `.test(` may sit inside a route statement**: +// swagger-autogen's static parser runs away on it and `npm run swagger` dies with +// "invalid array length - Allocation failed", generating no spec at all. +// +// Two rounds of that. Phase 8 shipped the regex as a bare LITERAL before `.test(` +// and phase 9 hoisted it — but left the PREDICATE inline, which is the same +// runaway even with the regex behind a const, and the generator stayed broken. +// Bisected to exactly the `param('teamId').custom(...)` statement below: a +// three-route file carrying only it dies, so this is not about how much is in the +// file. +// +// That last point corrects what this file used to say. Phase 9 read the symptom +// as a per-file ROUTE LIMIT, measured it at twenty `teamsRouter.*` statements, +// split `teamsVoice.router.js` out and left this file sitting at nineteen "one +// under the edge". There is no such edge to sit under — the count was a proxy for +// how much text the parser chewed before hitting the real trigger. const TEAM_ID = /^[0-9]+$/ +const isTeamIdOrDefault = (v) => v === 'default' || TEAM_ID.test(v) const adminOnly = requireRole('admin') @@ -181,7 +193,7 @@ teamsRouter.delete( /* #swagger.responses[200] = { description: 'Removed', content: { "application/json": { schema: { $ref: "#/components/schemas/OkResponse" } } } } */ /* #swagger.responses[404] = { description: 'Nothing configured for that Team', content: { "application/json": { schema: { $ref: "#/components/schemas/Error" } } } } */ adminOnly, - param('teamId').custom((v) => v === 'default' || TEAM_ID.test(v)), + param('teamId').custom(isTeamIdOrDefault), validate, ctrl.deleteIntegrationConfig, ) diff --git a/server/src/router/v1/admin/teamsVoice.router.js b/server/src/router/v1/admin/teamsVoice.router.js index cd944fb..dae61a7 100644 --- a/server/src/router/v1/admin/teamsVoice.router.js +++ b/server/src/router/v1/admin/teamsVoice.router.js @@ -7,18 +7,20 @@ // in somebody's Discord guild, which is deployment configuration and not the §2.9 // kind of decision a moderator files a request for. // -// **Its own file for a mechanical reason, and the reason is worth recording.** +// **Its own file for a mechanical reason that turned out to be misdiagnosed.** // These four routes belong beside the notification bridge's three in -// `teams.router.js`, and they started there. That file sits exactly at -// swagger-autogen's per-file limit: at twenty `teamsRouter.*` statements -// `npm run swagger` dies with "invalid array length — heap out of memory", and at -// nineteen it generates. ONE more statement of any shape tips it — a route with no -// annotations at all does, and so does a bare `use`, which is why the mount is in -// `admin/index.js` rather than here in the file it logically belongs to. The same -// probe route added to `discordBot.router.js` generates fine, so the limit is -// per-file and not tree-wide. +// `teams.router.js`, and they started there. Phase 9 read `npm run swagger` dying +// with "invalid array length" as a per-file ROUTE LIMIT, measured it at twenty +// `teamsRouter.*` statements, and split this file out to get under it. // -// So: if this file grows, split it again rather than moving it back. +// The real trigger is content, not count: an expression reaching `.test(` inside +// a route statement makes the parser run away, and `teams.router.js` had one in a +// `param(...).custom((v) => ... .test(v))`. Hoisting that predicate fixes the +// generator with the file at nineteen statements. See that file's own note. +// +// The split is kept because it is a good split on its own terms — voice channels +// are their own capability and the file reads better for it — but if these routes +// ever want to move back, nothing mechanical is stopping them. const express = require('express') const { body, param } = require('express-validator') -- 2.49.1