Skip to content

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

Open
iscekic wants to merge 1 commit into
mainfrom
kwf/explorer-the-backend-returned-a-server-error-while-a-user-55a86-ffa1
Open

iscekic wants to merge 1 commit into
mainfrom
kwf/explorer-the-backend-returned-a-server-error-while-a-user-55a86-ffa1

Conversation

@iscekic

@iscekic iscekic commented Sep 18, 2026

Copy link
Copy Markdown
Collaborator

Changelog for users

  • Starting a manual Code Reviewer job for a missing or private pull request now shows an actionable message instead of a server error.
  • A rate-limited GitHub or GitLab request now asks the user to try again in a few minutes.
  • Unreachable providers and unexpected provider responses now report a retryable failure.
  • GitLab messages now say "merge request" where they previously said "pull request".
  • A GitLab permission failure now reports an unexpected response instead of a rate limit.

Changelog for maintainers

  • apps/web/src/lib/code-reviews/manual-code-review-jobs.ts:398 — accepted: the error noun is now platform-aware, so GitLab says "merge request"; a unit test asserts the GitLab 404 message contains "merge request" and not "pull request".
  • apps/web/src/lib/code-reviews/manual-code-review-jobs.ts:401 — accepted: only GitHub 403 maps to TOO_MANY_REQUESTS; GitLab rate limits arrive as 429, and a GitLab 403 now falls through to BAD_GATEWAY. Tests cover both GitLab 429 and 403.
  • apps/web/src/lib/code-reviews/manual-code-review-jobs.ts:414 — accepted: JSON and schema parse failures now map to a BAD_GATEWAY "unexpected response" instead of "could not reach"; isProviderResponseParseError detects them, including a cross-realm SyntaxError.
  • Every typed provider error now keeps the original error as cause, including the network-failure path.
  • resolveLocalPublicSource converts public-provider failures to typed tRPC errors; the endpoint previously answered HTTP 500, and existing TRPCErrors and the connected-integration path remain unchanged.
  • Provider 404 maps to NOT_FOUND; other provider statuses map to BAD_GATEWAY; network failures, timeouts, and redirects report a retryable failure.
  • Unit tests cover GitHub 404, 403, and 500, GitLab 404, 429, and 403, unparseable JSON, schema-invalid JSON, and a network failure.
  • Review hint: start at toLocalSourceError on the DEBUG_SHOW_DEV_UI local path; the new branch separates a reached-but-unparseable response from a network failure.

E2E proof

The earlier e1 happy-path log excerpt is superseded by the current run's appended excerpts, which report the same review-creation path for vercel/next.js#98898 (android emulator-5554, 2026-09-18). The e1 error-toast and e2 happy-path captures remain valid, and the e2 fixture gap still applies: e2e-index.md names no github-stub PR fixture URLs and the stub 404s every /repos/.../pulls/N path, so the run pointed GITHUB_API_BASE_URL at https://api.github.com for the happy path.

[e1] Personal Code Reviewer → Manual review: submit https://github.com/kilo-seed/review-list/pull/3000 (a public PR URL that does not exist); the app shows an actionable error toast and no server 500. — the SCENE digest captured GitHub could not find that pull request. Check the URL, or make sure the repository is public., copy emitted only by toLocalSourceError, so the fixed local public path ran.

Personal Code Reviewer → Manual review: submit https://github.com/kilo-seed/review-list/pull/3000 (a public PR URL that does not exist); the app shows an actionable error toast and no server 500.

[e1] Personal Code Reviewer → Manual review: submit a real open public GitHub PR URL; the review is created and the app navigates to the new review detail (happy path unchanged). — the review detail shows vercel/next.js #98898 by lukesandberg and model inclusionai/ling-3.0-flash-vl:free.

[e1] Personal Code Reviewer → Manual review: submit a real open public GitHub PR URL; the review is created and the app navigates to the new review detail (happy path unchanged).

[e1] Personal Code Reviewer → Manual review: submit a real open public GitHub PR URL; the review is created and the app navigates to the new review detail (happy path unchanged).

[e2] Personal Code Reviewer → Manual review: submit a real open public GitHub PR URL; the review is created and the app navigates to the new review detail (happy path unchanged). — the review detail shows Queued and Created Just now.

[e2] Personal Code Reviewer → Manual review: submit a real open public GitHub PR URL; the review is created and the app navigates to the new review detail (happy path unchanged).

Owner request

Explorer finding: the backend returned a server error while a user drove the app

The user-agent explorer found this while using the app like a user.
One finding per item; the explorer never edits product code.

Flow: freestyle
Found on revision: 9d53b25

Repro:

  1. set this state first: account e2e-mobile-cloud-android@example.com with $10 credits, one top-level session permission (ses_f4a82ebf6ffe4w4712QAzyIJlj), Code Reviewer seeded with 3 pull requests whose checks are success, a second organization to switch to, system dark mode, font scale 1.5, battery 15% discharging, device portrait.
  2. open the app on emulator-5554
  3. reach the screen the capture names (the round's captures)
  4. the capture shows the defect named below

Observed: the dev stack logged: nextjs.log: POST /api/trpc/personalReviewAgent.createManualReviewJob?batch=1 500
Expected: no server error while a user uses the app

Evidence (from the device run):

Production record (automatic, 2026-09-18):

Production record for ses_f4a82ebf6ffe4w4712QAzyIJlj

Axiom, last 72 h

No line carries this term in: vercel, cloudflare-logpush, supabase-production.

The session row, read replica

No row in cli_sessions_v2 carries ses_f4a82ebf6ffe4w4712QAzyIJlj.

Log archives, R2 kilocode-sessions

NOT READ: R2 keys are indexed by the agent id, not the session id; take the agent_* from the worker logs above and query that.

Sentry, last 72h

No Sentry issue carries this term.

E2E proof — log excerpts

[e1] Personal Code Reviewer → Manual review: submit a real open public GitHub PR -> pass :: android emulator-5554: e1-create-200.log carries "POST /api/trpc/personalReviewAgent.createManualReviewJob?batch=1 200 in 664ms" and "[dispatchReview] Review dispatched successfully {" for the real public PR vercel/next.js #98898; the app then reached the new review detail, e1-story4.log carrying "SCENE e1 OK" and "android.widget.TextView vercel/next.js #98898 by lukesandberg tappable [55,593][1025,630]" and "android.widget.TextView Running tappable [55,667][1025,713]", and e1-scene-run2.log carrying "android.widget.TextView Queued tappable [55,667][1025,713]" on that same detail screen, so the review left Queued and the detail rendered; e1-db-review.log shows a sibling row reached completio

Comment thread apps/web/src/lib/code-reviews/manual-code-review-jobs.ts Outdated
Comment thread apps/web/src/lib/code-reviews/manual-code-review-jobs.ts Outdated
Comment thread apps/web/src/lib/code-reviews/manual-code-review-jobs.ts
@kilo-code-bot

kilo-code-bot Bot commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Executive Summary

The incremental changes resolve all three prior findings: the error noun is now platform-aware, only a GitHub 403 maps to a rate limit, and unparseable or schema-invalid provider responses are reported as reached-but-unexpected with the original error retained as cause. No new issues found in the changed lines.

Files Reviewed (2 files)
  • apps/web/src/lib/code-reviews/manual-code-review-jobs.ts
  • apps/web/src/routers/code-reviews-router.test.ts
Previous Review Summary (commit 8aaf801)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit 8aaf801)

Status: 3 Issues Found | Recommendation: Address before merge

Executive Summary

The local public-source error mapping is functionally correct and well tested; the remaining issues are inaccurate user-facing error copy for GitLab and a fallback branch that hides parse failures and discards the original error.

Overview

Severity Count
CRITICAL 0
WARNING 0
SUGGESTION 3
Issue Details (click to expand)

SUGGESTION

File Line Issue
apps/web/src/lib/code-reviews/manual-code-review-jobs.ts 398 GitLab errors say "pull request" instead of "merge request"
apps/web/src/lib/code-reviews/manual-code-review-jobs.ts 401 Bare 403 mapped to a rate-limit message, but GitLab returns 429 for rate limits
apps/web/src/lib/code-reviews/manual-code-review-jobs.ts 414 Fallback labels unparseable responses as "could not reach" and drops the original error
Files Reviewed (2 files)
  • apps/web/src/lib/code-reviews/manual-code-review-jobs.ts - 3 issues
  • apps/web/src/routers/code-reviews-router.test.ts - 0 issues

Fix these issues in Kilo Cloud


Reviewed by deepseek-v4.1-flash · Input: 0 · Output: 0 · Cached: 0

Review guidance: REVIEW.md from base branch main

@iscekic
iscekic marked this pull request as draft September 18, 2026 22:33
@iscekic
iscekic force-pushed the kwf/explorer-the-backend-returned-a-server-error-while-a-user-55a86-ffa1 branch from 8aaf801 to 0fb339a Compare September 18, 2026 23:26
@iscekic
iscekic force-pushed the kwf/explorer-the-backend-returned-a-server-error-while-a-user-55a86-ffa1 branch from a1a0765 to 446e3b6 Compare September 18, 2026 23:55
@iscekic
iscekic marked this pull request as ready for review September 19, 2026 00:16
@iscekic iscekic added the human-ready The PR is ready for human review. label Sep 19, 2026
@iscekic iscekic self-assigned this Sep 19, 2026
@iscekic

iscekic commented Sep 20, 2026

Copy link
Copy Markdown
Collaborator Author

Kept as the single PR for this defect; #6405 was closed in its favour.

One thing to pick up from it before merge:

apps/web/src/lib/code-reviews/manual-code-review-jobs.test.ts, a unit test co-located with the changed file

It is the only coverage #6405 had that this PR does not.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

human-ready The PR is ready for human review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant