fix(code-review): return actionable errors for public manual review sources

apps/web/src/lib/code-reviews/manual-code-review-jobs.ts

@@ -352,28 +352,100 @@ async function resolveLocalPublicSource(
   platform: CodeReviewPlatform,
   url: string
 ): Promise<ResolvedManualReviewSource> {
-  if (platform === PLATFORM.GITHUB) {
-    const parsed = parseGitHubPullRequestUrl(url);
-    const pullRequest = await fetchPublicGitHubPullRequest(parsed);
-    validateOpenGitHubPullRequest(pullRequest);
-    return buildGitHubSource(pullRequest, undefined);
+  try {
+    if (platform === PLATFORM.GITHUB) {
+      const parsed = parseGitHubPullRequestUrl(url);
+      const pullRequest = await fetchPublicGitHubPullRequest(parsed);
+      validateOpenGitHubPullRequest(pullRequest);
+      return buildGitHubSource(pullRequest, undefined);
+    }
+
+    const parsed = parseGitLabMergeRequestUrl(url);
+    if (new URL(parsed.origin).hostname !== 'gitlab.com') {
+      throw new TRPCError({
+        code: 'BAD_REQUEST',
+        message: 'Local Code Reviewer jobs only support public gitlab.com merge requests.',
+      });
+    }
+
+    const mergeRequest = await fetchPublicGitLabMergeRequest(parsed);
+    validateOpenGitLabMergeRequest(mergeRequest);
+    return buildGitLabSource({
+      mergeRequest,
+      projectPath: parsed.projectPath,
+      integrationId: undefined,
+      platformProjectId: mergeRequest.target_project_id ?? mergeRequest.project_id,
+    });
+  } catch (error) {
+    // The local (DEBUG_SHOW_DEV_UI) path reads the pull request from the provider's
+    // public API instead of a connected integration. A provider failure there used
+    // to escape as a raw Error, so tRPC answered 500 and the client showed a generic
+    // failure. Translate it into an actionable client error; the connected path
+    // already returns typed TRPCErrors. Existing TRPCErrors (invalid URL, closed or
+    // draft pull request) pass through unchanged.
+    throw toLocalSourceError(platform, error);
   }
+}
 
-  const parsed = parseGitLabMergeRequestUrl(url);
-  if (new URL(parsed.origin).hostname !== 'gitlab.com') {
-    throw new TRPCError({
-      code: 'BAD_REQUEST',
-      message: 'Local Code Reviewer jobs only support public gitlab.com merge requests.',
+// `Response.json()` rejects with a SyntaxError built outside this realm, so
+// `instanceof SyntaxError` misses it; match the error name instead. A ZodError
+// from the response schema is built in this realm and matches directly.
+function isProviderResponseParseError(error: unknown): boolean {
+  if (error instanceof z.ZodError) return true;
+  return (
+    typeof error === 'object' &&
+    error !== null &&
+    'name' in error &&
+    Reflect.get(error, 'name') === 'SyntaxError'
+  );
+}
+
+function toLocalSourceError(platform: CodeReviewPlatform, error: unknown): TRPCError {
+  if (error instanceof TRPCError) return error;
+
+  const provider = platform === PLATFORM.GITHUB ? 'GitHub' : 'GitLab';
+  // GitHub calls these pull requests; GitLab calls them merge requests.
+  const requestNoun = platform === PLATFORM.GITHUB ? 'pull request' : 'merge request';
+  if (error instanceof ProviderFetchError) {
+    if (error.status === 404) {
+      return new TRPCError({
+        code: 'NOT_FOUND',
+        message: `${provider} could not find that ${requestNoun}. Check the URL, or make sure the repository is public.`,
+        cause: error,
+      });
+    }
+    // GitHub signals primary and secondary rate limits with 403; GitLab uses 429.
+    // A GitLab 403 is a permission error, not a rate limit, so it falls through.
+    if (error.status === 429 || (error.status === 403 && platform === PLATFORM.GITHUB)) {
+      return new TRPCError({
+        code: 'TOO_MANY_REQUESTS',
+        message: `${provider} rate-limited the request. Try again in a few minutes.`,
+        cause: error,
+      });
+    }
+    return new TRPCError({
+      code: 'BAD_GATEWAY',
+      message: `${provider} returned an unexpected response for that ${requestNoun}.`,
+      cause: error,
+    });
+  }
+
+  // The provider answered, but with a body that is not valid JSON or does not
+  // match its documented shape. We reached it, so "could not reach" would
+  // misdescribe what happened.
+  if (isProviderResponseParseError(error)) {
+    return new TRPCError({
+      code: 'BAD_GATEWAY',
+      message: `${provider} returned an unexpected response for that ${requestNoun}.`,
+      cause: error,
     });
   }
 
-  const mergeRequest = await fetchPublicGitLabMergeRequest(parsed);
-  validateOpenGitLabMergeRequest(mergeRequest);
-  return buildGitLabSource({
-    mergeRequest,
-    projectPath: parsed.projectPath,
-    integrationId: undefined,
-    platformProjectId: mergeRequest.target_project_id ?? mergeRequest.project_id,
+  // Network failure, timeout, or redirect.
+  return new TRPCError({
+    code: 'BAD_GATEWAY',
+    message: `Could not reach ${provider} to read that ${requestNoun}. Try again.`,
+    cause: error,
   });
 }
 

apps/web/src/routers/code-reviews-router.test.ts

@@ -691,6 +691,196 @@ describe('personalReviewAgent.createManualReviewJob', () => {
       userId: testUser.id,
     });
   });
+
+  // The local (DEBUG_SHOW_DEV_UI) path reads the pull request from the provider's
+  // public API. A provider failure there used to escape as a raw Error, so tRPC
+  // answered 500 and the client showed a generic failure. Each case below must
+  // resolve to an actionable client error code instead of INTERNAL_SERVER_ERROR.
+  it.each([
+    { status: 404, expectedCode: 'NOT_FOUND', expectedMessage: 'could not find that pull request' },
+    {
+      status: 403,
+      expectedCode: 'TOO_MANY_REQUESTS',
+      expectedMessage: 'rate-limited the request',
+    },
+    { status: 500, expectedCode: 'BAD_GATEWAY', expectedMessage: 'unexpected response' },
+  ])(
+    'maps a public GitHub pull request fetch that fails with $status to $expectedCode',
+    async ({ status, expectedCode, expectedMessage }) => {
+      fetchSpy?.mockImplementation(async () => new Response('provider failure', { status }));
+      const caller = await createCallerForUser(testUser.id);
+
+      const rejection = await caller.personalReviewAgent
+        .createManualReviewJob({
+          platform: 'github',
+          url: prUrl,
+          modelSlug: 'test-model',
+        })
+        .then(
+          () => {
+            throw new Error('Expected createManualReviewJob to reject');
+          },
+          error => error as { code?: string; message?: string }
+        );
+
+      expect(rejection.code).toBe(expectedCode);
+      expect(rejection.code).not.toBe('INTERNAL_SERVER_ERROR');
+      expect(rejection.message).toContain(expectedMessage);
+    }
+  );
+
+  it('maps a missing public GitLab merge request to an actionable GitLab error', async () => {
+    fetchSpy?.mockImplementation(async () => new Response('Not Found', { status: 404 }));
+    const caller = await createCallerForUser(testUser.id);
+
+    const rejection = await caller.personalReviewAgent
+      .createManualReviewJob({
+        platform: 'gitlab',
+        url: 'https://gitlab.com/group/project/-/merge_requests/1',
+        modelSlug: 'test-model',
+      })
+      .then(
+        () => {
+          throw new Error('Expected createManualReviewJob to reject');
+        },
+        error => error as { code?: string; message?: string }
+      );
+
+    expect(rejection.code).toBe('NOT_FOUND');
+    expect(rejection.message).toContain('GitLab');
+    expect(rejection.message).toContain('merge request');
+    expect(rejection.message).not.toContain('pull request');
+  });
+
+  it('maps a public GitLab rate limit to TOO_MANY_REQUESTS', async () => {
+    fetchSpy?.mockImplementation(async () => new Response('Too Many Requests', { status: 429 }));
+    const caller = await createCallerForUser(testUser.id);
+
+    const rejection = await caller.personalReviewAgent
+      .createManualReviewJob({
+        platform: 'gitlab',
+        url: 'https://gitlab.com/group/project/-/merge_requests/1',
+        modelSlug: 'test-model',
+      })
+      .then(
+        () => {
+          throw new Error('Expected createManualReviewJob to reject');
+        },
+        error => error as { code?: string; message?: string }
+      );
+
+    expect(rejection.code).toBe('TOO_MANY_REQUESTS');
+    expect(rejection.message).toContain('rate-limited the request');
+  });
+
+  // GitLab returns 429 for rate limits; a 403 is a permission error and must not
+  // be reported to the user as a rate limit.
+  it('does not map a public GitLab 403 to a rate-limit error', async () => {
+    fetchSpy?.mockImplementation(async () => new Response('Forbidden', { status: 403 }));
+    const caller = await createCallerForUser(testUser.id);
+
+    const rejection = await caller.personalReviewAgent
+      .createManualReviewJob({
+        platform: 'gitlab',
+        url: 'https://gitlab.com/group/project/-/merge_requests/1',
+        modelSlug: 'test-model',
+      })
+      .then(
+        () => {
+          throw new Error('Expected createManualReviewJob to reject');
+        },
+        error => error as { code?: string; message?: string }
+      );
+
+    expect(rejection.code).toBe('BAD_GATEWAY');
+    expect(rejection.message).toContain('unexpected response');
+    expect(rejection.message).not.toContain('rate-limited');
+  });
+
+  // A provider that answers with an unparseable body was reached; the error must
+  // say so rather than claiming the provider was unreachable, and keep the cause.
+  it('reports an unparseable provider response as unexpected, not unreachable', async () => {
+    fetchSpy?.mockImplementation(
+      async () =>
+        new Response('not json', { status: 200, headers: { 'Content-Type': 'application/json' } })
+    );
+    const caller = await createCallerForUser(testUser.id);
+
+    const rejection = await caller.personalReviewAgent
+      .createManualReviewJob({
+        platform: 'github',
+        url: prUrl,
+        modelSlug: 'test-model',
+      })
+      .then(
+        () => {
+          throw new Error('Expected createManualReviewJob to reject');
+        },
+        error => error as { code?: string; message?: string; cause?: unknown }
+      );
+
+    expect(rejection.code).toBe('BAD_GATEWAY');
+    expect(rejection.message).toContain('unexpected response');
+    expect(rejection.message).not.toContain('Could not reach');
+    // The original parse error is kept as the cause rather than dropped.
+    expect((rejection.cause as Error | undefined)?.message).toContain('JSON');
+  });
+
+  // A well-formed JSON body that does not match the provider's documented shape
+  // is also a reached-but-unusable response, not an unreachable provider.
+  it('reports a provider response that fails schema validation as unexpected', async () => {
+    fetchSpy?.mockImplementation(
+      async () =>
+        new Response(JSON.stringify({ unexpected: true }), {
+          status: 200,
+          headers: { 'Content-Type': 'application/json' },
+        })
+    );
+    const caller = await createCallerForUser(testUser.id);
+
+    const rejection = await caller.personalReviewAgent
+      .createManualReviewJob({
+        platform: 'github',
+        url: prUrl,
+        modelSlug: 'test-model',
+      })
+      .then(
+        () => {
+          throw new Error('Expected createManualReviewJob to reject');
+        },
+        error => error as { code?: string; message?: string; cause?: unknown }
+      );
+
+    expect(rejection.code).toBe('BAD_GATEWAY');
+    expect(rejection.message).toContain('unexpected response');
+    expect(rejection.message).not.toContain('Could not reach');
+    expect(rejection.cause).toBeDefined();
+  });
+
+  it('maps a provider network failure to a client error instead of a 500', async () => {
+    const networkError = new TypeError('fetch failed');
+    fetchSpy?.mockImplementation(async () => {
+      throw networkError;
+    });
+    const caller = await createCallerForUser(testUser.id);
+
+    const rejection = await caller.personalReviewAgent
+      .createManualReviewJob({
+        platform: 'github',
+        url: prUrl,
+        modelSlug: 'test-model',
+      })
+      .then(
+        () => {
+          throw new Error('Expected createManualReviewJob to reject');
+        },
+        error => error as { code?: string; message?: string; cause?: unknown }
+      );
+
+    expect(rejection.code).toBe('BAD_GATEWAY');
+    expect(rejection.message).toContain('Could not reach GitHub');
+    expect(rejection.cause).toBe(networkError);
+  });
 });
 
 describe('review agent config REVIEW.md setting', () => {