Merge pull request #237 from oboroge0/fix/149-engine-reliability

fix: バックエンド信頼性の残項目を整備（シャットダウンテスト・validate不正JSON 400）

Author: oboroge0

apps/server-ts/src/index.ts

@@ -6,8 +6,9 @@ import { createBunWebSocket } from "hono/bun";
 import { serveStatic } from "hono/bun";
 import { cors } from "hono/cors";
 import { logger } from "hono/logger";
-import { closeDb, initDb } from "./db/database";
+import { initDb } from "./db/database";
 import { WorkflowExecutor } from "./engine/executor";
+import { shutdownGracefully } from "./shutdown";
 import { integrationRoutes } from "./routes/integrations";
 import { pluginRoutes } from "./routes/plugins";
 import { settingsRoutes } from "./routes/settings";
@@ -132,21 +133,7 @@ async function gracefulShutdown(signal: string): Promise<void> {
     timeoutHandle.unref();
   }
 
-  try {
-    const runningIds = executor.getRunningWorkflowIds();
-    if (runningIds.length > 0) {
-      console.log(`Stopping ${runningIds.length} running workflow(s)...`);
-      await Promise.allSettled(runningIds.map((id) => executor.stopWorkflow(id)));
-    }
-  } catch (err) {
-    console.error("Error stopping workflows on shutdown:", err);
-  }
-
-  try {
-    closeDb();
-  } catch (err) {
-    console.error("Error closing database on shutdown:", err);
-  }
+  await shutdownGracefully(executor);
 
   clearTimeout(timeoutHandle);
   process.exit(0);

apps/server-ts/src/routes/workflows.ts

@@ -290,16 +290,22 @@ app.post("/:id/validate", async (c) => {
   const [existing] = await _db.select().from(workflows).where(eq(workflows.id, id));
   if (!existing) return c.json({ detail: "Workflow not found" }, 404);
 
+  // Distinguish "no body" (fine — validate the saved workflow) from
+  // "malformed body" (reject), same as the /:id/start endpoint.
   let body: z.infer<typeof validateWorkflowBody> = {};
-  try {
-    const rawBody = await c.req.json();
+  const rawText = await c.req.text();
+  if (rawText.trim().length > 0) {
+    let rawBody: unknown;
+    try {
+      rawBody = JSON.parse(rawText);
+    } catch {
+      return c.json({ error: "Invalid JSON in request body" }, 400);
+    }
     const parsed = validateWorkflowBody.safeParse(rawBody);
     if (!parsed.success) {
       return c.json({ error: "Invalid request body", details: parsed.error.format() }, 400);
     }
     body = parsed.data ?? {};
-  } catch {
-    // No body provided - use saved workflow data
   }
 
   const nodes = body?.nodes ?? JSON.parse(existing.nodesJson || "[]");

apps/server-ts/src/shutdown.ts

@@ -0,0 +1,36 @@
+/**
+ * Graceful shutdown sequence, extracted from the index.ts signal handlers so
+ * it can be unit-tested. process.exit and the watchdog timeout stay in
+ * index.ts — this module only knows how to wind down cleanly.
+ */
+import { closeDb } from "./db/database";
+
+export interface ShutdownTarget {
+  getRunningWorkflowIds(): string[];
+  stopWorkflow(workflowId: string): Promise<void>;
+}
+
+export async function shutdownGracefully(
+  executor: ShutdownTarget,
+  options: { closeDatabase?: () => void } = {},
+): Promise<void> {
+  const closeDatabase = options.closeDatabase ?? closeDb;
+
+  try {
+    const runningIds = executor.getRunningWorkflowIds();
+    if (runningIds.length > 0) {
+      console.log(`Stopping ${runningIds.length} running workflow(s)...`);
+      // allSettled: one workflow failing to stop must not skip the others
+      // (or the DB close below)
+      await Promise.allSettled(runningIds.map((id) => executor.stopWorkflow(id)));
+    }
+  } catch (err) {
+    console.error("Error stopping workflows on shutdown:", err);
+  }
+
+  try {
+    closeDatabase();
+  } catch (err) {
+    console.error("Error closing database on shutdown:", err);
+  }
+}

tests/engine/shutdown.test.ts

@@ -0,0 +1,75 @@
+import { describe, it, expect, mock } from "bun:test";
+import {
+  shutdownGracefully,
+  type ShutdownTarget,
+} from "../../apps/server-ts/src/shutdown";
+
+function makeExecutor(runningIds: string[], stopImpl?: (id: string) => Promise<void>) {
+  const stopWorkflow = mock(stopImpl ?? (async (_id: string) => {}));
+  const target: ShutdownTarget = {
+    getRunningWorkflowIds: () => runningIds,
+    stopWorkflow,
+  };
+  return { target, stopWorkflow };
+}
+
+describe("TestGracefulShutdown", () => {
+  it("test_stops_all_running_workflows_then_closes_db", async () => {
+    const { target, stopWorkflow } = makeExecutor(["wf-a", "wf-b", "wf-c"]);
+    const order: string[] = [];
+    const closeDatabase = mock(() => {
+      order.push("close-db");
+    });
+
+    await shutdownGracefully(target, { closeDatabase });
+
+    expect(stopWorkflow).toHaveBeenCalledTimes(3);
+    expect(stopWorkflow.mock.calls.map((c) => c[0]).sort()).toEqual(["wf-a", "wf-b", "wf-c"]);
+    expect(closeDatabase).toHaveBeenCalledTimes(1);
+  });
+
+  it("test_closes_db_when_nothing_is_running", async () => {
+    const { target, stopWorkflow } = makeExecutor([]);
+    const closeDatabase = mock(() => {});
+
+    await shutdownGracefully(target, { closeDatabase });
+
+    expect(stopWorkflow).not.toHaveBeenCalled();
+    expect(closeDatabase).toHaveBeenCalledTimes(1);
+  });
+
+  it("test_one_failing_workflow_does_not_skip_others_or_db_close", async () => {
+    const { target, stopWorkflow } = makeExecutor(["wf-bad", "wf-ok"], async (id) => {
+      if (id === "wf-bad") throw new Error("stop failed");
+    });
+    const closeDatabase = mock(() => {});
+
+    await shutdownGracefully(target, { closeDatabase });
+
+    expect(stopWorkflow).toHaveBeenCalledTimes(2);
+    expect(closeDatabase).toHaveBeenCalledTimes(1);
+  });
+
+  it("test_db_close_failure_does_not_throw", async () => {
+    const { target } = makeExecutor([]);
+    const closeDatabase = mock(() => {
+      throw new Error("db close failed");
+    });
+
+    await expect(shutdownGracefully(target, { closeDatabase })).resolves.toBeUndefined();
+    expect(closeDatabase).toHaveBeenCalledTimes(1);
+  });
+
+  it("test_getRunningWorkflowIds_failure_still_closes_db", async () => {
+    const closeDatabase = mock(() => {});
+    const target: ShutdownTarget = {
+      getRunningWorkflowIds: () => {
+        throw new Error("executor broken");
+      },
+      stopWorkflow: async () => {},
+    };
+
+    await expect(shutdownGracefully(target, { closeDatabase })).resolves.toBeUndefined();
+    expect(closeDatabase).toHaveBeenCalledTimes(1);
+  });
+});

tests/routes/workflows.test.ts

@@ -828,3 +828,55 @@ describe("Edge Cases", () => {
     expect(imported.id).not.toBe(original.id);
   });
 });
+
+// ═══════════════════════════════════════════════════════════════
+// Workflow Validation Endpoint (body handling)
+// ═══════════════════════════════════════════════════════════════
+
+describe("Workflow validate body handling", () => {
+  it("should validate the saved workflow when no body is sent", async () => {
+    const created = await createWorkflowViaApi(app);
+
+    const res = await app.request(
+      new Request(`http://localhost/api/workflows/${created.id}/validate`, {
+        method: "POST",
+      })
+    );
+
+    expect(res.status).toBe(200);
+    const data = await res.json();
+    expect(data.valid).toBeDefined();
+    expect(Array.isArray(data.errors)).toBe(true);
+  });
+
+  it("should return 400 for malformed JSON body", async () => {
+    const created = await createWorkflowViaApi(app);
+
+    const res = await app.request(
+      new Request(`http://localhost/api/workflows/${created.id}/validate`, {
+        method: "POST",
+        headers: { "Content-Type": "application/json" },
+        body: "{ this is not json",
+      })
+    );
+
+    expect(res.status).toBe(400);
+    const data = await res.json();
+    expect(data.error).toContain("Invalid JSON");
+  });
+
+  it("should validate provided nodes when a valid body is sent", async () => {
+    const created = await createWorkflowViaApi(app);
+
+    const res = await app.request(
+      jsonRequest("POST", `/api/workflows/${created.id}/validate`, {
+        nodes: [],
+        connections: [],
+      })
+    );
+
+    expect(res.status).toBe(200);
+    const data = await res.json();
+    expect(data.valid).toBeDefined();
+  });
+});