feat(web): API & MCP improvements #795
Conversation
WalkthroughImplements offset pagination and RFC5988 Link headers for repos/commits, converts several POST endpoints to GET with query params, centralizes response validation, renames webUrl→externalWebUrl and searchCommits→listCommits, and introduces pagination/schema/typing updates across MCP and web packages. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/web/src/app/[domain]/search/components/filterPanel/index.tsx (1)
60-65: RenamewebUrltoexternalWebUrlinrepositoryInfoSchemafor consistency.The
repositoryInfoSchemaat line 29 ofpackages/web/src/features/search/types.tsstill defines the property aswebUrl, while the code at line 64 offilterPanel/index.tsxreads fromrepo.webUrlbut passes it as theexternalWebUrlparameter togetCodeHostInfoForRepo. To align with the PR-wide rename ofwebUrl→externalWebUrl, update the schema to useexternalWebUrl: z.string().optional()instead ofwebUrl. This will ensure naming consistency across the codebase and eliminate the property/parameter name mismatch.packages/web/src/app/[domain]/search/components/searchResultsPanel/fileMatchContainer.tsx (1)
94-99: Use repo.externalWebUrl for the external link target.
externalWebUrl: repo.webUrllikely points at the Sourcebot URL after the rename, so the code-host icon will open the wrong destination.🛠️ Proposed fix
- externalWebUrl: repo.webUrl, + externalWebUrl: repo.externalWebUrl,packages/web/src/ee/features/codeNav/components/exploreMenu/referenceList.tsx (1)
93-99: Use repoInfo.externalWebUrl for the external link target.
externalWebUrl: repoInfo.webUrlwill likely resolve to the Sourcebot URL after the rename, so the icon link won’t go to the external code host.🛠️ Proposed fix
- externalWebUrl: repoInfo.webUrl, + externalWebUrl: repoInfo.externalWebUrl,
🤖 Fix all issues with AI agents
In `@CHANGELOG.md`:
- Line 21: Fix the grammar in the CHANGELOG entry for the /api/commits endpoint:
change the phrase "to be a POST request to a GET request" to "from a POST
request to a GET request" so it reads that the /api/commits endpoint changed
from POST to GET; update the single line mentioning "/api/commits" accordingly.
In `@packages/mcp/CHANGELOG.md`:
- Line 13: Update the changelog entry text "Added server side pagination support
for `list_commits` and `list_repos`." to hyphenate the compound adjective by
changing "server side" to "server-side" so the line reads "Added server-side
pagination support for `list_commits` and `list_repos`."
In `@packages/mcp/src/index.ts`:
- Around line 29-37: The current zod schema unconditionally transforms query
(the query .transform(...) on the query schema) to a quoted literal and doesn't
escape backslashes, which breaks regex searches and can corrupt queries; remove
that unconditional transform and instead handle quoting/escaping in the request
handler where useRegex (the useRegex z.boolean().optional()) is available: when
useRegex is true pass the raw query through, and when false produce a literal
phrase by escaping double-quotes and backslashes then wrapping in quotes; apply
the same change to the second occurrence of the query transform (the other block
noted around lines 67–77) so both code paths behave the same.
- Around line 46-49: The schema field filterByFilepaths currently claims to
accept regexes but the code escapes input (via escapeStringRegexp), so either
update the description to state "literal path match" or stop escaping so real
regexes are allowed; to fix, locate the z.array(z.string()) definition named
filterByFilepaths and either change its .describe(...) text to "Scope the search
to the provided filepaths. Interpreted as literal path match." or remove the
escapeStringRegexp call where those values are later converted to regexes (so
the raw strings are passed through), and make the same change for the other
identical field instance referenced around the second occurrence.
- Around line 121-126: Update the snippet that builds the `text` string to
prefer `file.externalWebUrl` over `file.webUrl` (with `file.webUrl` as a
fallback) so the output matches the tool contract; locate the `text`
construction (the `dedent` block referencing `file.webUrl`, `numMatches`,
`file.repository`, `file.language`) and change the URL insertion to use
`file.externalWebUrl ?? file.webUrl` (or equivalent null/undefined-safe
fallback) while leaving the other fields unchanged.
In `@packages/web/src/app/api/`(server)/commits/route.ts:
- Around line 1-6: Add the missing 'use server' directive as the very first
statement in the file so the route runs in server context; place it before the
imports and keep the rest of the module (including the export GET handler and
uses of listCommits, buildLinkHeader, serviceErrorResponse, etc.) unchanged to
mirror other server routes like tree/route.ts and search/route.ts.
In `@packages/web/src/app/api/`(server)/repos/route.ts:
- Around line 77-82: The Link header built by buildLinkHeader is missing the
search query so pagination loses filters; update the call that constructs
linkHeader (where buildLinkHeader(request, { page, perPage, totalCount,
extraParams: { sort, direction } })) to include the active query parameter
(e.g., add query from request.searchParams or request.url into extraParams: {
sort, direction, query }) so next/prev links preserve the search term when using
the repos route.
- Around line 41-43: The totalCount calculation uses prisma.repo.count without
the search filter, so it returns an unfiltered total and breaks pagination;
update the prisma.repo.count call to use the same where clause used for
prisma.repo.findMany (include orgId plus the search conditions/filters derived
from query, e.g., name contains/insensitive or whatever filter logic is used) so
X-Total-Count and pagination reflect the filtered result set (modify the count
call at prisma.repo.count to mirror the findMany where object).
In `@packages/web/src/features/search/gitApi.ts`:
- Around line 99-105: The rev-list args currently append ref twice because
sharedOptions includes a computed key equal to ref; when building countArgs in
the loop over sharedOptions skip the entry whose key equals the ref variable (or
filter sharedOptions to remove that key) so only the explicit ref pushed at the
start is used; update the iteration around countArgs (the loop over
Object.entries(sharedOptions>) to continue/ignore when key === ref, leaving
git.raw(countArgs) unchanged.
🧹 Nitpick comments (6)
packages/web/src/actions.ts (1)
481-495: Consider ifgetReposshould guardheaders()for potential future non-request contexts.
headers()requires a request context and will throw outside of it (e.g., background jobs). While all current call sites are server-side page components within requests, ifgetReposis ever invoked in a background context, the function will hard-fail. A fallback toenv.AUTH_URLwould provide robustness:Optional defensive guard
- const headersList = await headers(); - const baseUrl = getBaseUrl(headersList); + let baseUrl = env.AUTH_URL; + try { + const headersList = await headers(); + baseUrl = getBaseUrl(headersList); + } catch { + // No request context; use fallback + }packages/mcp/src/types.ts (2)
35-36: Type naming inconsistency.
ListCommitsQueryParamsSchemaincludes "Schema" in the type name, which is inconsistent with other type naming conventions in this file (e.g.,ListReposQueryParams,SearchRequest). Consider renaming toListCommitsQueryParamsfor consistency.Suggested fix
-export type ListCommitsQueryParamsSchema = z.infer<typeof listCommitsQueryParamsSchema>; +export type ListCommitsQueryParams = z.infer<typeof listCommitsQueryParamsSchema>;
26-26: Inconsistent use ofz.inputvsz.infer.
ListReposQueryParamsusesz.inputwhileListCommitsQueryParamsSchemausesz.infer. For schemas with defaults:
z.inputmakes defaulted fields optional (useful for input/request types)z.infermakes defaulted fields required (useful for output/parsed types)If both are intended for client-side request building, consider using
z.inputconsistently. If both are for parsed responses, usez.inferconsistently.Also applies to: 35-35
packages/web/src/features/search/zoektSearcher.ts (1)
386-388: Verify request-scoped headers usage is safe in all call paths.
headers()is request-scoped in Next.js; iftransformZoektSearchResponseis ever invoked outside a route handler/server action (e.g., background tasks), this will throw. Consider passingbaseUrlfrom the caller if that’s a possibility.packages/web/src/app/api/(client)/client.ts (1)
54-68: Consider removing unnecessaryContent-Typeheader and extracting URL builder.
The
Content-Type: application/jsonheader on Line 62-64 is unnecessary for a GET request with no body (note:getFileSourcecorrectly omits it).The URL construction pattern (lines 55-58) is duplicated from
getFileSource. Consider extracting a helper.♻️ Suggested refactor
+const buildUrl = (path: string, params: Record<string, unknown>): URL => { + const url = new URL(path, window.location.origin); + for (const [key, value] of Object.entries(params)) { + if (value !== undefined) { + url.searchParams.set(key, String(value)); + } + } + return url; +}; + export const listRepos = async (queryParams: ListReposQueryParams): Promise<ListReposResponse | ServiceError> => { - const url = new URL("/api/repos", window.location.origin); - for (const [key, value] of Object.entries(queryParams)) { - url.searchParams.set(key, value.toString()); - } - + const url = buildUrl("/api/repos", queryParams); const result = await fetch(url, { method: "GET", - headers: { - "Content-Type": "application/json", - }, }).then(response => response.json()); return result as ListReposResponse | ServiceError; }packages/web/src/features/search/gitApi.ts (1)
78-90: Consider consolidating--regexp-ignore-casehandling.When both
authorandqueryare provided,--regexp-ignore-caseis set twice in the spread. While JavaScript objects deduplicate keys (last wins), the intent is clearer if case-insensitivity is handled once.♻️ Suggested refactor
const sharedOptions: Record<string, string | number | null> = { [ref]: null, ...(gitSince ? { '--since': gitSince } : {}), ...(gitUntil ? { '--until': gitUntil } : {}), - ...(author ? { - '--author': author, - '--regexp-ignore-case': null /// Case insensitive - } : {}), - ...(query ? { - '--grep': query, - '--regexp-ignore-case': null /// Case insensitive - } : {}), + ...(author ? { '--author': author } : {}), + ...(query ? { '--grep': query } : {}), + ...((author || query) ? { '--regexp-ignore-case': null } : {}), };
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@CHANGELOG.md`:
- Line 22: The CHANGELOG line uses "apis" in lowercase; update the sentence to
consistently capitalize it as "APIs" (preserve the rest of the text and the
identifiers `webUrl` and `externalWebUrl` unchanged) so the line reads with
"APIs" instead of "apis".
- Line 19: Update the changelog entry that reads "Made the code search `lang:`
filter case insensitive.
[`#795`](https://github.com/sourcebot-dev/sourcebot/pull/795)" by hyphenating
"case-insensitive" so it reads "Made the code search `lang:` filter
case-insensitive. [`#795`](https://github.com/sourcebot-dev/sourcebot/pull/795)";
locate the exact line containing that text in CHANGELOG.md and replace the
unhyphenated phrase with the hyphenated form to maintain style consistency.
In `@packages/web/src/app/api/`(server)/commits/route.ts:
- Around line 8-17: The schema listCommitsQueryParamsSchema currently allows an
empty repo string; update the repo validator to reject empty values by replacing
z.string() with a non-empty string check (e.g., z.string().nonempty() or
z.string().min(1)) so requests with repo == "" fail validation earlier and avoid
downstream "Repository \"\" not found" errors.
🧹 Nitpick comments (1)
packages/web/src/features/search/gitApi.ts (1)
107-107: Consider adding a type guard or mapper instead of double assertion.The
as unknown as Commit[]assertion bypasses TypeScript's type checking. While simple-git'sDefaultLogFieldslikely contains the same fields asCommit, this assertion hides any potential mismatches.♻️ Optional: Add explicit field mapping for type safety
- return { commits: log.all as unknown as Commit[], totalCount }; + const commits: Commit[] = log.all.map(entry => ({ + hash: entry.hash, + date: entry.date, + message: entry.message, + refs: entry.refs, + body: entry.body, + author_name: entry.author_name, + author_email: entry.author_email, + })); + return { commits, totalCount };This makes the mapping explicit and would surface any field mismatches at compile time.
This PR grew larger than it should of. It does a lot of things (see below). The two biggest things it does are the following:
/api/reposand/api/commits.In a follow-up PR, I will take some time to formalize our REST api into a openapi spec, which will allow us to generate some docs + (hopefully) allow us to more easily share types between MCP and the main package.
Sourcebot
Added
/api/reposendpoint. #795/api/commitsendpoint. #795Changed
lang:filter case insensitive. #795/api/sourceendpoint from a POST request to a GET request. Repo, path, and ref are specified as query params. #795/api/commitsendpoint to be a POST request to a GET request. #795webUrltoexternalWebUrlfor various apis. Moving forward,webUrlwill be used for URLs that point to Sourcebot, andexternalWebUrlwill be used for URLs that point to external code hosts. #795/api/sourceendpoint response body. #795@sourcebot/mcp
Added
list_commitsandlist_repos. #795filterByFilepathsanduseRegexparams to thesearch_codetool. #795Changed
search_commitstool tolist_commits. #795gitRevisionparam torefonsearch_codetool. #795Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.