Skip to content

feat(provider-review): expose the neutral review facade - #5763

Closed
iscekic wants to merge 6 commits into
mobile-provider-parity-1d93-s12from
mobile-provider-parity-1d93-s13
Closed

feat(provider-review): expose the neutral review facade#5763
iscekic wants to merge 6 commits into
mobile-provider-parity-1d93-s12from
mobile-provider-parity-1d93-s13

Conversation

@iscekic

@iscekic iscekic commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

The backend now exposes provider-neutral review endpoints. The app's screens and navigation remain unchanged.


Summary

The providerReview route exposes getAuthorization, resolveUrl, getReview, listInbox, listFiles, getFileContext, listChecks, listDiscussions, act, and getOperationStatus for GitHub, GitLab, and Bitbucket. The operationInput schema requires actorId, revision, and a universally unique identifier (UUID) for operationKey; reviewInput accepts legacy GitHub references. Bitbucket requires organization access; RootRouter and MobileRouter add the contract while retaining githubPrReview.

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.

withGitHubReviewIdentity introduces the server-only GitHubReviewIdentity scope for the admitted account, authorization, and actor. withGitHubUserTokenRetry validates the dispatched credential and actor before each attempt, including rotation, adding an authenticated-user read. An identity mismatch raises review_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 exposes hasGitHubReviewIdentity for scoped reconciliation.

createGitHubReviewBridge uses GitHubReviewAddressSchema, ReviewOverview, and ReviewMutationResult to adapt legacy reads and actions without changing endpoints, Terms gates, or fingerprint bytes. fetchGitHubReviewChecks preserves 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.ts modified (+139/−2 lines); apps/web/src/lib/provider-review/github-bridge.test.ts added (+1,466/−0 lines); apps/web/src/routers/provider-review-router.test.ts added (+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

  • Human steps: None required before or after merge.
  • Scope: level 13, mobile-provider-parity-1d93-s12...mobile-provider-parity-1d93-s13.
  • Repository: Kilo-Org/cloud.
  • Worktree: /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.

  1. mobile-provider-parity-1d93-s1feat(provider-review): add shared identity and stack checks #5663
  2. mobile-provider-parity-1d93-s2feat(provider-review): define review contracts and vocabulary #5665
  3. mobile-provider-parity-1d93-s3feat(provider-review): add safe provider SDK boundaries #5689
  4. mobile-provider-parity-1d93-s4feat(git-token): authorize exact provider review resources #5692
  5. mobile-provider-parity-1d93-s5feat(cloud-agent): preserve provider identity through checkout #5700
  6. mobile-provider-parity-1d93-s6feat(provider-review): expose exact repositories and branches #5705
  7. mobile-provider-parity-1d93-s7feat(mobile): preserve provider launch and recent identity #5711
  8. mobile-provider-parity-1d93-s8feat(mobile): select exact provider repositories and branches #5729
  9. mobile-provider-parity-1d93-s9feat(provider-review): add authorized GitLab review reads #5738
  10. mobile-provider-parity-1d93-s10feat(provider-review): add authorized Bitbucket review reads #5745
  11. mobile-provider-parity-1d93-s11feat(provider-review): add reconciled GitLab review actions #5748
  12. mobile-provider-parity-1d93-s12feat(provider-review): add Bitbucket review actions and recovery #5756
  13. mobile-provider-parity-1d93-s13feat(provider-review): expose the neutral review facade #5763 ← this PR (tip)

revision: overview.revision,
};
return {
token: cursor ? parseReviewCursor(cursor, scope).token : undefined,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@kilo-code-bot

kilo-code-bot Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: 3 Issues Found | Recommendation: Address before merge

Fix these issues in Kilo Cloud

Overview

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

WARNING

File Line Issue
apps/web/src/lib/provider-review/github-bridge.ts 403 Stale file/thread cursors surface as 500
apps/web/src/lib/provider-review/github-bridge.ts 427 Stale inbox cursors surface as 500
apps/web/src/lib/provider-review/github-bridge.ts 432 One inbox item failure fails the entire inbox
Files Reviewed (9 files)
  • apps/web/src/lib/github-pr-review/retry.test.ts
  • apps/web/src/lib/github-pr-review/retry.ts
  • apps/web/src/lib/provider-review/github-bridge.test.ts
  • apps/web/src/lib/provider-review/github-bridge.ts - 3 issues
  • apps/web/src/routers/github-pr-review-router.ts
  • apps/web/src/routers/provider-review-router.test.ts
  • apps/web/src/routers/provider-review-router.ts
  • apps/web/src/routers/root-router.ts
  • packages/trpc/src/mobile.ts
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

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

WARNING

File Line Issue
apps/web/src/lib/provider-review/github-bridge.ts 403 Stale file/thread cursors surface as 500
apps/web/src/lib/provider-review/github-bridge.ts 427 Stale inbox cursors surface as 500
apps/web/src/lib/provider-review/github-bridge.ts 432 One inbox item failure fails the entire inbox
Files Reviewed (9 files)
  • apps/web/src/lib/github-pr-review/retry.test.ts
  • apps/web/src/lib/github-pr-review/retry.ts
  • apps/web/src/lib/provider-review/github-bridge.test.ts
  • apps/web/src/lib/provider-review/github-bridge.ts - 3 issues
  • apps/web/src/routers/github-pr-review-router.ts
  • apps/web/src/routers/provider-review-router.test.ts
  • apps/web/src/routers/provider-review-router.ts
  • apps/web/src/routers/root-router.ts
  • packages/trpc/src/mobile.ts

Reviewed by grok-4.6 · Input: 100.7K · Output: 9.4K · Cached: 153K

Review guidance: REVIEW.md from base branch mobile-provider-parity-1d93-s12

@iscekic

iscekic commented Aug 31, 2026

Copy link
Copy Markdown
Contributor Author

Closing: the owner stopped this workflow section. The branch is retained.

@iscekic iscekic closed this Aug 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant