feat(provider-review): expose the neutral review facade - #5763
Conversation
| revision: overview.revision, | ||
| }; | ||
| return { | ||
| token: cursor ? parseReviewCursor(cursor, scope).token : undefined, |
There was a problem hiding this comment.
WARNING: Stale file/thread cursors surface as 500 instead of a client error
parseReviewCursor throws a generic Error ('Pagination identity mismatch') or a Zod error when the cursor scopeKey does not match this review and revision. GitLab and Bitbucket catch that failure and map it to invalid_request / invalid_pagination. This helper does not, so a leftover files or threads cursor after a push becomes INTERNAL_SERVER_ERROR.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| revision: null, | ||
| }; | ||
| const result = await caller.listInbox({ | ||
| cursor: cursor ? parseReviewCursor(cursor, scope).token : undefined, |
There was a problem hiding this comment.
WARNING: Stale inbox cursors surface as 500 instead of a client error
Inbox pagination calls parseReviewCursor without a try/catch. The inbox scopeKey includes the current authorization and actor, so a leftover cursor after token rotation throws an uncaught Error and tRPC maps it to 500 instead of a client-facing BAD_REQUEST or CONFLICT.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| const items = []; | ||
| for (const item of result.items) | ||
| items.push({ | ||
| identity: (await metadata(item, auth)).identity, |
There was a problem hiding this comment.
WARNING: One inbox item failing identity re-resolution fails the entire inbox
Each search hit is re-fetched with repos.get + pulls.get via metadata(). That helper throws CONFLICT on rename/identity mismatch and lets GitHub 404/403 propagate. The legacy buildInboxResult already drops malformed GraphQL nodes; a deleted PR or inaccessible repo that still appeared in search now takes down the whole page instead of skipping that row.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
Code Review SummaryStatus: 3 Issues Found | Recommendation: Address before merge Fix these issues in Kilo Cloud Overview
Issue Details (click to expand)WARNING
Files Reviewed (9 files)
Previous Review Summary (commit d3ab504)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit d3ab504)Status: 3 Issues Found | Recommendation: Address before merge Fix these issues in Kilo Cloud Overview
Issue Details (click to expand)WARNING
Files Reviewed (9 files)
Reviewed by grok-4.6 · Input: 100.7K · Output: 9.4K · Cached: 153K Review guidance: REVIEW.md from base branch |
|
Closing: the owner stopped this workflow section. The branch is retained. |
The backend now exposes provider-neutral review endpoints. The app's screens and navigation remain unchanged.
Summary
The
providerReviewroute exposesgetAuthorization,resolveUrl,getReview,listInbox,listFiles,getFileContext,listChecks,listDiscussions,act, andgetOperationStatusfor GitHub, GitLab, and Bitbucket. TheoperationInputschema requiresactorId,revision, and a universally unique identifier (UUID) foroperationKey;reviewInputaccepts legacy GitHub references. Bitbucket requires organization access;RootRouterandMobileRouteradd the contract while retaininggithubPrReview.Files
apps/web/src/routers/provider-review-router.ts— Source, added (+667/−0 lines). Selects one active integration and validates the owner, host, repository, actor, and positions. Resolves uncached GitLab projects, including archived projects, through authorized path lookup. Rejects unsafe URLs and oversized non-GitHub requests, normalizes errors, and returns authorization with review data. Limits context to 500 lines; inbox filters remain provider-specific, and only GitLab accepts diff versions.apps/web/src/routers/root-router.ts— Source, modified (+2/−0 lines). Registers the server route.packages/trpc/src/mobile.ts— Source, modified (+2/−0 lines). Adds the authored mobile contract.withGitHubReviewIdentityintroduces the server-onlyGitHubReviewIdentityscope for the admitted account, authorization, and actor.withGitHubUserTokenRetryvalidates the dispatched credential and actor before each attempt, including rotation, adding an authenticated-user read. An identity mismatch raisesreview_identity_or_revision_changed; unscoped callers keep their existing retry behavior.Files
apps/web/src/lib/github-pr-review/retry.ts— Source, modified (+33/−2 lines). Adds request-local identity checks and exposeshasGitHubReviewIdentityfor scoped reconciliation.createGitHubReviewBridgeusesGitHubReviewAddressSchema,ReviewOverview, andReviewMutationResultto adapt legacy reads and actions without changing endpoints, Terms gates, or fingerprint bytes.fetchGitHubReviewCheckspreserves check-run identifiers and status contexts; scoped merge reconciliation stays read-only and confirms only the admitted head. Old records retain casing and receipt fallbacks; missing evidence stays unresolved, and unconfirmed branch deletion produces partial success.Files
apps/web/src/lib/provider-review/github-bridge.ts— Source, added (+1,077/−0 lines). Supports comments, reviews, merges, thread actions, reactions, branch updates, and automatic merge. Scopes pagination by revision and validates file membership, action fields, and discussion targets. Fresh actions require available capabilities. Keeps legacy author and metadata fallbacks; old threads retain null positions. Compatibility remains until old clients and records disappear and the 30-day ledger window expires. Confirms branch updates through source and target ancestry, and reads operation status without writes.apps/web/src/routers/github-pr-review-router.ts— Source, modified (+47/−36 lines). Exposes raw checks without changing legacy responses. Waits for both check requests and prevents scoped merge redispatch during admission races.Tests: 3 files changed.
apps/web/src/lib/github-pr-review/retry.test.tsmodified (+139/−2 lines);apps/web/src/lib/provider-review/github-bridge.test.tsadded (+1,466/−0 lines);apps/web/src/routers/provider-review-router.test.tsadded (+796/−0 lines).Generated: 0 files changed.
Verification
No manual tests ran; runtime verification waits for the complete mobile stack.
Visual Changes
Visual Changes: N/A
Reviewer Notes
mobile-provider-parity-1d93-s12...mobile-provider-parity-1d93-s13.Kilo-Org/cloud./Users/igor/Projects/.worktrees/mobile-provider-parity-1d93.Notes
Runtime verification remains pending until the mobile stack is complete. The dispatcher owns backend, mobile, and web verification before human-ready.
Stacked PRs — merge bottom to top. Each level shows only its own diff.
Runtime verification (E2E, user advocacy, simplify) runs on the tip PR over every level.
Every level keeps its own checks, its own bot review, and its own threads; each one is answered on its own PR.
Each level is its own deliverable: it builds and passes its own checks alone.
A finding on a level is repaired on that level, then carried upward with stack.sh forward.
mobile-provider-parity-1d93-s1— feat(provider-review): add shared identity and stack checks #5663mobile-provider-parity-1d93-s2— feat(provider-review): define review contracts and vocabulary #5665mobile-provider-parity-1d93-s3— feat(provider-review): add safe provider SDK boundaries #5689mobile-provider-parity-1d93-s4— feat(git-token): authorize exact provider review resources #5692mobile-provider-parity-1d93-s5— feat(cloud-agent): preserve provider identity through checkout #5700mobile-provider-parity-1d93-s6— feat(provider-review): expose exact repositories and branches #5705mobile-provider-parity-1d93-s7— feat(mobile): preserve provider launch and recent identity #5711mobile-provider-parity-1d93-s8— feat(mobile): select exact provider repositories and branches #5729mobile-provider-parity-1d93-s9— feat(provider-review): add authorized GitLab review reads #5738mobile-provider-parity-1d93-s10— feat(provider-review): add authorized Bitbucket review reads #5745mobile-provider-parity-1d93-s11— feat(provider-review): add reconciled GitLab review actions #5748mobile-provider-parity-1d93-s12— feat(provider-review): add Bitbucket review actions and recovery #5756mobile-provider-parity-1d93-s13— feat(provider-review): expose the neutral review facade #5763 ← this PR (tip)