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')