From 94a4365e6dbc52594de542a97a081c1e5144d126 Mon Sep 17 00:00:00 2001 From: Tom Hale Date: Mon, 27 Apr 2026 19:17:40 +0700 Subject: [PATCH] fix(serve): serve web UI at root path instead of 404 (#1048) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(serve): serve web UI at root path instead of 404 gitnexus serve returned Cannot GET / because no route handler existed for the root path. Now serves the built gitnexus-web dist at / with SPA fallback for client-side routing. Falls back to a helpful landing page with API links when the web UI hasn't been built yet. Also updates the build script to build and copy gitnexus-web into gitnexus/web/ for the published npm package. * fix(serve): address Copilot review feedback - Use regex SPA fallback that excludes /api paths (avoids serving index.html for unknown API routes) - Add rel="noopener noreferrer" to external link (reverse-tabnabbing) - Move build "done" log after web UI step * fix(build): use npm run build for web UI, add npm install guard The build script ran `npx tsc -b && npx vite build` in gitnexus-web/, but CI only installs node_modules for gitnexus/ — not gitnexus-web/. npx then resolved the wrong `tsc` package (a trojan on npm), causing all CI jobs to fail. Fix: add an npm install guard when node_modules is missing, and use `npm run build` (which runs the local typescript) instead of npx. * feat(serve): styled fallback page, asset 404s, build script safety - Add landingPageHtml() with gitnexus-web design tokens (void bg, surface cards, accent color, terminal-style build command block). - Add resolveWebDistDir() helper with non-ENOENT error logging. - Register express.static with Cache-Control headers (no-cache HTML, immutable assets) and SPA fallback route. - Replace wildcard SPA fallback with regex that excludes /api/* AND asset-like file extensions (.js, .css, .ico, .woff2, .map, etc.). - Add ordering comment warning about SPA fallback route placement. scripts/build.js: - Change npm install to npm ci. - Add timeout: 120_000 to all execSync calls. Test coverage: - 26 new unit tests for design tokens, terminal block, external links, SPA regex acceptance/exclusion, cache headers, and fs.access edge cases. Closes #1048 (review feedback) * fix: format, lint, and add GITNEXUS_WEB_DIST env var - Remove unused fsType import from web-ui-serving.test.ts (lint error) - Run prettier on fallback-page-screenshot.html and test file - Add GITNEXUS_WEB_DIST env var as primary override in resolveWebDistDir - Add tests for env var: prefer when set, fallback when dir missing * fix: use cross-platform path matching in env var tests Path.includes('/env/dist') fails on Windows where path.join produces backslashed paths. Normalize via path.sep replacement before matching. * fix(serve): address PR #1048 review findings - Add uncaughtException/unhandledRejection crash guards to HTTP serve path - Export SPA_FALLBACK_REGEX so tests use the production constant (no drift) - Export staticCacheControlSetHeaders so tests verify the real production function - Add real Express dispatch tests for API 404 and asset 404 isolation - Delete committed debug artifact fallback-page-screenshot.html --- gitnexus/package.json | 3 +- gitnexus/scripts/build.js | 24 +- gitnexus/src/server/api.ts | 123 +++++++++ gitnexus/test/unit/web-ui-serving.test.ts | 320 ++++++++++++++++++++++ 4 files changed, 467 insertions(+), 3 deletions(-) create mode 100644 gitnexus/test/unit/web-ui-serving.test.ts diff --git a/gitnexus/package.json b/gitnexus/package.json index 8dc82d2e4..9045662aa 100644 --- a/gitnexus/package.json +++ b/gitnexus/package.json @@ -35,7 +35,8 @@ "hooks", "scripts", "skills", - "vendor" + "vendor", + "web" ], "scripts": { "build": "node scripts/build.js", diff --git a/gitnexus/scripts/build.js b/gitnexus/scripts/build.js index 68378c61d..84f43fc6b 100644 --- a/gitnexus/scripts/build.js +++ b/gitnexus/scripts/build.js @@ -21,11 +21,11 @@ const SHARED_DEST = path.join(DIST, '_shared'); // ── 1. Build gitnexus-shared ─────────────────────────────────────── console.log('[build] compiling gitnexus-shared…'); -execSync('npx tsc', { cwd: SHARED_ROOT, stdio: 'inherit' }); +execSync('npx tsc', { cwd: SHARED_ROOT, stdio: 'inherit', timeout: 120_000 }); // ── 2. Build gitnexus ────────────────────────────────────────────── console.log('[build] compiling gitnexus…'); -execSync('npx tsc', { cwd: ROOT, stdio: 'inherit' }); +execSync('npx tsc', { cwd: ROOT, stdio: 'inherit', timeout: 120_000 }); // ── 3. Copy shared dist ──────────────────────────────────────────── console.log('[build] copying shared module into dist/_shared…'); @@ -70,4 +70,24 @@ walk(DIST, ['.js', '.d.ts'], rewriteFile); const cliEntry = path.join(DIST, 'cli', 'index.js'); if (fs.existsSync(cliEntry)) fs.chmodSync(cliEntry, 0o755); +// ── 6. Build & copy web UI ────────────────────────────────────────── +const WEB_ROOT = path.resolve(ROOT, '..', 'gitnexus-web'); +const WEB_DEST = path.join(DIST, '..', 'web'); + +if (fs.existsSync(path.join(WEB_ROOT, 'package.json'))) { + console.log('[build] building gitnexus-web…'); + if (!fs.existsSync(path.join(WEB_ROOT, 'node_modules'))) { + console.log('[build] installing gitnexus-web dependencies…'); + execSync('npm ci', { cwd: WEB_ROOT, stdio: 'inherit', timeout: 120_000 }); + } + execSync('npm run build', { cwd: WEB_ROOT, stdio: 'inherit', timeout: 120_000 }); + + // Copy dist → gitnexus/web/ (shipped in the npm package) + fs.rmSync(WEB_DEST, { recursive: true, force: true }); + fs.cpSync(path.join(WEB_ROOT, 'dist'), WEB_DEST, { recursive: true }); + console.log('[build] copied web UI → gitnexus/web/'); +} else { + console.log('[build] skipping web UI (gitnexus-web not found)'); +} + console.log(`[build] done — rewrote ${rewritten} files.`); diff --git a/gitnexus/src/server/api.ts b/gitnexus/src/server/api.ts index 9c82477d4..d9aa00ae6 100644 --- a/gitnexus/src/server/api.ts +++ b/gitnexus/src/server/api.ts @@ -127,6 +127,105 @@ export const isIgnorableGraphQueryError = (err: unknown): boolean => { ); }; +export const SPA_FALLBACK_REGEX = /^(?!\/api(?:\/|$))(?!.*\.\w{1,10}$).*/; + +export const resolveWebDistDir = async ( + primaryDir: string, + fallbackDir: string, +): Promise => { + const envDir = process.env.GITNEXUS_WEB_DIST; + const dirs = envDir ? [envDir, primaryDir, fallbackDir] : [primaryDir, fallbackDir]; + for (const dir of dirs) { + try { + await fs.access(path.join(dir, 'index.html')); + return dir; + } catch (err: any) { + if (err?.code !== 'ENOENT') { + console.warn(`[serve] could not access web UI dir ${dir}:`, err.message); + } + } + } + return null; +}; + +export const landingPageHtml = (): string => ` + + + + +GitNexus + + + +
+ +
API server is running
+
Endpoints
+

/api/info — Server version & context

+

/api/repos — Indexed repositories

+

/api/heartbeat — SSE heartbeat

+

/api/graph /api/query /api/search — Data

+

/api/mcp — MCP over StreamableHTTP

+
+
Web UI not found
+
$ cd gitnexus-web && npm run build
+
+ + gitnexus.vercel.app + — connects to this server +
+
+ +`; + +export const staticCacheControlSetHeaders = (res: express.Response, filePath: string): void => { + if (filePath.endsWith('.html')) { + res.setHeader('Cache-Control', 'no-cache'); + } else { + res.setHeader('Cache-Control', 'public, max-age=31536000, immutable'); + } +}; + +export const registerWebUI = (app: express.Express, staticDir: string | null): void => { + if (staticDir) { + app.use( + express.static(staticDir, { + setHeaders: staticCacheControlSetHeaders, + }), + ); + // ⚠ This must remain the LAST route before the global error handler. + // The regex excludes /api paths AND paths with file extensions (.js, .css, etc.) + // so missing assets get real 404s instead of the SPA HTML. + // Adding routes below this will be unreachable for non-API, non-asset paths. + app.get(SPA_FALLBACK_REGEX, (_req, res) => { + res.sendFile(path.join(staticDir, 'index.html')); + }); + } else { + app.get('/', (_req, res) => { + res.type('html').send(landingPageHtml()); + }); + } +}; + const ensureStreamIsWritable = (res: express.Response, signal?: AbortSignal): void => { if (signal?.aborted || res.destroyed || res.writableEnded) { throw new ClientDisconnectedError(); @@ -1557,6 +1656,17 @@ export const createServer = async (port: number, host: string = '127.0.0.1') => res.json({ id: job.id, status: 'failed', error: 'Cancelled by user' }); }); + // ── Web UI (served at root) ─────────────────────────────────────── + + // Resolve the gitnexus-web dist directory relative to this file's location. + // In the published package: /dist/server/api.js → /web/ + // In dev (tsx): gitnexus/src/server/api.ts → gitnexus-web/dist/ + const __dirname = path.dirname(fileURLToPath(import.meta.url)); + const webDistDir = path.resolve(__dirname, '..', '..', 'web'); + const devWebDistDir = path.resolve(__dirname, '..', '..', '..', 'gitnexus-web', 'dist'); + const staticDir = await resolveWebDistDir(webDistDir, devWebDistDir); + registerWebUI(app, staticDir); + // Global error handler — catch anything the route handlers miss app.use((err: any, _req: express.Request, res: express.Response, _next: express.NextFunction) => { console.error('Unhandled error:', err); @@ -1586,5 +1696,18 @@ export const createServer = async (port: number, host: string = '127.0.0.1') => }; process.once('SIGINT', shutdown); process.once('SIGTERM', shutdown); + + // Catch-all crash guards (mirrors startMCPServer in mcp/server.ts) + let shuttingDown = false; + process.on('uncaughtException', (err) => { + console.error('GitNexus uncaughtException:', err?.stack || err); + if (!shuttingDown) { + shuttingDown = true; + shutdown().catch(() => {}); + } + }); + process.on('unhandledRejection', (reason: any) => { + console.error('GitNexus unhandledRejection:', reason?.stack || reason); + }); }); }; diff --git a/gitnexus/test/unit/web-ui-serving.test.ts b/gitnexus/test/unit/web-ui-serving.test.ts new file mode 100644 index 000000000..d3f085422 --- /dev/null +++ b/gitnexus/test/unit/web-ui-serving.test.ts @@ -0,0 +1,320 @@ +import path from 'node:path'; +import http from 'node:http'; +import express from 'express'; +import { describe, expect, it, vi, beforeEach } from 'vitest'; + +const { accessMock } = vi.hoisted(() => ({ + accessMock: vi.fn(), +})); + +vi.mock('fs/promises', () => ({ + default: { access: accessMock }, + access: accessMock, +})); + +import { + registerWebUI, + resolveWebDistDir, + landingPageHtml, + SPA_FALLBACK_REGEX, + staticCacheControlSetHeaders, +} from '../../src/server/api.js'; + +type MockRoute = { method: string; path: string | RegExp; handler: Function[] }; +type MockApp = { + use: ReturnType; + get: ReturnType; + _routes: MockRoute[]; +}; + +const createMockApp = (): MockApp => { + const _routes: MockRoute[] = []; + return { + use: vi.fn(), + get: vi.fn((p: string | RegExp, ...h: Function[]) => + _routes.push({ method: 'get', path: p, handler: h }), + ), + _routes, + }; +}; + +const invokeHandler = async (app: MockApp, method: string, reqPath: string) => { + for (const route of app._routes) { + if (route.method !== method) continue; + if (route.path instanceof RegExp) { + if (!route.path.test(reqPath)) continue; + } else { + if (route.path !== reqPath) continue; + } + const res: any = { + sendFile: vi.fn(), + type: vi.fn().mockReturnThis(), + send: vi.fn().mockReturnThis(), + setHeader: vi.fn(), + }; + await route.handler[0]({ path: reqPath } as any, res, vi.fn()); + return res; + } + return null; +}; + +describe('landingPageHtml', () => { + const html = landingPageHtml(); + + it('contains void background colour from gitnexus-web design tokens', () => { + expect(html).toContain('#06060a'); + }); + + it('contains surface card colour from gitnexus-web design tokens', () => { + expect(html).toContain('#101018'); + }); + + it('contains accent colour from gitnexus-web design tokens', () => { + expect(html).toContain('#7c3aed'); + }); + + it('uses Outfit font with system-ui fallback', () => { + expect(html).toContain('Outfit'); + expect(html).toContain('system-ui'); + }); + + it('contains the build command in a terminal-style block', () => { + expect(html).toContain('cd gitnexus-web'); + expect(html).toContain('npm run build'); + }); + + it('contains the Vercel link with safe external attributes', () => { + expect(html).toContain('https://gitnexus.vercel.app'); + expect(html).toContain('target="_blank"'); + expect(html).toContain('rel="noopener noreferrer"'); + }); + + it('contains the Web UI not found message', () => { + expect(html).toContain('Web UI not found'); + }); +}); + +describe('SPA fallback regex', () => { + it('allows root path', () => { + expect(SPA_FALLBACK_REGEX.test('/')).toBe(true); + }); + + it('allows SPA routes', () => { + expect(SPA_FALLBACK_REGEX.test('/processes')).toBe(true); + expect(SPA_FALLBACK_REGEX.test('/settings')).toBe(true); + expect(SPA_FALLBACK_REGEX.test('/clusters')).toBe(true); + }); + + it('excludes /api paths', () => { + expect(SPA_FALLBACK_REGEX.test('/api')).toBe(false); + expect(SPA_FALLBACK_REGEX.test('/api/')).toBe(false); + expect(SPA_FALLBACK_REGEX.test('/api/info')).toBe(false); + expect(SPA_FALLBACK_REGEX.test('/api/does-not-exist')).toBe(false); + }); + + it('excludes asset-like paths with file extensions', () => { + expect(SPA_FALLBACK_REGEX.test('/assets/missing.js')).toBe(false); + expect(SPA_FALLBACK_REGEX.test('/assets/missing.css')).toBe(false); + expect(SPA_FALLBACK_REGEX.test('/favicon.ico')).toBe(false); + expect(SPA_FALLBACK_REGEX.test('/assets/font.woff2')).toBe(false); + expect(SPA_FALLBACK_REGEX.test('/static/app.map')).toBe(false); + expect(SPA_FALLBACK_REGEX.test('/images/logo.png')).toBe(false); + }); + + it('allows SPA routes with dots not at the end', () => { + expect(SPA_FALLBACK_REGEX.test('/v1.0/api')).toBe(true); + expect(SPA_FALLBACK_REGEX.test('/docs/v2.0/guide')).toBe(true); + }); + + it('excludes paths ending in dot-plus-extension regardless of content before it', () => { + expect(SPA_FALLBACK_REGEX.test('/user@example.com')).toBe(false); + }); +}); + +describe('registerWebUI', () => { + it('registers express.static and SPA fallback when staticDir provided', () => { + const app = createMockApp(); + registerWebUI(app as any, '/some/dir'); + expect(app.use).toHaveBeenCalledTimes(1); + expect(app.get).toHaveBeenCalledTimes(1); + const [regex] = app.get.mock.calls[0] as [RegExp, ...Function[]]; + expect(regex.source).toBe(SPA_FALLBACK_REGEX.source); + }); + + it('registers landing page route when staticDir is null', () => { + const app = createMockApp(); + registerWebUI(app as any, null); + expect(app.use).not.toHaveBeenCalled(); + expect(app.get).toHaveBeenCalledTimes(1); + const [path] = app.get.mock.calls[0] as [string, ...Function[]]; + expect(path).toBe('/'); + }); + + it('landing page handler returns styled HTML', async () => { + const app = createMockApp(); + registerWebUI(app as any, null); + const res = await invokeHandler(app, 'get', '/'); + expect(res.type).toHaveBeenCalledWith('html'); + expect(res.send).toHaveBeenCalledWith(expect.stringContaining('Web UI not found')); + expect(res.send).toHaveBeenCalledWith(expect.stringContaining('#06060a')); + expect(res.send).toHaveBeenCalledWith(expect.stringContaining('#7c3aed')); + }); + + it('Cache-Control setHeaders sets no-cache for HTML, immutable for assets', () => { + const captureHeaders = (filePath: string) => { + const headers: Record = {}; + const res = { + setHeader: (k: string, v: string) => { + headers[k] = v; + }, + }; + staticCacheControlSetHeaders(res as express.Response, filePath); + return headers; + }; + expect(captureHeaders('index.html')).toEqual({ 'Cache-Control': 'no-cache' }); + expect(captureHeaders('app.js')).toEqual({ + 'Cache-Control': 'public, max-age=31536000, immutable', + }); + expect(captureHeaders('style.css')).toEqual({ + 'Cache-Control': 'public, max-age=31536000, immutable', + }); + }); +}); + +describe('resolveWebDistDir', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it('returns primary dir when index.html exists', async () => { + accessMock.mockImplementation(async (p: string) => { + if (p.includes('primary')) return undefined; + throw Object.assign(new Error('not found'), { code: 'ENOENT' }); + }); + const result = await resolveWebDistDir('/primary', '/fallback'); + expect(result).toBe('/primary'); + }); + + it('returns fallback dir when primary missing', async () => { + accessMock.mockImplementation(async (p: string) => { + if (p.includes('fallback')) return undefined; + throw Object.assign(new Error('not found'), { code: 'ENOENT' }); + }); + const result = await resolveWebDistDir('/primary', '/fallback'); + expect(result).toBe('/fallback'); + }); + + it('returns null when both dirs missing', async () => { + accessMock.mockImplementation(async () => { + throw Object.assign(new Error('not found'), { code: 'ENOENT' }); + }); + const result = await resolveWebDistDir('/primary', '/fallback'); + expect(result).toBeNull(); + }); + + it('warns on non-ENOENT errors but continues', async () => { + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + accessMock.mockImplementation(async (p: string) => { + if (p.includes('primary')) + throw Object.assign(new Error('permission denied'), { code: 'EACCES' }); + if (p.includes('fallback')) return undefined; + throw Object.assign(new Error('not found'), { code: 'ENOENT' }); + }); + const result = await resolveWebDistDir('/primary', '/fallback'); + expect(result).toBe('/fallback'); + expect(warnSpy).toHaveBeenCalledWith( + expect.stringContaining('could not access web UI dir /primary'), + 'permission denied', + ); + warnSpy.mockRestore(); + }); + + it('prefers GITNEXUS_WEB_DIST env var when set', async () => { + const original = process.env.GITNEXUS_WEB_DIST; + process.env.GITNEXUS_WEB_DIST = '/env/dist'; + try { + accessMock.mockImplementation(async (p: string) => { + const normalized = p.split(path.sep).join('/'); + if (normalized.includes('/env/dist')) return undefined; + throw Object.assign(new Error('not found'), { code: 'ENOENT' }); + }); + const result = await resolveWebDistDir('/primary', '/fallback'); + expect(result).toBe('/env/dist'); + } finally { + if (original === undefined) { + delete process.env.GITNEXUS_WEB_DIST; + } else { + process.env.GITNEXUS_WEB_DIST = original; + } + } + }); + + it('falls back to primary when GITNEXUS_WEB_DIST dir missing', async () => { + const original = process.env.GITNEXUS_WEB_DIST; + process.env.GITNEXUS_WEB_DIST = '/env/dist'; + try { + accessMock.mockImplementation(async (p: string) => { + const normalized = p.split(path.sep).join('/'); + if (normalized.includes('/env/dist')) + throw Object.assign(new Error('not found'), { code: 'ENOENT' }); + if (normalized.includes('/primary')) return undefined; + throw Object.assign(new Error('not found'), { code: 'ENOENT' }); + }); + const result = await resolveWebDistDir('/primary', '/fallback'); + expect(result).toBe('/primary'); + } finally { + if (original === undefined) { + delete process.env.GITNEXUS_WEB_DIST; + } else { + process.env.GITNEXUS_WEB_DIST = original; + } + } + }); +}); + +describe('Real Express dispatch — API and asset isolation', () => { + const makeRequest = (app: express.Express, method: string, url: string): Promise => { + return new Promise((resolve) => { + const server = app.listen(0, () => { + const opts = { + hostname: 'localhost', + port: (server.address() as any).port, + method, + path: url, + }; + const req = http.request(opts, (res) => { + server.close(); + resolve(res.statusCode ?? 0); + }); + req.on('error', () => { + server.close(); + resolve(0); + }); + req.end(); + }); + }); + }; + + it('GET /api/does-not-exist returns 404, not SPA HTML', async () => { + const app = express(); + app.get('/api/info', (_req, res) => res.json({ ok: true })); + registerWebUI(app, '/nonexistent'); + const status = await makeRequest(app, 'GET', '/api/does-not-exist'); + expect(status).toBe(404); + }); + + it('GET /assets/missing.js returns 404, not SPA HTML', async () => { + const app = express(); + app.get('/api/info', (_req, res) => res.json({ ok: true })); + registerWebUI(app, '/nonexistent'); + const status = await makeRequest(app, 'GET', '/assets/missing.js'); + expect(status).toBe(404); + }); + + it('GET / returns the landing page when no web build exists', async () => { + const app = express(); + registerWebUI(app, null); + const status = await makeRequest(app, 'GET', '/'); + expect(status).toBe(200); + }); +});