fix(web): address review feedback on citation SHA pinning

whoisthey · cursoragent · whoisthey · commit 5da3214f86bb · 2026-06-29T14:57:53.000-07:00
- Resolve the git ref to a concrete commit before reading file content and
  .gitattributes, so all of source, language, and commitSha come from the
  same revision even if the ref moves mid-request.
- Narrow the evidence-panel fallback to only retry at the symbolic ref when
  the pinned commit is unresolvable (INVALID_GIT_REF); surface other errors.
- Pin symbol-search citations from the indexed commit carried by the same
  search snapshot instead of a follow-up getRepoInfoByName lookup.
- Regenerate the public OpenAPI spec for the new commitSha/indexedCommitHash
  fields.

Co-authored-by: Cursor &lt;cursoragent@cursor.com&gt;
diff --git a/docs/api-reference/sourcebot-public.openapi.json b/docs/api-reference/sourcebot-public.openapi.json
@@ -381,6 +381,9 @@
                 },
                 "webUrl": {
                   "type": "string"
+                },
+                "indexedCommitHash": {
+                  "type": "string"
                 }
               },
               "required": [
@@ -576,6 +579,9 @@
           },
           "externalWebUrl": {
             "type": "string"
+          },
+          "commitSha": {
+            "type": "string"
           }
         },
         "required": [
@@ -931,6 +937,9 @@
                 },
                 "webUrl": {
                   "type": "string"
+                },
+                "indexedCommitHash": {
+                  "type": "string"
                 }
               },
               "required": [
diff --git a/packages/web/src/ee/features/chat/components/chatThread/referencedFileSourceListItemContainer.tsx b/packages/web/src/ee/features/chat/components/chatThread/referencedFileSourceListItemContainer.tsx
@@ -4,6 +4,7 @@ import { getFileSource } from "@/app/api/(client)/client";
 import { VscodeFileIcon } from "@/app/components/vscodeFileIcon";
 import { Skeleton } from "@/components/ui/skeleton";
 import { isServiceError, unwrapServiceError } from "@/lib/utils";
+import { ErrorCode } from "@/lib/errorCodes";
 import { useQuery } from "@tanstack/react-query";
 import { ReactCodeMirrorRef } from '@uiw/react-codemirror';
 import { memo, useCallback } from "react";
@@ -51,9 +52,15 @@ const ReferencedFileSourceListItemContainerComponent = ({
                 ref: fetchRef,
             });
 
-            // The pinned commit can disappear (e.g. a force-push + GC prunes it).
-            // Fall back once to the symbolic ref so the file still renders.
-            if (isServiceError(pinned) && fetchRef !== fileSource.revision) {
+            // The pinned commit can disappear (e.g. a force-push + GC prunes it),
+            // which surfaces as an unresolvable git ref. Only that case falls
+            // back to the symbolic ref; other errors (repo/path/access) are
+            // surfaced as-is so we don't silently render the wrong revision.
+            if (
+                isServiceError(pinned) &&
+                pinned.errorCode === ErrorCode.INVALID_GIT_REF &&
+                fetchRef !== fileSource.revision
+            ) {
                 return unwrapServiceError(getFileSource({
                     path: fileSource.path,
                     repo: fileSource.repo,
diff --git a/packages/web/src/features/git/getFileSourceApi.test.ts b/packages/web/src/features/git/getFileSourceApi.test.ts
@@ -92,8 +92,13 @@ describe('getFileSourceForRepo', () => {
         mockSimpleGit.mockReturnValue({ cwd: mockCwd });
         mockFindFirst.mockResolvedValue(MOCK_REPO);
 
-        // Default: file show succeeds; .gitattributes not present
+        // Default: ref resolves to a concrete sha, file show succeeds, and
+        // .gitattributes is absent. The SUT resolves the ref first (rev-parse),
+        // then reads content + .gitattributes at the resolved sha.
         mockGitRaw.mockImplementation(async (args: string[]) => {
+            if (args[0] === 'rev-parse') {
+                return 'resolvedsha\n';
+            }
             if (args[1]?.endsWith('.gitattributes')) {
                 throw new Error('does not exist in HEAD');
             }
@@ -170,7 +175,7 @@ describe('getFileSourceForRepo', () => {
 
     describe('git error handling', () => {
         it('returns FILE_NOT_FOUND when git reports the file does not exist', async () => {
-            mockGitRaw.mockRejectedValueOnce(
+            mockGitRaw.mockRejectedValue(
                 new Error("fatal: path 'src/missing.ts' does not exist in 'main'"),
             );
 
@@ -183,7 +188,7 @@ describe('getFileSourceForRepo', () => {
         });
 
         it('returns FILE_NOT_FOUND for "fatal: path" errors', async () => {
-            mockGitRaw.mockRejectedValueOnce(new Error('fatal: path not found'));
+            mockGitRaw.mockRejectedValue(new Error('fatal: path not found'));
 
             const result = await getFileSourceForRepo(
                 { path: 'src/index.ts', repo: 'github.com/owner/repo' },
@@ -196,9 +201,9 @@ describe('getFileSourceForRepo', () => {
         it('returns INVALID_GIT_REF with an unresolved-ref message when head_sha has not been fetched ("unknown revision")', async () => {
             // This is the scenario from the v4.16.14 regression: the review agent passes
             // pr_payload.head_sha as ref, but the bare clone hasn't fetched it yet.
-            mockGitRaw.mockRejectedValueOnce(
+            mockGitRaw.mockRejectedValue(
                 new Error("fatal: ambiguous argument 'deadbeef': unknown revision or path not in the working tree"),
-            );
+            ); // rejects rev-parse (swallowed) and the show, which drives the result
 
             const result = await getFileSourceForRepo(
                 { path: 'src/index.ts', repo: 'github.com/owner/repo', ref: 'deadbeef' },
@@ -212,7 +217,7 @@ describe('getFileSourceForRepo', () => {
         });
 
         it('returns INVALID_GIT_REF with an unresolved-ref message for "bad revision" errors', async () => {
-            mockGitRaw.mockRejectedValueOnce(new Error('fatal: bad revision'));
+            mockGitRaw.mockRejectedValue(new Error('fatal: bad revision'));
 
             const result = await getFileSourceForRepo(
                 { path: 'src/index.ts', repo: 'github.com/owner/repo', ref: 'nonexistent' },
@@ -226,7 +231,7 @@ describe('getFileSourceForRepo', () => {
         });
 
         it('returns INVALID_GIT_REF with an unresolved-ref message for "invalid object name" errors', async () => {
-            mockGitRaw.mockRejectedValueOnce(new Error('fatal: invalid object name HEAD'));
+            mockGitRaw.mockRejectedValue(new Error('fatal: invalid object name HEAD'));
 
             const result = await getFileSourceForRepo(
                 { path: 'src/index.ts', repo: 'github.com/owner/repo' },
@@ -243,7 +248,7 @@ describe('getFileSourceForRepo', () => {
             // Before the fix, getFileSourceForRepo re-threw unknown errors.
             // Outside sew(), this caused a fatal Next.js task-runner exception.
             // After the fix, all errors are returned as ServiceError.
-            mockGitRaw.mockRejectedValueOnce(new Error('I/O error: device busy'));
+            mockGitRaw.mockRejectedValue(new Error('I/O error: device busy'));
 
             const result = await getFileSourceForRepo(
                 { path: 'src/index.ts', repo: 'github.com/owner/repo' },
@@ -254,7 +259,7 @@ describe('getFileSourceForRepo', () => {
         });
 
         it('never rejects its returned promise for unrecognised git errors', async () => {
-            mockGitRaw.mockRejectedValueOnce(new Error('transient I/O error'));
+            mockGitRaw.mockRejectedValue(new Error('transient I/O error'));
 
             await expect(
                 getFileSourceForRepo(
@@ -280,13 +285,16 @@ describe('getFileSourceForRepo', () => {
             });
         });
 
-        it('uses the provided ref for the git show command', async () => {
+        it('resolves the provided ref to a commit, then reads content at it', async () => {
             await getFileSourceForRepo(
                 { path: 'src/index.ts', repo: 'github.com/owner/repo', ref: 'abc123sha' },
                 { org: MOCK_ORG, prisma: mockPrisma },
             );
 
-            expect(mockGitRaw).toHaveBeenCalledWith(['show', 'abc123sha:src/index.ts']);
+            // The provided ref is resolved up front...
+            expect(mockGitRaw).toHaveBeenCalledWith(['rev-parse', 'abc123sha^{commit}']);
+            // ...and content is read at the resolved sha, not the symbolic ref.
+            expect(mockGitRaw).toHaveBeenCalledWith(['show', 'resolvedsha:src/index.ts']);
         });
 
         it('falls back to defaultBranch when ref is omitted', async () => {
@@ -295,7 +303,7 @@ describe('getFileSourceForRepo', () => {
                 { org: MOCK_ORG, prisma: mockPrisma },
             );
 
-            expect(mockGitRaw).toHaveBeenCalledWith(['show', 'main:src/index.ts']);
+            expect(mockGitRaw).toHaveBeenCalledWith(['rev-parse', 'main^{commit}']);
         });
 
         it('falls back to HEAD when both ref and defaultBranch are absent', async () => {
@@ -306,7 +314,28 @@ describe('getFileSourceForRepo', () => {
                 { org: MOCK_ORG, prisma: mockPrisma },
             );
 
-            expect(mockGitRaw).toHaveBeenCalledWith(['show', 'HEAD:src/index.ts']);
+            expect(mockGitRaw).toHaveBeenCalledWith(['rev-parse', 'HEAD^{commit}']);
+        });
+
+        it('reads content at the symbolic ref when rev-parse fails', async () => {
+            mockGitRaw.mockImplementation(async (args: string[]) => {
+                if (args[0] === 'rev-parse') {
+                    throw new Error('unknown revision');
+                }
+                if (args[1]?.endsWith('.gitattributes')) {
+                    throw new Error('does not exist');
+                }
+                return 'console.log("hello");';
+            });
+
+            const result = await getFileSourceForRepo(
+                { path: 'src/index.ts', repo: 'github.com/owner/repo', ref: 'main' },
+                { org: MOCK_ORG, prisma: mockPrisma },
+            );
+
+            expect(mockGitRaw).toHaveBeenCalledWith(['show', 'main:src/index.ts']);
+            expect(result).toMatchObject({ source: 'console.log("hello");' });
+            expect((result as { commitSha?: string }).commitSha).toBeUndefined();
         });
 
         it('uses the repo path from getRepoPath for the git working directory', async () => {
diff --git a/packages/web/src/features/git/getFileSourceApi.ts b/packages/web/src/features/git/getFileSourceApi.ts
@@ -47,9 +47,21 @@ export const getFileSourceForRepo = async (
 
     const gitRef = ref ?? repo.defaultBranch ?? 'HEAD';
 
+    // Resolve the symbolic ref to a concrete commit up front so the content,
+    // language, and commitSha all come from the same revision even if the ref
+    // moves mid-request. `^{commit}` peels annotated tags. Reads below fall back
+    // to the symbolic ref when resolution fails.
+    let commitSha: string | undefined;
+    try {
+        commitSha = (await git.raw(['rev-parse', `${gitRef}^{commit}`])).trim();
+    } catch {
+        // Leave unpinned; the reads below use the symbolic ref.
+    }
+    const readRef = commitSha ?? gitRef;
+
     let fileContent: string;
     try {
-        fileContent = await git.raw(['show', `${gitRef}:${filePath}`]);
+        fileContent = await git.raw(['show', `${readRef}:${filePath}`]);
     } catch (error: unknown) {
         const errorMessage = error instanceof Error ? error.message : String(error);
         if (errorMessage.includes('does not exist') || errorMessage.includes('fatal: path')) {
@@ -61,18 +73,9 @@ export const getFileSourceForRepo = async (
         return unexpectedError(errorMessage);
     }
 
-    // Resolve the symbolic ref to a concrete commit SHA so callers can pin a
-    // citation to the exact code read. `^{commit}` peels annotated tags.
-    let commitSha: string | undefined;
-    try {
-        commitSha = (await git.raw(['rev-parse', `${gitRef}^{commit}`])).trim();
-    } catch {
-        // Leave unpinned if the ref can't be resolved.
-    }
-
     let gitattributesContent: string | undefined;
     try {
-        gitattributesContent = await git.raw(['show', `${gitRef}:.gitattributes`]);
+        gitattributesContent = await git.raw(['show', `${readRef}:.gitattributes`]);
     } catch {
         // No .gitattributes in this repo/ref, that's fine
     }
diff --git a/packages/web/src/features/tools/findSymbolDefinitions.ts b/packages/web/src/features/tools/findSymbolDefinitions.ts
@@ -63,6 +63,13 @@ export const findSymbolDefinitionsDefinition: ToolDefinition<
             codeHostType: repoInfoResult.codeHostType,
         };
 
+        // Pin to the indexed commit carried by the same search snapshot that
+        // produced these matches, rather than a follow-up repo-info lookup that
+        // could drift if the index advances in between.
+        const indexedCommitShaByRepo = new Map(
+            response.repositoryInfo.map((info) => [info.name, info.indexedCommitHash]),
+        );
+
         const metadata: FindSymbolDefinitionsMetadata = {
             symbol,
             matchCount,
@@ -72,7 +79,7 @@ export const findSymbolDefinitionsDefinition: ToolDefinition<
                 fileName: file.fileName,
                 repo: file.repository,
                 revision,
-                commitSha: repoInfoResult.indexedCommitHash,
+                commitSha: indexedCommitShaByRepo.get(file.repository),
             })),
         };
 
diff --git a/packages/web/src/features/tools/findSymbolReferences.ts b/packages/web/src/features/tools/findSymbolReferences.ts
@@ -74,6 +74,13 @@ export const findSymbolReferencesDefinition: ToolDefinition<
             codeHostType: repoInfoResult.codeHostType,
         };
 
+        // Pin to the indexed commit carried by the same search snapshot that
+        // produced these matches, rather than a follow-up repo-info lookup that
+        // could drift if the index advances in between.
+        const indexedCommitShaByRepo = new Map(
+            response.repositoryInfo.map((info) => [info.name, info.indexedCommitHash]),
+        );
+
         const metadata: FindSymbolReferencesMetadata = {
             symbol,
             matchCount,
@@ -83,7 +90,7 @@ export const findSymbolReferencesDefinition: ToolDefinition<
                 fileName: file.fileName,
                 repo: file.repository,
                 revision,
-                commitSha: repoInfoResult.indexedCommitHash,
+                commitSha: indexedCommitShaByRepo.get(file.repository),
             })),
         };
 
