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) <noreply@anthropic.com>

Author: sweetmantech

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);
+});
+});

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);
+});
+});

lib/github/isValidGitHubRepoName.ts

@@ -1,10 +1,18 @@
-constGITHUB_REPO_PATH_SEGMENT_PATTERN=/^[.\w-]+$/;
+constMAX_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.
  */
 exportfunctionisValidGitHubRepoName(repoName: string): boolean{
-returnGITHUB_REPO_PATH_SEGMENT_PATTERN.test(repoName);
+if(repoName.length===0||repoName.length>MAX_REPO_NAME_LENGTH)returnfalse;
+if(repoName==="."||repoName==="..")returnfalse;
+if(!/^[\w.-]+$/.test(repoName))returnfalse;
+if(repoName.toLowerCase().endsWith(".git"))returnfalse;
+returntrue;
 }

lib/github/isValidGitHubRepoOwner.ts

@@ -1,12 +1,17 @@
-constGITHUB_REPO_PATH_SEGMENT_PATTERN=/^[.\w-]+$/;
+constMAX_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.
  */
 exportfunctionisValidGitHubRepoOwner(owner: string): boolean{
-returnGITHUB_REPO_PATH_SEGMENT_PATTERN.test(owner);
+if(owner.length===0||owner.length>MAX_OWNER_LENGTH)returnfalse;
+if(!/^[a-zA-Z0-9-]+$/.test(owner))returnfalse;
+if(owner.startsWith("-")||owner.endsWith("-"))returnfalse;
+if(owner.includes("--"))returnfalse;
+returntrue;
 }

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",()=>{
+constrow=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",()=>{
+constrow=buildSessionInsertRow({body: {title: " Hello "},accountId: "acc-1"});
+expect(row.title).toBe("Hello");
+});
+
+it("falls back to default when title is whitespace-only",()=>{
+constrow=buildSessionInsertRow({body: {title: " "},accountId: "acc-1"});
+expect(row.title).toBe("New session");
+});
+
+it("forwards repo + branch + clone fields verbatim",()=>{
+constrow=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");
+});
+});

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());
+
+constres=awaitcreateSessionHandler(makeCreateSessionReq({}));
+expect(res.status).toBe(200);
+
+constbody=(awaitres.json())as{session: {userId: string};chat: {sessionId: string}};
+expect(body.session.userId).toBe("acc-uuid-1");
+expect(body.chat.sessionId).toBe("sess_1");
+
+constinsertArgs=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"});
+
+constchatArgs=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());
+
+awaitcreateSessionHandler(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);
+
+constres=awaitcreateSessionHandler(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);
+
+constres=awaitcreateSessionHandler(makeCreateSessionReq({}));
+expect(res.status).toBe(500);
+expect(deleteSessionById).toHaveBeenCalledWith("sess_rollback");
+});
+});