fix(web): map manual review provider failures to client errors

[iscekic]

Changelog for users

• Manual code review of a missing or private GitHub pull request or GitLab merge request now shows a 'couldn't find' message instead of a server error.
• A provider that is rate-limiting requests now shows a 'try again in a few minutes' message instead of a server error.
• A provider that is unreachable or takes too long now shows a gateway error message instead of a server error.
• A provider response the app cannot read now shows a 'try again in a moment' message instead of a server error.

Changelog for maintainers

• apps/web/src/lib/code-reviews/manual-code-review-jobs.ts:754 — accepted: the GitLab adapter's own request timeout raises a plain Error named 'Error', so the timeout check now also matches the adapter message 'GitLab request timed out'; a connected-GitLab timeout now returns GATEWAY_TIMEOUT (504) instead of BAD_GATEWAY (502).
• Provider round-trip failures while resolving a manual review now map to tRPC client errors instead of INTERNAL_SERVER_ERROR (HTTP 500).
• Mapping: 404 to NOT_FOUND, 403/429 to TOO_MANY_REQUESTS, other 4xx to BAD_REQUEST, timeout/abort to GATEWAY_TIMEOUT, otherwise BAD_GATEWAY.
• Upstream status is read from ProviderFetchError.status, or from an Error message ending in a three-digit status (the GitLab adapter contract).
• Provider response schemas now use safeParse; an unreadable body raises BAD_GATEWAY through unreadableProviderResponse.
• Review hint: the timeout detection is the new helper near the bottom of manual-code-review-jobs.ts; the unit test 'maps a connected GitLab timeout to GATEWAY_TIMEOUT, not BAD_GATEWAY' covers the repair.
• Risk: timeout detection now depends on the GitLab adapter keeping the exact message 'GitLab request timed out'.
• Evidence note: the connected-GitLab timeout (504) is covered by the added unit test; it replaces the earlier timeout claim that only recognized AbortSignal timeouts.

E2E proof

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: 59ef790

Repro:

1. set this state first: the setup failed: de t=1789804358 login.sh: phase=app-settle t=1789804396 login.sh: verify-code failed and the app is not signed in as e2e-firstrun-1789804110@example.com login.sh: failed with exit 1 (phase=app-settle); the device in light mode
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 nextjs.log: caused by [trpc] mutation personalReviewAgent.createManualReviewJob failed: INTERNAL_SERVER_ERROR
Expected: no server error while a user uses the app

Evidence (from the device run):

E2E proof — log excerpts

[e1] With GitHub connected (the explorer's state), sign in on emulator-5554, ope -> pass :: jev read the digest: pass (confidence 0.91)

[kilo-code-bot]

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Executive Summary

Incremental review of 24d87e8 (delta from 6a5eb19): the new isProviderTimeout helper correctly maps the connected-GitLab adapter timeout to GATEWAY_TIMEOUT (504), resolving the prior audit finding, with no new issues in the changed lines.

Files Reviewed (2 files)

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

Incremental Findings

• Resolved: the connected-GitLab request timeout was mapped to BAD_GATEWAY (502). The adapter destroys the request with a plain Error('GitLab request timed out') (apps/web/src/lib/integrations/platforms/gitlab/adapter.ts:172), whose name is Error, so the old TimeoutError/AbortError name-only check missed it. isProviderTimeout now also matches that exact message and the helper is used at manual-code-review-jobs.ts:769, producing 504.
• Verified the adapter throws that message unwrapped through fetchGitLabMergeRequest (adapter.ts:1146 does not catch), and that providerErrorStatus (checked first) does not intercept it since the message has no : <status> suffix.
• New unit test 'maps a connected GitLab timeout to GATEWAY_TIMEOUT, not BAD_GATEWAY' asserts code GATEWAY_TIMEOUT and HTTP 504, matching the implementation.
• Residual coupling noted in the PR description: timeout detection depends on the adapter preserving the exact string GitLab request timed out. This is documented in-code and covered by the test; treated as an accepted tradeoff, not a defect.

No new inline comments were raised. The review's prior summary contained no open findings.

Previous Review Summary (commit 6a5eb19)

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

Previous review (commit 6a5eb19)

Status: No Issues Found | Recommendation: Merge

Reviewed PR head 6a5eb19. The change wraps manual-review provider round-trips (fetchPublicGitHubPullRequest, fetchPublicGitLabMergeRequest, and the connected GitLab path) so ProviderFetchError/network/timeout/body-shape failures map to typed tRPC client errors instead of an unmapped 500. The status-extraction regex mirrors the established classifyGitLabError contract (GitLab MR fetch failed: <status>), the adapter was verified to throw that shape (apps/web/src/lib/integrations/platforms/gitlab/adapter.ts:1143), safeParse failures route through unreadableProviderResponse to BAD_GATEWAY, TRPCError passthrough preserves existing validations, and no provider body/token is echoed into the new messages. The new test file covers 404/403/network/timeout/unreadable-shape plus the connected GitLab and happy paths, and its mocks/imports match the module's real dependencies.

Files Reviewed (2 files)

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

---

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

_{Review guidance: REVIEW.md from base branch main}

[iscekic]

kilo-review — independent audit of the published diff.

Status: 1 Issues

File	Line	Issue
apps/web/src/lib/code-reviews/manual-code-review-jobs.ts	754	A connected-GitLab timeout is mapped to BAD_GATEWAY (502) instead of the documented GATEWAY_TIMEOUT (504), because the adapter's own timeout raises a plain Error whose name is 'Error'.

[iscekic]

bot: Accepted. Pushed 24d87e8 (kwf kwf-fix-review-9631).

What changed:

• apps/web/src/lib/code-reviews/manual-code-review-jobs.ts:754 — accepted: the GitLab adapter's own request timeout raises a plain Error named 'Error', so the timeout check now also matches the adapter message 'GitLab request timed out'; a connected-GitLab timeout now returns GATEWAY_TIMEOUT (504) instead of BAD_GATEWAY (502).
• Provider round-trip failures while resolving a manual review map to tRPC client errors instead of INTERNAL_SERVER_ERROR (HTTP 500).
• Mapping: 404 to NOT_FOUND, 403/429 to TOO_MANY_REQUESTS, other 4xx to BAD_REQUEST, timeout/abort to GATEWAY_TIMEOUT, otherwise BAD_GATEWAY.
• Upstream status is read from ProviderFetchError.status, or from an Error message ending in a three-digit status (the GitLab adapter contract).
• Provider response schemas now use safeParse; an unreadable body raises BAD_GATEWAY through unreadableProviderResponse.
• Review hint: the timeout detection is the new helper near the bottom of manual-code-review-jobs.ts; a unit test covers the GitLab timeout.
• Risk: timeout detection now depends on the GitLab adapter keeping the exact message 'GitLab request timed out'.

[iscekic]

Closing as a duplicate of #6325.

Both came from the same explorer finding, the-backend-returned-a-server-error-while-a-user, and both rewrite apps/web/src/lib/code-reviews/manual-code-review-jobs.ts, so they cannot both merge.

#6325 is kept as the original (18 Sep, against 19 Sep here). This PR's extra work is the co-located unit test manual-code-review-jobs.test.ts; if that coverage is wanted it is worth adding to #6325 rather than keeping two PRs on one file.

The intake now refuses a finding whose slug already has an open PR or a live section (kwf 07c842fd), so this finding will not be filed a third time.

[iscekic]

Reopened — I closed this in error.

I closed it as a duplicate of #6325. Both do rewrite manual-code-review-jobs.ts, but this PR is not a subset of #6325: it adds manual-code-review-jobs.test.ts, a co-located unit test #6325 does not have. Closing it discarded that coverage.

The two still cannot both merge as they stand, since they rewrite the same source file. Consolidating this PR's test onto #6325, or taking this one instead, are both better outcomes than the close I did.

[iscekic]

Closing in favour of #6325 — one PR per defect.

Both rewrite manual-code-review-jobs.ts for the same 500. #6325 is the original (18 Sep) and is the one the workflow already records as owning this finding.

I checked this is not work thrown away: the only thing this PR changes that #6325 does not is

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

Worth porting that onto #6325 rather than keeping two PRs on the same files. I have left the same note there.

Consolidating by hand because these two rewrite the same logic in different ways, so they cannot be merged mechanically. Going forward the explorer pools findings for an hour and dispatches one item per defect (kwf d25ae7de), and the intake refuses a slug that already has open work (kwf 07c842fd).