Explorar o código

feat: enhance tool call validation with pre-checks and better error messages #21

- Created new validation.ts module with helper functions for validating
  users, repos, branches, issues, labels, milestones, orgs, and teams
- Added formatGogsError() to provide context-aware error messages with
  HTTP status code handling and resource identification
- Added pre-validation to high-priority tools:
  * get_commits: validates branch exists when SHA provided
  * create_issue/update_issue: validates assignee, milestone, labels
  * add_collaborator: validates username exists
  * add_organization_member: validates org and username
  * add_team_member: validates team and username
  * add_issue_labels/replace_issue_labels: validates label IDs
- Improved error messages with actionable suggestions and better context
- All tools now return user-friendly errors instead of generic axios messages

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
claude hai 8 meses
pai
achega
8ea9776cd4
Modificáronse 2 ficheiros con 349 adicións e 12 borrados
  1. 87 12
      src/server.ts
  2. 262 0
      src/validation.ts

+ 87 - 12
src/server.ts

@@ -12,6 +12,18 @@ import {
 } from '@modelcontextprotocol/sdk/types.js';
 import { z } from 'zod';
 import { GogsClient } from './gogs-client.js';
+import {
+  formatGogsError,
+  validateUserExists,
+  validateRepoExists,
+  validateBranchExists,
+  validateIssueExists,
+  validateLabelExists,
+  validateLabelsExist,
+  validateMilestoneExists,
+  validateOrgExists,
+  validateTeamExists,
+} from './validation.js';
 
 /**
  * Helper function to create owner/repo schema fields that are optional when restricted
@@ -2617,6 +2629,12 @@ export function createMcpServer(gogsClient: GogsClient, restrictedOwner?: string
         case 'get_commits': {
           const parsed = GetCommitsSchema.parse(args);
           const { owner, repo } = resolveRepoParams(parsed.owner, parsed.repo, restrictedOwner, restrictedRepo);
+
+          // Validate branch exists if sha is provided
+          if (parsed.sha) {
+            await validateBranchExists(gogsClient, owner, repo, parsed.sha);
+          }
+
           const commits = await gogsClient.getCommits(owner, repo, { sha: parsed.sha, page: parsed.page });
           return {
             content: [
@@ -2673,6 +2691,22 @@ export function createMcpServer(gogsClient: GogsClient, restrictedOwner?: string
         case 'create_issue': {
           const parsed = CreateIssueSchema.parse(args);
           const { owner, repo } = resolveRepoParams(parsed.owner, parsed.repo, restrictedOwner, restrictedRepo);
+
+          // Validate assignee exists if provided
+          if (parsed.assignee) {
+            await validateUserExists(gogsClient, parsed.assignee, 'assignee');
+          }
+
+          // Validate milestone exists if provided
+          if (parsed.milestone) {
+            await validateMilestoneExists(gogsClient, owner, repo, parsed.milestone);
+          }
+
+          // Validate labels exist if provided
+          if (parsed.labels && parsed.labels.length > 0) {
+            await validateLabelsExist(gogsClient, owner, repo, parsed.labels);
+          }
+
           const issue = await gogsClient.createIssue(owner, repo, {
             title: parsed.title,
             body: parsed.body,
@@ -2693,6 +2727,22 @@ export function createMcpServer(gogsClient: GogsClient, restrictedOwner?: string
         case 'update_issue': {
           const parsed = UpdateIssueSchema.parse(args);
           const { owner, repo } = resolveRepoParams(parsed.owner, parsed.repo, restrictedOwner, restrictedRepo);
+
+          // Validate assignee exists if provided
+          if (parsed.assignee) {
+            await validateUserExists(gogsClient, parsed.assignee, 'assignee');
+          }
+
+          // Validate milestone exists if provided
+          if (parsed.milestone) {
+            await validateMilestoneExists(gogsClient, owner, repo, parsed.milestone);
+          }
+
+          // Validate labels exist if provided
+          if (parsed.labels && parsed.labels.length > 0) {
+            await validateLabelsExist(gogsClient, owner, repo, parsed.labels);
+          }
+
           const issue = await gogsClient.updateIssue(owner, repo, parsed.number, {
             title: parsed.title,
             body: parsed.body,
@@ -2840,6 +2890,10 @@ export function createMcpServer(gogsClient: GogsClient, restrictedOwner?: string
         case 'add_issue_labels': {
           const parsed = AddIssueLabelsSchema.parse(args);
           const { owner, repo } = resolveRepoParams(parsed.owner, parsed.repo, restrictedOwner, restrictedRepo);
+
+          // Validate labels exist
+          await validateLabelsExist(gogsClient, owner, repo, parsed.labels);
+
           const result = await gogsClient.addIssueLabels(owner, repo, parsed.number, parsed.labels);
           return {
             content: [
@@ -2868,6 +2922,10 @@ export function createMcpServer(gogsClient: GogsClient, restrictedOwner?: string
         case 'replace_issue_labels': {
           const parsed = ReplaceIssueLabelsSchema.parse(args);
           const { owner, repo } = resolveRepoParams(parsed.owner, parsed.repo, restrictedOwner, restrictedRepo);
+
+          // Validate labels exist
+          await validateLabelsExist(gogsClient, owner, repo, parsed.labels);
+
           const result = await gogsClient.replaceIssueLabels(owner, repo, parsed.number, parsed.labels);
           return {
             content: [
@@ -3049,6 +3107,13 @@ export function createMcpServer(gogsClient: GogsClient, restrictedOwner?: string
 
         case 'add_organization_member': {
           const { orgname, username, role } = AddOrganizationMemberSchema.parse(args);
+
+          // Validate organization exists
+          await validateOrgExists(gogsClient, orgname);
+
+          // Validate user exists before adding to organization
+          await validateUserExists(gogsClient, username, 'username');
+
           await gogsClient.addOrganizationMember(orgname, username, role);
           return {
             content: [
@@ -3105,6 +3170,13 @@ export function createMcpServer(gogsClient: GogsClient, restrictedOwner?: string
 
         case 'add_team_member': {
           const { teamId, username } = AddTeamMemberSchema.parse(args);
+
+          // Validate team exists
+          await validateTeamExists(gogsClient, teamId);
+
+          // Validate user exists before adding to team
+          await validateUserExists(gogsClient, username, 'username');
+
           await gogsClient.addTeamMember(teamId, username);
           return {
             content: [
@@ -3174,6 +3246,10 @@ export function createMcpServer(gogsClient: GogsClient, restrictedOwner?: string
         case 'add_collaborator': {
           const parsed = AddCollaboratorSchema.parse(args);
           const { owner, repo } = resolveRepoParams(parsed.owner, parsed.repo, restrictedOwner, restrictedRepo);
+
+          // Validate user exists before adding as collaborator
+          await validateUserExists(gogsClient, parsed.username, 'username');
+
           await gogsClient.addCollaborator(owner, repo, parsed.username, parsed.permission);
           return {
             content: [
@@ -3697,18 +3773,17 @@ export function createMcpServer(gogsClient: GogsClient, restrictedOwner?: string
           throw new Error(`Unknown tool: ${name}`);
       }
     } catch (error) {
-      if (error instanceof Error) {
-        return {
-          content: [
-            {
-              type: 'text',
-              text: `Error: ${error.message}`,
-            },
-          ],
-          isError: true,
-        };
-      }
-      throw error;
+      // Use enhanced error formatting
+      const errorMessage = formatGogsError(error);
+      return {
+        content: [
+          {
+            type: 'text',
+            text: errorMessage,
+          },
+        ],
+        isError: true,
+      };
     }
   });
 

+ 262 - 0
src/validation.ts

@@ -0,0 +1,262 @@
+/**
+ * Validation utilities for Gogs MCP Server
+ * Provides pre-validation checks to prevent API errors and improve error messages
+ */
+
+import { AxiosError } from 'axios';
+import { GogsClient } from './gogs-client.js';
+
+export interface ValidationError {
+  message: string;
+  field?: string;
+  suggestion?: string;
+}
+
+/**
+ * Enhanced error formatter that extracts meaningful information from axios errors
+ */
+export function formatGogsError(error: unknown, context?: string): string {
+  if (error instanceof Error) {
+    // Check if it's an axios error
+    const axiosError = error as AxiosError;
+    if (axiosError.response) {
+      const status = axiosError.response.status;
+      const method = axiosError.config?.method?.toUpperCase();
+      const url = axiosError.config?.url;
+
+      // Extract error message from response if available
+      let apiMessage = '';
+      if (axiosError.response.data && typeof axiosError.response.data === 'object') {
+        const data = axiosError.response.data as any;
+        apiMessage = data.message || data.error || '';
+      }
+
+      const contextPrefix = context ? `${context}: ` : '';
+
+      switch (status) {
+        case 400:
+          return `${contextPrefix}Bad request - ${apiMessage || 'Invalid parameters provided'}`;
+        case 401:
+          return `${contextPrefix}Authentication failed - Check your GOGS_ACCESS_TOKEN is valid`;
+        case 403:
+          return `${contextPrefix}Access forbidden - You don't have permission for this operation${apiMessage ? `. ${apiMessage}` : ''}`;
+        case 404: {
+          // Try to extract what wasn't found from the URL
+          const urlParts = url?.split('/').filter(Boolean) || [];
+          let notFoundEntity = 'Resource';
+
+          if (url?.includes('/users/')) {
+            const userIdx = urlParts.indexOf('users');
+            notFoundEntity = `User '${urlParts[userIdx + 1]}'`;
+          } else if (url?.includes('/repos/')) {
+            const repoIdx = urlParts.indexOf('repos');
+            const owner = urlParts[repoIdx + 1];
+            const repo = urlParts[repoIdx + 2];
+            if (url.includes('/branches')) {
+              notFoundEntity = `Repository '${owner}/${repo}' or branch`;
+            } else if (url.includes('/issues/')) {
+              const issueNum = urlParts[urlParts.indexOf('issues') + 1];
+              notFoundEntity = `Issue #${issueNum} in '${owner}/${repo}'`;
+            } else if (url.includes('/labels/')) {
+              notFoundEntity = `Label in '${owner}/${repo}'`;
+            } else if (url.includes('/milestones/')) {
+              notFoundEntity = `Milestone in '${owner}/${repo}'`;
+            } else {
+              notFoundEntity = `Repository '${owner}/${repo}'`;
+            }
+          } else if (url?.includes('/orgs/')) {
+            const orgIdx = urlParts.indexOf('orgs');
+            notFoundEntity = `Organization '${urlParts[orgIdx + 1]}'`;
+          } else if (url?.includes('/teams/')) {
+            const teamIdx = urlParts.indexOf('teams');
+            notFoundEntity = `Team '${urlParts[teamIdx + 1]}'`;
+          }
+
+          return `${contextPrefix}${notFoundEntity} not found`;
+        }
+        case 409:
+          return `${contextPrefix}Conflict - ${apiMessage || 'Resource already exists or operation conflicts with current state'}`;
+        case 422:
+          return `${contextPrefix}Validation error - ${apiMessage || 'The provided data is invalid'}`;
+        case 500:
+          return `${contextPrefix}Server error - The Gogs server encountered an internal error`;
+        default:
+          return `${contextPrefix}HTTP ${status} error${apiMessage ? `: ${apiMessage}` : ''}`;
+      }
+    } else if (axiosError.code === 'ECONNREFUSED') {
+      return `Cannot connect to Gogs server - Check GOGS_SERVER_URL is correct and server is running`;
+    } else if (axiosError.code === 'ETIMEDOUT') {
+      return `Connection timeout - Gogs server is not responding`;
+    }
+
+    return error.message;
+  }
+
+  return String(error);
+}
+
+/**
+ * Validates that a user exists
+ * @throws ValidationError if user doesn't exist
+ */
+export async function validateUserExists(
+  client: GogsClient,
+  username: string,
+  fieldName: string = 'username'
+): Promise<void> {
+  try {
+    await client.getUser(username);
+  } catch (error) {
+    throw new Error(`${fieldName}: User '${username}' not found. Please check the username is correct.`);
+  }
+}
+
+/**
+ * Validates that a repository exists
+ * @throws ValidationError if repository doesn't exist
+ */
+export async function validateRepoExists(
+  client: GogsClient,
+  owner: string,
+  repo: string
+): Promise<void> {
+  try {
+    await client.getRepository(owner, repo);
+  } catch (error) {
+    throw new Error(`Repository '${owner}/${repo}' not found. Please check the owner and repository names are correct.`);
+  }
+}
+
+/**
+ * Validates that a branch exists in a repository
+ * @throws ValidationError if branch doesn't exist
+ */
+export async function validateBranchExists(
+  client: GogsClient,
+  owner: string,
+  repo: string,
+  branchName: string
+): Promise<void> {
+  try {
+    const branches = await client.listBranches(owner, repo);
+    const branchExists = branches.some(b => b.name === branchName);
+
+    if (!branchExists) {
+      const availableBranches = branches.map(b => b.name).join(', ');
+      throw new Error(
+        `Branch '${branchName}' not found in repository '${owner}/${repo}'. ` +
+        `Available branches: ${availableBranches || 'none'}`
+      );
+    }
+  } catch (error) {
+    if (error instanceof Error && error.message.includes('not found in repository')) {
+      throw error;
+    }
+    // If we can't list branches, the repo might not exist
+    throw new Error(`Cannot validate branch '${branchName}' - repository '${owner}/${repo}' may not exist or is inaccessible.`);
+  }
+}
+
+/**
+ * Validates that an issue exists in a repository
+ * @throws ValidationError if issue doesn't exist
+ */
+export async function validateIssueExists(
+  client: GogsClient,
+  owner: string,
+  repo: string,
+  issueNumber: number
+): Promise<void> {
+  try {
+    await client.getIssue(owner, repo, issueNumber);
+  } catch (error) {
+    throw new Error(`Issue #${issueNumber} not found in repository '${owner}/${repo}'.`);
+  }
+}
+
+/**
+ * Validates that a label exists in a repository
+ * @throws ValidationError if label doesn't exist
+ */
+export async function validateLabelExists(
+  client: GogsClient,
+  owner: string,
+  repo: string,
+  labelId: number
+): Promise<void> {
+  try {
+    await client.getLabel(owner, repo, labelId);
+  } catch (error) {
+    throw new Error(`Label ID ${labelId} not found in repository '${owner}/${repo}'.`);
+  }
+}
+
+/**
+ * Validates that a milestone exists in a repository
+ * @throws ValidationError if milestone doesn't exist
+ */
+export async function validateMilestoneExists(
+  client: GogsClient,
+  owner: string,
+  repo: string,
+  milestoneId: number
+): Promise<void> {
+  try {
+    await client.getMilestone(owner, repo, milestoneId);
+  } catch (error) {
+    throw new Error(`Milestone ID ${milestoneId} not found in repository '${owner}/${repo}'.`);
+  }
+}
+
+/**
+ * Validates that an organization exists
+ * @throws ValidationError if organization doesn't exist
+ */
+export async function validateOrgExists(
+  client: GogsClient,
+  orgname: string
+): Promise<void> {
+  try {
+    await client.getOrganization(orgname);
+  } catch (error) {
+    throw new Error(`Organization '${orgname}' not found. Please check the organization name is correct.`);
+  }
+}
+
+/**
+ * Validates that a team exists
+ * @throws ValidationError if team doesn't exist
+ */
+export async function validateTeamExists(
+  client: GogsClient,
+  teamId: number
+): Promise<void> {
+  try {
+    await client.getTeam(teamId);
+  } catch (error) {
+    throw new Error(`Team ID ${teamId} not found.`);
+  }
+}
+
+/**
+ * Validates multiple label IDs exist in a repository
+ * @throws ValidationError if any label doesn't exist
+ */
+export async function validateLabelsExist(
+  client: GogsClient,
+  owner: string,
+  repo: string,
+  labelIds: number[]
+): Promise<void> {
+  const labels = await client.listLabels(owner, repo);
+  const existingIds = new Set(labels.map(l => l.id));
+
+  const invalidIds = labelIds.filter(id => !existingIds.has(id));
+
+  if (invalidIds.length > 0) {
+    throw new Error(
+      `Label ID(s) ${invalidIds.join(', ')} not found in repository '${owner}/${repo}'. ` +
+      `Available label IDs: ${Array.from(existingIds).join(', ')}`
+    );
+  }
+}