feat: add dual repository identifier support to search_commits

waynesun09 · waynesun09 · commit b30bbd398532 · 2025-11-21T12:09:33.000-05:00
Add support for both numeric database IDs and string repository names
in the search_commits tool, allowing users to directly use repository
identifiers from list_repos output.

**Implementation:**
- Add resolveRepoId utility function to convert string repo names to
  numeric IDs via database lookup
- Update SearchCommitsRequest interface to accept repoId: string | number
- Wrap searchCommits in withOptionalAuthV2 for database access
- Update MCP schema and API route schema with z.union([z.number(), z.string()])

**Testing:**
- Add comprehensive unit tests for repository identifier resolution
- Test both numeric ID and string name inputs
- Test error handling for non-existent repositories
- Verify database query parameters
- All 29 tests passing

**Documentation:**
- Update README to document both identifier types
- Update CHANGELOG with new functionality
- Clear error messages guide users to use list_repos for valid identifiers

This enhancement improves the user experience by eliminating the need
to manually convert repository names to numeric IDs when using
search_commits after list_repos.

Signed-off-by: Wayne Sun &lt;gsun@redhat.com&gt;
diff --git a/packages/mcp/CHANGELOG.md b/packages/mcp/CHANGELOG.md
@@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Added
 - Added comprehensive relative date support for all temporal parameters (e.g., "30 days ago", "last week", "yesterday")
-- Added `search_commits` tool to search commits by actual commit time with full temporal filtering
+- Added `search_commits` tool to search commits by actual commit time with full temporal filtering. Accepts both numeric database IDs (e.g., 123) and string repository names (e.g., "github.com/owner/repo") for the `repoId` parameter, allowing direct use of repository names from `list_repos` output
 - Added `since`/`until` parameters to `search_code` (filters by index time - when Sourcebot indexed the repo)
 - Added `gitRevision` parameter to `search_code`
 - Added `activeAfter`/`activeBefore` parameters to `list_repos` (filters by commit time - actual git commit dates)
@@ -21,12 +21,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Added comprehensive unit tests for date parsing utilities (90+ test cases)
 - Added unit tests for git commit search functionality with mocking
 - Added integration tests for temporal parameter validation
-
-### Changed
-- Enhanced `list_repos` tool with better pagination feedback messages
-
-### Fixed
-- Fixed `list_repos` pagination bug where `take` limit was applied before activity filtering, returning fewer results than requested
+- Added unit tests for repository identifier resolution (both string and number types)
 
 ## [1.0.9] - 2025-11-17
 
diff --git a/packages/mcp/README.md b/packages/mcp/README.md
@@ -230,7 +230,7 @@ Searches for commits in a specific repository based on actual commit time (NOT i
 
 | Name       | Required | Description                                                                                    |
 |:-----------|:---------|:-----------------------------------------------------------------------------------------------|
-| `repoId`   | yes      | Repository ID (obtain from `list_repos`).                                                      |
+| `repoId`   | yes      | Repository identifier: either numeric database ID (e.g., 123) or full repository name (e.g., "github.com/owner/repo") as returned by `list_repos`. |
 | `query`    | no       | Search query to filter commits by message (case-insensitive).                                  |
 | `since`    | no       | Show commits after this date (by commit time). Supports ISO 8601 or relative formats.          |
 | `until`    | no       | Show commits before this date (by commit time). Supports ISO 8601 or relative formats.         |
diff --git a/packages/mcp/src/index.ts b/packages/mcp/src/index.ts
@@ -194,7 +194,11 @@ server.tool(
 
     If you receive an error that indicates that you're not authenticated, please inform the user to set the SOURCEBOT_API_KEY environment variable.`,
     {
-        repoId: z.number().describe(`The ID of the repository to search in. Obtain this by calling 'list_repos' first.`),
+        repoId: z.union([z.number(), z.string()]).describe(`Repository identifier. Can be either:
+        - Numeric database ID (e.g., 123)
+        - Full repository name (e.g., "github.com/owner/repo") as returned by 'list_repos'
+
+        **YOU MUST** call 'list_repos' first to obtain the repository identifier.`),
         query: z.string().describe(`Search query to filter commits by message content (case-insensitive).`).optional(),
         since: z.string().describe(`Show commits more recent than this date. Filters by actual commit time. Supports ISO 8601 (e.g., '2024-01-01') or relative formats (e.g., '30 days ago', 'last week').`).optional(),
         until: z.string().describe(`Show commits older than this date. Filters by actual commit time. Supports ISO 8601 (e.g., '2024-12-31') or relative formats (e.g., 'yesterday').`).optional(),
diff --git a/packages/web/src/features/search/gitApi.test.ts b/packages/web/src/features/search/gitApi.test.ts
@@ -27,6 +27,26 @@ vi.mock('@/actions', () => ({
         }
     },
 }));
+// Create a mock findFirst function that we can configure per-test
+const mockFindFirst = vi.fn();
+
+vi.mock('@/withAuthV2', () => ({
+    withOptionalAuthV2: async (fn: (args: any) => any) => {
+        // Mock withOptionalAuthV2 to provide org and prisma context
+        const mockOrg = { id: 1, name: 'test-org' };
+        const mockPrisma = {
+            repo: {
+                findFirst: mockFindFirst,
+            },
+        };
+        return await fn({ org: mockOrg, prisma: mockPrisma });
+    },
+}));
+vi.mock('@/lib/utils', () => ({
+    isServiceError: (obj: any) => {
+        return obj && typeof obj === 'object' && 'errorCode' in obj;
+    },
+}));
 
 // Import mocked modules
 import { simpleGit } from 'simple-git';
@@ -451,4 +471,105 @@ describe('searchCommits', () => {
             expect(message).toContain('cloning process may not be finished yet');
         });
     });
+
+    describe('repository identifier resolution', () => {
+        beforeEach(() => {
+            // Reset mockFindFirst before each test in this suite
+            mockFindFirst.mockReset();
+        });
+
+        it('should accept numeric repository ID', async () => {
+            mockGitLog.mockResolvedValue({ all: [] });
+
+            const result = await searchCommits({
+                repoId: 123,
+            });
+
+            expect(Array.isArray(result)).toBe(true);
+            expect(mockExistsSync).toHaveBeenCalledWith('/mock/cache/dir/123');
+            // mockFindFirst should not be called for numeric IDs
+            expect(mockFindFirst).not.toHaveBeenCalled();
+        });
+
+        it('should accept string repository name and resolve to numeric ID', async () => {
+            mockFindFirst.mockResolvedValue({ id: 456 });
+            mockGitLog.mockResolvedValue({ all: [] });
+
+            const result = await searchCommits({
+                repoId: 'github.com/owner/repo',
+            });
+
+            expect(Array.isArray(result)).toBe(true);
+            expect(mockExistsSync).toHaveBeenCalledWith('/mock/cache/dir/456');
+            expect(mockFindFirst).toHaveBeenCalledWith({
+                where: {
+                    name: 'github.com/owner/repo',
+                    orgId: 1,
+                },
+                select: { id: true },
+            });
+        });
+
+        it('should return error when string repository name is not found', async () => {
+            mockFindFirst.mockResolvedValue(null);
+
+            const result = await searchCommits({
+                repoId: 'github.com/nonexistent/repo',
+            });
+
+            expect(result).toMatchObject({
+                errorCode: 'UNEXPECTED_ERROR',
+                message: expect.stringContaining('Repository "github.com/nonexistent/repo" not found'),
+            });
+            expect(result).toMatchObject({
+                message: expect.stringContaining('Use \'list_repos\' to get valid repository identifiers'),
+            });
+        });
+
+        it('should query database with correct parameters for string repo name', async () => {
+            mockFindFirst.mockResolvedValue({ id: 789 });
+            mockGitLog.mockResolvedValue({ all: [] });
+
+            await searchCommits({
+                repoId: 'github.com/example/project',
+            });
+
+            expect(mockFindFirst).toHaveBeenCalledWith({
+                where: {
+                    name: 'github.com/example/project',
+                    orgId: 1,
+                },
+                select: { id: true },
+            });
+        });
+
+        it('should work with string repo name in full search scenario', async () => {
+            const mockCommits = [
+                {
+                    hash: 'xyz789',
+                    date: '2024-06-20T10:00:00Z',
+                    message: 'feat: new feature',
+                    refs: 'main',
+                    body: 'Added new functionality',
+                    author_name: 'Developer',
+                    author_email: 'dev@example.com',
+                },
+            ];
+
+            mockFindFirst.mockResolvedValue({ id: 555 });
+            vi.spyOn(dateUtils, 'validateDateRange').mockReturnValue(null);
+            vi.spyOn(dateUtils, 'toGitDate').mockImplementation((date) => date);
+            mockGitLog.mockResolvedValue({ all: mockCommits });
+
+            const result = await searchCommits({
+                repoId: 'github.com/test/repository',
+                query: 'feature',
+                since: '7 days ago',
+                author: 'Developer',
+            });
+
+            expect(result).toEqual(mockCommits);
+            expect(mockExistsSync).toHaveBeenCalledWith('/mock/cache/dir/555');
+        });
+    });
 });
diff --git a/packages/web/src/features/search/gitApi.ts b/packages/web/src/features/search/gitApi.ts
@@ -5,6 +5,8 @@ import path from 'path';
 import { ServiceError, unexpectedError } from '@/lib/serviceError';
 import { sew } from '@/actions';
 import { toGitDate, validateDateRange } from './dateUtils';
+import { withOptionalAuthV2 } from '@/withAuthV2';
+import { isServiceError } from '@/lib/utils';
 
 const createGitClientForPath = (repoPath: string) => {
     return simpleGit({
@@ -17,8 +19,46 @@ const createGitClientForPath = (repoPath: string) => {
     });
 }
 
+/**
+ * Resolves a repository identifier to a numeric ID.
+ * Accepts both numeric IDs and string repository names.
+ *
+ * @param identifier - Either a numeric repo ID or a string repo name (e.g., "github.com/owner/repo")
+ * @param orgId - Organization ID to scope the lookup
+ * @param prisma - Prisma client instance
+ * @returns Numeric repository ID or ServiceError if not found
+ */
+const resolveRepoId = async (
+    identifier: string | number,
+    orgId: number,
+    prisma: any
+): Promise<number | ServiceError> => {
+    // If already numeric, return as-is
+    if (typeof identifier === 'number') {
+        return identifier;
+    }
+
+    // Convert string name to numeric ID
+    const repo = await prisma.repo.findFirst({
+        where: {
+            name: identifier,
+            orgId: orgId,
+        },
+        select: { id: true }
+    });
+
+    if (!repo) {
+        return unexpectedError(
+            `Repository "${identifier}" not found. ` +
+            `Use 'list_repos' to get valid repository identifiers.`
+        );
+    }
+
+    return repo.id;
+}
+
 export interface SearchCommitsRequest {
-    repoId: number;
+    repoId: string | number;
     query?: string;
     since?: string;
     until?: string;
@@ -48,18 +88,28 @@ export interface Commit {
  * process might not be finished when this query is executed. If the repository
  * is not found on the server disk, an error will be returned.
  *
+ * **Repository ID**: Accepts either a numeric database ID or a string repository name
+ * (e.g., "github.com/owner/repo") as returned by list_repos.
+ *
  * @param request - Search parameters including timeframe filters
  * @returns Array of commits or ServiceError
  */
 export const searchCommits = async ({
-    repoId,
+    repoId: repoIdInput,
     query,
     since,
     until,
     author,
     maxCount = 50,
-}: SearchCommitsRequest): Promise<Commit[] | ServiceError> => sew(async () => {
-    const repoPath = path.join(REPOS_CACHE_DIR, repoId.toString());
+}: SearchCommitsRequest): Promise<Commit[] | ServiceError> => sew(() =>
+    withOptionalAuthV2(async ({ org, prisma }) => {
+        // Resolve repository identifier to numeric ID
+        const repoId = await resolveRepoId(repoIdInput, org.id, prisma);
+        if (isServiceError(repoId)) {
+            return repoId;
+        }
+
+        const repoPath = path.join(REPOS_CACHE_DIR, repoId.toString());
 
     // Check if repository exists on Sourcebot server disk
     if (!existsSync(repoPath)) {
@@ -143,4 +193,4 @@ export const searchCommits = async ({
             );
         }
     }
-});
+}));
diff --git a/packages/web/src/features/search/schemas.ts b/packages/web/src/features/search/schemas.ts
@@ -169,7 +169,7 @@ export const fileSourceResponseSchema = z.object({
 });
 
 export const searchCommitsRequestSchema = z.object({
-    repoId: z.number(),
+    repoId: z.union([z.number(), z.string()]),
     query: z.string().optional(),
     since: z.string().optional(),
     until: z.string().optional(),
