Refactor UI modules and harden run/API behavior

src/ui/server.ts

@@ -1,27 +1,14 @@
 import "dotenv/config";
-import { createServer, type IncomingMessage, type ServerResponse } from "node:http";
 import { pathToFileURL } from "node:url";
 import { resolve } from "node:path";
-import { buildSessionGraphInsight, buildSessionSummaries } from "./session-insights.js";
-import { UiConfigStore, type LimitSettings, type RuntimeNotificationSettings, type SecurityPolicySettings } from "./config-store.js";
-import { ManifestStore } from "./manifest-store.js";
-import { filterRuntimeEvents, readRuntimeEvents } from "./runtime-events-store.js";
-import { parseJsonBody, sendJson, methodNotAllowed, notFound, serveStaticFile } from "./http-utils.js";
-import { readRunMetaBySession, UiRunService, type RunExecutionMode } from "./run-service.js";
-import type { RunProvider } from "./provider-executor.js";
-
-type StartRunRequest = {
-  prompt: string;
-  manifestPath?: string;
-  manifest?: unknown;
-  sessionId?: string;
-  topologyHint?: string;
-  initialFlags?: Record<string, boolean>;
-  runtimeContextOverrides?: Record<string, string | number | boolean>;
-  simulateValidationNodeIds?: string[];
-  executionMode?: RunExecutionMode;
-  provider?: RunProvider;
-};
+import express from "express";
+import cors from "cors";
+import { z } from "zod";
+import { buildSessionGraphInsight, buildSessionSummaries } from "../telemetry/session-insights.js";
+import { UiConfigStore, type LimitSettings, type RuntimeNotificationSettings, type SecurityPolicySettings } from "../store/config-store.js";
+import { ManifestStore } from "../agents/manifest-store.js";
+import { filterRuntimeEvents, readRuntimeEvents } from "../telemetry/runtime-events-store.js";
+import { readRunMetaBySession, UiRunService } from "../runs/run-service.js";
 
 function parsePort(value: string | undefined): number {
   const parsed = Number(value ?? "4317");
@@ -31,7 +18,7 @@ function parsePort(value: string | undefined): number {
   return parsed;
 }
 
-function parseLimit(value: string | null, fallback: number): number {
+function parseLimit(value: string | null | undefined, fallback: number): number {
   if (!value) {
     return fallback;
   }
@@ -42,66 +29,6 @@ function parseLimit(value: string | null, fallback: number): number {
   return parsed;
 }
 
-function toRelativePathFromApi(urlPath: string): string {
-  return decodeURIComponent(urlPath);
-}
-
-function ensureBooleanRecord(value: unknown): Record<string, boolean> {
-  if (!value || typeof value !== "object") {
-    return {};
-  }
-
-  const output: Record<string, boolean> = {};
-  for (const [key, raw] of Object.entries(value as Record<string, unknown>)) {
-    if (typeof raw === "boolean") {
-      output[key] = raw;
-    }
-  }
-  return output;
-}
-
-function ensureStringArray(value: unknown): string[] {
-  if (!Array.isArray(value)) {
-    return [];
-  }
-
-  const output: string[] = [];
-  for (const item of value) {
-    if (typeof item !== "string") {
-      continue;
-    }
-    const normalized = item.trim();
-    if (!normalized) {
-      continue;
-    }
-    output.push(normalized);
-  }
-
-  return output;
-}
-
-function ensureRuntimeContext(value: unknown): Record<string, string | number | boolean> {
-  if (!value || typeof value !== "object") {
-    return {};
-  }
-
-  const output: Record<string, string | number | boolean> = {};
-  for (const [key, raw] of Object.entries(value as Record<string, unknown>)) {
-    if (typeof raw === "string" || typeof raw === "number" || typeof raw === "boolean") {
-      output[key] = raw;
-    }
-  }
-  return output;
-}
-
-function ensureExecutionMode(value: unknown): RunExecutionMode {
-  return value === "provider" ? "provider" : "mock";
-}
-
-function ensureProvider(value: unknown): RunProvider {
-  return value === "claude" ? "claude" : "codex";
-}
-
 async function readRuntimePaths(configStore: UiConfigStore, workspaceRoot: string): Promise<{
   stateRoot: string;
   runtimeEventLogPath: string;
@@ -113,237 +40,144 @@ async function readRuntimePaths(configStore: UiConfigStore, workspaceRoot: strin
   };
 }
 
-async function handleApiRequest(input: {
-  request: IncomingMessage;
-  response: ServerResponse;
+const StartRunSchema = z.object({
+  prompt: z.string().min(1, 'Field "prompt" is required.'),
+  manifestPath: z.string().optional(),
+  manifest: z.unknown().optional(),
+  sessionId: z.string().optional(),
+  topologyHint: z.string().optional(),
+  initialFlags: z.record(z.string(), z.boolean()).optional(),
+  runtimeContextOverrides: z.record(z.string(), z.union([z.string(), z.number(), z.boolean()])).optional(),
+  simulateValidationNodeIds: z.array(z.string()).optional(),
+  executionMode: z.enum(["mock", "provider"]).optional(),
+  provider: z.enum(["claude", "codex"]).optional(),
+});
+
+export async function startUiServer(input: {
   workspaceRoot: string;
-  configStore: UiConfigStore;
-  manifestStore: ManifestStore;
-  runService: UiRunService;
-}): Promise<boolean> {
-  const { request, response, workspaceRoot, configStore, manifestStore, runService } = input;
-  const requestUrl = new URL(request.url ?? "/", `http://${request.headers.host ?? "localhost"}`);
-  const { pathname } = requestUrl;
-  const method = request.method ?? "GET";
+  port?: number;
+  host?: string;
+}): Promise<{
+  close: () => Promise<void>;
+}> {
+  const workspaceRoot = resolve(input.workspaceRoot);
+  const staticRoot = resolve(workspaceRoot, "ui/dist");
 
-  if (!pathname.startsWith("/api/")) {
-    return false;
-  }
+  const configStore = new UiConfigStore({ workspaceRoot });
+  const manifestStore = new ManifestStore({ workspaceRoot });
+  const runService = new UiRunService({ workspaceRoot });
 
-  try {
-    if (pathname === "/api/health") {
-      if (method !== "GET") {
-        methodNotAllowed(response);
-        return true;
-      }
-      sendJson(response, 200, {
-        ok: true,
-        now: new Date().toISOString(),
-      });
-      return true;
-    }
+  const app = express();
+  app.use(cors());
+  app.use(express.json({ limit: "50mb" }));
 
-    if (pathname === "/api/config") {
-      if (method !== "GET") {
-        methodNotAllowed(response);
-        return true;
-      }
+  app.get("/api/health", (req, res) => {
+    res.json({ ok: true, now: new Date().toISOString() });
+  });
 
+  app.get("/api/config", async (req, res, next) => {
+    try {
       const snapshot = await configStore.readSnapshot();
-      sendJson(response, 200, {
-        ok: true,
-        config: snapshot,
-      });
-      return true;
-    }
+      res.json({ ok: true, config: snapshot });
+    } catch (error) { next(error); }
+  });
 
-    if (pathname === "/api/config/runtime-events") {
-      if (method !== "PUT") {
-        methodNotAllowed(response);
-        return true;
-      }
+  app.put("/api/config/runtime-events", async (req, res, next) => {
+    try {
+      const snapshot = await configStore.updateRuntimeEvents(req.body as RuntimeNotificationSettings);
+      res.json({ ok: true, config: snapshot });
+    } catch (error) { next(error); }
+  });
 
-      const body = await parseJsonBody<RuntimeNotificationSettings>(request);
-      const snapshot = await configStore.updateRuntimeEvents(body);
-      sendJson(response, 200, {
-        ok: true,
-        config: snapshot,
-      });
-      return true;
-    }
+  app.put("/api/config/security", async (req, res, next) => {
+    try {
+      const snapshot = await configStore.updateSecurityPolicy(req.body as SecurityPolicySettings);
+      res.json({ ok: true, config: snapshot });
+    } catch (error) { next(error); }
+  });
 
-    if (pathname === "/api/config/security") {
-      if (method !== "PUT") {
-        methodNotAllowed(response);
-        return true;
-      }
-
-      const body = await parseJsonBody<SecurityPolicySettings>(request);
-      const snapshot = await configStore.updateSecurityPolicy(body);
-      sendJson(response, 200, {
-        ok: true,
-        config: snapshot,
-      });
-      return true;
-    }
-
-    if (pathname === "/api/config/limits") {
-      if (method !== "PUT") {
-        methodNotAllowed(response);
-        return true;
-      }
-
-      const body = await parseJsonBody<LimitSettings>(request);
-      const snapshot = await configStore.updateLimits(body);
-      sendJson(response, 200, {
-        ok: true,
-        config: snapshot,
-      });
-      return true;
-    }
-
-    if (pathname === "/api/manifests") {
-      if (method !== "GET") {
-        methodNotAllowed(response);
-        return true;
-      }
+  app.put("/api/config/limits", async (req, res, next) => {
+    try {
+      const snapshot = await configStore.updateLimits(req.body as LimitSettings);
+      res.json({ ok: true, config: snapshot });
+    } catch (error) { next(error); }
+  });
 
+  app.get("/api/manifests", async (req, res, next) => {
+    try {
       const listing = await manifestStore.list();
-      sendJson(response, 200, {
-        ok: true,
-        manifests: listing.paths,
-      });
-      return true;
-    }
+      res.json({ ok: true, manifests: listing.paths });
+    } catch (error) { next(error); }
+  });
 
-    if (pathname === "/api/manifests/read") {
-      if (method !== "GET") {
-        methodNotAllowed(response);
-        return true;
-      }
-
-      const manifestPath = requestUrl.searchParams.get("path");
+  app.get("/api/manifests/read", async (req, res, next) => {
+    try {
+      const manifestPath = req.query.path as string;
       if (!manifestPath) {
-        sendJson(response, 400, {
-          ok: false,
-          error: 'Query parameter "path" is required.',
-        });
-        return true;
+        res.status(400).json({ ok: false, error: 'Query parameter "path" is required.' });
+        return;
       }
-
       const record = await manifestStore.read(manifestPath);
-      sendJson(response, 200, {
-        ok: true,
-        manifest: record,
-      });
-      return true;
-    }
+      res.json({ ok: true, manifest: record });
+    } catch (error) { next(error); }
+  });
 
-    if (pathname === "/api/manifests/validate") {
-      if (method !== "POST") {
-        methodNotAllowed(response);
-        return true;
-      }
-
-      const body = await parseJsonBody<{ manifest: unknown }>(request);
-      const manifest = await manifestStore.validate(body.manifest);
-      sendJson(response, 200, {
-        ok: true,
-        manifest,
-      });
-      return true;
-    }
-
-    if (pathname === "/api/manifests/save") {
-      if (method !== "PUT") {
-        methodNotAllowed(response);
-        return true;
-      }
-
-      const body = await parseJsonBody<{ path: string; manifest: unknown }>(request);
-      if (!body.path || typeof body.path !== "string") {
-        sendJson(response, 400, {
-          ok: false,
-          error: 'Field "path" is required.',
-        });
-        return true;
-      }
-
-      const record = await manifestStore.save(body.path, body.manifest);
-      sendJson(response, 200, {
-        ok: true,
-        manifest: record,
-      });
-      return true;
-    }
-
-    if (pathname === "/api/runtime-events") {
-      if (method !== "GET") {
-        methodNotAllowed(response);
-        return true;
+  app.post("/api/manifests/validate", async (req, res, next) => {
+    try {
+      const manifest = await manifestStore.validate(req.body.manifest);
+      res.json({ ok: true, manifest });
+    } catch (error) { next(error); }
+  });
+
+  app.put("/api/manifests/save", async (req, res, next) => {
+    try {
+      const { path, manifest } = req.body;
+      if (!path || typeof path !== "string") {
+        res.status(400).json({ ok: false, error: 'Field "path" is required.' });
+        return;
       }
+      const record = await manifestStore.save(path, manifest);
+      res.json({ ok: true, manifest: record });
+    } catch (error) { next(error); }
+  });
 
+  app.get("/api/runtime-events", async (req, res, next) => {
+    try {
       const { runtimeEventLogPath } = await readRuntimePaths(configStore, workspaceRoot);
-      const limit = parseLimit(requestUrl.searchParams.get("limit"), 200);
-      const sessionId = requestUrl.searchParams.get("sessionId") ?? undefined;
+      const limit = parseLimit(req.query.limit as string | undefined, 200);
+      const sessionId = (req.query.sessionId as string) || undefined;
       const events = filterRuntimeEvents(await readRuntimeEvents(runtimeEventLogPath), {
         ...(sessionId ? { sessionId } : {}),
         limit,
       });
+      res.json({ ok: true, events });
+    } catch (error) { next(error); }
+  });
 
-      sendJson(response, 200, {
-        ok: true,
-        events,
-      });
-      return true;
-    }
-
-    if (pathname === "/api/sessions") {
-      if (method !== "GET") {
-        methodNotAllowed(response);
-        return true;
-      }
-
+  app.get("/api/sessions", async (req, res, next) => {
+    try {
       const { stateRoot, runtimeEventLogPath } = await readRuntimePaths(configStore, workspaceRoot);
-      const sessions = await buildSessionSummaries({
-        stateRoot,
-        runtimeEventLogPath,
-      });
+      const sessions = await buildSessionSummaries({ stateRoot, runtimeEventLogPath });
+      res.json({ ok: true, sessions, runs: runService.listRuns() });
+    } catch (error) { next(error); }
+  });
 
-      sendJson(response, 200, {
-        ok: true,
-        sessions,
-        runs: runService.listRuns(),
-      });
-      return true;
-    }
-
-    if (pathname === "/api/sessions/graph") {
-      if (method !== "GET") {
-        methodNotAllowed(response);
-        return true;
-      }
-
-      const sessionId = requestUrl.searchParams.get("sessionId") ?? "";
+  app.get("/api/sessions/graph", async (req, res, next) => {
+    try {
+      const sessionId = (req.query.sessionId as string) ?? "";
       if (!sessionId) {
-        sendJson(response, 400, {
-          ok: false,
-          error: 'Query parameter "sessionId" is required.',
-        });
-        return true;
+        res.status(400).json({ ok: false, error: 'Query parameter "sessionId" is required.' });
+        return;
       }
 
       const { stateRoot, runtimeEventLogPath } = await readRuntimePaths(configStore, workspaceRoot);
-      const explicitManifestPath = requestUrl.searchParams.get("manifestPath");
+      const explicitManifestPath = req.query.manifestPath as string | undefined;
       const runMeta = await readRunMetaBySession({ stateRoot, sessionId });
       const manifestPath = explicitManifestPath ?? runMeta?.manifestPath;
 
       if (!manifestPath) {
-        sendJson(response, 400, {
-          ok: false,
-          error: "No manifestPath available for this session. Provide one in query string.",
-        });
-        return true;
+        res.status(400).json({ ok: false, error: "No manifestPath available for this session. Provide one in query string." });
+        return;
       }
 
       const manifestRecord = await manifestStore.read(manifestPath);
@@ -354,212 +188,126 @@ async function handleApiRequest(input: {
         manifest: manifestRecord.manifest,
       });
 
-      sendJson(response, 200, {
-        ok: true,
-        graph,
-        manifestPath: manifestRecord.path,
+      res.json({ ok: true, graph, manifestPath: manifestRecord.path });
+    } catch (error) { next(error); }
+  });
+
+  app.get("/api/runs", (req, res) => {
+    res.json({ ok: true, runs: runService.listRuns() });
+  });
+
+  app.post("/api/runs", async (req, res, next) => {
+    try {
+      const parseResult = StartRunSchema.safeParse(req.body);
+      if (!parseResult.success) {
+        res.status(400).json({ ok: false, error: parseResult.error.issues[0]?.message || 'Invalid body' });
+        return;
+      }
+      const body = parseResult.data;
+
+      let manifest: unknown;
+      if (body.manifest !== undefined) {
+        manifest = body.manifest;
+      } else if (body.manifestPath) {
+        manifest = (await manifestStore.read(body.manifestPath)).source;
+      }
+
+      if (!manifest) {
+        res.status(400).json({ ok: false, error: "A manifest or manifestPath is required to start a run." });
+        return;
+      }
+
+      const record = await runService.startRun({
+        prompt: body.prompt,
+        manifest,
+        manifestPath: body.manifestPath,
+        sessionId: body.sessionId,
+        topologyHint: body.topologyHint,
+        initialFlags: body.initialFlags ?? {},
+        runtimeContextOverrides: body.runtimeContextOverrides ?? {},
+        simulateValidationNodeIds: body.simulateValidationNodeIds ?? [],
+        executionMode: body.executionMode ?? "mock",
+        provider: body.provider ?? "codex",
       });
-      return true;
-    }
 
-    if (pathname === "/api/runs") {
-      if (method === "GET") {
-        sendJson(response, 200, {
-          ok: true,
-          runs: runService.listRuns(),
-        });
-        return true;
+      res.status(202).json({ ok: true, run: record });
+    } catch (error) { next(error); }
+  });
+
+  app.post("/api/runs/:runId/cancel", async (req, res, next) => {
+    try {
+      const runId = req.params.runId;
+      if (!runId) {
+        res.status(400).json({ ok: false, error: "runId required" });
+        return;
       }
-
-      if (method === "POST") {
-        const body = await parseJsonBody<StartRunRequest>(request);
-        if (typeof body.prompt !== "string" || body.prompt.trim().length === 0) {
-          sendJson(response, 400, {
-            ok: false,
-            error: 'Field "prompt" is required.',
-          });
-          return true;
-        }
-
-        const manifestSource = (() => {
-          if (body.manifest !== undefined) {
-            return body.manifest;
-          }
-          if (typeof body.manifestPath === "string" && body.manifestPath.trim().length > 0) {
-            return undefined;
-          }
-          return undefined;
-        })();
-
-        const resolvedManifest = manifestSource ?? (() => {
-          if (!body.manifestPath) {
-            return undefined;
-          }
-          return body.manifestPath;
-        })();
-
-        let manifest: unknown;
-        if (typeof resolvedManifest === "string") {
-          manifest = (await manifestStore.read(resolvedManifest)).source;
-        } else if (resolvedManifest !== undefined) {
-          manifest = resolvedManifest;
-        }
-
-        if (!manifest) {
-          sendJson(response, 400, {
-            ok: false,
-            error: "A manifest or manifestPath is required to start a run.",
-          });
-          return true;
-        }
-
-        const record = await runService.startRun({
-          prompt: body.prompt,
-          manifest,
-          manifestPath: body.manifestPath,
-          sessionId: body.sessionId,
-          topologyHint: body.topologyHint,
-          initialFlags: ensureBooleanRecord(body.initialFlags),
-          runtimeContextOverrides: ensureRuntimeContext(body.runtimeContextOverrides),
-          simulateValidationNodeIds: ensureStringArray(body.simulateValidationNodeIds),
-          executionMode: ensureExecutionMode(body.executionMode),
-          provider: ensureProvider(body.provider),
-        });
-
-        sendJson(response, 202, {
-          ok: true,
-          run: record,
-        });
-        return true;
-      }
-
-      methodNotAllowed(response);
-      return true;
-    }
-
-    if (pathname.startsWith("/api/runs/") && pathname.endsWith("/cancel")) {
-      if (method !== "POST") {
-        methodNotAllowed(response);
-        return true;
-      }
-
-      const runId = toRelativePathFromApi(pathname.slice("/api/runs/".length, -"/cancel".length));
       const run = await runService.cancelRun(runId);
-      sendJson(response, 200, {
-        ok: true,
-        run,
-      });
-      return true;
-    }
+      res.json({ ok: true, run });
+    } catch (error) { next(error); }
+  });
 
-    if (pathname.startsWith("/api/runs/")) {
-      if (method !== "GET") {
-        methodNotAllowed(response);
-        return true;
+  app.get("/api/runs/:runId", async (req, res, next) => {
+    try {
+      const runId = req.params.runId;
+      if (!runId) {
+        res.status(400).json({ ok: false, error: "runId required" });
+        return;
       }
-
-      const runId = toRelativePathFromApi(pathname.slice("/api/runs/".length));
       const run = runService.getRun(runId);
       if (!run) {
-        sendJson(response, 404, {
-          ok: false,
-          error: `Run \"${runId}\" was not found.`,
-        });
-        return true;
+        res.status(404).json({ ok: false, error: `Run "${runId}" was not found.` });
+        return;
       }
+      res.json({ ok: true, run });
+    } catch (error) { next(error); }
+  });
 
-      sendJson(response, 200, {
-        ok: true,
-        run,
-      });
-      return true;
-    }
-
-    notFound(response);
-    return true;
-  } catch (error) {
-    sendJson(response, 400, {
+  app.use("/api", (req, res) => {
+    res.status(404).json({
       ok: false,
-      error: error instanceof Error ? error.message : String(error),
+      error: `API route "${req.method} ${req.originalUrl}" was not found.`,
     });
-    return true;
-  }
-}
+  });
 
-export async function startUiServer(input: {
-  workspaceRoot: string;
-  port?: number;
-  host?: string;
-}): Promise<{
-  close: () => Promise<void>;
-}> {
-  const workspaceRoot = resolve(input.workspaceRoot);
-  const staticRoot = resolve(workspaceRoot, "src/ui/public");
-
-  const configStore = new UiConfigStore({ workspaceRoot });
-  const manifestStore = new ManifestStore({ workspaceRoot });
-  const runService = new UiRunService({ workspaceRoot });
-
-  const server = createServer(async (request, response) => {
-    const handledApi = await handleApiRequest({
-      request,
-      response,
-      workspaceRoot,
-      configStore,
-      manifestStore,
-      runService,
+  // Default error handler
+  app.use((err: any, req: express.Request, res: express.Response, next: express.NextFunction) => {
+    res.status(400).json({
+      ok: false,
+      error: err instanceof Error ? err.message : String(err),
     });
+  });
 
-    if (handledApi) {
-      return;
-    }
-
-    const requestUrl = new URL(request.url ?? "/", `http://${request.headers.host ?? "localhost"}`);
-    const pathname = requestUrl.pathname === "/" ? "/index.html" : requestUrl.pathname;
-    const cleanPath = pathname.replace(/^\//, "");
-    if (cleanPath.includes("..")) {
-      notFound(response);
-      return;
-    }
-
-    const staticPath = resolve(staticRoot, cleanPath);
-    const served = await serveStaticFile({
-      response,
-      filePath: staticPath,
-    });
-
-    if (served) {
-      return;
-    }
-
-    const fallbackServed = await serveStaticFile({
-      response,
-      filePath: resolve(staticRoot, "index.html"),
-    });
-
-    if (!fallbackServed) {
-      notFound(response);
-    }
+  // Serve static files
+  app.use(express.static(staticRoot));
+  app.get(/(.*)/, (req, res) => {
+    res.sendFile(resolve(staticRoot, "index.html"));
   });
 
   const host = input.host ?? "127.0.0.1";
   const port = input.port ?? parsePort(process.env.AGENT_UI_PORT);
 
+  const server = app.listen(port, host);
   await new Promise<void>((resolveReady, rejectReady) => {
     server.once("error", rejectReady);
-    server.listen(port, host, () => {
+    server.once("listening", () => {
       server.off("error", rejectReady);
+      console.log(`AI Ops UI listening at http://${host}:${String(port)}`);
       resolveReady();
     });
   });
 
-  console.log(`AI Ops UI listening at http://${host}:${String(port)}`);
-
   return {
     close: async () => {
+      if (!server.listening) {
+        return;
+      }
       await new Promise<void>((resolveClose, rejectClose) => {
         server.close((error) => {
           if (error) {
+            if ((error as NodeJS.ErrnoException).code === "ERR_SERVER_NOT_RUNNING") {
+              resolveClose();
+              return;
+            }
             rejectClose(error);
             return;
           }