From c28f46111c62014dbd11c5830a4418a2f6edb671 Mon Sep 17 00:00:00 2001 From: Sweets Sweetman Date: Mon, 4 May 2026 12:32:12 -0500 Subject: [PATCH 01/12] =?UTF-8?q?feat(sessions):=20port=20GET=20/api/sessi?= =?UTF-8?q?ons/[sessionId]=20from=20open-agents=20(Phase=202.4=20=E2=80=94?= =?UTF-8?q?=20first=20route)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit First route in the route-by-route cutover plan. Strategy: open-agents frontend stays unchanged in shape; api ports each route it calls in priority order (simplest first), and the open-agents frontend gets cut over to api one route at a time. Why this route first: - Pure DB read (single-row select by id) — no agent runner, no Vercel Workflow, no sandbox runtime - Hits sessions table already migrated in database PR #20 - Frontend usage: agents-frontend hits /api/sessions/{id} on session detail page navigation - Smallest possible blast radius for proving the cutover pattern Files added: lib/supabase/sessions/selectSession.ts Single-row helper + SessionRow type (hand-typed; database.types.ts regen pending — flagged in code comment) app/api/sessions/[sessionId]/route.ts GET handler matching open-agents response shape exactly (camelCase fields, "userId" preserved on the wire even though stored as account_id internally) app/api/sessions/[sessionId]/__tests__/route.test.ts (5 tests) Auth: validateAuthContext (Privy Bearer or x-api-key). Response codes match open-agents: 200 happy path, 401 no auth, 403 not owner, 404 not found. Wire-format translation: snake_case Supabase row -> camelCase response, with account_id surfaced as userId so the existing open-agents frontend fetches with zero code changes. Translation lives at the route boundary (toSessionResponse) where it is easy to remove once chat absorbs this UI and we can switch to schema-natural naming. Verification: - pnpm lint:check: clean - pnpm test: 2379/2379 pass (5 new for this route) Up next: - Cutover step (separate PR in open-agents): point the frontend at api's URL for this single route. Validate end-to-end before porting the next route. - Next routes in priority order (still pure DB, no agent/workflow): GET /api/sessions (list with unread — needs Postgres RPC for the multi-table aggregation), GET /api/sessions/[id]/chats, GET /api/sessions/[id]/chats/[chatId]. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../[sessionId]/__tests__/route.test.ts | 153 ++++++++++++++++++ app/api/sessions/[sessionId]/route.ts | 106 ++++++++++++ lib/supabase/sessions/selectSession.ts | 63 ++++++++ 3 files changed, 322 insertions(+) create mode 100644 app/api/sessions/[sessionId]/__tests__/route.test.ts create mode 100644 app/api/sessions/[sessionId]/route.ts create mode 100644 lib/supabase/sessions/selectSession.ts diff --git a/app/api/sessions/[sessionId]/__tests__/route.test.ts b/app/api/sessions/[sessionId]/__tests__/route.test.ts new file mode 100644 index 000000000..ba5581b5d --- /dev/null +++ b/app/api/sessions/[sessionId]/__tests__/route.test.ts @@ -0,0 +1,153 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { NextRequest, NextResponse } from "next/server"; +import { GET, OPTIONS } from "../route"; +import type { SessionRow } from "@/lib/supabase/sessions/selectSession"; + +vi.mock("@/lib/supabase/sessions/selectSession", () => ({ + selectSession: vi.fn(), +})); + +vi.mock("@/lib/auth/validateAuthContext", () => ({ + validateAuthContext: vi.fn(), +})); + +vi.mock("@/lib/networking/getCorsHeaders", () => ({ + getCorsHeaders: vi.fn(() => ({ "Access-Control-Allow-Origin": "*" })), +})); + +const { selectSession } = await import("@/lib/supabase/sessions/selectSession"); +const { validateAuthContext } = await import("@/lib/auth/validateAuthContext"); + +function makeReq(url = "https://example.com/api/sessions/sess_1"): NextRequest { + return new NextRequest(url); +} + +const mockRow: SessionRow = { + id: "sess_1", + account_id: "acc-uuid-1", + title: "Test session", + status: "running", + repo_owner: "acme", + repo_name: "demo", + branch: "main", + clone_url: "https://github.com/acme/demo.git", + is_new_branch: false, + global_skill_refs: [], + sandbox_state: { type: "vercel" }, + lifecycle_state: "active", + lifecycle_version: 1, + last_activity_at: "2026-05-04T00:00:00.000Z", + sandbox_expires_at: null, + hibernate_after: null, + lifecycle_run_id: null, + lifecycle_error: null, + lines_added: 12, + lines_removed: 3, + snapshot_url: null, + snapshot_created_at: null, + snapshot_size_bytes: null, + cached_diff: null, + cached_diff_updated_at: null, + created_at: "2026-05-01T00:00:00.000Z", + updated_at: "2026-05-04T00:00:00.000Z", +}; + +describe("OPTIONS /api/sessions/[sessionId]", () => { + it("returns 200 with CORS headers", async () => { + const res = await OPTIONS(); + expect(res.status).toBe(200); + }); +}); + +describe("GET /api/sessions/[sessionId]", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("returns 401 when auth fails", async () => { + vi.mocked(validateAuthContext).mockResolvedValue( + NextResponse.json({ error: "Unauthorized" }, { status: 401 }), + ); + + const res = await GET(makeReq(), { + params: Promise.resolve({ sessionId: "sess_1" }), + }); + expect(res.status).toBe(401); + expect(selectSession).not.toHaveBeenCalled(); + }); + + it("returns 404 when session does not exist", async () => { + vi.mocked(validateAuthContext).mockResolvedValue({ + accountId: "acc-uuid-1", + orgId: null, + authToken: "tok", + }); + vi.mocked(selectSession).mockResolvedValue(null); + + const res = await GET(makeReq(), { + params: Promise.resolve({ sessionId: "sess_missing" }), + }); + expect(res.status).toBe(404); + expect(selectSession).toHaveBeenCalledWith("sess_missing"); + }); + + it("returns 403 when session is owned by a different account", async () => { + vi.mocked(validateAuthContext).mockResolvedValue({ + accountId: "acc-uuid-OTHER", + orgId: null, + authToken: "tok", + }); + vi.mocked(selectSession).mockResolvedValue(mockRow); + + const res = await GET(makeReq(), { + params: Promise.resolve({ sessionId: "sess_1" }), + }); + expect(res.status).toBe(403); + }); + + it("returns 200 with camelCase session shape on happy path", async () => { + vi.mocked(validateAuthContext).mockResolvedValue({ + accountId: "acc-uuid-1", + orgId: null, + authToken: "tok", + }); + vi.mocked(selectSession).mockResolvedValue(mockRow); + + const res = await GET(makeReq(), { + params: Promise.resolve({ sessionId: "sess_1" }), + }); + expect(res.status).toBe(200); + const body = await res.json(); + expect(body).toEqual({ + session: { + id: "sess_1", + userId: "acc-uuid-1", + title: "Test session", + status: "running", + repoOwner: "acme", + repoName: "demo", + branch: "main", + cloneUrl: "https://github.com/acme/demo.git", + isNewBranch: false, + globalSkillRefs: [], + sandboxState: { type: "vercel" }, + lifecycleState: "active", + lifecycleVersion: 1, + lastActivityAt: "2026-05-04T00:00:00.000Z", + sandboxExpiresAt: null, + hibernateAfter: null, + lifecycleRunId: null, + lifecycleError: null, + linesAdded: 12, + linesRemoved: 3, + snapshotUrl: null, + snapshotCreatedAt: null, + snapshotSizeBytes: null, + cachedDiff: null, + cachedDiffUpdatedAt: null, + createdAt: "2026-05-01T00:00:00.000Z", + updatedAt: "2026-05-04T00:00:00.000Z", + }, + }); + }); +}); diff --git a/app/api/sessions/[sessionId]/route.ts b/app/api/sessions/[sessionId]/route.ts new file mode 100644 index 000000000..df354e30c --- /dev/null +++ b/app/api/sessions/[sessionId]/route.ts @@ -0,0 +1,106 @@ +import { NextRequest, NextResponse } from "next/server"; +import { getCorsHeaders } from "@/lib/networking/getCorsHeaders"; +import { validateAuthContext } from "@/lib/auth/validateAuthContext"; +import { selectSession, type SessionRow } from "@/lib/supabase/sessions/selectSession"; + +/** + * OPTIONS handler for CORS preflight requests. + * + * @returns A NextResponse with CORS headers. + */ +export async function OPTIONS() { + return new NextResponse(null, { + status: 200, + headers: getCorsHeaders(), + }); +} + +/** + * Translates the snake_case Supabase row into the camelCase shape that + * open-agents' frontend expects, preserving its existing field names + * (e.g. `userId` for what is now `account_id`). This keeps the wire + * format identical so the open-agents frontend can cut over to api + * with zero frontend code changes. + * + * @param row - The Supabase sessions row. + * @returns The camelCase session payload for HTTP responses. + */ +function toSessionResponse(row: SessionRow) { + return { + id: row.id, + userId: row.account_id, + title: row.title, + status: row.status, + repoOwner: row.repo_owner, + repoName: row.repo_name, + branch: row.branch, + cloneUrl: row.clone_url, + isNewBranch: row.is_new_branch, + globalSkillRefs: row.global_skill_refs, + sandboxState: row.sandbox_state, + lifecycleState: row.lifecycle_state, + lifecycleVersion: row.lifecycle_version, + lastActivityAt: row.last_activity_at, + sandboxExpiresAt: row.sandbox_expires_at, + hibernateAfter: row.hibernate_after, + lifecycleRunId: row.lifecycle_run_id, + lifecycleError: row.lifecycle_error, + linesAdded: row.lines_added, + linesRemoved: row.lines_removed, + snapshotUrl: row.snapshot_url, + snapshotCreatedAt: row.snapshot_created_at, + snapshotSizeBytes: row.snapshot_size_bytes, + cachedDiff: row.cached_diff, + cachedDiffUpdatedAt: row.cached_diff_updated_at, + createdAt: row.created_at, + updatedAt: row.updated_at, + }; +} + +/** + * GET /api/sessions/{sessionId} + * + * Reads a single agent session by id. Authenticates via Privy Bearer + * token or x-api-key header. Returns 404 if the session does not exist + * and 403 if it exists but is not owned by the authenticated account. + * + * Response shape mirrors open-agents' /api/sessions/[sessionId] so that + * its frontend can hit this endpoint without changes. + * + * @param request - The request object + * @param options - Route options containing the async params + * @param options.params - Route params containing the session id + * @returns A NextResponse with `{ session }` on 200, or an error. + */ +export async function GET( + request: NextRequest, + options: { params: Promise<{ sessionId: string }> }, +) { + const auth = await validateAuthContext(request); + if (auth instanceof NextResponse) { + return auth; + } + + const { sessionId } = await options.params; + const row = await selectSession(sessionId); + + if (!row) { + return NextResponse.json( + { error: "Session not found" }, + { status: 404, headers: getCorsHeaders() }, + ); + } + + if (row.account_id !== auth.accountId) { + return NextResponse.json({ error: "Forbidden" }, { status: 403, headers: getCorsHeaders() }); + } + + return NextResponse.json( + { session: toSessionResponse(row) }, + { status: 200, headers: getCorsHeaders() }, + ); +} + +export const dynamic = "force-dynamic"; +export const fetchCache = "force-no-store"; +export const revalidate = 0; diff --git a/lib/supabase/sessions/selectSession.ts b/lib/supabase/sessions/selectSession.ts new file mode 100644 index 000000000..3f3e6f093 --- /dev/null +++ b/lib/supabase/sessions/selectSession.ts @@ -0,0 +1,63 @@ +import supabase from "@/lib/supabase/serverClient"; + +/** + * Row shape for the `sessions` table (added in database PR #20 — open-agents + * Phase 0 schema port). Hand-typed here pending types/database.types.ts + * regeneration; remove once `Tables<"sessions">` is available there. + */ +export interface SessionRow { + id: string; + account_id: string; + title: string; + status: "running" | "completed" | "failed" | "archived"; + repo_owner: string | null; + repo_name: string | null; + branch: string | null; + clone_url: string | null; + is_new_branch: boolean; + global_skill_refs: unknown; + sandbox_state: unknown | null; + lifecycle_state: + | "provisioning" + | "active" + | "hibernating" + | "hibernated" + | "restoring" + | "archived" + | "failed" + | null; + lifecycle_version: number; + last_activity_at: string | null; + sandbox_expires_at: string | null; + hibernate_after: string | null; + lifecycle_run_id: string | null; + lifecycle_error: string | null; + lines_added: number | null; + lines_removed: number | null; + snapshot_url: string | null; + snapshot_created_at: string | null; + snapshot_size_bytes: number | null; + cached_diff: unknown | null; + cached_diff_updated_at: string | null; + created_at: string; + updated_at: string; +} + +/** + * Select a single session row by its id, or null if not found. + * Caller is responsible for any ownership / access-control checks. + */ +export async function selectSession(sessionId: string): Promise { + const { data, error } = await supabase + .from("sessions") + .select("*") + .eq("id", sessionId) + .maybeSingle(); + + if (error) { + console.error(`[selectSession] error for sessionId=${sessionId}:`, error); + return null; + } + + return (data as SessionRow | null) ?? null; +} From 00e2823aa42941d4990bf4357126a0eb25428541 Mon Sep 17 00:00:00 2001 From: Sweets Sweetman Date: Mon, 4 May 2026 14:40:57 -0500 Subject: [PATCH 02/12] =?UTF-8?q?fix:=20address=20PR=20review=20=E2=80=94?= =?UTF-8?q?=20SRP=20splits=20+=20use=20Tables<\"sessions\">=20from=20regen?= =?UTF-8?q?'d=20types?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three review comments on PR #514: 1. SRP: extract toSessionResponse to its own file was: defined inline in app/api/sessions/[sessionId]/route.ts now: lib/sessions/toSessionResponse.ts (one exported fn per file) 2. SRP: add a handler function (mirroring api convention) was: GET handler logic inline in route.ts now: lib/sessions/getSessionByIdHandler.ts contains all the auth + ownership + DB lookup + response logic; route.ts is a thin shell that awaits options.params and delegates. Matches the pattern used by every other api route (e.g. socials/[id]/scrape, artists/[id]/...). 3. DRY: use existing db schema type was: hand-typed SessionRow interface in selectSession.ts now: Tables<\"sessions\"> from types/database.types.ts (regenerated via npx supabase gen types typescript --project-id ... --schema public) The types regen also resolved the preview-build failure (\"Type instantiation is excessively deep and possibly infinite\") on the .from(\"sessions\") call — Supabase's type inference was choking because the table was unknown to the generic. Files added: lib/sessions/toSessionResponse.ts lib/sessions/getSessionByIdHandler.ts Files modified: app/api/sessions/[sessionId]/route.ts thin shell now app/api/sessions/[sessionId]/__tests__/ route.test.ts type alias updated lib/supabase/sessions/selectSession.ts Tables<\"sessions\"> types/database.types.ts Supabase regen Verification: - pnpm lint:check: clean - pnpm test: 2379/2379 pass (no test changes; same 5 route tests) - tsc compile clean (the local pnpm build progresses past compile into page-data collection where it fails on missing local env vars — Vercel preview will have those set, so the preview rebuild should now succeed) Co-Authored-By: Claude Opus 4.7 (1M context) --- .../[sessionId]/__tests__/route.test.ts | 4 +- app/api/sessions/[sessionId]/route.ts | 72 +--- lib/sessions/getSessionByIdHandler.ts | 47 +++ lib/sessions/toSessionResponse.ts | 43 +++ lib/supabase/sessions/selectSession.ts | 51 +-- types/database.types.ts | 307 +++++++++++++++++- 6 files changed, 404 insertions(+), 120 deletions(-) create mode 100644 lib/sessions/getSessionByIdHandler.ts create mode 100644 lib/sessions/toSessionResponse.ts diff --git a/app/api/sessions/[sessionId]/__tests__/route.test.ts b/app/api/sessions/[sessionId]/__tests__/route.test.ts index ba5581b5d..120d65d57 100644 --- a/app/api/sessions/[sessionId]/__tests__/route.test.ts +++ b/app/api/sessions/[sessionId]/__tests__/route.test.ts @@ -1,7 +1,9 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; import { NextRequest, NextResponse } from "next/server"; import { GET, OPTIONS } from "../route"; -import type { SessionRow } from "@/lib/supabase/sessions/selectSession"; +import type { Tables } from "@/types/database.types"; + +type SessionRow = Tables<"sessions">; vi.mock("@/lib/supabase/sessions/selectSession", () => ({ selectSession: vi.fn(), diff --git a/app/api/sessions/[sessionId]/route.ts b/app/api/sessions/[sessionId]/route.ts index df354e30c..dbc04994a 100644 --- a/app/api/sessions/[sessionId]/route.ts +++ b/app/api/sessions/[sessionId]/route.ts @@ -1,7 +1,6 @@ import { NextRequest, NextResponse } from "next/server"; import { getCorsHeaders } from "@/lib/networking/getCorsHeaders"; -import { validateAuthContext } from "@/lib/auth/validateAuthContext"; -import { selectSession, type SessionRow } from "@/lib/supabase/sessions/selectSession"; +import { getSessionByIdHandler } from "@/lib/sessions/getSessionByIdHandler"; /** * OPTIONS handler for CORS preflight requests. @@ -15,48 +14,6 @@ export async function OPTIONS() { }); } -/** - * Translates the snake_case Supabase row into the camelCase shape that - * open-agents' frontend expects, preserving its existing field names - * (e.g. `userId` for what is now `account_id`). This keeps the wire - * format identical so the open-agents frontend can cut over to api - * with zero frontend code changes. - * - * @param row - The Supabase sessions row. - * @returns The camelCase session payload for HTTP responses. - */ -function toSessionResponse(row: SessionRow) { - return { - id: row.id, - userId: row.account_id, - title: row.title, - status: row.status, - repoOwner: row.repo_owner, - repoName: row.repo_name, - branch: row.branch, - cloneUrl: row.clone_url, - isNewBranch: row.is_new_branch, - globalSkillRefs: row.global_skill_refs, - sandboxState: row.sandbox_state, - lifecycleState: row.lifecycle_state, - lifecycleVersion: row.lifecycle_version, - lastActivityAt: row.last_activity_at, - sandboxExpiresAt: row.sandbox_expires_at, - hibernateAfter: row.hibernate_after, - lifecycleRunId: row.lifecycle_run_id, - lifecycleError: row.lifecycle_error, - linesAdded: row.lines_added, - linesRemoved: row.lines_removed, - snapshotUrl: row.snapshot_url, - snapshotCreatedAt: row.snapshot_created_at, - snapshotSizeBytes: row.snapshot_size_bytes, - cachedDiff: row.cached_diff, - cachedDiffUpdatedAt: row.cached_diff_updated_at, - createdAt: row.created_at, - updatedAt: row.updated_at, - }; -} - /** * GET /api/sessions/{sessionId} * @@ -64,8 +21,8 @@ function toSessionResponse(row: SessionRow) { * token or x-api-key header. Returns 404 if the session does not exist * and 403 if it exists but is not owned by the authenticated account. * - * Response shape mirrors open-agents' /api/sessions/[sessionId] so that - * its frontend can hit this endpoint without changes. + * Response shape mirrors open-agents' /api/sessions/[sessionId] so the + * existing frontend can cut over to api without code changes. * * @param request - The request object * @param options - Route options containing the async params @@ -76,29 +33,8 @@ export async function GET( request: NextRequest, options: { params: Promise<{ sessionId: string }> }, ) { - const auth = await validateAuthContext(request); - if (auth instanceof NextResponse) { - return auth; - } - const { sessionId } = await options.params; - const row = await selectSession(sessionId); - - if (!row) { - return NextResponse.json( - { error: "Session not found" }, - { status: 404, headers: getCorsHeaders() }, - ); - } - - if (row.account_id !== auth.accountId) { - return NextResponse.json({ error: "Forbidden" }, { status: 403, headers: getCorsHeaders() }); - } - - return NextResponse.json( - { session: toSessionResponse(row) }, - { status: 200, headers: getCorsHeaders() }, - ); + return getSessionByIdHandler(request, sessionId); } export const dynamic = "force-dynamic"; diff --git a/lib/sessions/getSessionByIdHandler.ts b/lib/sessions/getSessionByIdHandler.ts new file mode 100644 index 000000000..4ff871220 --- /dev/null +++ b/lib/sessions/getSessionByIdHandler.ts @@ -0,0 +1,47 @@ +import { NextRequest, NextResponse } from "next/server"; +import { getCorsHeaders } from "@/lib/networking/getCorsHeaders"; +import { validateAuthContext } from "@/lib/auth/validateAuthContext"; +import { selectSession } from "@/lib/supabase/sessions/selectSession"; +import { toSessionResponse } from "@/lib/sessions/toSessionResponse"; + +/** + * Handles GET /api/sessions/{sessionId}. + * + * Reads a single agent session by id. Authenticates via Privy Bearer + * token or x-api-key header. Returns 404 if the session does not exist + * and 403 if it exists but is not owned by the authenticated account. + * + * Response shape mirrors open-agents' /api/sessions/[sessionId] so the + * existing frontend can cut over to api without code changes. + * + * @param request - The incoming request. + * @param sessionId - The id of the session to fetch. + * @returns A NextResponse with `{ session }` on 200, or an error. + */ +export async function getSessionByIdHandler( + request: NextRequest, + sessionId: string, +): Promise { + const auth = await validateAuthContext(request); + if (auth instanceof NextResponse) { + return auth; + } + + const row = await selectSession(sessionId); + + if (!row) { + return NextResponse.json( + { error: "Session not found" }, + { status: 404, headers: getCorsHeaders() }, + ); + } + + if (row.account_id !== auth.accountId) { + return NextResponse.json({ error: "Forbidden" }, { status: 403, headers: getCorsHeaders() }); + } + + return NextResponse.json( + { session: toSessionResponse(row) }, + { status: 200, headers: getCorsHeaders() }, + ); +} diff --git a/lib/sessions/toSessionResponse.ts b/lib/sessions/toSessionResponse.ts new file mode 100644 index 000000000..d7c08cbac --- /dev/null +++ b/lib/sessions/toSessionResponse.ts @@ -0,0 +1,43 @@ +import type { Tables } from "@/types/database.types"; + +/** + * Translates the snake_case Supabase row into the camelCase shape that + * open-agents' frontend expects, preserving its existing field names + * (e.g. `userId` for what is now `account_id`). This keeps the wire + * format identical so the open-agents frontend can cut over to api + * with zero frontend code changes. + * + * @param row - The Supabase sessions row. + * @returns The camelCase session payload for HTTP responses. + */ +export function toSessionResponse(row: Tables<"sessions">) { + return { + id: row.id, + userId: row.account_id, + title: row.title, + status: row.status, + repoOwner: row.repo_owner, + repoName: row.repo_name, + branch: row.branch, + cloneUrl: row.clone_url, + isNewBranch: row.is_new_branch, + globalSkillRefs: row.global_skill_refs, + sandboxState: row.sandbox_state, + lifecycleState: row.lifecycle_state, + lifecycleVersion: row.lifecycle_version, + lastActivityAt: row.last_activity_at, + sandboxExpiresAt: row.sandbox_expires_at, + hibernateAfter: row.hibernate_after, + lifecycleRunId: row.lifecycle_run_id, + lifecycleError: row.lifecycle_error, + linesAdded: row.lines_added, + linesRemoved: row.lines_removed, + snapshotUrl: row.snapshot_url, + snapshotCreatedAt: row.snapshot_created_at, + snapshotSizeBytes: row.snapshot_size_bytes, + cachedDiff: row.cached_diff, + cachedDiffUpdatedAt: row.cached_diff_updated_at, + createdAt: row.created_at, + updatedAt: row.updated_at, + }; +} diff --git a/lib/supabase/sessions/selectSession.ts b/lib/supabase/sessions/selectSession.ts index 3f3e6f093..8f7bfaefd 100644 --- a/lib/supabase/sessions/selectSession.ts +++ b/lib/supabase/sessions/selectSession.ts @@ -1,53 +1,14 @@ import supabase from "@/lib/supabase/serverClient"; - -/** - * Row shape for the `sessions` table (added in database PR #20 — open-agents - * Phase 0 schema port). Hand-typed here pending types/database.types.ts - * regeneration; remove once `Tables<"sessions">` is available there. - */ -export interface SessionRow { - id: string; - account_id: string; - title: string; - status: "running" | "completed" | "failed" | "archived"; - repo_owner: string | null; - repo_name: string | null; - branch: string | null; - clone_url: string | null; - is_new_branch: boolean; - global_skill_refs: unknown; - sandbox_state: unknown | null; - lifecycle_state: - | "provisioning" - | "active" - | "hibernating" - | "hibernated" - | "restoring" - | "archived" - | "failed" - | null; - lifecycle_version: number; - last_activity_at: string | null; - sandbox_expires_at: string | null; - hibernate_after: string | null; - lifecycle_run_id: string | null; - lifecycle_error: string | null; - lines_added: number | null; - lines_removed: number | null; - snapshot_url: string | null; - snapshot_created_at: string | null; - snapshot_size_bytes: number | null; - cached_diff: unknown | null; - cached_diff_updated_at: string | null; - created_at: string; - updated_at: string; -} +import type { Tables } from "@/types/database.types"; /** * Select a single session row by its id, or null if not found. * Caller is responsible for any ownership / access-control checks. + * + * @param sessionId - The id of the session to fetch. + * @returns The session row, or null when no row matches. */ -export async function selectSession(sessionId: string): Promise { +export async function selectSession(sessionId: string): Promise | null> { const { data, error } = await supabase .from("sessions") .select("*") @@ -59,5 +20,5 @@ export async function selectSession(sessionId: string): Promise Date: Mon, 4 May 2026 16:10:46 -0500 Subject: [PATCH 03/12] fix(sessions): make 404/403 errors emit status:"error" for shape consistency MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The 401 returned by validateAuthContext shaped like {status:"error", error:"..."} but 404/403 from this handler returned {error:"..."} only. Same endpoint, two error shapes — inconsistent for clients. Align all error responses on the validateAuthContext shape. Tests now assert the full error body, not just the status code. Co-Authored-By: Claude Opus 4.7 (1M context) --- app/api/sessions/[sessionId]/__tests__/route.test.ts | 8 ++++++++ lib/sessions/getSessionByIdHandler.ts | 7 +++++-- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/app/api/sessions/[sessionId]/__tests__/route.test.ts b/app/api/sessions/[sessionId]/__tests__/route.test.ts index 120d65d57..83c8f879a 100644 --- a/app/api/sessions/[sessionId]/__tests__/route.test.ts +++ b/app/api/sessions/[sessionId]/__tests__/route.test.ts @@ -90,6 +90,10 @@ describe("GET /api/sessions/[sessionId]", () => { params: Promise.resolve({ sessionId: "sess_missing" }), }); expect(res.status).toBe(404); + expect(await res.json()).toEqual({ + status: "error", + error: "Session not found", + }); expect(selectSession).toHaveBeenCalledWith("sess_missing"); }); @@ -105,6 +109,10 @@ describe("GET /api/sessions/[sessionId]", () => { params: Promise.resolve({ sessionId: "sess_1" }), }); expect(res.status).toBe(403); + expect(await res.json()).toEqual({ + status: "error", + error: "Forbidden", + }); }); it("returns 200 with camelCase session shape on happy path", async () => { diff --git a/lib/sessions/getSessionByIdHandler.ts b/lib/sessions/getSessionByIdHandler.ts index 4ff871220..3b5a7db5f 100644 --- a/lib/sessions/getSessionByIdHandler.ts +++ b/lib/sessions/getSessionByIdHandler.ts @@ -31,13 +31,16 @@ export async function getSessionByIdHandler( if (!row) { return NextResponse.json( - { error: "Session not found" }, + { status: "error", error: "Session not found" }, { status: 404, headers: getCorsHeaders() }, ); } if (row.account_id !== auth.accountId) { - return NextResponse.json({ error: "Forbidden" }, { status: 403, headers: getCorsHeaders() }); + return NextResponse.json( + { status: "error", error: "Forbidden" }, + { status: 403, headers: getCorsHeaders() }, + ); } return NextResponse.json( From 455cb67afec7833b13bbc015f0cbb1b70df517c7 Mon Sep 17 00:00:00 2001 From: Sweets Sweetman Date: Mon, 4 May 2026 17:14:06 -0500 Subject: [PATCH 04/12] feat(sessions): port POST /api/sessions from open-agents MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implements the POST /api/sessions contract documented in recoupable/docs PR #186 + #187. Creates a session row and an initial chat row; rolls back the session if chat insert fails so callers never observe an orphaned session. Auth: validateAuthContext (Privy Bearer or x-api-key). Validation: Zod schema + GitHub repo segment regex. Body is optional — empty body creates a session with sensible defaults (status=running, lifecycle_state=provisioning, sandbox_state.type= vercel, title="New session"). Out of scope (will follow once database catches up): auto_commit_push_override, auto_create_pr_override, pr_number, pr_status — these columns don't yet exist on api's sessions table, so the docs spec was trimmed accordingly in docs PR #187. TDD: 9 handler tests cover 401, 400 (sandboxType / repoOwner / repoName), 200 happy path, branch generation, title pass-through, 500 (insertSession failure), and 500-with-rollback (insertChat failure). Plus 1 thin test on the route shell. Co-Authored-By: Claude Opus 4.7 (1M context) --- app/api/sessions/__tests__/route.test.ts | 22 ++ app/api/sessions/route.ts | 12 + lib/github/isValidGitHubRepoName.ts | 10 + lib/github/isValidGitHubRepoOwner.ts | 12 + .../__tests__/createSessionHandler.test.ts | 212 ++++++++++++++++++ lib/sessions/createSessionHandler.ts | 81 +++++++ lib/sessions/generateSessionBranchName.ts | 10 + lib/sessions/toChatResponse.ts | 22 ++ lib/sessions/validateCreateSessionBody.ts | 50 +++++ lib/supabase/chats/insertChat.ts | 19 ++ lib/supabase/sessions/deleteSessionById.ts | 20 ++ lib/supabase/sessions/insertSession.ts | 21 ++ 12 files changed, 491 insertions(+) create mode 100644 app/api/sessions/__tests__/route.test.ts create mode 100644 app/api/sessions/route.ts create mode 100644 lib/github/isValidGitHubRepoName.ts create mode 100644 lib/github/isValidGitHubRepoOwner.ts create mode 100644 lib/sessions/__tests__/createSessionHandler.test.ts create mode 100644 lib/sessions/createSessionHandler.ts create mode 100644 lib/sessions/generateSessionBranchName.ts create mode 100644 lib/sessions/toChatResponse.ts create mode 100644 lib/sessions/validateCreateSessionBody.ts create mode 100644 lib/supabase/chats/insertChat.ts create mode 100644 lib/supabase/sessions/deleteSessionById.ts create mode 100644 lib/supabase/sessions/insertSession.ts diff --git a/app/api/sessions/__tests__/route.test.ts b/app/api/sessions/__tests__/route.test.ts new file mode 100644 index 000000000..6b70a3bd7 --- /dev/null +++ b/app/api/sessions/__tests__/route.test.ts @@ -0,0 +1,22 @@ +import { describe, it, expect, vi } from "vitest"; +import { NextRequest, NextResponse } from "next/server"; + +import { POST } from "@/app/api/sessions/route"; +import { createSessionHandler } from "@/lib/sessions/createSessionHandler"; + +vi.mock("@/lib/sessions/createSessionHandler", () => ({ + createSessionHandler: vi.fn(), +})); + +describe("POST /api/sessions", () => { + it("delegates to createSessionHandler", async () => { + const expected = NextResponse.json({ ok: true }, { status: 200 }); + vi.mocked(createSessionHandler).mockResolvedValue(expected); + + const req = new NextRequest("http://localhost/api/sessions", { method: "POST" }); + const res = await POST(req); + + expect(createSessionHandler).toHaveBeenCalledWith(req); + expect(res).toBe(expected); + }); +}); diff --git a/app/api/sessions/route.ts b/app/api/sessions/route.ts new file mode 100644 index 000000000..e853c239f --- /dev/null +++ b/app/api/sessions/route.ts @@ -0,0 +1,12 @@ +import { NextRequest } from "next/server"; +import { createSessionHandler } from "@/lib/sessions/createSessionHandler"; + +/** + * `POST /api/sessions` — create a session and an initial chat. + * + * @param request - The incoming request. + * @returns A NextResponse with `{ session, chat }` on 200, or an error. + */ +export async function POST(request: NextRequest) { + return createSessionHandler(request); +} diff --git a/lib/github/isValidGitHubRepoName.ts b/lib/github/isValidGitHubRepoName.ts new file mode 100644 index 000000000..c67913827 --- /dev/null +++ b/lib/github/isValidGitHubRepoName.ts @@ -0,0 +1,10 @@ +const GITHUB_REPO_PATH_SEGMENT_PATTERN = /^[.\w-]+$/; + +/** + * Returns true if `repoName` is a valid GitHub repository name segment. + * + * @param repoName - Candidate repo name to validate. + */ +export function isValidGitHubRepoName(repoName: string): boolean { + return GITHUB_REPO_PATH_SEGMENT_PATTERN.test(repoName); +} diff --git a/lib/github/isValidGitHubRepoOwner.ts b/lib/github/isValidGitHubRepoOwner.ts new file mode 100644 index 000000000..209fe2cb4 --- /dev/null +++ b/lib/github/isValidGitHubRepoOwner.ts @@ -0,0 +1,12 @@ +const GITHUB_REPO_PATH_SEGMENT_PATTERN = /^[.\w-]+$/; + +/** + * Returns true if `owner` is a valid GitHub repository owner segment + * (alphanumerics, underscore, hyphen, dot — the same characters GitHub + * itself accepts in URL path segments). + * + * @param owner - Candidate owner segment to validate. + */ +export function isValidGitHubRepoOwner(owner: string): boolean { + return GITHUB_REPO_PATH_SEGMENT_PATTERN.test(owner); +} diff --git a/lib/sessions/__tests__/createSessionHandler.test.ts b/lib/sessions/__tests__/createSessionHandler.test.ts new file mode 100644 index 000000000..f23a535a4 --- /dev/null +++ b/lib/sessions/__tests__/createSessionHandler.test.ts @@ -0,0 +1,212 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { NextRequest, NextResponse } from "next/server"; +import type { Tables } from "@/types/database.types"; + +import { validateAuthContext } from "@/lib/auth/validateAuthContext"; +import { insertSession } from "@/lib/supabase/sessions/insertSession"; +import { deleteSessionById } from "@/lib/supabase/sessions/deleteSessionById"; +import { insertChat } from "@/lib/supabase/chats/insertChat"; +import { createSessionHandler } from "@/lib/sessions/createSessionHandler"; + +vi.mock("@/lib/networking/getCorsHeaders", () => ({ + getCorsHeaders: () => ({ "Access-Control-Allow-Origin": "*" }), +})); + +vi.mock("@/lib/auth/validateAuthContext", () => ({ + validateAuthContext: vi.fn(), +})); + +vi.mock("@/lib/supabase/sessions/insertSession", () => ({ + insertSession: vi.fn(), +})); + +vi.mock("@/lib/supabase/sessions/deleteSessionById", () => ({ + deleteSessionById: vi.fn(), +})); + +vi.mock("@/lib/supabase/chats/insertChat", () => ({ + insertChat: vi.fn(), +})); + +vi.mock("@/lib/sessions/generateSessionBranchName", () => ({ + generateSessionBranchName: vi.fn(() => "ag/abcd1234"), +})); + +type SessionRow = Tables<"sessions">; +type ChatRow = Tables<"chats">; + +function makeReq(body: unknown, opts?: { contentType?: string | null }): NextRequest { + const headers = new Headers(); + if (opts?.contentType !== null) { + headers.set("Content-Type", opts?.contentType ?? "application/json"); + } + headers.set("x-api-key", "key_test"); + return new NextRequest("http://localhost/api/sessions", { + method: "POST", + headers, + body: typeof body === "string" ? body : JSON.stringify(body), + }); +} + +const baseSessionRow = (overrides: Partial = {}): SessionRow => ({ + id: "sess_1", + account_id: "acc-uuid-1", + title: "Test session", + status: "running", + repo_owner: null, + repo_name: null, + branch: null, + clone_url: null, + is_new_branch: false, + global_skill_refs: [], + sandbox_state: { type: "vercel" }, + lifecycle_state: "provisioning", + lifecycle_version: 0, + last_activity_at: null, + sandbox_expires_at: null, + hibernate_after: null, + lifecycle_run_id: null, + lifecycle_error: null, + lines_added: 0, + lines_removed: 0, + snapshot_url: null, + snapshot_created_at: null, + snapshot_size_bytes: null, + cached_diff: null, + cached_diff_updated_at: null, + created_at: "2026-05-04T00:00:00.000Z", + updated_at: "2026-05-04T00:00:00.000Z", + ...overrides, +}); + +const baseChatRow = (overrides: Partial = {}): ChatRow => ({ + id: "chat_1", + session_id: "sess_1", + title: "New chat", + model_id: null, + active_stream_id: null, + last_assistant_message_at: null, + created_at: "2026-05-04T00:00:00.000Z", + updated_at: "2026-05-04T00:00:00.000Z", + ...overrides, +}); + +const okAuth = { + accountId: "acc-uuid-1", + orgId: null, + authToken: "key_test", +}; + +describe("createSessionHandler", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("returns 401 when validateAuthContext rejects", async () => { + vi.mocked(validateAuthContext).mockResolvedValue( + NextResponse.json({ status: "error", error: "no auth" }, { status: 401 }), + ); + + const res = await createSessionHandler(makeReq({})); + + expect(res.status).toBe(401); + expect(insertSession).not.toHaveBeenCalled(); + }); + + it("returns 400 when sandboxType is not 'vercel'", async () => { + vi.mocked(validateAuthContext).mockResolvedValue(okAuth); + + const res = await createSessionHandler(makeReq({ sandboxType: "wrong" })); + expect(res.status).toBe(400); + expect(insertSession).not.toHaveBeenCalled(); + }); + + it("returns 400 when repoOwner has invalid github format", async () => { + vi.mocked(validateAuthContext).mockResolvedValue(okAuth); + + const res = await createSessionHandler(makeReq({ repoOwner: "bad@@@owner" })); + expect(res.status).toBe(400); + }); + + it("returns 400 when repoName has invalid github format", async () => { + vi.mocked(validateAuthContext).mockResolvedValue(okAuth); + + const res = await createSessionHandler(makeReq({ repoName: "spaces in name" })); + expect(res.status).toBe(400); + }); + + it("creates session and chat with defaults on empty body", async () => { + vi.mocked(validateAuthContext).mockResolvedValue(okAuth); + vi.mocked(insertSession).mockResolvedValue(baseSessionRow()); + vi.mocked(insertChat).mockResolvedValue(baseChatRow()); + + const res = await createSessionHandler(makeReq({})); + + expect(res.status).toBe(200); + const body = (await res.json()) as { session: { userId: string }; chat: { sessionId: string } }; + expect(body.session.userId).toBe("acc-uuid-1"); + expect(body.chat.sessionId).toBe("sess_1"); + + expect(insertSession).toHaveBeenCalledOnce(); + const insertArgs = vi.mocked(insertSession).mock.calls[0][0]; + expect(insertArgs.account_id).toBe("acc-uuid-1"); + expect(insertArgs.status).toBe("running"); + expect(insertArgs.lifecycle_state).toBe("provisioning"); + expect(insertArgs.lifecycle_version).toBe(0); + expect(insertArgs.is_new_branch).toBe(false); + expect(insertArgs.sandbox_state).toEqual({ type: "vercel" }); + + expect(insertChat).toHaveBeenCalledOnce(); + const chatArgs = vi.mocked(insertChat).mock.calls[0][0]; + expect(chatArgs.session_id).toBe("sess_1"); + expect(chatArgs.title).toBe("New chat"); + }); + + it("generates a branch when isNewBranch is true", async () => { + vi.mocked(validateAuthContext).mockResolvedValue(okAuth); + vi.mocked(insertSession).mockResolvedValue( + baseSessionRow({ branch: "ag/abcd1234", is_new_branch: true }), + ); + vi.mocked(insertChat).mockResolvedValue(baseChatRow()); + + const res = await createSessionHandler( + makeReq({ isNewBranch: true, branch: "ignored-because-new" }), + ); + + expect(res.status).toBe(200); + const insertArgs = vi.mocked(insertSession).mock.calls[0][0]; + expect(insertArgs.branch).toBe("ag/abcd1234"); + expect(insertArgs.is_new_branch).toBe(true); + }); + + it("uses provided title when present", async () => { + vi.mocked(validateAuthContext).mockResolvedValue(okAuth); + vi.mocked(insertSession).mockResolvedValue(baseSessionRow({ title: "Hello world" })); + vi.mocked(insertChat).mockResolvedValue(baseChatRow()); + + await createSessionHandler(makeReq({ title: "Hello world" })); + + expect(vi.mocked(insertSession).mock.calls[0][0].title).toBe("Hello world"); + }); + + it("returns 500 when insertSession fails", async () => { + vi.mocked(validateAuthContext).mockResolvedValue(okAuth); + vi.mocked(insertSession).mockResolvedValue(null); + + const res = await createSessionHandler(makeReq({})); + + expect(res.status).toBe(500); + expect(insertChat).not.toHaveBeenCalled(); + }); + + it("rolls back the session and returns 500 when insertChat fails", async () => { + vi.mocked(validateAuthContext).mockResolvedValue(okAuth); + vi.mocked(insertSession).mockResolvedValue(baseSessionRow({ id: "sess_rollback" })); + vi.mocked(insertChat).mockResolvedValue(null); + + const res = await createSessionHandler(makeReq({})); + + expect(res.status).toBe(500); + expect(deleteSessionById).toHaveBeenCalledWith("sess_rollback"); + }); +}); diff --git a/lib/sessions/createSessionHandler.ts b/lib/sessions/createSessionHandler.ts new file mode 100644 index 000000000..adbd1ca60 --- /dev/null +++ b/lib/sessions/createSessionHandler.ts @@ -0,0 +1,81 @@ +import { NextRequest, NextResponse } from "next/server"; +import { getCorsHeaders } from "@/lib/networking/getCorsHeaders"; +import { safeParseJson } from "@/lib/networking/safeParseJson"; +import { validateAuthContext } from "@/lib/auth/validateAuthContext"; +import { generateUUID } from "@/lib/uuid/generateUUID"; +import { validateCreateSessionBody } from "@/lib/sessions/validateCreateSessionBody"; +import { generateSessionBranchName } from "@/lib/sessions/generateSessionBranchName"; +import { insertSession } from "@/lib/supabase/sessions/insertSession"; +import { deleteSessionById } from "@/lib/supabase/sessions/deleteSessionById"; +import { insertChat } from "@/lib/supabase/chats/insertChat"; +import { toSessionResponse } from "@/lib/sessions/toSessionResponse"; +import { toChatResponse } from "@/lib/sessions/toChatResponse"; + +/** + * Handles `POST /api/sessions`. + * + * Authenticates the caller, validates the optional request body, then + * creates a session row and an initial chat row. If the chat insert + * fails after the session row is persisted, the session is rolled + * back so callers never observe an orphaned session. + * + * @param request - The incoming request. + * @returns A NextResponse with `{ session, chat }` on 200, or an error. + */ +export async function createSessionHandler(request: NextRequest): Promise { + const auth = await validateAuthContext(request); + if (auth instanceof NextResponse) { + return auth; + } + + const body = await safeParseJson(request); + const validated = validateCreateSessionBody(body); + if (validated instanceof NextResponse) { + return validated; + } + + const branch = validated.isNewBranch ? generateSessionBranchName() : (validated.branch ?? null); + const sessionId = generateUUID(); + + const sessionRow = await insertSession({ + id: sessionId, + account_id: auth.accountId, + title: validated.title?.trim() || "New session", + status: "running", + repo_owner: validated.repoOwner ?? null, + repo_name: validated.repoName ?? null, + branch, + clone_url: validated.cloneUrl ?? null, + is_new_branch: validated.isNewBranch ?? false, + global_skill_refs: [], + sandbox_state: { type: validated.sandboxType ?? "vercel" }, + lifecycle_state: "provisioning", + lifecycle_version: 0, + }); + + if (!sessionRow) { + return NextResponse.json( + { status: "error", error: "Failed to create session" }, + { status: 500, headers: getCorsHeaders() }, + ); + } + + const chatRow = await insertChat({ + id: generateUUID(), + session_id: sessionRow.id, + title: "New chat", + }); + + if (!chatRow) { + await deleteSessionById(sessionRow.id); + return NextResponse.json( + { status: "error", error: "Failed to create session" }, + { status: 500, headers: getCorsHeaders() }, + ); + } + + return NextResponse.json( + { session: toSessionResponse(sessionRow), chat: toChatResponse(chatRow) }, + { status: 200, headers: getCorsHeaders() }, + ); +} diff --git a/lib/sessions/generateSessionBranchName.ts b/lib/sessions/generateSessionBranchName.ts new file mode 100644 index 000000000..6719cba04 --- /dev/null +++ b/lib/sessions/generateSessionBranchName.ts @@ -0,0 +1,10 @@ +/** + * Generates a fresh branch name for a session that opted in to + * `isNewBranch`. Format: `ag/<8-hex-chars>` — the `ag` prefix marks + * agent-authored branches and the suffix is random enough to avoid + * collisions across concurrent sessions. + */ +export function generateSessionBranchName(): string { + const suffix = crypto.randomUUID().replace(/-/g, "").slice(0, 8); + return `ag/${suffix}`; +} diff --git a/lib/sessions/toChatResponse.ts b/lib/sessions/toChatResponse.ts new file mode 100644 index 000000000..2b89105aa --- /dev/null +++ b/lib/sessions/toChatResponse.ts @@ -0,0 +1,22 @@ +import type { Tables } from "@/types/database.types"; + +/** + * Translates a Supabase `chats` row into the camelCase shape returned + * by the API. Mirrors `toSessionResponse` so wire format stays aligned + * with what open-agents' frontend already consumes. + * + * @param row - The Supabase chats row. + * @returns The camelCase chat payload for HTTP responses. + */ +export function toChatResponse(row: Tables<"chats">) { + return { + id: row.id, + sessionId: row.session_id, + title: row.title, + modelId: row.model_id, + activeStreamId: row.active_stream_id, + lastAssistantMessageAt: row.last_assistant_message_at, + createdAt: row.created_at, + updatedAt: row.updated_at, + }; +} diff --git a/lib/sessions/validateCreateSessionBody.ts b/lib/sessions/validateCreateSessionBody.ts new file mode 100644 index 000000000..ec767f4bf --- /dev/null +++ b/lib/sessions/validateCreateSessionBody.ts @@ -0,0 +1,50 @@ +import { NextResponse } from "next/server"; +import { z } from "zod"; +import { getCorsHeaders } from "@/lib/networking/getCorsHeaders"; +import { isValidGitHubRepoOwner } from "@/lib/github/isValidGitHubRepoOwner"; +import { isValidGitHubRepoName } from "@/lib/github/isValidGitHubRepoName"; + +export const createSessionBodySchema = z.object({ + title: z.string().optional(), + repoOwner: z + .string() + .refine(isValidGitHubRepoOwner, { message: "Invalid repository owner" }) + .optional(), + repoName: z + .string() + .refine(isValidGitHubRepoName, { message: "Invalid repository name" }) + .optional(), + branch: z.string().optional(), + cloneUrl: z.string().optional(), + isNewBranch: z.boolean().optional(), + sandboxType: z.literal("vercel", { message: "Invalid sandbox type" }).optional(), +}); + +export type CreateSessionBody = z.infer; + +/** + * Validates the request body for `POST /api/sessions`. + * + * @param body - The parsed JSON body (or `{}` for an empty body). + * @returns The validated body, or a 400 NextResponse describing the first failure. + */ +export function validateCreateSessionBody(body: unknown): NextResponse | CreateSessionBody { + const result = createSessionBodySchema.safeParse(body); + + if (!result.success) { + const firstError = result.error.issues[0]; + return NextResponse.json( + { + status: "error", + missing_fields: firstError.path, + error: firstError.message, + }, + { + status: 400, + headers: getCorsHeaders(), + }, + ); + } + + return result.data; +} diff --git a/lib/supabase/chats/insertChat.ts b/lib/supabase/chats/insertChat.ts new file mode 100644 index 000000000..6b6102198 --- /dev/null +++ b/lib/supabase/chats/insertChat.ts @@ -0,0 +1,19 @@ +import supabase from "@/lib/supabase/serverClient"; +import type { Tables, TablesInsert } from "@/types/database.types"; + +/** + * Inserts a new row into the `chats` table and returns it. + * + * @param row - The chat row to insert. + * @returns The inserted row, or `null` if the insert failed. + */ +export async function insertChat(row: TablesInsert<"chats">): Promise | null> { + const { data, error } = await supabase.from("chats").insert(row).select().maybeSingle(); + + if (error) { + console.error("Error inserting chat:", error); + return null; + } + + return data; +} diff --git a/lib/supabase/sessions/deleteSessionById.ts b/lib/supabase/sessions/deleteSessionById.ts new file mode 100644 index 000000000..731f75a34 --- /dev/null +++ b/lib/supabase/sessions/deleteSessionById.ts @@ -0,0 +1,20 @@ +import supabase from "@/lib/supabase/serverClient"; + +/** + * Deletes the session with the given id. Used for rollback when a + * subsequent insert (e.g. the initial chat) fails after the session + * row was already persisted. + * + * @param id - The session id to delete. + * @returns `true` on success, `false` if the delete failed. + */ +export async function deleteSessionById(id: string): Promise { + const { error } = await supabase.from("sessions").delete().eq("id", id); + + if (error) { + console.error("Error deleting session:", error); + return false; + } + + return true; +} diff --git a/lib/supabase/sessions/insertSession.ts b/lib/supabase/sessions/insertSession.ts new file mode 100644 index 000000000..7ac6f7bf7 --- /dev/null +++ b/lib/supabase/sessions/insertSession.ts @@ -0,0 +1,21 @@ +import supabase from "@/lib/supabase/serverClient"; +import type { Tables, TablesInsert } from "@/types/database.types"; + +/** + * Inserts a new row into the `sessions` table and returns it. + * + * @param row - The session row to insert. + * @returns The inserted row, or `null` if the insert failed. + */ +export async function insertSession( + row: TablesInsert<"sessions">, +): Promise | null> { + const { data, error } = await supabase.from("sessions").insert(row).select().maybeSingle(); + + if (error) { + console.error("Error inserting session:", error); + return null; + } + + return data; +} From 5fb85d50be3075ac007ba421da19afa178a69b13 Mon Sep 17 00:00:00 2001 From: Sweets Sweetman Date: Mon, 4 May 2026 17:18:04 -0500 Subject: [PATCH 05/12] feat(sessions): add OPTIONS handler + cache directives to POST route Match the convention from app/api/sessions/[sessionId]/route.ts: - OPTIONS handler returning 200 + CORS headers (preflight) - dynamic="force-dynamic", fetchCache="force-no-store", revalidate=0 POST routes that mutate DB shouldn't be cached, and browsers issuing preflight checks (POST with JSON body + custom auth headers) need OPTIONS to respond. Co-Authored-By: Claude Opus 4.7 (1M context) --- app/api/sessions/__tests__/route.test.ts | 9 ++++++++- app/api/sessions/route.ts | 19 ++++++++++++++++++- 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/app/api/sessions/__tests__/route.test.ts b/app/api/sessions/__tests__/route.test.ts index 6b70a3bd7..09170af70 100644 --- a/app/api/sessions/__tests__/route.test.ts +++ b/app/api/sessions/__tests__/route.test.ts @@ -1,7 +1,7 @@ import { describe, it, expect, vi } from "vitest"; import { NextRequest, NextResponse } from "next/server"; -import { POST } from "@/app/api/sessions/route"; +import { POST, OPTIONS } from "@/app/api/sessions/route"; import { createSessionHandler } from "@/lib/sessions/createSessionHandler"; vi.mock("@/lib/sessions/createSessionHandler", () => ({ @@ -20,3 +20,10 @@ describe("POST /api/sessions", () => { expect(res).toBe(expected); }); }); + +describe("OPTIONS /api/sessions", () => { + it("returns 200 for CORS preflight", async () => { + const res = await OPTIONS(); + expect(res.status).toBe(200); + }); +}); diff --git a/app/api/sessions/route.ts b/app/api/sessions/route.ts index e853c239f..d4a458782 100644 --- a/app/api/sessions/route.ts +++ b/app/api/sessions/route.ts @@ -1,6 +1,19 @@ -import { NextRequest } from "next/server"; +import { NextRequest, NextResponse } from "next/server"; +import { getCorsHeaders } from "@/lib/networking/getCorsHeaders"; import { createSessionHandler } from "@/lib/sessions/createSessionHandler"; +/** + * OPTIONS handler for CORS preflight requests. + * + * @returns A NextResponse with CORS headers. + */ +export async function OPTIONS() { + return new NextResponse(null, { + status: 200, + headers: getCorsHeaders(), + }); +} + /** * `POST /api/sessions` — create a session and an initial chat. * @@ -10,3 +23,7 @@ import { createSessionHandler } from "@/lib/sessions/createSessionHandler"; export async function POST(request: NextRequest) { return createSessionHandler(request); } + +export const dynamic = "force-dynamic"; +export const fetchCache = "force-no-store"; +export const revalidate = 0; From 93da2ac32adcf26271873b7ea5a096d7b8cbea9d Mon Sep 17 00:00:00 2001 From: Sweets Sweetman Date: Mon, 4 May 2026 17:26:37 -0500 Subject: [PATCH 06/12] fix(sessions): address PR review feedback - SRP: extract insert-row construction to lib/sessions/buildSessionInsertRow.ts - YAGNI: drop generateSessionBranchName + isNewBranch handling (sessions commit to whatever branch the client provides; auto-generation was speculative) - Tighten isValidGitHubRepoOwner: GitHub's actual rules are alphanumeric + hyphen only (no `_` or `.`), 1-39 chars, no leading/trailing or consecutive hyphens - Tighten isValidGitHubRepoName: reject reserved `.` and `..`, reject `.git` suffix, cap at 100 chars - Add unit tests for both validators (15 cases) and for the new buildSessionInsertRow (4 cases) - Split createSessionHandler tests into auth/validation + persistence files; share fixtures via createSessionHandlerFixtures.ts. All test files now under 100 lines. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../__tests__/isValidGitHubRepoName.test.ts | 34 ++++ .../__tests__/isValidGitHubRepoOwner.test.ts | 33 +++ lib/github/isValidGitHubRepoName.ts | 14 +- lib/github/isValidGitHubRepoOwner.ts | 15 +- .../__tests__/buildSessionInsertRow.test.ts | 45 ++++ .../createSessionHandler.persistence.test.ts | 76 +++++++ .../__tests__/createSessionHandler.test.ts | 192 ++---------------- .../__tests__/createSessionHandlerFixtures.ts | 81 ++++++++ lib/sessions/buildSessionInsertRow.ts | 36 ++++ lib/sessions/createSessionHandler.ts | 44 ++-- lib/sessions/generateSessionBranchName.ts | 10 - lib/sessions/validateCreateSessionBody.ts | 1 - 12 files changed, 357 insertions(+), 224 deletions(-) create mode 100644 lib/github/__tests__/isValidGitHubRepoName.test.ts create mode 100644 lib/github/__tests__/isValidGitHubRepoOwner.test.ts create mode 100644 lib/sessions/__tests__/buildSessionInsertRow.test.ts create mode 100644 lib/sessions/__tests__/createSessionHandler.persistence.test.ts create mode 100644 lib/sessions/__tests__/createSessionHandlerFixtures.ts create mode 100644 lib/sessions/buildSessionInsertRow.ts delete mode 100644 lib/sessions/generateSessionBranchName.ts diff --git a/lib/github/__tests__/isValidGitHubRepoName.test.ts b/lib/github/__tests__/isValidGitHubRepoName.test.ts new file mode 100644 index 000000000..a2fadfede --- /dev/null +++ b/lib/github/__tests__/isValidGitHubRepoName.test.ts @@ -0,0 +1,34 @@ +import { describe, it, expect } from "vitest"; +import { isValidGitHubRepoName } from "@/lib/github/isValidGitHubRepoName"; + +describe("isValidGitHubRepoName", () => { + it("accepts standard repo names", () => { + expect(isValidGitHubRepoName("api")).toBe(true); + expect(isValidGitHubRepoName("hello-world")).toBe(true); + expect(isValidGitHubRepoName("snake_case")).toBe(true); + expect(isValidGitHubRepoName("dot.in.middle")).toBe(true); + expect(isValidGitHubRepoName("v1.2.3")).toBe(true); + }); + + it("rejects empty and too-long names", () => { + expect(isValidGitHubRepoName("")).toBe(false); + expect(isValidGitHubRepoName("a".repeat(101))).toBe(false); + }); + + it("rejects reserved dot-only names", () => { + expect(isValidGitHubRepoName(".")).toBe(false); + expect(isValidGitHubRepoName("..")).toBe(false); + }); + + it("rejects names ending in .git", () => { + expect(isValidGitHubRepoName("repo.git")).toBe(false); + expect(isValidGitHubRepoName("Repo.GIT")).toBe(false); + expect(isValidGitHubRepoName(".git")).toBe(false); + }); + + it("rejects characters outside [\\w.-]", () => { + expect(isValidGitHubRepoName("with space")).toBe(false); + expect(isValidGitHubRepoName("repo@bad")).toBe(false); + expect(isValidGitHubRepoName("slash/repo")).toBe(false); + }); +}); diff --git a/lib/github/__tests__/isValidGitHubRepoOwner.test.ts b/lib/github/__tests__/isValidGitHubRepoOwner.test.ts new file mode 100644 index 000000000..26067ba2c --- /dev/null +++ b/lib/github/__tests__/isValidGitHubRepoOwner.test.ts @@ -0,0 +1,33 @@ +import { describe, it, expect } from "vitest"; +import { isValidGitHubRepoOwner } from "@/lib/github/isValidGitHubRepoOwner"; + +describe("isValidGitHubRepoOwner", () => { + it("accepts standard alphanumeric owners", () => { + expect(isValidGitHubRepoOwner("vercel")).toBe(true); + expect(isValidGitHubRepoOwner("Recoupable")).toBe(true); + expect(isValidGitHubRepoOwner("octo-cat-42")).toBe(true); + expect(isValidGitHubRepoOwner("a")).toBe(true); + }); + + it("rejects empty and too-long owners", () => { + expect(isValidGitHubRepoOwner("")).toBe(false); + expect(isValidGitHubRepoOwner("a".repeat(40))).toBe(false); + }); + + it("rejects underscores and dots (allowed in repo names but not owner logins)", () => { + expect(isValidGitHubRepoOwner("snake_case")).toBe(false); + expect(isValidGitHubRepoOwner("dot.case")).toBe(false); + }); + + it("rejects leading, trailing, and consecutive hyphens", () => { + expect(isValidGitHubRepoOwner("-leading")).toBe(false); + expect(isValidGitHubRepoOwner("trailing-")).toBe(false); + expect(isValidGitHubRepoOwner("double--hyphen")).toBe(false); + }); + + it("rejects characters outside [a-zA-Z0-9-]", () => { + expect(isValidGitHubRepoOwner("bad@owner")).toBe(false); + expect(isValidGitHubRepoOwner("with space")).toBe(false); + expect(isValidGitHubRepoOwner("slash/owner")).toBe(false); + }); +}); diff --git a/lib/github/isValidGitHubRepoName.ts b/lib/github/isValidGitHubRepoName.ts index c67913827..2457fd5c5 100644 --- a/lib/github/isValidGitHubRepoName.ts +++ b/lib/github/isValidGitHubRepoName.ts @@ -1,10 +1,18 @@ -const GITHUB_REPO_PATH_SEGMENT_PATTERN = /^[.\w-]+$/; +const MAX_REPO_NAME_LENGTH = 100; /** - * Returns true if `repoName` is a valid GitHub repository name segment. + * Returns true if `repoName` is a valid GitHub repository name. + * + * GitHub's actual rules: 1–100 characters, alphanumerics plus `.`, + * `-`, and `_`. The reserved names `.` and `..` are not allowed, + * and a name cannot end with `.git`. * * @param repoName - Candidate repo name to validate. */ export function isValidGitHubRepoName(repoName: string): boolean { - return GITHUB_REPO_PATH_SEGMENT_PATTERN.test(repoName); + if (repoName.length === 0 || repoName.length > MAX_REPO_NAME_LENGTH) return false; + if (repoName === "." || repoName === "..") return false; + if (!/^[\w.-]+$/.test(repoName)) return false; + if (repoName.toLowerCase().endsWith(".git")) return false; + return true; } diff --git a/lib/github/isValidGitHubRepoOwner.ts b/lib/github/isValidGitHubRepoOwner.ts index 209fe2cb4..4d4648fe5 100644 --- a/lib/github/isValidGitHubRepoOwner.ts +++ b/lib/github/isValidGitHubRepoOwner.ts @@ -1,12 +1,17 @@ -const GITHUB_REPO_PATH_SEGMENT_PATTERN = /^[.\w-]+$/; +const MAX_OWNER_LENGTH = 39; /** - * Returns true if `owner` is a valid GitHub repository owner segment - * (alphanumerics, underscore, hyphen, dot — the same characters GitHub - * itself accepts in URL path segments). + * Returns true if `owner` is a valid GitHub user / org login. + * + * GitHub's actual rules: 1–39 characters, alphanumerics and hyphens + * only, cannot start or end with a hyphen, no consecutive hyphens. * * @param owner - Candidate owner segment to validate. */ export function isValidGitHubRepoOwner(owner: string): boolean { - return GITHUB_REPO_PATH_SEGMENT_PATTERN.test(owner); + if (owner.length === 0 || owner.length > MAX_OWNER_LENGTH) return false; + if (!/^[a-zA-Z0-9-]+$/.test(owner)) return false; + if (owner.startsWith("-") || owner.endsWith("-")) return false; + if (owner.includes("--")) return false; + return true; } diff --git a/lib/sessions/__tests__/buildSessionInsertRow.test.ts b/lib/sessions/__tests__/buildSessionInsertRow.test.ts new file mode 100644 index 000000000..bd02d37ad --- /dev/null +++ b/lib/sessions/__tests__/buildSessionInsertRow.test.ts @@ -0,0 +1,45 @@ +import { describe, it, expect } from "vitest"; +import { buildSessionInsertRow } from "@/lib/sessions/buildSessionInsertRow"; + +describe("buildSessionInsertRow", () => { + it("returns sane defaults for an empty body", () => { + const row = buildSessionInsertRow({ body: {}, accountId: "acc-1" }); + expect(row.account_id).toBe("acc-1"); + expect(row.title).toBe("New session"); + expect(row.status).toBe("running"); + expect(row.lifecycle_state).toBe("provisioning"); + expect(row.lifecycle_version).toBe(0); + expect(row.sandbox_state).toEqual({ type: "vercel" }); + expect(row.repo_owner).toBeNull(); + expect(row.repo_name).toBeNull(); + expect(row.branch).toBeNull(); + expect(row.clone_url).toBeNull(); + expect(row.id).toMatch(/^[0-9a-f-]{36}$/i); + }); + + it("trims and forwards a provided title", () => { + const row = buildSessionInsertRow({ body: { title: " Hello " }, accountId: "acc-1" }); + expect(row.title).toBe("Hello"); + }); + + it("falls back to default when title is whitespace-only", () => { + const row = buildSessionInsertRow({ body: { title: " " }, accountId: "acc-1" }); + expect(row.title).toBe("New session"); + }); + + it("forwards repo + branch + clone fields verbatim", () => { + const row = buildSessionInsertRow({ + body: { + repoOwner: "vercel", + repoName: "ai", + branch: "main", + cloneUrl: "https://github.com/vercel/ai.git", + }, + accountId: "acc-1", + }); + expect(row.repo_owner).toBe("vercel"); + expect(row.repo_name).toBe("ai"); + expect(row.branch).toBe("main"); + expect(row.clone_url).toBe("https://github.com/vercel/ai.git"); + }); +}); diff --git a/lib/sessions/__tests__/createSessionHandler.persistence.test.ts b/lib/sessions/__tests__/createSessionHandler.persistence.test.ts new file mode 100644 index 000000000..7cef69e95 --- /dev/null +++ b/lib/sessions/__tests__/createSessionHandler.persistence.test.ts @@ -0,0 +1,76 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; + +import { validateAuthContext } from "@/lib/auth/validateAuthContext"; +import { insertSession } from "@/lib/supabase/sessions/insertSession"; +import { deleteSessionById } from "@/lib/supabase/sessions/deleteSessionById"; +import { insertChat } from "@/lib/supabase/chats/insertChat"; +import { createSessionHandler } from "@/lib/sessions/createSessionHandler"; +import { + baseChatRow, + baseSessionRow, + makeCreateSessionReq, + okAuth, +} from "@/lib/sessions/__tests__/createSessionHandlerFixtures"; + +vi.mock("@/lib/networking/getCorsHeaders", () => ({ + getCorsHeaders: () => ({ "Access-Control-Allow-Origin": "*" }), +})); +vi.mock("@/lib/auth/validateAuthContext", () => ({ validateAuthContext: vi.fn() })); +vi.mock("@/lib/supabase/sessions/insertSession", () => ({ insertSession: vi.fn() })); +vi.mock("@/lib/supabase/sessions/deleteSessionById", () => ({ deleteSessionById: vi.fn() })); +vi.mock("@/lib/supabase/chats/insertChat", () => ({ insertChat: vi.fn() })); + +describe("createSessionHandler — persistence", () => { + beforeEach(() => vi.clearAllMocks()); + + it("creates session and chat with defaults on empty body", async () => { + vi.mocked(validateAuthContext).mockResolvedValue(okAuth); + vi.mocked(insertSession).mockResolvedValue(baseSessionRow()); + vi.mocked(insertChat).mockResolvedValue(baseChatRow()); + + const res = await createSessionHandler(makeCreateSessionReq({})); + expect(res.status).toBe(200); + + const body = (await res.json()) as { session: { userId: string }; chat: { sessionId: string } }; + expect(body.session.userId).toBe("acc-uuid-1"); + expect(body.chat.sessionId).toBe("sess_1"); + + const insertArgs = vi.mocked(insertSession).mock.calls[0][0]; + expect(insertArgs.account_id).toBe("acc-uuid-1"); + expect(insertArgs.status).toBe("running"); + expect(insertArgs.lifecycle_state).toBe("provisioning"); + expect(insertArgs.sandbox_state).toEqual({ type: "vercel" }); + + const chatArgs = vi.mocked(insertChat).mock.calls[0][0]; + expect(chatArgs.session_id).toBe("sess_1"); + expect(chatArgs.title).toBe("New chat"); + }); + + it("uses provided title when present", async () => { + vi.mocked(validateAuthContext).mockResolvedValue(okAuth); + vi.mocked(insertSession).mockResolvedValue(baseSessionRow({ title: "Hello world" })); + vi.mocked(insertChat).mockResolvedValue(baseChatRow()); + + await createSessionHandler(makeCreateSessionReq({ title: "Hello world" })); + expect(vi.mocked(insertSession).mock.calls[0][0].title).toBe("Hello world"); + }); + + it("returns 500 when insertSession fails", async () => { + vi.mocked(validateAuthContext).mockResolvedValue(okAuth); + vi.mocked(insertSession).mockResolvedValue(null); + + const res = await createSessionHandler(makeCreateSessionReq({})); + expect(res.status).toBe(500); + expect(insertChat).not.toHaveBeenCalled(); + }); + + it("rolls back the session and returns 500 when insertChat fails", async () => { + vi.mocked(validateAuthContext).mockResolvedValue(okAuth); + vi.mocked(insertSession).mockResolvedValue(baseSessionRow({ id: "sess_rollback" })); + vi.mocked(insertChat).mockResolvedValue(null); + + const res = await createSessionHandler(makeCreateSessionReq({})); + expect(res.status).toBe(500); + expect(deleteSessionById).toHaveBeenCalledWith("sess_rollback"); + }); +}); diff --git a/lib/sessions/__tests__/createSessionHandler.test.ts b/lib/sessions/__tests__/createSessionHandler.test.ts index f23a535a4..4304038bf 100644 --- a/lib/sessions/__tests__/createSessionHandler.test.ts +++ b/lib/sessions/__tests__/createSessionHandler.test.ts @@ -1,212 +1,50 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; -import { NextRequest, NextResponse } from "next/server"; -import type { Tables } from "@/types/database.types"; +import { NextResponse } from "next/server"; import { validateAuthContext } from "@/lib/auth/validateAuthContext"; import { insertSession } from "@/lib/supabase/sessions/insertSession"; -import { deleteSessionById } from "@/lib/supabase/sessions/deleteSessionById"; -import { insertChat } from "@/lib/supabase/chats/insertChat"; import { createSessionHandler } from "@/lib/sessions/createSessionHandler"; +import { + makeCreateSessionReq, + okAuth, +} from "@/lib/sessions/__tests__/createSessionHandlerFixtures"; vi.mock("@/lib/networking/getCorsHeaders", () => ({ getCorsHeaders: () => ({ "Access-Control-Allow-Origin": "*" }), })); +vi.mock("@/lib/auth/validateAuthContext", () => ({ validateAuthContext: vi.fn() })); +vi.mock("@/lib/supabase/sessions/insertSession", () => ({ insertSession: vi.fn() })); +vi.mock("@/lib/supabase/sessions/deleteSessionById", () => ({ deleteSessionById: vi.fn() })); +vi.mock("@/lib/supabase/chats/insertChat", () => ({ insertChat: vi.fn() })); -vi.mock("@/lib/auth/validateAuthContext", () => ({ - validateAuthContext: vi.fn(), -})); - -vi.mock("@/lib/supabase/sessions/insertSession", () => ({ - insertSession: vi.fn(), -})); - -vi.mock("@/lib/supabase/sessions/deleteSessionById", () => ({ - deleteSessionById: vi.fn(), -})); - -vi.mock("@/lib/supabase/chats/insertChat", () => ({ - insertChat: vi.fn(), -})); - -vi.mock("@/lib/sessions/generateSessionBranchName", () => ({ - generateSessionBranchName: vi.fn(() => "ag/abcd1234"), -})); - -type SessionRow = Tables<"sessions">; -type ChatRow = Tables<"chats">; - -function makeReq(body: unknown, opts?: { contentType?: string | null }): NextRequest { - const headers = new Headers(); - if (opts?.contentType !== null) { - headers.set("Content-Type", opts?.contentType ?? "application/json"); - } - headers.set("x-api-key", "key_test"); - return new NextRequest("http://localhost/api/sessions", { - method: "POST", - headers, - body: typeof body === "string" ? body : JSON.stringify(body), - }); -} - -const baseSessionRow = (overrides: Partial = {}): SessionRow => ({ - id: "sess_1", - account_id: "acc-uuid-1", - title: "Test session", - status: "running", - repo_owner: null, - repo_name: null, - branch: null, - clone_url: null, - is_new_branch: false, - global_skill_refs: [], - sandbox_state: { type: "vercel" }, - lifecycle_state: "provisioning", - lifecycle_version: 0, - last_activity_at: null, - sandbox_expires_at: null, - hibernate_after: null, - lifecycle_run_id: null, - lifecycle_error: null, - lines_added: 0, - lines_removed: 0, - snapshot_url: null, - snapshot_created_at: null, - snapshot_size_bytes: null, - cached_diff: null, - cached_diff_updated_at: null, - created_at: "2026-05-04T00:00:00.000Z", - updated_at: "2026-05-04T00:00:00.000Z", - ...overrides, -}); - -const baseChatRow = (overrides: Partial = {}): ChatRow => ({ - id: "chat_1", - session_id: "sess_1", - title: "New chat", - model_id: null, - active_stream_id: null, - last_assistant_message_at: null, - created_at: "2026-05-04T00:00:00.000Z", - updated_at: "2026-05-04T00:00:00.000Z", - ...overrides, -}); - -const okAuth = { - accountId: "acc-uuid-1", - orgId: null, - authToken: "key_test", -}; - -describe("createSessionHandler", () => { - beforeEach(() => { - vi.clearAllMocks(); - }); +describe("createSessionHandler — auth & validation", () => { + beforeEach(() => vi.clearAllMocks()); it("returns 401 when validateAuthContext rejects", async () => { vi.mocked(validateAuthContext).mockResolvedValue( NextResponse.json({ status: "error", error: "no auth" }, { status: 401 }), ); - - const res = await createSessionHandler(makeReq({})); - + const res = await createSessionHandler(makeCreateSessionReq({})); expect(res.status).toBe(401); expect(insertSession).not.toHaveBeenCalled(); }); it("returns 400 when sandboxType is not 'vercel'", async () => { vi.mocked(validateAuthContext).mockResolvedValue(okAuth); - - const res = await createSessionHandler(makeReq({ sandboxType: "wrong" })); + const res = await createSessionHandler(makeCreateSessionReq({ sandboxType: "wrong" })); expect(res.status).toBe(400); expect(insertSession).not.toHaveBeenCalled(); }); it("returns 400 when repoOwner has invalid github format", async () => { vi.mocked(validateAuthContext).mockResolvedValue(okAuth); - - const res = await createSessionHandler(makeReq({ repoOwner: "bad@@@owner" })); + const res = await createSessionHandler(makeCreateSessionReq({ repoOwner: "bad@@@owner" })); expect(res.status).toBe(400); }); it("returns 400 when repoName has invalid github format", async () => { vi.mocked(validateAuthContext).mockResolvedValue(okAuth); - - const res = await createSessionHandler(makeReq({ repoName: "spaces in name" })); + const res = await createSessionHandler(makeCreateSessionReq({ repoName: "spaces in name" })); expect(res.status).toBe(400); }); - - it("creates session and chat with defaults on empty body", async () => { - vi.mocked(validateAuthContext).mockResolvedValue(okAuth); - vi.mocked(insertSession).mockResolvedValue(baseSessionRow()); - vi.mocked(insertChat).mockResolvedValue(baseChatRow()); - - const res = await createSessionHandler(makeReq({})); - - expect(res.status).toBe(200); - const body = (await res.json()) as { session: { userId: string }; chat: { sessionId: string } }; - expect(body.session.userId).toBe("acc-uuid-1"); - expect(body.chat.sessionId).toBe("sess_1"); - - expect(insertSession).toHaveBeenCalledOnce(); - const insertArgs = vi.mocked(insertSession).mock.calls[0][0]; - expect(insertArgs.account_id).toBe("acc-uuid-1"); - expect(insertArgs.status).toBe("running"); - expect(insertArgs.lifecycle_state).toBe("provisioning"); - expect(insertArgs.lifecycle_version).toBe(0); - expect(insertArgs.is_new_branch).toBe(false); - expect(insertArgs.sandbox_state).toEqual({ type: "vercel" }); - - expect(insertChat).toHaveBeenCalledOnce(); - const chatArgs = vi.mocked(insertChat).mock.calls[0][0]; - expect(chatArgs.session_id).toBe("sess_1"); - expect(chatArgs.title).toBe("New chat"); - }); - - it("generates a branch when isNewBranch is true", async () => { - vi.mocked(validateAuthContext).mockResolvedValue(okAuth); - vi.mocked(insertSession).mockResolvedValue( - baseSessionRow({ branch: "ag/abcd1234", is_new_branch: true }), - ); - vi.mocked(insertChat).mockResolvedValue(baseChatRow()); - - const res = await createSessionHandler( - makeReq({ isNewBranch: true, branch: "ignored-because-new" }), - ); - - expect(res.status).toBe(200); - const insertArgs = vi.mocked(insertSession).mock.calls[0][0]; - expect(insertArgs.branch).toBe("ag/abcd1234"); - expect(insertArgs.is_new_branch).toBe(true); - }); - - it("uses provided title when present", async () => { - vi.mocked(validateAuthContext).mockResolvedValue(okAuth); - vi.mocked(insertSession).mockResolvedValue(baseSessionRow({ title: "Hello world" })); - vi.mocked(insertChat).mockResolvedValue(baseChatRow()); - - await createSessionHandler(makeReq({ title: "Hello world" })); - - expect(vi.mocked(insertSession).mock.calls[0][0].title).toBe("Hello world"); - }); - - it("returns 500 when insertSession fails", async () => { - vi.mocked(validateAuthContext).mockResolvedValue(okAuth); - vi.mocked(insertSession).mockResolvedValue(null); - - const res = await createSessionHandler(makeReq({})); - - expect(res.status).toBe(500); - expect(insertChat).not.toHaveBeenCalled(); - }); - - it("rolls back the session and returns 500 when insertChat fails", async () => { - vi.mocked(validateAuthContext).mockResolvedValue(okAuth); - vi.mocked(insertSession).mockResolvedValue(baseSessionRow({ id: "sess_rollback" })); - vi.mocked(insertChat).mockResolvedValue(null); - - const res = await createSessionHandler(makeReq({})); - - expect(res.status).toBe(500); - expect(deleteSessionById).toHaveBeenCalledWith("sess_rollback"); - }); }); diff --git a/lib/sessions/__tests__/createSessionHandlerFixtures.ts b/lib/sessions/__tests__/createSessionHandlerFixtures.ts new file mode 100644 index 000000000..001c99f14 --- /dev/null +++ b/lib/sessions/__tests__/createSessionHandlerFixtures.ts @@ -0,0 +1,81 @@ +import { NextRequest } from "next/server"; +import type { Tables } from "@/types/database.types"; + +export type SessionRow = Tables<"sessions">; +export type ChatRow = Tables<"chats">; + +/** + * Builds a NextRequest pointing at `/api/sessions` with a JSON body. + * + * @param body - The body to send (object literal or raw string). + */ +export function makeCreateSessionReq(body: unknown): NextRequest { + const headers = new Headers({ + "Content-Type": "application/json", + "x-api-key": "key_test", + }); + return new NextRequest("http://localhost/api/sessions", { + method: "POST", + headers, + body: typeof body === "string" ? body : JSON.stringify(body), + }); +} + +/** + * A minimal valid `sessions` row that tests can override per case. + */ +export function baseSessionRow(overrides: Partial = {}): SessionRow { + return { + id: "sess_1", + account_id: "acc-uuid-1", + title: "Test session", + status: "running", + repo_owner: null, + repo_name: null, + branch: null, + clone_url: null, + is_new_branch: false, + global_skill_refs: [], + sandbox_state: { type: "vercel" }, + lifecycle_state: "provisioning", + lifecycle_version: 0, + last_activity_at: null, + sandbox_expires_at: null, + hibernate_after: null, + lifecycle_run_id: null, + lifecycle_error: null, + lines_added: 0, + lines_removed: 0, + snapshot_url: null, + snapshot_created_at: null, + snapshot_size_bytes: null, + cached_diff: null, + cached_diff_updated_at: null, + created_at: "2026-05-04T00:00:00.000Z", + updated_at: "2026-05-04T00:00:00.000Z", + ...overrides, + }; +} + +/** + * A minimal valid `chats` row that tests can override per case. + */ +export function baseChatRow(overrides: Partial = {}): ChatRow { + return { + id: "chat_1", + session_id: "sess_1", + title: "New chat", + model_id: null, + active_stream_id: null, + last_assistant_message_at: null, + created_at: "2026-05-04T00:00:00.000Z", + updated_at: "2026-05-04T00:00:00.000Z", + ...overrides, + }; +} + +export const okAuth = { + accountId: "acc-uuid-1", + orgId: null, + authToken: "key_test", +}; diff --git a/lib/sessions/buildSessionInsertRow.ts b/lib/sessions/buildSessionInsertRow.ts new file mode 100644 index 000000000..d6eef43e8 --- /dev/null +++ b/lib/sessions/buildSessionInsertRow.ts @@ -0,0 +1,36 @@ +import type { TablesInsert } from "@/types/database.types"; +import type { CreateSessionBody } from "@/lib/sessions/validateCreateSessionBody"; +import { generateUUID } from "@/lib/uuid/generateUUID"; + +const DEFAULT_TITLE = "New session"; + +interface BuildSessionInsertRowInput { + body: CreateSessionBody; + accountId: string; +} + +/** + * Normalizes a validated `POST /api/sessions` body into a `sessions` + * insert row. Centralizes the trim / default / null-coalescing rules + * so the handler can stay focused on HTTP and persistence flow. + * + * @param input - The validated body and the authenticated account id. + * @returns A row ready to pass to `insertSession`. + */ +export function buildSessionInsertRow(input: BuildSessionInsertRowInput): TablesInsert<"sessions"> { + const { body, accountId } = input; + return { + id: generateUUID(), + account_id: accountId, + title: body.title?.trim() || DEFAULT_TITLE, + status: "running", + repo_owner: body.repoOwner ?? null, + repo_name: body.repoName ?? null, + branch: body.branch ?? null, + clone_url: body.cloneUrl ?? null, + global_skill_refs: [], + sandbox_state: { type: body.sandboxType ?? "vercel" }, + lifecycle_state: "provisioning", + lifecycle_version: 0, + }; +} diff --git a/lib/sessions/createSessionHandler.ts b/lib/sessions/createSessionHandler.ts index adbd1ca60..2cc56e79e 100644 --- a/lib/sessions/createSessionHandler.ts +++ b/lib/sessions/createSessionHandler.ts @@ -4,13 +4,22 @@ import { safeParseJson } from "@/lib/networking/safeParseJson"; import { validateAuthContext } from "@/lib/auth/validateAuthContext"; import { generateUUID } from "@/lib/uuid/generateUUID"; import { validateCreateSessionBody } from "@/lib/sessions/validateCreateSessionBody"; -import { generateSessionBranchName } from "@/lib/sessions/generateSessionBranchName"; +import { buildSessionInsertRow } from "@/lib/sessions/buildSessionInsertRow"; import { insertSession } from "@/lib/supabase/sessions/insertSession"; import { deleteSessionById } from "@/lib/supabase/sessions/deleteSessionById"; import { insertChat } from "@/lib/supabase/chats/insertChat"; import { toSessionResponse } from "@/lib/sessions/toSessionResponse"; import { toChatResponse } from "@/lib/sessions/toChatResponse"; +const INITIAL_CHAT_TITLE = "New chat"; + +function failedToCreateSession(): NextResponse { + return NextResponse.json( + { status: "error", error: "Failed to create session" }, + { status: 500, headers: getCorsHeaders() }, + ); +} + /** * Handles `POST /api/sessions`. * @@ -34,44 +43,23 @@ export async function createSessionHandler(request: NextRequest): Promise` — the `ag` prefix marks - * agent-authored branches and the suffix is random enough to avoid - * collisions across concurrent sessions. - */ -export function generateSessionBranchName(): string { - const suffix = crypto.randomUUID().replace(/-/g, "").slice(0, 8); - return `ag/${suffix}`; -} diff --git a/lib/sessions/validateCreateSessionBody.ts b/lib/sessions/validateCreateSessionBody.ts index ec767f4bf..93b0e430e 100644 --- a/lib/sessions/validateCreateSessionBody.ts +++ b/lib/sessions/validateCreateSessionBody.ts @@ -16,7 +16,6 @@ export const createSessionBodySchema = z.object({ .optional(), branch: z.string().optional(), cloneUrl: z.string().optional(), - isNewBranch: z.boolean().optional(), sandboxType: z.literal("vercel", { message: "Invalid sandbox type" }).optional(), }); From 23f469db82ee7fc2d6497ea7081396e74c8ae115 Mon Sep 17 00:00:00 2001 From: Sweets Sweetman Date: Mon, 4 May 2026 17:38:11 -0500 Subject: [PATCH 07/12] fix(sessions): address second round of PR review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 500 message: "Failed to create session" → "Internal server error" (per cubic.dev standardized 500 envelope feedback) - SRP: extract failedToCreateSession to lib/sessions/failedToCreateSession.ts - YAGNI: drop repoOwner from request body and remove isValidGitHubRepoOwner helper entirely (recoupable is the only owner; no need to validate) - YAGNI: drop repoName from request body and remove isValidGitHubRepoName helper (repo identity is derived server-side from the authenticated account, not accepted from user input) - Single-export per file: split createSessionHandlerFixtures.ts into makeCreateSessionReq.ts, baseSessionRow.ts, baseChatRow.ts. okAuth constant inlined where used. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../__tests__/isValidGitHubRepoName.test.ts | 34 -------- .../__tests__/isValidGitHubRepoOwner.test.ts | 33 -------- lib/github/isValidGitHubRepoName.ts | 18 ----- lib/github/isValidGitHubRepoOwner.ts | 17 ---- lib/sessions/__tests__/baseChatRow.ts | 19 +++++ lib/sessions/__tests__/baseSessionRow.ts | 39 +++++++++ .../__tests__/buildSessionInsertRow.test.ts | 12 +-- .../createSessionHandler.persistence.test.ts | 11 ++- .../__tests__/createSessionHandler.test.ts | 19 +---- .../__tests__/createSessionHandlerFixtures.ts | 81 ------------------- .../__tests__/makeCreateSessionReq.ts | 19 +++++ lib/sessions/buildSessionInsertRow.ts | 2 - lib/sessions/createSessionHandler.ts | 8 +- lib/sessions/failedToCreateSession.ts | 15 ++++ lib/sessions/validateCreateSessionBody.ts | 14 +--- 15 files changed, 108 insertions(+), 233 deletions(-) delete mode 100644 lib/github/__tests__/isValidGitHubRepoName.test.ts delete mode 100644 lib/github/__tests__/isValidGitHubRepoOwner.test.ts delete mode 100644 lib/github/isValidGitHubRepoName.ts delete mode 100644 lib/github/isValidGitHubRepoOwner.ts create mode 100644 lib/sessions/__tests__/baseChatRow.ts create mode 100644 lib/sessions/__tests__/baseSessionRow.ts delete mode 100644 lib/sessions/__tests__/createSessionHandlerFixtures.ts create mode 100644 lib/sessions/__tests__/makeCreateSessionReq.ts create mode 100644 lib/sessions/failedToCreateSession.ts diff --git a/lib/github/__tests__/isValidGitHubRepoName.test.ts b/lib/github/__tests__/isValidGitHubRepoName.test.ts deleted file mode 100644 index a2fadfede..000000000 --- a/lib/github/__tests__/isValidGitHubRepoName.test.ts +++ /dev/null @@ -1,34 +0,0 @@ -import { describe, it, expect } from "vitest"; -import { isValidGitHubRepoName } from "@/lib/github/isValidGitHubRepoName"; - -describe("isValidGitHubRepoName", () => { - it("accepts standard repo names", () => { - expect(isValidGitHubRepoName("api")).toBe(true); - expect(isValidGitHubRepoName("hello-world")).toBe(true); - expect(isValidGitHubRepoName("snake_case")).toBe(true); - expect(isValidGitHubRepoName("dot.in.middle")).toBe(true); - expect(isValidGitHubRepoName("v1.2.3")).toBe(true); - }); - - it("rejects empty and too-long names", () => { - expect(isValidGitHubRepoName("")).toBe(false); - expect(isValidGitHubRepoName("a".repeat(101))).toBe(false); - }); - - it("rejects reserved dot-only names", () => { - expect(isValidGitHubRepoName(".")).toBe(false); - expect(isValidGitHubRepoName("..")).toBe(false); - }); - - it("rejects names ending in .git", () => { - expect(isValidGitHubRepoName("repo.git")).toBe(false); - expect(isValidGitHubRepoName("Repo.GIT")).toBe(false); - expect(isValidGitHubRepoName(".git")).toBe(false); - }); - - it("rejects characters outside [\\w.-]", () => { - expect(isValidGitHubRepoName("with space")).toBe(false); - expect(isValidGitHubRepoName("repo@bad")).toBe(false); - expect(isValidGitHubRepoName("slash/repo")).toBe(false); - }); -}); diff --git a/lib/github/__tests__/isValidGitHubRepoOwner.test.ts b/lib/github/__tests__/isValidGitHubRepoOwner.test.ts deleted file mode 100644 index 26067ba2c..000000000 --- a/lib/github/__tests__/isValidGitHubRepoOwner.test.ts +++ /dev/null @@ -1,33 +0,0 @@ -import { describe, it, expect } from "vitest"; -import { isValidGitHubRepoOwner } from "@/lib/github/isValidGitHubRepoOwner"; - -describe("isValidGitHubRepoOwner", () => { - it("accepts standard alphanumeric owners", () => { - expect(isValidGitHubRepoOwner("vercel")).toBe(true); - expect(isValidGitHubRepoOwner("Recoupable")).toBe(true); - expect(isValidGitHubRepoOwner("octo-cat-42")).toBe(true); - expect(isValidGitHubRepoOwner("a")).toBe(true); - }); - - it("rejects empty and too-long owners", () => { - expect(isValidGitHubRepoOwner("")).toBe(false); - expect(isValidGitHubRepoOwner("a".repeat(40))).toBe(false); - }); - - it("rejects underscores and dots (allowed in repo names but not owner logins)", () => { - expect(isValidGitHubRepoOwner("snake_case")).toBe(false); - expect(isValidGitHubRepoOwner("dot.case")).toBe(false); - }); - - it("rejects leading, trailing, and consecutive hyphens", () => { - expect(isValidGitHubRepoOwner("-leading")).toBe(false); - expect(isValidGitHubRepoOwner("trailing-")).toBe(false); - expect(isValidGitHubRepoOwner("double--hyphen")).toBe(false); - }); - - it("rejects characters outside [a-zA-Z0-9-]", () => { - expect(isValidGitHubRepoOwner("bad@owner")).toBe(false); - expect(isValidGitHubRepoOwner("with space")).toBe(false); - expect(isValidGitHubRepoOwner("slash/owner")).toBe(false); - }); -}); diff --git a/lib/github/isValidGitHubRepoName.ts b/lib/github/isValidGitHubRepoName.ts deleted file mode 100644 index 2457fd5c5..000000000 --- a/lib/github/isValidGitHubRepoName.ts +++ /dev/null @@ -1,18 +0,0 @@ -const MAX_REPO_NAME_LENGTH = 100; - -/** - * Returns true if `repoName` is a valid GitHub repository name. - * - * GitHub's actual rules: 1–100 characters, alphanumerics plus `.`, - * `-`, and `_`. The reserved names `.` and `..` are not allowed, - * and a name cannot end with `.git`. - * - * @param repoName - Candidate repo name to validate. - */ -export function isValidGitHubRepoName(repoName: string): boolean { - if (repoName.length === 0 || repoName.length > MAX_REPO_NAME_LENGTH) return false; - if (repoName === "." || repoName === "..") return false; - if (!/^[\w.-]+$/.test(repoName)) return false; - if (repoName.toLowerCase().endsWith(".git")) return false; - return true; -} diff --git a/lib/github/isValidGitHubRepoOwner.ts b/lib/github/isValidGitHubRepoOwner.ts deleted file mode 100644 index 4d4648fe5..000000000 --- a/lib/github/isValidGitHubRepoOwner.ts +++ /dev/null @@ -1,17 +0,0 @@ -const MAX_OWNER_LENGTH = 39; - -/** - * Returns true if `owner` is a valid GitHub user / org login. - * - * GitHub's actual rules: 1–39 characters, alphanumerics and hyphens - * only, cannot start or end with a hyphen, no consecutive hyphens. - * - * @param owner - Candidate owner segment to validate. - */ -export function isValidGitHubRepoOwner(owner: string): boolean { - if (owner.length === 0 || owner.length > MAX_OWNER_LENGTH) return false; - if (!/^[a-zA-Z0-9-]+$/.test(owner)) return false; - if (owner.startsWith("-") || owner.endsWith("-")) return false; - if (owner.includes("--")) return false; - return true; -} diff --git a/lib/sessions/__tests__/baseChatRow.ts b/lib/sessions/__tests__/baseChatRow.ts new file mode 100644 index 000000000..4c295ed49 --- /dev/null +++ b/lib/sessions/__tests__/baseChatRow.ts @@ -0,0 +1,19 @@ +import type { Tables } from "@/types/database.types"; + +/** + * Returns a fully-populated `chats` row suitable for mocking + * `insertChat` in tests. Pass `overrides` to customize fields per case. + */ +export function baseChatRow(overrides: Partial> = {}): Tables<"chats"> { + return { + id: "chat_1", + session_id: "sess_1", + title: "New chat", + model_id: null, + active_stream_id: null, + last_assistant_message_at: null, + created_at: "2026-05-04T00:00:00.000Z", + updated_at: "2026-05-04T00:00:00.000Z", + ...overrides, + }; +} diff --git a/lib/sessions/__tests__/baseSessionRow.ts b/lib/sessions/__tests__/baseSessionRow.ts new file mode 100644 index 000000000..8c5fe8095 --- /dev/null +++ b/lib/sessions/__tests__/baseSessionRow.ts @@ -0,0 +1,39 @@ +import type { Tables } from "@/types/database.types"; + +/** + * Returns a fully-populated `sessions` row suitable for mocking + * `insertSession` / `selectSession` in tests. Pass `overrides` to + * customize fields per case. + */ +export function baseSessionRow(overrides: Partial> = {}): Tables<"sessions"> { + return { + id: "sess_1", + account_id: "acc-uuid-1", + title: "Test session", + status: "running", + repo_owner: null, + repo_name: null, + branch: null, + clone_url: null, + is_new_branch: false, + global_skill_refs: [], + sandbox_state: { type: "vercel" }, + lifecycle_state: "provisioning", + lifecycle_version: 0, + last_activity_at: null, + sandbox_expires_at: null, + hibernate_after: null, + lifecycle_run_id: null, + lifecycle_error: null, + lines_added: 0, + lines_removed: 0, + snapshot_url: null, + snapshot_created_at: null, + snapshot_size_bytes: null, + cached_diff: null, + cached_diff_updated_at: null, + created_at: "2026-05-04T00:00:00.000Z", + updated_at: "2026-05-04T00:00:00.000Z", + ...overrides, + }; +} diff --git a/lib/sessions/__tests__/buildSessionInsertRow.test.ts b/lib/sessions/__tests__/buildSessionInsertRow.test.ts index bd02d37ad..b1f4bec94 100644 --- a/lib/sessions/__tests__/buildSessionInsertRow.test.ts +++ b/lib/sessions/__tests__/buildSessionInsertRow.test.ts @@ -10,8 +10,6 @@ describe("buildSessionInsertRow", () => { expect(row.lifecycle_state).toBe("provisioning"); expect(row.lifecycle_version).toBe(0); expect(row.sandbox_state).toEqual({ type: "vercel" }); - expect(row.repo_owner).toBeNull(); - expect(row.repo_name).toBeNull(); expect(row.branch).toBeNull(); expect(row.clone_url).toBeNull(); expect(row.id).toMatch(/^[0-9a-f-]{36}$/i); @@ -27,19 +25,15 @@ describe("buildSessionInsertRow", () => { expect(row.title).toBe("New session"); }); - it("forwards repo + branch + clone fields verbatim", () => { + it("forwards branch + clone fields verbatim", () => { const row = buildSessionInsertRow({ body: { - repoOwner: "vercel", - repoName: "ai", branch: "main", - cloneUrl: "https://github.com/vercel/ai.git", + cloneUrl: "https://github.com/recoupable/ai.git", }, accountId: "acc-1", }); - expect(row.repo_owner).toBe("vercel"); - expect(row.repo_name).toBe("ai"); expect(row.branch).toBe("main"); - expect(row.clone_url).toBe("https://github.com/vercel/ai.git"); + expect(row.clone_url).toBe("https://github.com/recoupable/ai.git"); }); }); diff --git a/lib/sessions/__tests__/createSessionHandler.persistence.test.ts b/lib/sessions/__tests__/createSessionHandler.persistence.test.ts index 7cef69e95..b3803f4db 100644 --- a/lib/sessions/__tests__/createSessionHandler.persistence.test.ts +++ b/lib/sessions/__tests__/createSessionHandler.persistence.test.ts @@ -5,12 +5,9 @@ import { insertSession } from "@/lib/supabase/sessions/insertSession"; import { deleteSessionById } from "@/lib/supabase/sessions/deleteSessionById"; import { insertChat } from "@/lib/supabase/chats/insertChat"; import { createSessionHandler } from "@/lib/sessions/createSessionHandler"; -import { - baseChatRow, - baseSessionRow, - makeCreateSessionReq, - okAuth, -} from "@/lib/sessions/__tests__/createSessionHandlerFixtures"; +import { baseSessionRow } from "@/lib/sessions/__tests__/baseSessionRow"; +import { baseChatRow } from "@/lib/sessions/__tests__/baseChatRow"; +import { makeCreateSessionReq } from "@/lib/sessions/__tests__/makeCreateSessionReq"; vi.mock("@/lib/networking/getCorsHeaders", () => ({ getCorsHeaders: () => ({ "Access-Control-Allow-Origin": "*" }), @@ -20,6 +17,8 @@ vi.mock("@/lib/supabase/sessions/insertSession", () => ({ insertSession: vi.fn() vi.mock("@/lib/supabase/sessions/deleteSessionById", () => ({ deleteSessionById: vi.fn() })); vi.mock("@/lib/supabase/chats/insertChat", () => ({ insertChat: vi.fn() })); +const okAuth = { accountId: "acc-uuid-1", orgId: null, authToken: "key_test" }; + describe("createSessionHandler — persistence", () => { beforeEach(() => vi.clearAllMocks()); diff --git a/lib/sessions/__tests__/createSessionHandler.test.ts b/lib/sessions/__tests__/createSessionHandler.test.ts index 4304038bf..1ae903937 100644 --- a/lib/sessions/__tests__/createSessionHandler.test.ts +++ b/lib/sessions/__tests__/createSessionHandler.test.ts @@ -4,10 +4,7 @@ import { NextResponse } from "next/server"; import { validateAuthContext } from "@/lib/auth/validateAuthContext"; import { insertSession } from "@/lib/supabase/sessions/insertSession"; import { createSessionHandler } from "@/lib/sessions/createSessionHandler"; -import { - makeCreateSessionReq, - okAuth, -} from "@/lib/sessions/__tests__/createSessionHandlerFixtures"; +import { makeCreateSessionReq } from "@/lib/sessions/__tests__/makeCreateSessionReq"; vi.mock("@/lib/networking/getCorsHeaders", () => ({ getCorsHeaders: () => ({ "Access-Control-Allow-Origin": "*" }), @@ -17,6 +14,8 @@ vi.mock("@/lib/supabase/sessions/insertSession", () => ({ insertSession: vi.fn() vi.mock("@/lib/supabase/sessions/deleteSessionById", () => ({ deleteSessionById: vi.fn() })); vi.mock("@/lib/supabase/chats/insertChat", () => ({ insertChat: vi.fn() })); +const okAuth = { accountId: "acc-uuid-1", orgId: null, authToken: "key_test" }; + describe("createSessionHandler — auth & validation", () => { beforeEach(() => vi.clearAllMocks()); @@ -35,16 +34,4 @@ describe("createSessionHandler — auth & validation", () => { expect(res.status).toBe(400); expect(insertSession).not.toHaveBeenCalled(); }); - - it("returns 400 when repoOwner has invalid github format", async () => { - vi.mocked(validateAuthContext).mockResolvedValue(okAuth); - const res = await createSessionHandler(makeCreateSessionReq({ repoOwner: "bad@@@owner" })); - expect(res.status).toBe(400); - }); - - it("returns 400 when repoName has invalid github format", async () => { - vi.mocked(validateAuthContext).mockResolvedValue(okAuth); - const res = await createSessionHandler(makeCreateSessionReq({ repoName: "spaces in name" })); - expect(res.status).toBe(400); - }); }); diff --git a/lib/sessions/__tests__/createSessionHandlerFixtures.ts b/lib/sessions/__tests__/createSessionHandlerFixtures.ts deleted file mode 100644 index 001c99f14..000000000 --- a/lib/sessions/__tests__/createSessionHandlerFixtures.ts +++ /dev/null @@ -1,81 +0,0 @@ -import { NextRequest } from "next/server"; -import type { Tables } from "@/types/database.types"; - -export type SessionRow = Tables<"sessions">; -export type ChatRow = Tables<"chats">; - -/** - * Builds a NextRequest pointing at `/api/sessions` with a JSON body. - * - * @param body - The body to send (object literal or raw string). - */ -export function makeCreateSessionReq(body: unknown): NextRequest { - const headers = new Headers({ - "Content-Type": "application/json", - "x-api-key": "key_test", - }); - return new NextRequest("http://localhost/api/sessions", { - method: "POST", - headers, - body: typeof body === "string" ? body : JSON.stringify(body), - }); -} - -/** - * A minimal valid `sessions` row that tests can override per case. - */ -export function baseSessionRow(overrides: Partial = {}): SessionRow { - return { - id: "sess_1", - account_id: "acc-uuid-1", - title: "Test session", - status: "running", - repo_owner: null, - repo_name: null, - branch: null, - clone_url: null, - is_new_branch: false, - global_skill_refs: [], - sandbox_state: { type: "vercel" }, - lifecycle_state: "provisioning", - lifecycle_version: 0, - last_activity_at: null, - sandbox_expires_at: null, - hibernate_after: null, - lifecycle_run_id: null, - lifecycle_error: null, - lines_added: 0, - lines_removed: 0, - snapshot_url: null, - snapshot_created_at: null, - snapshot_size_bytes: null, - cached_diff: null, - cached_diff_updated_at: null, - created_at: "2026-05-04T00:00:00.000Z", - updated_at: "2026-05-04T00:00:00.000Z", - ...overrides, - }; -} - -/** - * A minimal valid `chats` row that tests can override per case. - */ -export function baseChatRow(overrides: Partial = {}): ChatRow { - return { - id: "chat_1", - session_id: "sess_1", - title: "New chat", - model_id: null, - active_stream_id: null, - last_assistant_message_at: null, - created_at: "2026-05-04T00:00:00.000Z", - updated_at: "2026-05-04T00:00:00.000Z", - ...overrides, - }; -} - -export const okAuth = { - accountId: "acc-uuid-1", - orgId: null, - authToken: "key_test", -}; diff --git a/lib/sessions/__tests__/makeCreateSessionReq.ts b/lib/sessions/__tests__/makeCreateSessionReq.ts new file mode 100644 index 000000000..e7b368f68 --- /dev/null +++ b/lib/sessions/__tests__/makeCreateSessionReq.ts @@ -0,0 +1,19 @@ +import { NextRequest } from "next/server"; + +/** + * Builds a NextRequest pointing at `/api/sessions` with a JSON body + * and the standard test API key already attached. + * + * @param body - The body to send (object literal or raw string). + */ +export function makeCreateSessionReq(body: unknown): NextRequest { + const headers = new Headers({ + "Content-Type": "application/json", + "x-api-key": "key_test", + }); + return new NextRequest("http://localhost/api/sessions", { + method: "POST", + headers, + body: typeof body === "string" ? body : JSON.stringify(body), + }); +} diff --git a/lib/sessions/buildSessionInsertRow.ts b/lib/sessions/buildSessionInsertRow.ts index d6eef43e8..ebfb82c27 100644 --- a/lib/sessions/buildSessionInsertRow.ts +++ b/lib/sessions/buildSessionInsertRow.ts @@ -24,8 +24,6 @@ export function buildSessionInsertRow(input: BuildSessionInsertRowInput): Tables account_id: accountId, title: body.title?.trim() || DEFAULT_TITLE, status: "running", - repo_owner: body.repoOwner ?? null, - repo_name: body.repoName ?? null, branch: body.branch ?? null, clone_url: body.cloneUrl ?? null, global_skill_refs: [], diff --git a/lib/sessions/createSessionHandler.ts b/lib/sessions/createSessionHandler.ts index 2cc56e79e..c1b68f3e7 100644 --- a/lib/sessions/createSessionHandler.ts +++ b/lib/sessions/createSessionHandler.ts @@ -5,6 +5,7 @@ import { validateAuthContext } from "@/lib/auth/validateAuthContext"; import { generateUUID } from "@/lib/uuid/generateUUID"; import { validateCreateSessionBody } from "@/lib/sessions/validateCreateSessionBody"; import { buildSessionInsertRow } from "@/lib/sessions/buildSessionInsertRow"; +import { failedToCreateSession } from "@/lib/sessions/failedToCreateSession"; import { insertSession } from "@/lib/supabase/sessions/insertSession"; import { deleteSessionById } from "@/lib/supabase/sessions/deleteSessionById"; import { insertChat } from "@/lib/supabase/chats/insertChat"; @@ -13,13 +14,6 @@ import { toChatResponse } from "@/lib/sessions/toChatResponse"; const INITIAL_CHAT_TITLE = "New chat"; -function failedToCreateSession(): NextResponse { - return NextResponse.json( - { status: "error", error: "Failed to create session" }, - { status: 500, headers: getCorsHeaders() }, - ); -} - /** * Handles `POST /api/sessions`. * diff --git a/lib/sessions/failedToCreateSession.ts b/lib/sessions/failedToCreateSession.ts new file mode 100644 index 000000000..a9e010c04 --- /dev/null +++ b/lib/sessions/failedToCreateSession.ts @@ -0,0 +1,15 @@ +import { NextResponse } from "next/server"; +import { getCorsHeaders } from "@/lib/networking/getCorsHeaders"; + +/** + * 500 response shared by every internal failure on `POST /api/sessions`. + * Returns the standard `{status, error}` envelope with the generic + * `"Internal server error"` message — specific failure modes are logged + * server-side rather than leaked to clients. + */ +export function failedToCreateSession(): NextResponse { + return NextResponse.json( + { status: "error", error: "Internal server error" }, + { status: 500, headers: getCorsHeaders() }, + ); +} diff --git a/lib/sessions/validateCreateSessionBody.ts b/lib/sessions/validateCreateSessionBody.ts index 93b0e430e..e6958eb7d 100644 --- a/lib/sessions/validateCreateSessionBody.ts +++ b/lib/sessions/validateCreateSessionBody.ts @@ -1,19 +1,9 @@ import { NextResponse } from "next/server"; import { z } from "zod"; import { getCorsHeaders } from "@/lib/networking/getCorsHeaders"; -import { isValidGitHubRepoOwner } from "@/lib/github/isValidGitHubRepoOwner"; -import { isValidGitHubRepoName } from "@/lib/github/isValidGitHubRepoName"; export const createSessionBodySchema = z.object({ title: z.string().optional(), - repoOwner: z - .string() - .refine(isValidGitHubRepoOwner, { message: "Invalid repository owner" }) - .optional(), - repoName: z - .string() - .refine(isValidGitHubRepoName, { message: "Invalid repository name" }) - .optional(), branch: z.string().optional(), cloneUrl: z.string().optional(), sandboxType: z.literal("vercel", { message: "Invalid sandbox type" }).optional(), @@ -24,6 +14,10 @@ export type CreateSessionBody = z.infer; /** * Validates the request body for `POST /api/sessions`. * + * Repo identity (owner + name) is derived server-side from the + * authenticated account, not accepted from the body, so this schema + * stays minimal. + * * @param body - The parsed JSON body (or `{}` for an empty body). * @returns The validated body, or a 400 NextResponse describing the first failure. */ From 8073e3094f4c18475657ffe7378f47b99bec8400 Mon Sep 17 00:00:00 2001 From: Sweets Sweetman Date: Mon, 4 May 2026 17:56:23 -0500 Subject: [PATCH 08/12] feat(sessions): port random-city title fallback from open-agents MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Generated session titles now match the open-agents UX — names like "Anchorage", "Vienna", "Philadelphia" — instead of every untitled session being called "New session". Closes a wire-shape gap with open-agents production identified by the head-to-head test on PR. Pieces: - lib/sessions/cityNames.ts: ~200-city curated list (verbatim port) - lib/sessions/getRandomCityName.ts: pick a city not in `usedNames`, numeric-suffix fallback when the curated list is exhausted - lib/supabase/sessions/selectSessionTitlesByAccountId.ts: Supabase helper for collision avoidance - lib/sessions/resolveSessionTitle.ts: orchestrates provided title (trimmed) > random city fallback. Async. Kept separate from the insert-row builder so that stays synchronous + pure. - buildSessionInsertRow now takes `title` as a parameter - createSessionHandler awaits resolveSessionTitle before building the row TDD: 4 tests for getRandomCityName, 4 for resolveSessionTitle. Handler tests updated to mock resolveSessionTitle. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../__tests__/buildSessionInsertRow.test.ts | 24 +-- .../createSessionHandler.persistence.test.ts | 12 +- .../__tests__/createSessionHandler.test.ts | 3 + .../__tests__/getRandomCityName.test.ts | 39 ++++ .../__tests__/resolveSessionTitle.test.ts | 47 +++++ lib/sessions/buildSessionInsertRow.ts | 19 +- lib/sessions/cityNames.ts | 198 ++++++++++++++++++ lib/sessions/createSessionHandler.ts | 17 +- lib/sessions/getRandomCityName.ts | 26 +++ lib/sessions/resolveSessionTitle.ts | 29 +++ .../selectSessionTitlesByAccountId.ts | 23 ++ 11 files changed, 411 insertions(+), 26 deletions(-) create mode 100644 lib/sessions/__tests__/getRandomCityName.test.ts create mode 100644 lib/sessions/__tests__/resolveSessionTitle.test.ts create mode 100644 lib/sessions/cityNames.ts create mode 100644 lib/sessions/getRandomCityName.ts create mode 100644 lib/sessions/resolveSessionTitle.ts create mode 100644 lib/supabase/sessions/selectSessionTitlesByAccountId.ts diff --git a/lib/sessions/__tests__/buildSessionInsertRow.test.ts b/lib/sessions/__tests__/buildSessionInsertRow.test.ts index b1f4bec94..a786871f5 100644 --- a/lib/sessions/__tests__/buildSessionInsertRow.test.ts +++ b/lib/sessions/__tests__/buildSessionInsertRow.test.ts @@ -3,9 +3,9 @@ import { buildSessionInsertRow } from "@/lib/sessions/buildSessionInsertRow"; describe("buildSessionInsertRow", () => { it("returns sane defaults for an empty body", () => { - const row = buildSessionInsertRow({ body: {}, accountId: "acc-1" }); + const row = buildSessionInsertRow({ body: {}, accountId: "acc-1", title: "Berlin" }); expect(row.account_id).toBe("acc-1"); - expect(row.title).toBe("New session"); + expect(row.title).toBe("Berlin"); expect(row.status).toBe("running"); expect(row.lifecycle_state).toBe("provisioning"); expect(row.lifecycle_version).toBe(0); @@ -15,16 +15,6 @@ describe("buildSessionInsertRow", () => { expect(row.id).toMatch(/^[0-9a-f-]{36}$/i); }); - it("trims and forwards a provided title", () => { - const row = buildSessionInsertRow({ body: { title: " Hello " }, accountId: "acc-1" }); - expect(row.title).toBe("Hello"); - }); - - it("falls back to default when title is whitespace-only", () => { - const row = buildSessionInsertRow({ body: { title: " " }, accountId: "acc-1" }); - expect(row.title).toBe("New session"); - }); - it("forwards branch + clone fields verbatim", () => { const row = buildSessionInsertRow({ body: { @@ -32,8 +22,18 @@ describe("buildSessionInsertRow", () => { cloneUrl: "https://github.com/recoupable/ai.git", }, accountId: "acc-1", + title: "Berlin", }); expect(row.branch).toBe("main"); expect(row.clone_url).toBe("https://github.com/recoupable/ai.git"); }); + + it("uses the provided sandboxType when set", () => { + const row = buildSessionInsertRow({ + body: { sandboxType: "vercel" }, + accountId: "acc-1", + title: "Berlin", + }); + expect(row.sandbox_state).toEqual({ type: "vercel" }); + }); }); diff --git a/lib/sessions/__tests__/createSessionHandler.persistence.test.ts b/lib/sessions/__tests__/createSessionHandler.persistence.test.ts index b3803f4db..dcc2f9ae5 100644 --- a/lib/sessions/__tests__/createSessionHandler.persistence.test.ts +++ b/lib/sessions/__tests__/createSessionHandler.persistence.test.ts @@ -4,6 +4,7 @@ import { validateAuthContext } from "@/lib/auth/validateAuthContext"; import { insertSession } from "@/lib/supabase/sessions/insertSession"; import { deleteSessionById } from "@/lib/supabase/sessions/deleteSessionById"; import { insertChat } from "@/lib/supabase/chats/insertChat"; +import { resolveSessionTitle } from "@/lib/sessions/resolveSessionTitle"; import { createSessionHandler } from "@/lib/sessions/createSessionHandler"; import { baseSessionRow } from "@/lib/sessions/__tests__/baseSessionRow"; import { baseChatRow } from "@/lib/sessions/__tests__/baseChatRow"; @@ -16,6 +17,9 @@ vi.mock("@/lib/auth/validateAuthContext", () => ({ validateAuthContext: vi.fn() vi.mock("@/lib/supabase/sessions/insertSession", () => ({ insertSession: vi.fn() })); vi.mock("@/lib/supabase/sessions/deleteSessionById", () => ({ deleteSessionById: vi.fn() })); vi.mock("@/lib/supabase/chats/insertChat", () => ({ insertChat: vi.fn() })); +vi.mock("@/lib/sessions/resolveSessionTitle", () => ({ + resolveSessionTitle: vi.fn(async () => "Anchorage"), +})); const okAuth = { accountId: "acc-uuid-1", orgId: null, authToken: "key_test" }; @@ -45,12 +49,18 @@ describe("createSessionHandler — persistence", () => { expect(chatArgs.title).toBe("New chat"); }); - it("uses provided title when present", async () => { + it("forwards body title to resolveSessionTitle and writes the resolved title", async () => { vi.mocked(validateAuthContext).mockResolvedValue(okAuth); + vi.mocked(resolveSessionTitle).mockResolvedValueOnce("Hello world"); vi.mocked(insertSession).mockResolvedValue(baseSessionRow({ title: "Hello world" })); vi.mocked(insertChat).mockResolvedValue(baseChatRow()); await createSessionHandler(makeCreateSessionReq({ title: "Hello world" })); + + expect(resolveSessionTitle).toHaveBeenCalledWith({ + providedTitle: "Hello world", + accountId: "acc-uuid-1", + }); expect(vi.mocked(insertSession).mock.calls[0][0].title).toBe("Hello world"); }); diff --git a/lib/sessions/__tests__/createSessionHandler.test.ts b/lib/sessions/__tests__/createSessionHandler.test.ts index 1ae903937..4c24400fe 100644 --- a/lib/sessions/__tests__/createSessionHandler.test.ts +++ b/lib/sessions/__tests__/createSessionHandler.test.ts @@ -13,6 +13,9 @@ vi.mock("@/lib/auth/validateAuthContext", () => ({ validateAuthContext: vi.fn() vi.mock("@/lib/supabase/sessions/insertSession", () => ({ insertSession: vi.fn() })); vi.mock("@/lib/supabase/sessions/deleteSessionById", () => ({ deleteSessionById: vi.fn() })); vi.mock("@/lib/supabase/chats/insertChat", () => ({ insertChat: vi.fn() })); +vi.mock("@/lib/sessions/resolveSessionTitle", () => ({ + resolveSessionTitle: vi.fn(async () => "Anchorage"), +})); const okAuth = { accountId: "acc-uuid-1", orgId: null, authToken: "key_test" }; diff --git a/lib/sessions/__tests__/getRandomCityName.test.ts b/lib/sessions/__tests__/getRandomCityName.test.ts new file mode 100644 index 000000000..5925efc40 --- /dev/null +++ b/lib/sessions/__tests__/getRandomCityName.test.ts @@ -0,0 +1,39 @@ +import { describe, it, expect } from "vitest"; +import { getRandomCityName } from "@/lib/sessions/getRandomCityName"; +import { cityNames } from "@/lib/sessions/cityNames"; + +describe("getRandomCityName", () => { + it("returns a city from the curated list when none are used", () => { + const result = getRandomCityName(new Set()); + expect(cityNames).toContain(result); + }); + + it("avoids cities that are already in use", () => { + const used = new Set(cityNames.slice(0, cityNames.length - 1)); + const result = getRandomCityName(used); + expect(result).toBe(cityNames[cityNames.length - 1]); + }); + + it("appends a numeric suffix once every city is used", () => { + const used = new Set(cityNames); + const result = getRandomCityName(used); + expect(result).toMatch(/ \d+$/); + const base = result.replace(/ \d+$/, ""); + expect(cityNames).toContain(base); + }); + + it("increments the suffix when the next number is also used", () => { + const baseCity = cityNames[0]; + const used = new Set([...cityNames, `${baseCity} 2`, `${baseCity} 3`]); + let attempts = 0; + while (attempts < 50) { + const result = getRandomCityName(used); + const match = result.match(/^(.*) (\d+)$/); + if (match && match[1] === baseCity) { + expect(Number(match[2])).toBeGreaterThanOrEqual(4); + return; + } + attempts++; + } + }); +}); diff --git a/lib/sessions/__tests__/resolveSessionTitle.test.ts b/lib/sessions/__tests__/resolveSessionTitle.test.ts new file mode 100644 index 000000000..e24944f3b --- /dev/null +++ b/lib/sessions/__tests__/resolveSessionTitle.test.ts @@ -0,0 +1,47 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; + +import { selectSessionTitlesByAccountId } from "@/lib/supabase/sessions/selectSessionTitlesByAccountId"; +import { getRandomCityName } from "@/lib/sessions/getRandomCityName"; +import { resolveSessionTitle } from "@/lib/sessions/resolveSessionTitle"; + +vi.mock("@/lib/supabase/sessions/selectSessionTitlesByAccountId", () => ({ + selectSessionTitlesByAccountId: vi.fn(), +})); +vi.mock("@/lib/sessions/getRandomCityName", () => ({ + getRandomCityName: vi.fn(() => "Anchorage"), +})); + +describe("resolveSessionTitle", () => { + beforeEach(() => vi.clearAllMocks()); + + it("uses the provided title verbatim when present", async () => { + const result = await resolveSessionTitle({ providedTitle: "Hello", accountId: "acc-1" }); + expect(result).toBe("Hello"); + expect(selectSessionTitlesByAccountId).not.toHaveBeenCalled(); + expect(getRandomCityName).not.toHaveBeenCalled(); + }); + + it("trims whitespace around a provided title", async () => { + const result = await resolveSessionTitle({ providedTitle: " Hi ", accountId: "acc-1" }); + expect(result).toBe("Hi"); + }); + + it("falls back to getRandomCityName when no title is provided", async () => { + vi.mocked(selectSessionTitlesByAccountId).mockResolvedValue(["Berlin", "Paris"]); + + const result = await resolveSessionTitle({ accountId: "acc-1" }); + + expect(result).toBe("Anchorage"); + expect(selectSessionTitlesByAccountId).toHaveBeenCalledWith("acc-1"); + expect(vi.mocked(getRandomCityName).mock.calls[0][0]).toEqual(new Set(["Berlin", "Paris"])); + }); + + it("falls back to getRandomCityName when title is whitespace-only", async () => { + vi.mocked(selectSessionTitlesByAccountId).mockResolvedValue([]); + + const result = await resolveSessionTitle({ providedTitle: " ", accountId: "acc-1" }); + + expect(result).toBe("Anchorage"); + expect(selectSessionTitlesByAccountId).toHaveBeenCalledWith("acc-1"); + }); +}); diff --git a/lib/sessions/buildSessionInsertRow.ts b/lib/sessions/buildSessionInsertRow.ts index ebfb82c27..8c718f57f 100644 --- a/lib/sessions/buildSessionInsertRow.ts +++ b/lib/sessions/buildSessionInsertRow.ts @@ -2,27 +2,30 @@ import type { TablesInsert } from "@/types/database.types"; import type { CreateSessionBody } from "@/lib/sessions/validateCreateSessionBody"; import { generateUUID } from "@/lib/uuid/generateUUID"; -const DEFAULT_TITLE = "New session"; - interface BuildSessionInsertRowInput { body: CreateSessionBody; accountId: string; + title: string; } /** - * Normalizes a validated `POST /api/sessions` body into a `sessions` - * insert row. Centralizes the trim / default / null-coalescing rules - * so the handler can stay focused on HTTP and persistence flow. + * Normalizes a validated `POST /api/sessions` body plus a resolved + * title into a `sessions` insert row. Centralizes the default / + * null-coalescing rules so the handler can stay focused on HTTP and + * persistence flow. + * + * Title resolution is intentionally not done here — that lives in + * `resolveSessionTitle` so this function stays synchronous and pure. * - * @param input - The validated body and the authenticated account id. + * @param input - The validated body, owning account id, and resolved title. * @returns A row ready to pass to `insertSession`. */ export function buildSessionInsertRow(input: BuildSessionInsertRowInput): TablesInsert<"sessions"> { - const { body, accountId } = input; + const { body, accountId, title } = input; return { id: generateUUID(), account_id: accountId, - title: body.title?.trim() || DEFAULT_TITLE, + title, status: "running", branch: body.branch ?? null, clone_url: body.cloneUrl ?? null, diff --git a/lib/sessions/cityNames.ts b/lib/sessions/cityNames.ts new file mode 100644 index 000000000..92cd18a17 --- /dev/null +++ b/lib/sessions/cityNames.ts @@ -0,0 +1,198 @@ +/** + * Curated list of ~200 notable cities used as memorable, unique + * fallback titles for sessions that don't have a user-provided title. + * Ported verbatim from open-agents so generated titles look familiar + * to the existing frontend. + */ +export const cityNames: readonly string[] = [ + // Africa + "Abidjan", + "Accra", + "Addis Ababa", + "Algiers", + "Antananarivo", + "Cairo", + "Cape Town", + "Casablanca", + "Dakar", + "Dar es Salaam", + "Johannesburg", + "Kampala", + "Kigali", + "Kinshasa", + "Lagos", + "Luanda", + "Lusaka", + "Maputo", + "Marrakech", + "Nairobi", + "Tunis", + "Windhoek", + + // Asia + "Almaty", + "Amman", + "Baku", + "Bangkok", + "Beijing", + "Beirut", + "Bengaluru", + "Bishkek", + "Colombo", + "Dhaka", + "Dubai", + "Hanoi", + "Ho Chi Minh City", + "Hong Kong", + "Hyderabad", + "Islamabad", + "Istanbul", + "Jakarta", + "Jeddah", + "Jerusalem", + "Kabul", + "Karachi", + "Kathmandu", + "Kolkata", + "Kuala Lumpur", + "Kyoto", + "Manila", + "Mumbai", + "Muscat", + "New Delhi", + "Osaka", + "Phnom Penh", + "Riyadh", + "Seoul", + "Shanghai", + "Shenzhen", + "Singapore", + "Taipei", + "Tashkent", + "Tbilisi", + "Tehran", + "Tel Aviv", + "Thimphu", + "Tokyo", + "Ulaanbaatar", + "Vientiane", + "Yangon", + "Yerevan", + + // Europe + "Amsterdam", + "Athens", + "Barcelona", + "Belgrade", + "Berlin", + "Bern", + "Bordeaux", + "Bratislava", + "Brussels", + "Bucharest", + "Budapest", + "Copenhagen", + "Dublin", + "Edinburgh", + "Florence", + "Geneva", + "Hamburg", + "Helsinki", + "Kraków", + "Kyiv", + "Lisbon", + "Ljubljana", + "London", + "Lyon", + "Madrid", + "Marseille", + "Milan", + "Minsk", + "Monaco", + "Moscow", + "Munich", + "Naples", + "Oslo", + "Paris", + "Porto", + "Prague", + "Reykjavik", + "Riga", + "Rome", + "Sarajevo", + "Seville", + "Sofia", + "Stockholm", + "Tallinn", + "Valencia", + "Venice", + "Vienna", + "Vilnius", + "Warsaw", + "Zagreb", + "Zurich", + + // North America + "Anchorage", + "Atlanta", + "Austin", + "Boston", + "Calgary", + "Chicago", + "Denver", + "Detroit", + "Guadalajara", + "Havana", + "Honolulu", + "Houston", + "Kingston", + "Los Angeles", + "Mexico City", + "Miami", + "Minneapolis", + "Montreal", + "Nashville", + "New Orleans", + "New York", + "Ottawa", + "Philadelphia", + "Phoenix", + "Portland", + "San Diego", + "San Francisco", + "San Juan", + "Seattle", + "Toronto", + "Vancouver", + "Washington", + + // South America + "Bogotá", + "Brasília", + "Buenos Aires", + "Caracas", + "Cartagena", + "Cusco", + "Guayaquil", + "La Paz", + "Lima", + "Medellín", + "Montevideo", + "Quito", + "Recife", + "Rio de Janeiro", + "Santiago", + "São Paulo", + "Valparaíso", + + // Oceania + "Adelaide", + "Auckland", + "Brisbane", + "Christchurch", + "Fiji", + "Melbourne", + "Perth", + "Sydney", + "Wellington", +]; diff --git a/lib/sessions/createSessionHandler.ts b/lib/sessions/createSessionHandler.ts index c1b68f3e7..488c42cb8 100644 --- a/lib/sessions/createSessionHandler.ts +++ b/lib/sessions/createSessionHandler.ts @@ -4,6 +4,7 @@ import { safeParseJson } from "@/lib/networking/safeParseJson"; import { validateAuthContext } from "@/lib/auth/validateAuthContext"; import { generateUUID } from "@/lib/uuid/generateUUID"; import { validateCreateSessionBody } from "@/lib/sessions/validateCreateSessionBody"; +import { resolveSessionTitle } from "@/lib/sessions/resolveSessionTitle"; import { buildSessionInsertRow } from "@/lib/sessions/buildSessionInsertRow"; import { failedToCreateSession } from "@/lib/sessions/failedToCreateSession"; import { insertSession } from "@/lib/supabase/sessions/insertSession"; @@ -17,10 +18,11 @@ const INITIAL_CHAT_TITLE = "New chat"; /** * Handles `POST /api/sessions`. * - * Authenticates the caller, validates the optional request body, then - * creates a session row and an initial chat row. If the chat insert - * fails after the session row is persisted, the session is rolled - * back so callers never observe an orphaned session. + * Authenticates the caller, validates the optional request body, + * resolves a final session title (provided > random city fallback), + * then creates a session row and an initial chat row. If the chat + * insert fails after the session row is persisted, the session is + * rolled back so callers never observe an orphaned session. * * @param request - The incoming request. * @returns A NextResponse with `{ session, chat }` on 200, or an error. @@ -37,8 +39,13 @@ export async function createSessionHandler(request: NextRequest): Promise): string { + const available = cityNames.filter(city => !usedNames.has(city)); + if (available.length > 0) { + return available[Math.floor(Math.random() * available.length)]!; + } + + const base = cityNames[Math.floor(Math.random() * cityNames.length)]!; + let suffix = 2; + while (usedNames.has(`${base} ${suffix}`)) { + suffix++; + } + return `${base} ${suffix}`; +} diff --git a/lib/sessions/resolveSessionTitle.ts b/lib/sessions/resolveSessionTitle.ts new file mode 100644 index 000000000..2c728eeea --- /dev/null +++ b/lib/sessions/resolveSessionTitle.ts @@ -0,0 +1,29 @@ +import { selectSessionTitlesByAccountId } from "@/lib/supabase/sessions/selectSessionTitlesByAccountId"; +import { getRandomCityName } from "@/lib/sessions/getRandomCityName"; + +interface ResolveSessionTitleInput { + providedTitle?: string; + accountId: string; +} + +/** + * Resolves the final title for a new session. + * + * If the caller provided a non-blank title, returns it trimmed. + * Otherwise queries the account's existing session titles and picks + * a random city name that doesn't collide with them — mirroring the + * open-agents fallback so generated titles look familiar after + * frontend cutover. + * + * @param input - Provided title (optional) and the owning account id. + * @returns The resolved title. + */ +export async function resolveSessionTitle(input: ResolveSessionTitleInput): Promise { + const trimmed = input.providedTitle?.trim(); + if (trimmed) { + return trimmed; + } + + const usedTitles = await selectSessionTitlesByAccountId(input.accountId); + return getRandomCityName(new Set(usedTitles)); +} diff --git a/lib/supabase/sessions/selectSessionTitlesByAccountId.ts b/lib/supabase/sessions/selectSessionTitlesByAccountId.ts new file mode 100644 index 000000000..53acef94e --- /dev/null +++ b/lib/supabase/sessions/selectSessionTitlesByAccountId.ts @@ -0,0 +1,23 @@ +import supabase from "@/lib/supabase/serverClient"; + +/** + * Returns the titles of every session belonging to the given account. + * Used by `resolveSessionTitle` to avoid collisions when generating + * fallback session titles. + * + * @param accountId - The account whose session titles to fetch. + * @returns The list of titles, or an empty array if the query failed. + */ +export async function selectSessionTitlesByAccountId(accountId: string): Promise { + const { data, error } = await supabase + .from("sessions") + .select("title") + .eq("account_id", accountId); + + if (error || !data) { + if (error) console.error("Error fetching session titles:", error); + return []; + } + + return data.map(row => row.title); +} From 4adb35d1765bbe1ff20db89c1d9ff848f60b0202 Mon Sep 17 00:00:00 2001 From: Sweets Sweetman Date: Mon, 4 May 2026 17:59:09 -0500 Subject: [PATCH 09/12] chore: remove GET-only files (scope this PR to POST) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The GET endpoint + handler + tests live in PR #514 and were inadvertently brought in when this branch was rebased after #514's work. This PR is scoped to POST only; GET ships in #514. Shared infrastructure stays (types/database.types.ts regen + lib/sessions/toSessionResponse.ts) — both are required by the POST handler too. When either #514 or this PR merges to test first, the other will see those files already present and resolve cleanly. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../[sessionId]/__tests__/route.test.ts | 163 ------------------ app/api/sessions/[sessionId]/route.ts | 42 ----- lib/sessions/getSessionByIdHandler.ts | 50 ------ 3 files changed, 255 deletions(-) delete mode 100644 app/api/sessions/[sessionId]/__tests__/route.test.ts delete mode 100644 app/api/sessions/[sessionId]/route.ts delete mode 100644 lib/sessions/getSessionByIdHandler.ts diff --git a/app/api/sessions/[sessionId]/__tests__/route.test.ts b/app/api/sessions/[sessionId]/__tests__/route.test.ts deleted file mode 100644 index 83c8f879a..000000000 --- a/app/api/sessions/[sessionId]/__tests__/route.test.ts +++ /dev/null @@ -1,163 +0,0 @@ -import { describe, it, expect, vi, beforeEach } from "vitest"; -import { NextRequest, NextResponse } from "next/server"; -import { GET, OPTIONS } from "../route"; -import type { Tables } from "@/types/database.types"; - -type SessionRow = Tables<"sessions">; - -vi.mock("@/lib/supabase/sessions/selectSession", () => ({ - selectSession: vi.fn(), -})); - -vi.mock("@/lib/auth/validateAuthContext", () => ({ - validateAuthContext: vi.fn(), -})); - -vi.mock("@/lib/networking/getCorsHeaders", () => ({ - getCorsHeaders: vi.fn(() => ({ "Access-Control-Allow-Origin": "*" })), -})); - -const { selectSession } = await import("@/lib/supabase/sessions/selectSession"); -const { validateAuthContext } = await import("@/lib/auth/validateAuthContext"); - -function makeReq(url = "https://example.com/api/sessions/sess_1"): NextRequest { - return new NextRequest(url); -} - -const mockRow: SessionRow = { - id: "sess_1", - account_id: "acc-uuid-1", - title: "Test session", - status: "running", - repo_owner: "acme", - repo_name: "demo", - branch: "main", - clone_url: "https://github.com/acme/demo.git", - is_new_branch: false, - global_skill_refs: [], - sandbox_state: { type: "vercel" }, - lifecycle_state: "active", - lifecycle_version: 1, - last_activity_at: "2026-05-04T00:00:00.000Z", - sandbox_expires_at: null, - hibernate_after: null, - lifecycle_run_id: null, - lifecycle_error: null, - lines_added: 12, - lines_removed: 3, - snapshot_url: null, - snapshot_created_at: null, - snapshot_size_bytes: null, - cached_diff: null, - cached_diff_updated_at: null, - created_at: "2026-05-01T00:00:00.000Z", - updated_at: "2026-05-04T00:00:00.000Z", -}; - -describe("OPTIONS /api/sessions/[sessionId]", () => { - it("returns 200 with CORS headers", async () => { - const res = await OPTIONS(); - expect(res.status).toBe(200); - }); -}); - -describe("GET /api/sessions/[sessionId]", () => { - beforeEach(() => { - vi.clearAllMocks(); - }); - - it("returns 401 when auth fails", async () => { - vi.mocked(validateAuthContext).mockResolvedValue( - NextResponse.json({ error: "Unauthorized" }, { status: 401 }), - ); - - const res = await GET(makeReq(), { - params: Promise.resolve({ sessionId: "sess_1" }), - }); - expect(res.status).toBe(401); - expect(selectSession).not.toHaveBeenCalled(); - }); - - it("returns 404 when session does not exist", async () => { - vi.mocked(validateAuthContext).mockResolvedValue({ - accountId: "acc-uuid-1", - orgId: null, - authToken: "tok", - }); - vi.mocked(selectSession).mockResolvedValue(null); - - const res = await GET(makeReq(), { - params: Promise.resolve({ sessionId: "sess_missing" }), - }); - expect(res.status).toBe(404); - expect(await res.json()).toEqual({ - status: "error", - error: "Session not found", - }); - expect(selectSession).toHaveBeenCalledWith("sess_missing"); - }); - - it("returns 403 when session is owned by a different account", async () => { - vi.mocked(validateAuthContext).mockResolvedValue({ - accountId: "acc-uuid-OTHER", - orgId: null, - authToken: "tok", - }); - vi.mocked(selectSession).mockResolvedValue(mockRow); - - const res = await GET(makeReq(), { - params: Promise.resolve({ sessionId: "sess_1" }), - }); - expect(res.status).toBe(403); - expect(await res.json()).toEqual({ - status: "error", - error: "Forbidden", - }); - }); - - it("returns 200 with camelCase session shape on happy path", async () => { - vi.mocked(validateAuthContext).mockResolvedValue({ - accountId: "acc-uuid-1", - orgId: null, - authToken: "tok", - }); - vi.mocked(selectSession).mockResolvedValue(mockRow); - - const res = await GET(makeReq(), { - params: Promise.resolve({ sessionId: "sess_1" }), - }); - expect(res.status).toBe(200); - const body = await res.json(); - expect(body).toEqual({ - session: { - id: "sess_1", - userId: "acc-uuid-1", - title: "Test session", - status: "running", - repoOwner: "acme", - repoName: "demo", - branch: "main", - cloneUrl: "https://github.com/acme/demo.git", - isNewBranch: false, - globalSkillRefs: [], - sandboxState: { type: "vercel" }, - lifecycleState: "active", - lifecycleVersion: 1, - lastActivityAt: "2026-05-04T00:00:00.000Z", - sandboxExpiresAt: null, - hibernateAfter: null, - lifecycleRunId: null, - lifecycleError: null, - linesAdded: 12, - linesRemoved: 3, - snapshotUrl: null, - snapshotCreatedAt: null, - snapshotSizeBytes: null, - cachedDiff: null, - cachedDiffUpdatedAt: null, - createdAt: "2026-05-01T00:00:00.000Z", - updatedAt: "2026-05-04T00:00:00.000Z", - }, - }); - }); -}); diff --git a/app/api/sessions/[sessionId]/route.ts b/app/api/sessions/[sessionId]/route.ts deleted file mode 100644 index dbc04994a..000000000 --- a/app/api/sessions/[sessionId]/route.ts +++ /dev/null @@ -1,42 +0,0 @@ -import { NextRequest, NextResponse } from "next/server"; -import { getCorsHeaders } from "@/lib/networking/getCorsHeaders"; -import { getSessionByIdHandler } from "@/lib/sessions/getSessionByIdHandler"; - -/** - * OPTIONS handler for CORS preflight requests. - * - * @returns A NextResponse with CORS headers. - */ -export async function OPTIONS() { - return new NextResponse(null, { - status: 200, - headers: getCorsHeaders(), - }); -} - -/** - * GET /api/sessions/{sessionId} - * - * Reads a single agent session by id. Authenticates via Privy Bearer - * token or x-api-key header. Returns 404 if the session does not exist - * and 403 if it exists but is not owned by the authenticated account. - * - * Response shape mirrors open-agents' /api/sessions/[sessionId] so the - * existing frontend can cut over to api without code changes. - * - * @param request - The request object - * @param options - Route options containing the async params - * @param options.params - Route params containing the session id - * @returns A NextResponse with `{ session }` on 200, or an error. - */ -export async function GET( - request: NextRequest, - options: { params: Promise<{ sessionId: string }> }, -) { - const { sessionId } = await options.params; - return getSessionByIdHandler(request, sessionId); -} - -export const dynamic = "force-dynamic"; -export const fetchCache = "force-no-store"; -export const revalidate = 0; diff --git a/lib/sessions/getSessionByIdHandler.ts b/lib/sessions/getSessionByIdHandler.ts deleted file mode 100644 index 3b5a7db5f..000000000 --- a/lib/sessions/getSessionByIdHandler.ts +++ /dev/null @@ -1,50 +0,0 @@ -import { NextRequest, NextResponse } from "next/server"; -import { getCorsHeaders } from "@/lib/networking/getCorsHeaders"; -import { validateAuthContext } from "@/lib/auth/validateAuthContext"; -import { selectSession } from "@/lib/supabase/sessions/selectSession"; -import { toSessionResponse } from "@/lib/sessions/toSessionResponse"; - -/** - * Handles GET /api/sessions/{sessionId}. - * - * Reads a single agent session by id. Authenticates via Privy Bearer - * token or x-api-key header. Returns 404 if the session does not exist - * and 403 if it exists but is not owned by the authenticated account. - * - * Response shape mirrors open-agents' /api/sessions/[sessionId] so the - * existing frontend can cut over to api without code changes. - * - * @param request - The incoming request. - * @param sessionId - The id of the session to fetch. - * @returns A NextResponse with `{ session }` on 200, or an error. - */ -export async function getSessionByIdHandler( - request: NextRequest, - sessionId: string, -): Promise { - const auth = await validateAuthContext(request); - if (auth instanceof NextResponse) { - return auth; - } - - const row = await selectSession(sessionId); - - if (!row) { - return NextResponse.json( - { status: "error", error: "Session not found" }, - { status: 404, headers: getCorsHeaders() }, - ); - } - - if (row.account_id !== auth.accountId) { - return NextResponse.json( - { status: "error", error: "Forbidden" }, - { status: 403, headers: getCorsHeaders() }, - ); - } - - return NextResponse.json( - { session: toSessionResponse(row) }, - { status: 200, headers: getCorsHeaders() }, - ); -} From c57718347d0665b97d9a95c36c35f4142b7bc5fb Mon Sep 17 00:00:00 2001 From: Sweets Sweetman Date: Mon, 4 May 2026 18:06:49 -0500 Subject: [PATCH 10/12] fix(sessions): consolidate request validation + DRY supabase select MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two reviewer asks rolled into one commit: SRP — validateCreateSessionBody now owns the full validation flow. The handler used to call safeParseJson, validateAuthContext, and the Zod body schema separately; that was three places to short-circuit and three places to duplicate the error envelope. Folded them into validateCreateSessionBody so the handler does one call → success or NextResponse error. Returns { body, auth } on success. DRY — replaced lib/supabase/sessions/selectSession.ts and selectSessionTitlesByAccountId.ts with a single selectSessions({ id?, accountId? }) that supports both call sites. resolveSessionTitle now derives titles from the general fetch. Tests: - New validateCreateSessionBody.test.ts covers auth-failure / 400 / success / malformed-JSON tolerance (4 cases) - Handler tests now mock validateCreateSessionBody (single mock surface instead of three) - resolveSessionTitle tests mock selectSessions Co-Authored-By: Claude Opus 4.7 (1M context) --- .../createSessionHandler.persistence.test.ts | 25 +++++-- .../__tests__/createSessionHandler.test.ts | 27 ++++---- .../__tests__/resolveSessionTitle.test.ts | 20 +++--- .../validateCreateSessionBody.test.ts | 65 +++++++++++++++++++ lib/sessions/createSessionHandler.ts | 25 +++---- lib/sessions/resolveSessionTitle.ts | 5 +- lib/sessions/validateCreateSessionBody.ts | 37 ++++++++--- lib/supabase/sessions/selectSession.ts | 24 ------- .../selectSessionTitlesByAccountId.ts | 23 ------- lib/supabase/sessions/selectSessions.ts | 36 ++++++++++ 10 files changed, 185 insertions(+), 102 deletions(-) create mode 100644 lib/sessions/__tests__/validateCreateSessionBody.test.ts delete mode 100644 lib/supabase/sessions/selectSession.ts delete mode 100644 lib/supabase/sessions/selectSessionTitlesByAccountId.ts create mode 100644 lib/supabase/sessions/selectSessions.ts diff --git a/lib/sessions/__tests__/createSessionHandler.persistence.test.ts b/lib/sessions/__tests__/createSessionHandler.persistence.test.ts index dcc2f9ae5..279109a7f 100644 --- a/lib/sessions/__tests__/createSessionHandler.persistence.test.ts +++ b/lib/sessions/__tests__/createSessionHandler.persistence.test.ts @@ -1,6 +1,6 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; -import { validateAuthContext } from "@/lib/auth/validateAuthContext"; +import { validateCreateSessionBody } from "@/lib/sessions/validateCreateSessionBody"; import { insertSession } from "@/lib/supabase/sessions/insertSession"; import { deleteSessionById } from "@/lib/supabase/sessions/deleteSessionById"; import { insertChat } from "@/lib/supabase/chats/insertChat"; @@ -13,7 +13,9 @@ import { makeCreateSessionReq } from "@/lib/sessions/__tests__/makeCreateSession vi.mock("@/lib/networking/getCorsHeaders", () => ({ getCorsHeaders: () => ({ "Access-Control-Allow-Origin": "*" }), })); -vi.mock("@/lib/auth/validateAuthContext", () => ({ validateAuthContext: vi.fn() })); +vi.mock("@/lib/sessions/validateCreateSessionBody", () => ({ + validateCreateSessionBody: vi.fn(), +})); vi.mock("@/lib/supabase/sessions/insertSession", () => ({ insertSession: vi.fn() })); vi.mock("@/lib/supabase/sessions/deleteSessionById", () => ({ deleteSessionById: vi.fn() })); vi.mock("@/lib/supabase/chats/insertChat", () => ({ insertChat: vi.fn() })); @@ -21,13 +23,20 @@ vi.mock("@/lib/sessions/resolveSessionTitle", () => ({ resolveSessionTitle: vi.fn(async () => "Anchorage"), })); -const okAuth = { accountId: "acc-uuid-1", orgId: null, authToken: "key_test" }; +const okValidated = (overrides: { body?: object; accountId?: string } = {}) => ({ + body: overrides.body ?? {}, + auth: { + accountId: overrides.accountId ?? "acc-uuid-1", + orgId: null, + authToken: "key_test", + }, +}); describe("createSessionHandler — persistence", () => { beforeEach(() => vi.clearAllMocks()); it("creates session and chat with defaults on empty body", async () => { - vi.mocked(validateAuthContext).mockResolvedValue(okAuth); + vi.mocked(validateCreateSessionBody).mockResolvedValue(okValidated()); vi.mocked(insertSession).mockResolvedValue(baseSessionRow()); vi.mocked(insertChat).mockResolvedValue(baseChatRow()); @@ -50,7 +59,9 @@ describe("createSessionHandler — persistence", () => { }); it("forwards body title to resolveSessionTitle and writes the resolved title", async () => { - vi.mocked(validateAuthContext).mockResolvedValue(okAuth); + vi.mocked(validateCreateSessionBody).mockResolvedValue( + okValidated({ body: { title: "Hello world" } }), + ); vi.mocked(resolveSessionTitle).mockResolvedValueOnce("Hello world"); vi.mocked(insertSession).mockResolvedValue(baseSessionRow({ title: "Hello world" })); vi.mocked(insertChat).mockResolvedValue(baseChatRow()); @@ -65,7 +76,7 @@ describe("createSessionHandler — persistence", () => { }); it("returns 500 when insertSession fails", async () => { - vi.mocked(validateAuthContext).mockResolvedValue(okAuth); + vi.mocked(validateCreateSessionBody).mockResolvedValue(okValidated()); vi.mocked(insertSession).mockResolvedValue(null); const res = await createSessionHandler(makeCreateSessionReq({})); @@ -74,7 +85,7 @@ describe("createSessionHandler — persistence", () => { }); it("rolls back the session and returns 500 when insertChat fails", async () => { - vi.mocked(validateAuthContext).mockResolvedValue(okAuth); + vi.mocked(validateCreateSessionBody).mockResolvedValue(okValidated()); vi.mocked(insertSession).mockResolvedValue(baseSessionRow({ id: "sess_rollback" })); vi.mocked(insertChat).mockResolvedValue(null); diff --git a/lib/sessions/__tests__/createSessionHandler.test.ts b/lib/sessions/__tests__/createSessionHandler.test.ts index 4c24400fe..a239ccd82 100644 --- a/lib/sessions/__tests__/createSessionHandler.test.ts +++ b/lib/sessions/__tests__/createSessionHandler.test.ts @@ -1,7 +1,7 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; import { NextResponse } from "next/server"; -import { validateAuthContext } from "@/lib/auth/validateAuthContext"; +import { validateCreateSessionBody } from "@/lib/sessions/validateCreateSessionBody"; import { insertSession } from "@/lib/supabase/sessions/insertSession"; import { createSessionHandler } from "@/lib/sessions/createSessionHandler"; import { makeCreateSessionReq } from "@/lib/sessions/__tests__/makeCreateSessionReq"; @@ -9,7 +9,9 @@ import { makeCreateSessionReq } from "@/lib/sessions/__tests__/makeCreateSession vi.mock("@/lib/networking/getCorsHeaders", () => ({ getCorsHeaders: () => ({ "Access-Control-Allow-Origin": "*" }), })); -vi.mock("@/lib/auth/validateAuthContext", () => ({ validateAuthContext: vi.fn() })); +vi.mock("@/lib/sessions/validateCreateSessionBody", () => ({ + validateCreateSessionBody: vi.fn(), +})); vi.mock("@/lib/supabase/sessions/insertSession", () => ({ insertSession: vi.fn() })); vi.mock("@/lib/supabase/sessions/deleteSessionById", () => ({ deleteSessionById: vi.fn() })); vi.mock("@/lib/supabase/chats/insertChat", () => ({ insertChat: vi.fn() })); @@ -17,22 +19,23 @@ vi.mock("@/lib/sessions/resolveSessionTitle", () => ({ resolveSessionTitle: vi.fn(async () => "Anchorage"), })); -const okAuth = { accountId: "acc-uuid-1", orgId: null, authToken: "key_test" }; - -describe("createSessionHandler — auth & validation", () => { +describe("createSessionHandler — short-circuits on validation failure", () => { beforeEach(() => vi.clearAllMocks()); - it("returns 401 when validateAuthContext rejects", async () => { - vi.mocked(validateAuthContext).mockResolvedValue( - NextResponse.json({ status: "error", error: "no auth" }, { status: 401 }), - ); + it("returns the NextResponse from validateCreateSessionBody as-is", async () => { + const failure = NextResponse.json({ status: "error", error: "bad" }, { status: 401 }); + vi.mocked(validateCreateSessionBody).mockResolvedValue(failure); + const res = await createSessionHandler(makeCreateSessionReq({})); - expect(res.status).toBe(401); + expect(res).toBe(failure); expect(insertSession).not.toHaveBeenCalled(); }); - it("returns 400 when sandboxType is not 'vercel'", async () => { - vi.mocked(validateAuthContext).mockResolvedValue(okAuth); + it("returns 400 when validateCreateSessionBody rejects with 400", async () => { + vi.mocked(validateCreateSessionBody).mockResolvedValue( + NextResponse.json({ status: "error", error: "Invalid sandbox type" }, { status: 400 }), + ); + const res = await createSessionHandler(makeCreateSessionReq({ sandboxType: "wrong" })); expect(res.status).toBe(400); expect(insertSession).not.toHaveBeenCalled(); diff --git a/lib/sessions/__tests__/resolveSessionTitle.test.ts b/lib/sessions/__tests__/resolveSessionTitle.test.ts index e24944f3b..07615db28 100644 --- a/lib/sessions/__tests__/resolveSessionTitle.test.ts +++ b/lib/sessions/__tests__/resolveSessionTitle.test.ts @@ -1,11 +1,12 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; -import { selectSessionTitlesByAccountId } from "@/lib/supabase/sessions/selectSessionTitlesByAccountId"; +import { selectSessions } from "@/lib/supabase/sessions/selectSessions"; import { getRandomCityName } from "@/lib/sessions/getRandomCityName"; import { resolveSessionTitle } from "@/lib/sessions/resolveSessionTitle"; +import { baseSessionRow } from "@/lib/sessions/__tests__/baseSessionRow"; -vi.mock("@/lib/supabase/sessions/selectSessionTitlesByAccountId", () => ({ - selectSessionTitlesByAccountId: vi.fn(), +vi.mock("@/lib/supabase/sessions/selectSessions", () => ({ + selectSessions: vi.fn(), })); vi.mock("@/lib/sessions/getRandomCityName", () => ({ getRandomCityName: vi.fn(() => "Anchorage"), @@ -17,7 +18,7 @@ describe("resolveSessionTitle", () => { it("uses the provided title verbatim when present", async () => { const result = await resolveSessionTitle({ providedTitle: "Hello", accountId: "acc-1" }); expect(result).toBe("Hello"); - expect(selectSessionTitlesByAccountId).not.toHaveBeenCalled(); + expect(selectSessions).not.toHaveBeenCalled(); expect(getRandomCityName).not.toHaveBeenCalled(); }); @@ -27,21 +28,24 @@ describe("resolveSessionTitle", () => { }); it("falls back to getRandomCityName when no title is provided", async () => { - vi.mocked(selectSessionTitlesByAccountId).mockResolvedValue(["Berlin", "Paris"]); + vi.mocked(selectSessions).mockResolvedValue([ + baseSessionRow({ title: "Berlin" }), + baseSessionRow({ id: "sess_2", title: "Paris" }), + ]); const result = await resolveSessionTitle({ accountId: "acc-1" }); expect(result).toBe("Anchorage"); - expect(selectSessionTitlesByAccountId).toHaveBeenCalledWith("acc-1"); + expect(selectSessions).toHaveBeenCalledWith({ accountId: "acc-1" }); expect(vi.mocked(getRandomCityName).mock.calls[0][0]).toEqual(new Set(["Berlin", "Paris"])); }); it("falls back to getRandomCityName when title is whitespace-only", async () => { - vi.mocked(selectSessionTitlesByAccountId).mockResolvedValue([]); + vi.mocked(selectSessions).mockResolvedValue([]); const result = await resolveSessionTitle({ providedTitle: " ", accountId: "acc-1" }); expect(result).toBe("Anchorage"); - expect(selectSessionTitlesByAccountId).toHaveBeenCalledWith("acc-1"); + expect(selectSessions).toHaveBeenCalledWith({ accountId: "acc-1" }); }); }); diff --git a/lib/sessions/__tests__/validateCreateSessionBody.test.ts b/lib/sessions/__tests__/validateCreateSessionBody.test.ts new file mode 100644 index 000000000..e5e863b73 --- /dev/null +++ b/lib/sessions/__tests__/validateCreateSessionBody.test.ts @@ -0,0 +1,65 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { NextRequest, NextResponse } from "next/server"; + +import { validateCreateSessionBody } from "@/lib/sessions/validateCreateSessionBody"; +import { validateAuthContext } from "@/lib/auth/validateAuthContext"; + +vi.mock("@/lib/networking/getCorsHeaders", () => ({ + getCorsHeaders: () => ({ "Access-Control-Allow-Origin": "*" }), +})); +vi.mock("@/lib/auth/validateAuthContext", () => ({ validateAuthContext: vi.fn() })); + +const okAuth = { accountId: "acc-1", orgId: null, authToken: "key" }; + +function req(body: unknown): NextRequest { + return new NextRequest("http://localhost/api/sessions", { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: typeof body === "string" ? body : JSON.stringify(body), + }); +} + +describe("validateCreateSessionBody", () => { + beforeEach(() => vi.clearAllMocks()); + + it("returns the auth NextResponse when validateAuthContext rejects", async () => { + const failure = NextResponse.json({ status: "error", error: "no auth" }, { status: 401 }); + vi.mocked(validateAuthContext).mockResolvedValue(failure); + + const result = await validateCreateSessionBody(req({})); + expect(result).toBe(failure); + }); + + it("returns 400 when sandboxType is not 'vercel'", async () => { + vi.mocked(validateAuthContext).mockResolvedValue(okAuth); + + const result = await validateCreateSessionBody(req({ sandboxType: "wrong" })); + expect(result).toBeInstanceOf(NextResponse); + if (result instanceof NextResponse) { + expect(result.status).toBe(400); + const body = (await result.json()) as { status: string; error: string }; + expect(body.error).toBe("Invalid sandbox type"); + } + }); + + it("returns body + auth on success", async () => { + vi.mocked(validateAuthContext).mockResolvedValue(okAuth); + + const result = await validateCreateSessionBody(req({ title: "Hello", sandboxType: "vercel" })); + expect(result).not.toBeInstanceOf(NextResponse); + if (!(result instanceof NextResponse)) { + expect(result.body).toEqual({ title: "Hello", sandboxType: "vercel" }); + expect(result.auth).toBe(okAuth); + } + }); + + it("treats malformed JSON as an empty body and accepts it", async () => { + vi.mocked(validateAuthContext).mockResolvedValue(okAuth); + + const result = await validateCreateSessionBody(req("{not valid")); + expect(result).not.toBeInstanceOf(NextResponse); + if (!(result instanceof NextResponse)) { + expect(result.body).toEqual({}); + } + }); +}); diff --git a/lib/sessions/createSessionHandler.ts b/lib/sessions/createSessionHandler.ts index 488c42cb8..eb843b213 100644 --- a/lib/sessions/createSessionHandler.ts +++ b/lib/sessions/createSessionHandler.ts @@ -1,7 +1,5 @@ import { NextRequest, NextResponse } from "next/server"; import { getCorsHeaders } from "@/lib/networking/getCorsHeaders"; -import { safeParseJson } from "@/lib/networking/safeParseJson"; -import { validateAuthContext } from "@/lib/auth/validateAuthContext"; import { generateUUID } from "@/lib/uuid/generateUUID"; import { validateCreateSessionBody } from "@/lib/sessions/validateCreateSessionBody"; import { resolveSessionTitle } from "@/lib/sessions/resolveSessionTitle"; @@ -18,34 +16,29 @@ const INITIAL_CHAT_TITLE = "New chat"; /** * Handles `POST /api/sessions`. * - * Authenticates the caller, validates the optional request body, - * resolves a final session title (provided > random city fallback), - * then creates a session row and an initial chat row. If the chat - * insert fails after the session row is persisted, the session is - * rolled back so callers never observe an orphaned session. + * Authenticates, validates the request, resolves a final session + * title (provided > random city fallback), then creates a session + * row and an initial chat row. If the chat insert fails after the + * session row is persisted, the session is rolled back so callers + * never observe an orphaned session. * * @param request - The incoming request. * @returns A NextResponse with `{ session, chat }` on 200, or an error. */ export async function createSessionHandler(request: NextRequest): Promise { - const auth = await validateAuthContext(request); - if (auth instanceof NextResponse) { - return auth; - } - - const body = await safeParseJson(request); - const validated = validateCreateSessionBody(body); + const validated = await validateCreateSessionBody(request); if (validated instanceof NextResponse) { return validated; } + const { body, auth } = validated; const title = await resolveSessionTitle({ - providedTitle: validated.title, + providedTitle: body.title, accountId: auth.accountId, }); const sessionRow = await insertSession( - buildSessionInsertRow({ body: validated, accountId: auth.accountId, title }), + buildSessionInsertRow({ body, accountId: auth.accountId, title }), ); if (!sessionRow) { diff --git a/lib/sessions/resolveSessionTitle.ts b/lib/sessions/resolveSessionTitle.ts index 2c728eeea..1e2270b40 100644 --- a/lib/sessions/resolveSessionTitle.ts +++ b/lib/sessions/resolveSessionTitle.ts @@ -1,4 +1,4 @@ -import { selectSessionTitlesByAccountId } from "@/lib/supabase/sessions/selectSessionTitlesByAccountId"; +import { selectSessions } from "@/lib/supabase/sessions/selectSessions"; import { getRandomCityName } from "@/lib/sessions/getRandomCityName"; interface ResolveSessionTitleInput { @@ -24,6 +24,7 @@ export async function resolveSessionTitle(input: ResolveSessionTitleInput): Prom return trimmed; } - const usedTitles = await selectSessionTitlesByAccountId(input.accountId); + const rows = await selectSessions({ accountId: input.accountId }); + const usedTitles = rows.map(row => row.title); return getRandomCityName(new Set(usedTitles)); } diff --git a/lib/sessions/validateCreateSessionBody.ts b/lib/sessions/validateCreateSessionBody.ts index e6958eb7d..6bd3f17d8 100644 --- a/lib/sessions/validateCreateSessionBody.ts +++ b/lib/sessions/validateCreateSessionBody.ts @@ -1,6 +1,9 @@ -import { NextResponse } from "next/server"; +import { NextRequest, NextResponse } from "next/server"; import { z } from "zod"; import { getCorsHeaders } from "@/lib/networking/getCorsHeaders"; +import { safeParseJson } from "@/lib/networking/safeParseJson"; +import { validateAuthContext } from "@/lib/auth/validateAuthContext"; +import type { AuthContext } from "@/lib/auth/validateAuthContext"; export const createSessionBodySchema = z.object({ title: z.string().optional(), @@ -11,19 +14,33 @@ export const createSessionBodySchema = z.object({ export type CreateSessionBody = z.infer; +export interface ValidatedCreateSessionRequest { + body: CreateSessionBody; + auth: AuthContext; +} + /** - * Validates the request body for `POST /api/sessions`. + * Validates a `POST /api/sessions` request end-to-end: + * 1. Authenticates the caller via Privy Bearer / x-api-key + * 2. Parses the JSON body (treating malformed JSON as an empty body) + * 3. Validates the body against the Zod schema * - * Repo identity (owner + name) is derived server-side from the - * authenticated account, not accepted from the body, so this schema - * stays minimal. + * Returns either a 4xx NextResponse describing the first failure, or + * the validated `{ body, auth }` ready for the handler to consume. * - * @param body - The parsed JSON body (or `{}` for an empty body). - * @returns The validated body, or a 400 NextResponse describing the first failure. + * @param request - The incoming request. + * @returns A NextResponse on validation failure, or the validated body + auth. */ -export function validateCreateSessionBody(body: unknown): NextResponse | CreateSessionBody { - const result = createSessionBodySchema.safeParse(body); +export async function validateCreateSessionBody( + request: NextRequest, +): Promise { + const auth = await validateAuthContext(request); + if (auth instanceof NextResponse) { + return auth; + } + const rawBody = await safeParseJson(request); + const result = createSessionBodySchema.safeParse(rawBody); if (!result.success) { const firstError = result.error.issues[0]; return NextResponse.json( @@ -39,5 +56,5 @@ export function validateCreateSessionBody(body: unknown): NextResponse | CreateS ); } - return result.data; + return { body: result.data, auth }; } diff --git a/lib/supabase/sessions/selectSession.ts b/lib/supabase/sessions/selectSession.ts deleted file mode 100644 index 8f7bfaefd..000000000 --- a/lib/supabase/sessions/selectSession.ts +++ /dev/null @@ -1,24 +0,0 @@ -import supabase from "@/lib/supabase/serverClient"; -import type { Tables } from "@/types/database.types"; - -/** - * Select a single session row by its id, or null if not found. - * Caller is responsible for any ownership / access-control checks. - * - * @param sessionId - The id of the session to fetch. - * @returns The session row, or null when no row matches. - */ -export async function selectSession(sessionId: string): Promise | null> { - const { data, error } = await supabase - .from("sessions") - .select("*") - .eq("id", sessionId) - .maybeSingle(); - - if (error) { - console.error(`[selectSession] error for sessionId=${sessionId}:`, error); - return null; - } - - return data ?? null; -} diff --git a/lib/supabase/sessions/selectSessionTitlesByAccountId.ts b/lib/supabase/sessions/selectSessionTitlesByAccountId.ts deleted file mode 100644 index 53acef94e..000000000 --- a/lib/supabase/sessions/selectSessionTitlesByAccountId.ts +++ /dev/null @@ -1,23 +0,0 @@ -import supabase from "@/lib/supabase/serverClient"; - -/** - * Returns the titles of every session belonging to the given account. - * Used by `resolveSessionTitle` to avoid collisions when generating - * fallback session titles. - * - * @param accountId - The account whose session titles to fetch. - * @returns The list of titles, or an empty array if the query failed. - */ -export async function selectSessionTitlesByAccountId(accountId: string): Promise { - const { data, error } = await supabase - .from("sessions") - .select("title") - .eq("account_id", accountId); - - if (error || !data) { - if (error) console.error("Error fetching session titles:", error); - return []; - } - - return data.map(row => row.title); -} diff --git a/lib/supabase/sessions/selectSessions.ts b/lib/supabase/sessions/selectSessions.ts new file mode 100644 index 000000000..a180102ab --- /dev/null +++ b/lib/supabase/sessions/selectSessions.ts @@ -0,0 +1,36 @@ +import supabase from "@/lib/supabase/serverClient"; +import type { Tables } from "@/types/database.types"; + +interface SelectSessionsFilter { + /** Optional id filter — when set, returns at most one row. */ + id?: string; + /** Optional account filter — when set, returns every session owned by the account. */ + accountId?: string; +} + +/** + * General-purpose `sessions` reader. Pass any combination of filters + * to narrow the result set; an unset filter is ignored. Returns an + * empty array on Supabase error after logging. + * + * Callers project to whatever shape they need (single row by id, + * titles by account, etc.) — keeping this single function as the + * sole entry point keeps `lib/supabase/sessions/` DRY. + * + * @param filter - Optional filters narrowing the query. + * @returns Matching rows, or `[]` on error / no match. + */ +export async function selectSessions( + filter: SelectSessionsFilter = {}, +): Promise[]> { + let query = supabase.from("sessions").select("*"); + if (filter.id) query = query.eq("id", filter.id); + if (filter.accountId) query = query.eq("account_id", filter.accountId); + + const { data, error } = await query; + if (error) { + console.error("[selectSessions] error:", error); + return []; + } + return data ?? []; +} From c7dff7c5481f610faf6ac168a476db11b9926de7 Mon Sep 17 00:00:00 2001 From: Sweets Sweetman Date: Mon, 4 May 2026 18:16:20 -0500 Subject: [PATCH 11/12] fix(sessions): address automated review feedback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four small fixes from the latest round: 1. Zod v4 migration: { message } → { error } on the sandboxType literal. v4 unified the error customization API; { message } is deprecated. 2. Orphan rollback observability: when insertChat fails AND the session-rollback delete also fails, log the session id so ops can detect orphaned rows. New persistence test asserts the log. 3. Defensive try/catch in selectSessions so a thrown exception (network-level rejection, not a Supabase {error} return) doesn't bubble up and 500 the entire session-creation flow. 4. Deterministic test for getRandomCityName suffix-increment: pin Math.random instead of looping until the random pick lands on baseCity. Previous test could pass without ever asserting if the loop cap was hit. Skipped: cubic-dev-ai's note about logging raw sessionId in selectSession.ts — that file was deleted earlier in this PR. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../createSessionHandler.persistence.test.ts | 17 ++++++++++++++ .../__tests__/getRandomCityName.test.ts | 23 ++++++++++--------- lib/sessions/createSessionHandler.ts | 8 ++++++- lib/sessions/validateCreateSessionBody.ts | 2 +- lib/supabase/sessions/selectSessions.ts | 13 +++++++---- 5 files changed, 46 insertions(+), 17 deletions(-) diff --git a/lib/sessions/__tests__/createSessionHandler.persistence.test.ts b/lib/sessions/__tests__/createSessionHandler.persistence.test.ts index 279109a7f..dc52a2bdd 100644 --- a/lib/sessions/__tests__/createSessionHandler.persistence.test.ts +++ b/lib/sessions/__tests__/createSessionHandler.persistence.test.ts @@ -88,9 +88,26 @@ describe("createSessionHandler — persistence", () => { vi.mocked(validateCreateSessionBody).mockResolvedValue(okValidated()); vi.mocked(insertSession).mockResolvedValue(baseSessionRow({ id: "sess_rollback" })); vi.mocked(insertChat).mockResolvedValue(null); + vi.mocked(deleteSessionById).mockResolvedValue(true); const res = await createSessionHandler(makeCreateSessionReq({})); expect(res.status).toBe(500); expect(deleteSessionById).toHaveBeenCalledWith("sess_rollback"); }); + + it("logs an orphan-session error when rollback also fails", async () => { + vi.mocked(validateCreateSessionBody).mockResolvedValue(okValidated()); + vi.mocked(insertSession).mockResolvedValue(baseSessionRow({ id: "sess_orphan" })); + vi.mocked(insertChat).mockResolvedValue(null); + vi.mocked(deleteSessionById).mockResolvedValue(false); + const errSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + + const res = await createSessionHandler(makeCreateSessionReq({})); + expect(res.status).toBe(500); + expect(errSpy).toHaveBeenCalledWith( + expect.stringContaining("orphaned session"), + "sess_orphan", + ); + errSpy.mockRestore(); + }); }); diff --git a/lib/sessions/__tests__/getRandomCityName.test.ts b/lib/sessions/__tests__/getRandomCityName.test.ts index 5925efc40..a9d2e0754 100644 --- a/lib/sessions/__tests__/getRandomCityName.test.ts +++ b/lib/sessions/__tests__/getRandomCityName.test.ts @@ -1,7 +1,11 @@ -import { describe, it, expect } from "vitest"; +import { describe, it, expect, vi, afterEach } from "vitest"; import { getRandomCityName } from "@/lib/sessions/getRandomCityName"; import { cityNames } from "@/lib/sessions/cityNames"; +afterEach(() => { + vi.restoreAllMocks(); +}); + describe("getRandomCityName", () => { it("returns a city from the curated list when none are used", () => { const result = getRandomCityName(new Set()); @@ -25,15 +29,12 @@ describe("getRandomCityName", () => { it("increments the suffix when the next number is also used", () => { const baseCity = cityNames[0]; const used = new Set([...cityNames, `${baseCity} 2`, `${baseCity} 3`]); - let attempts = 0; - while (attempts < 50) { - const result = getRandomCityName(used); - const match = result.match(/^(.*) (\d+)$/); - if (match && match[1] === baseCity) { - expect(Number(match[2])).toBeGreaterThanOrEqual(4); - return; - } - attempts++; - } + // Pin Math.random so the fallback always picks index 0 (= baseCity), + // making the suffix-increment behavior deterministic to assert. + vi.spyOn(Math, "random").mockReturnValue(0); + + const result = getRandomCityName(used); + + expect(result).toBe(`${baseCity} 4`); }); }); diff --git a/lib/sessions/createSessionHandler.ts b/lib/sessions/createSessionHandler.ts index eb843b213..d27b0b9e1 100644 --- a/lib/sessions/createSessionHandler.ts +++ b/lib/sessions/createSessionHandler.ts @@ -52,7 +52,13 @@ export async function createSessionHandler(request: NextRequest): Promise; diff --git a/lib/supabase/sessions/selectSessions.ts b/lib/supabase/sessions/selectSessions.ts index a180102ab..69477b9cb 100644 --- a/lib/supabase/sessions/selectSessions.ts +++ b/lib/supabase/sessions/selectSessions.ts @@ -27,10 +27,15 @@ export async function selectSessions( if (filter.id) query = query.eq("id", filter.id); if (filter.accountId) query = query.eq("account_id", filter.accountId); - const { data, error } = await query; - if (error) { - console.error("[selectSessions] error:", error); + try { + const { data, error } = await query; + if (error) { + console.error("[selectSessions] error:", error); + return []; + } + return data ?? []; + } catch (e) { + console.error("[selectSessions] threw:", e); return []; } - return data ?? []; } From cb091b0869294bc3312b3bd2f4dae45fa9d51ff3 Mon Sep 17 00:00:00 2001 From: Sweets Sweetman Date: Mon, 4 May 2026 18:21:31 -0500 Subject: [PATCH 12/12] chore: prettier format fix on persistence test The new orphan-session test had a line that exceeded prettier's wrap width. Auto-format fixed it; format-check now clean. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../__tests__/createSessionHandler.persistence.test.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/lib/sessions/__tests__/createSessionHandler.persistence.test.ts b/lib/sessions/__tests__/createSessionHandler.persistence.test.ts index dc52a2bdd..038889a3d 100644 --- a/lib/sessions/__tests__/createSessionHandler.persistence.test.ts +++ b/lib/sessions/__tests__/createSessionHandler.persistence.test.ts @@ -104,10 +104,7 @@ describe("createSessionHandler — persistence", () => { const res = await createSessionHandler(makeCreateSessionReq({})); expect(res.status).toBe(500); - expect(errSpy).toHaveBeenCalledWith( - expect.stringContaining("orphaned session"), - "sess_orphan", - ); + expect(errSpy).toHaveBeenCalledWith(expect.stringContaining("orphaned session"), "sess_orphan"); errSpy.mockRestore(); }); });