feat: enhance pull request operations with validation and error handling

- Add branch existence validation
- Add duplicate PR check
- Add comprehensive error handling
- Improve type safety with zod transforms
- Add input sanitization

Author: txbm

src/github/operations/pulls.ts

@@ -1,16 +1,28 @@
 import { z } from "zod";
-import { githubRequest } from "../common/utils.js";
+import {
+  githubRequest,
+  validateBranchName,
+  validateOwnerName,
+  validateRepositoryName,
+  checkBranchExists,
+} from "../common/utils.js";
 import { 
   GitHubIssueAssigneeSchema, 
   GitHubRepositorySchema 
 } from "../common/types.js";
+import {
+  GitHubError,
+  GitHubValidationError,
+  GitHubResourceNotFoundError,
+  GitHubConflictError,
+} from "../common/errors.js";
 
 const GITHUB_TITLE_MAX_LENGTH = 256;
 const GITHUB_BODY_MAX_LENGTH = 65536;
 
 export const RepositoryParamsSchema = z.object({
-  owner: z.string().min(1),
-  repo: z.string().min(1),
+  owner: z.string().min(1).transform(validateOwnerName),
+  repo: z.string().min(1).transform(validateRepositoryName),
 });
 
 export const GitHubPullRequestStateSchema = z.enum([
@@ -34,7 +46,7 @@ export const GitHubDirectionSchema = z.enum([
 
 export const GitHubPullRequestRefSchema = z.object({
   label: z.string(),
-  ref: z.string().min(1),
+  ref: z.string().min(1).transform(validateBranchName),
   sha: z.string().length(40),
   user: GitHubIssueAssigneeSchema,
   repo: GitHubRepositorySchema,
@@ -73,8 +85,8 @@ export const GitHubPullRequestSchema = z.object({
 
 export const ListPullRequestsOptionsSchema = z.object({
   state: GitHubPullRequestStateSchema.optional(),
-  head: z.string().optional(),
-  base: z.string().optional(),
+  head: z.string().transform(validateBranchName).optional(),
+  base: z.string().transform(validateBranchName).optional(),
   sort: GitHubPullRequestSortSchema.optional(),
   direction: GitHubDirectionSchema.optional(),
   per_page: z.number().min(1).max(100).optional(),
@@ -84,8 +96,8 @@ export const ListPullRequestsOptionsSchema = z.object({
 export const CreatePullRequestOptionsSchema = z.object({
   title: z.string().max(GITHUB_TITLE_MAX_LENGTH),
   body: z.string().max(GITHUB_BODY_MAX_LENGTH).optional(),
-  head: z.string().min(1),
-  base: z.string().min(1),
+  head: z.string().min(1).transform(validateBranchName),
+  base: z.string().min(1).transform(validateBranchName),
   maintainer_can_modify: z.boolean().optional(),
   draft: z.boolean().optional(),
 });
@@ -100,20 +112,86 @@ export type ListPullRequestsOptions = z.infer<typeof ListPullRequestsOptionsSche
 export type GitHubPullRequest = z.infer<typeof GitHubPullRequestSchema>;
 export type GitHubPullRequestRef = z.infer<typeof GitHubPullRequestRefSchema>;
 
+async function validatePullRequestBranches(
+  owner: string,
+  repo: string,
+  head: string,
+  base: string
+): Promise<void> {
+  const [headExists, baseExists] = await Promise.all([
+    checkBranchExists(owner, repo, head),
+    checkBranchExists(owner, repo, base),
+  ]);
+
+  if (!headExists) {
+    throw new GitHubResourceNotFoundError(`Branch '${head}' not found`);
+  }
+
+  if (!baseExists) {
+    throw new GitHubResourceNotFoundError(`Branch '${base}' not found`);
+  }
+
+  if (head === base) {
+    throw new GitHubValidationError(
+      "Head and base branches cannot be the same",
+      422,
+      { message: "Head and base branches must be different" }
+    );
+  }
+}
+
+async function checkForExistingPullRequest(
+  owner: string,
+  repo: string,
+  head: string,
+  base: string
+): Promise<void> {
+  const existingPRs = await listPullRequests({
+    owner,
+    repo,
+    head,
+    base,
+    state: "open",
+  });
+
+  if (existingPRs.length > 0) {
+    throw new GitHubConflictError(
+      `A pull request already exists for ${head} into ${base}`
+    );
+  }
+}
+
 export async function createPullRequest(
   params: z.infer<typeof CreatePullRequestSchema>
 ): Promise<GitHubPullRequest> {
   const { owner, repo, ...options } = CreatePullRequestSchema.parse(params);
-  
-  const response = await githubRequest(
-    `https://api.github.com/repos/${owner}/${repo}/pulls`,
-    {
-      method: "POST",
-      body: options,
-    }
-  );
 
-  return GitHubPullRequestSchema.parse(response);
+  try {
+    await validatePullRequestBranches(owner, repo, options.head, options.base);
+    await checkForExistingPullRequest(owner, repo, options.head, options.base);
+
+    const response = await githubRequest(
+      `https://api.github.com/repos/${owner}/${repo}/pulls`,
+      {
+        method: "POST",
+        body: options,
+      }
+    );
+
+    return GitHubPullRequestSchema.parse(response);
+  } catch (error) {
+    if (error instanceof GitHubError) {
+      throw error;
+    }
+    if (error instanceof z.ZodError) {
+      throw new GitHubValidationError(
+        "Invalid pull request data",
+        422,
+        { errors: error.errors }
+      );
+    }
+    throw error;
+  }
 }
 
 export async function getPullRequest(
@@ -124,11 +202,25 @@ export async function getPullRequest(
     pullNumber: z.number().positive(),
   }).parse(params);
 
-  const response = await githubRequest(
-    `https://api.github.com/repos/${owner}/${repo}/pulls/${pullNumber}`
-  );
+  try {
+    const response = await githubRequest(
+      `https://api.github.com/repos/${owner}/${repo}/pulls/${pullNumber}`
+    );
 
-  return GitHubPullRequestSchema.parse(response);
+    return GitHubPullRequestSchema.parse(response);
+  } catch (error) {
+    if (error instanceof GitHubError) {
+      throw error;
+    }
+    if (error instanceof z.ZodError) {
+      throw new GitHubValidationError(
+        "Invalid pull request response data",
+        422,
+        { errors: error.errors }
+      );
+    }
+    throw error;
+  }
 }
 
 export async function listPullRequests(
@@ -139,14 +231,28 @@ export async function listPullRequests(
     ...ListPullRequestsOptionsSchema.partial().shape,
   }).parse(params);
 
-  const url = new URL(`https://api.github.com/repos/${owner}/${repo}/pulls`);
-  
-  Object.entries(options).forEach(([key, value]) => {
-    if (value !== undefined) {
-      url.searchParams.append(key, value.toString());
-    }
-  });
+  try {
+    const url = new URL(`https://api.github.com/repos/${owner}/${repo}/pulls`);
+    
+    Object.entries(options).forEach(([key, value]) => {
+      if (value !== undefined) {
+        url.searchParams.append(key, value.toString());
+      }
+    });
 
-  const response = await githubRequest(url.toString());
-  return z.array(GitHubPullRequestSchema).parse(response);
+    const response = await githubRequest(url.toString());
+    return z.array(GitHubPullRequestSchema).parse(response);
+  } catch (error) {
+    if (error instanceof GitHubError) {
+      throw error;
+    }
+    if (error instanceof z.ZodError) {
+      throw new GitHubValidationError(
+        "Invalid pull request list response data",
+        422,
+        { errors: error.errors }
+      );
+    }
+    throw error;
+  }
 }