fix(server): make reopenSession idempotent and return 404 for missing

saltbo · saltbo · commit 6f17bc024a69 · 2026-04-21T09:15:24.000-04:00
Reopen used a single UPDATE gated on status='closed' and threw a plain
Error when nothing changed, collapsing "not found" and "already active"
into a 500. After a daemon crash (e.g. SIGKILL mid-closeSession), the
server session stays active while the daemon still thinks it owes a
reopen; on restart the daemon entered a 5-min backoff loop hammering
this endpoint. Split into SELECT + early-return on active + UPDATE, and
throw HTTPException(404) for genuinely missing sessions.
diff --git a/apps/web/server/agentSessionRepo.ts b/apps/web/server/agentSessionRepo.ts
@@ -1,5 +1,6 @@
 import type { AgentSession, AgentSessionWithMachine, SessionUsageInput } from "@agent-kanban/shared";
 import { signDelegation } from "@agent-kanban/shared";
+import { HTTPException } from "hono/http-exception";
 import { getAgentPrivateKey } from "./agentRepo";
 import { createAuth } from "./betterAuth";
 import type { D1 } from "./db";
@@ -107,11 +108,10 @@ export async function closeSession(db: D1, sessionId: string): Promise<void> {
 }
 
 export async function reopenSession(db: D1, sessionId: string): Promise<void> {
-  const result = await db
-    .prepare("UPDATE agent_sessions SET status = 'active', closed_at = NULL WHERE id = ? AND status = 'closed'")
-    .bind(sessionId)
-    .run();
-  if (!result.meta.changes) throw new Error(`Session ${sessionId} not found or not closed`);
+  const row = await db.prepare("SELECT status FROM agent_sessions WHERE id = ?").bind(sessionId).first<{ status: string }>();
+  if (!row) throw new HTTPException(404, { message: `Session ${sessionId} not found` });
+  if (row.status === "active") return;
+  await db.prepare("UPDATE agent_sessions SET status = 'active', closed_at = NULL WHERE id = ?").bind(sessionId).run();
 }
 
 export async function updateSessionUsage(db: D1, sessionId: string, usage: SessionUsageInput): Promise<void> {
diff --git a/tests/agent-redesign.test.ts b/tests/agent-redesign.test.ts
@@ -280,6 +280,29 @@ describe("session lifecycle", () => {
     agentId = agent.id;
   });
 
+  it("createSession auto-creates agentHost when it does not exist yet", async () => {
+    // Use a fresh machine with no pre-seeded agentHost to exercise the create-if-not-exists branch
+    const { upsertMachine } = await import("../apps/web/server/machineRepo");
+    const freshMachine = await upsertMachine(env.DB, userId, {
+      name: "fresh-machine-no-host",
+      os: "test",
+      version: "1.0",
+      runtimes: [],
+      device_id: "test-device-no-host",
+    });
+    const freshAgent = await createTestAgent(env.DB, userId, {
+      name: "FreshAgent",
+      username: "fresh-agent-no-host",
+      runtime: "claude",
+    });
+    const { createSession, getSession } = await import("../apps/web/server/agentSessionRepo");
+    const { publicKey } = await createSessionKeypair();
+    const freshSessionId = randomUUID();
+    await createSession(env.DB, env, freshAgent.id, freshMachine.id, freshSessionId, publicKey, userId);
+    const session = await getSession(env.DB, freshSessionId);
+    expect(session!.status).toBe("active");
+  });
+
   it("close session sets status and closed_at", async () => {
     const { createSession, closeSession, getSession } = await import("../apps/web/server/agentSessionRepo");
     const { publicKey } = await createSessionKeypair();
diff --git a/tests/routes.test.ts b/tests/routes.test.ts
@@ -729,6 +729,62 @@ describe("routes", () => {
     expect(res.status).toBe(200);
   });
 
+  it("POST /api/agents/:agentId/sessions/:sessionId/reopen returns 404 for nonexistent session", async () => {
+    const nonexistentSessionId = randomUUID();
+    const res = await apiRequest("POST", `/api/agents/${agentId}/sessions/${nonexistentSessionId}/reopen`, {}, apiKey);
+    expect(res.status).toBe(404);
+  });
+
+  it("POST /api/agents/:agentId/sessions/:sessionId/reopen is idempotent when session is already active", async () => {
+    // Create a fresh session that starts active (status='active', closed_at=NULL)
+    const freshSessionId = randomUUID();
+    const freshKeypair = await crypto.subtle.generateKey({ name: "Ed25519" } as any, true, ["sign", "verify"]);
+    const freshPubJwk = await crypto.subtle.exportKey("jwk", (freshKeypair as any).publicKey);
+    await apiRequest("POST", `/api/agents/${agentId}/sessions`, { session_id: freshSessionId, session_public_key: freshPubJwk.x! }, apiKey);
+
+    // Inject a sentinel closed_at while keeping status='active'. This state is not reachable
+    // via the public API — it exists solely to discriminate the no-op path from an erroneous
+    // UPDATE: if reopen runs the UPDATE it would set closed_at to NULL, failing the assertion;
+    // if it correctly skips the UPDATE the sentinel value survives unchanged.
+    const sentinelClosedAt = "2000-01-01T00:00:00.000Z";
+    await env.DB.prepare("UPDATE agent_sessions SET closed_at = ? WHERE id = ?").bind(sentinelClosedAt, freshSessionId).run();
+
+    const res = await apiRequest("POST", `/api/agents/${agentId}/sessions/${freshSessionId}/reopen`, {}, apiKey);
+    expect(res.status).toBe(200);
+
+    const row = await env.DB.prepare("SELECT status, closed_at FROM agent_sessions WHERE id = ?")
+      .bind(freshSessionId)
+      .first<{ status: string; closed_at: string | null }>();
+    expect(row?.status).toBe("active");
+    // The sentinel must survive — proves the UPDATE branch was skipped entirely
+    expect(row?.closed_at).toBe(sentinelClosedAt);
+  });
+
+  it("POST /api/agents/:agentId/sessions/:sessionId/reopen clears closed_at after close", async () => {
+    // Create a session, close it, then reopen and verify closed_at is cleared
+    const freshSessionId = randomUUID();
+    const freshKeypair = await crypto.subtle.generateKey({ name: "Ed25519" } as any, true, ["sign", "verify"]);
+    const freshPubJwk = await crypto.subtle.exportKey("jwk", (freshKeypair as any).publicKey);
+    await apiRequest("POST", `/api/agents/${agentId}/sessions`, { session_id: freshSessionId, session_public_key: freshPubJwk.x! }, apiKey);
+
+    await apiRequest("DELETE", `/api/agents/${agentId}/sessions/${freshSessionId}`, undefined, apiKey);
+
+    const closedRow = await env.DB.prepare("SELECT status, closed_at FROM agent_sessions WHERE id = ?")
+      .bind(freshSessionId)
+      .first<{ status: string; closed_at: string | null }>();
+    expect(closedRow?.status).toBe("closed");
+    expect(closedRow?.closed_at).not.toBeNull();
+
+    const res = await apiRequest("POST", `/api/agents/${agentId}/sessions/${freshSessionId}/reopen`, {}, apiKey);
+    expect(res.status).toBe(200);
+
+    const reopenedRow = await env.DB.prepare("SELECT status, closed_at FROM agent_sessions WHERE id = ?")
+      .bind(freshSessionId)
+      .first<{ status: string; closed_at: string | null }>();
+    expect(reopenedRow?.status).toBe("active");
+    expect(reopenedRow?.closed_at).toBeNull();
+  });
+
   // ─── Agent PATCH/DELETE ───
 
   it("PATCH /api/agents/:id returns 404 for nonexistent agent", async () => {
