From 5bdb6a7e10a96dbcc492f351970ed3638b958b38 Mon Sep 17 00:00:00 2001 From: wtclaude Date: Tue, 11 Aug 2026 18:54:31 -0500 Subject: [PATCH] fix(modules): guard the portal's nav icon, resolve MODULES_DIR absolutely MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both found by the §7.7 browser smoke, running the slice-3 pair together, and neither is visible to any test in either repo. `PlayerPortalLayout` rendered `` unguarded while `AdminLayout` guarded its equivalent. `icon` is optional in the nav contract, and every core row in that sidebar has always had one — so the difference cost nothing until a module registered a row without, and then it was not a missing glyph, it was React error #130 and a blank player portal. Guarded now, like its neighbour. `MODULES_DIR` is resolved absolute. `resolveClient` checks containment by comparing an absolute `path.resolve(dir, entry)` against the module directory, so a RELATIVE `MODULES_DIR` — which is what §7.7's own recipe produces when run from `server/` — failed every module with "client.entry escapes the module directory". A perfectly-placed entry, and a message pointing at the module. Co-Authored-By: Claude --- client/src/routes/player/PlayerPortalLayout.jsx | 8 +++++++- server/src/modules/loader.js | 9 ++++++++- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/client/src/routes/player/PlayerPortalLayout.jsx b/client/src/routes/player/PlayerPortalLayout.jsx index d5134d9..12e9153 100644 --- a/client/src/routes/player/PlayerPortalLayout.jsx +++ b/client/src/routes/player/PlayerPortalLayout.jsx @@ -137,7 +137,13 @@ export default function PlayerPortalLayout() { borderLeft: `2px solid ${isActive ? 'var(--accent)' : 'transparent'}`, })} > - + {/* Guarded, like AdminLayout's. `icon` is optional in the nav + contract (§3.3) and every CORE row here has always had one, so + an unguarded `` was fine right up until a module + registered a row without — and then it was not a missing glyph, + it was React error #130 and a blank portal. Found by the §7.7 + browser smoke; no DOM-less test can see it. */} + {n.icon && } {n.label} ))} diff --git a/server/src/modules/loader.js b/server/src/modules/loader.js index f44a547..61d3603 100644 --- a/server/src/modules/loader.js +++ b/server/src/modules/loader.js @@ -45,7 +45,14 @@ const { splitStatements } = require('../utils/sqlStatements') const log = require('../utils/logger')('modules') const REPO_ROOT = path.join(__dirname, '..', '..', '..') -const MODULES_DIR = process.env.MODULES_DIR || path.join(REPO_ROOT, 'modules') +// Resolved absolute, and the `path.resolve` is load-bearing rather than tidy. +// `resolveClient` compares an absolute `path.resolve(dir, entry)` against this +// directory to check containment, so a RELATIVE `MODULES_DIR` — which is what +// anyone following §7.7's smoke recipe from `server/` naturally types — makes +// that comparison fail for every module, with the thoroughly misleading +// "client.entry escapes the module directory". Found the first time the smoke +// was run against a module with a real client half. +const MODULES_DIR = path.resolve(process.env.MODULES_DIR || path.join(REPO_ROOT, 'modules')) // One segment, lowercase, no parameters. A module prefix that could contain a // `/` or a `:` would let a module reach outside the slot it was given.