From 886216560e68455954a4d519f6044b532713e509 Mon Sep 17 00:00:00 2001
From: buihongduc132 <buihongduc132@gmail.com>
Date: Tue, 19 May 2026 02:43:36 +0700
Subject: [PATCH] fix(server): return JSON 404 for unknown /api/* routes
 instead of HTML
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

All three servers (plan, review, annotate) serve the SPA HTML for any
unmatched route. This means hitting /api/nonexistent returns the full
HTML page with 200 OK — confusing for API clients and wasteful.

Now unknown /api/* paths get a proper JSON 404 response before the SPA
fallback kicks in. Non-API paths (like /some/random/path) still serve
HTML for SPA routing as before.

Tested on all three server types.
---
 packages/server/annotate.ts           |  8 ++++
 packages/server/api-404-guard.test.ts | 60 +++++++++++++++++++++++++++
 packages/server/index.ts              |  8 ++++
 packages/server/review.ts             |  8 ++++
 4 files changed, 84 insertions(+)
 create mode 100644 packages/server/api-404-guard.test.ts

diff --git a/packages/server/annotate.ts b/packages/server/annotate.ts
index 3029a0e9f..7f0200531 100644
--- a/packages/server/annotate.ts
+++ b/packages/server/annotate.ts
@@ -300,6 +300,14 @@ export async function startAnnotateServer(
           // Favicon
           if (url.pathname === "/favicon.svg") return handleFavicon();
 
+          // API 404 guard: unknown /api/* routes should return JSON, not HTML
+          if (url.pathname.startsWith("/api/")) {
+            return Response.json(
+              { error: "Not found", path: url.pathname },
+              { status: 404 },
+            );
+          }
+
           // Serve embedded HTML for all other routes (SPA)
           return new Response(htmlContent, {
             headers: { "Content-Type": "text/html" },
diff --git a/packages/server/api-404-guard.test.ts b/packages/server/api-404-guard.test.ts
new file mode 100644
index 000000000..59f7096b3
--- /dev/null
+++ b/packages/server/api-404-guard.test.ts
@@ -0,0 +1,60 @@
+/**
+ * PR B: Tests for API route 404 guards
+ *
+ * Unknown /api/* paths should return JSON 404, not HTML.
+ */
+import { describe, test, expect, afterAll, beforeAll } from "bun:test";
+
+describe("API route 404 guards", () => {
+  const controllers: AbortController[] = [];
+  let savedPort: string | undefined;
+  let savedRemote: string | undefined;
+
+  beforeAll(() => {
+    savedPort = process.env.PLANNOTATOR_PORT;
+    savedRemote = process.env.PLANNOTATOR_REMOTE;
+    delete process.env.PLANNOTATOR_PORT;
+    delete process.env.PLANNOTATOR_REMOTE;
+    delete process.env.PLANNOTATOR_SERVER_URL;
+  });
+
+  afterAll(() => {
+    for (const c of controllers) c.abort();
+    if (savedPort) process.env.PLANNOTATOR_PORT = savedPort;
+    if (savedRemote) process.env.PLANNOTATOR_REMOTE = savedRemote;
+  });
+
+  test("/api/nonexistent on plan server returns JSON 404", async () => {
+    const { startPlannotatorServer } = await import("./index");
+
+    const controller = new AbortController();
+    controllers.push(controller);
+    const server = await startPlannotatorServer({
+      plan: "# Test Plan\n\nHello",
+      signal: controller.signal,
+    });
+
+    const response = await fetch(`${server.url}/api/nonexistent-route`);
+    expect(response.status).toBe(404);
+    expect(response.headers.get("content-type")).toContain("application/json");
+
+    const body = await response.json() as any;
+    expect(body.error).toBe("Not found");
+    expect(body.path).toBe("/api/nonexistent-route");
+  });
+
+  test("non-API route still serves HTML (SPA fallback)", async () => {
+    const { startPlannotatorServer } = await import("./index");
+
+    const controller = new AbortController();
+    controllers.push(controller);
+    const server = await startPlannotatorServer({
+      plan: "# Test",
+      signal: controller.signal,
+    });
+
+    const response = await fetch(`${server.url}/some/random/path`);
+    expect(response.status).toBe(200);
+    expect(response.headers.get("content-type")).toContain("text/html");
+  });
+});
diff --git a/packages/server/index.ts b/packages/server/index.ts
index 8b1356a49..17c1e0ffe 100644
--- a/packages/server/index.ts
+++ b/packages/server/index.ts
@@ -568,6 +568,14 @@ export async function startPlannotatorServer(
           // Favicon
           if (url.pathname === "/favicon.svg") return handleFavicon();
 
+          // API 404 guard: unknown /api/* routes should return JSON, not HTML
+          if (url.pathname.startsWith("/api/")) {
+            return Response.json(
+              { error: "Not found", path: url.pathname },
+              { status: 404 },
+            );
+          }
+
           // Serve embedded HTML for all other routes (SPA)
           return new Response(htmlContent, {
             headers: { "Content-Type": "text/html" },
diff --git a/packages/server/review.ts b/packages/server/review.ts
index c31646398..66679bdb2 100644
--- a/packages/server/review.ts
+++ b/packages/server/review.ts
@@ -1146,6 +1146,14 @@ export async function startReviewServer(
           // Favicon
           if (url.pathname === "/favicon.svg") return handleFavicon();
 
+          // API 404 guard: unknown /api/* routes should return JSON, not HTML
+          if (url.pathname.startsWith("/api/")) {
+            return Response.json(
+              { error: "Not found", path: url.pathname },
+              { status: 404 },
+            );
+          }
+
           // Serve embedded HTML for all other routes (SPA)
           return new Response(htmlContent, {
             headers: { "Content-Type": "text/html" },