Skip to content

feat(provider-review): define review contracts and vocabulary - #5665

Open
iscekic wants to merge 3 commits into
mobile-provider-parity-1d93-s1from
mobile-provider-parity-1d93-s2
Open

feat(provider-review): define review contracts and vocabulary#5665
iscekic wants to merge 3 commits into
mobile-provider-parity-1d93-s1from
mobile-provider-parity-1d93-s2

Conversation

@iscekic

@iscekic iscekic commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

No new behavior — This change prepares GitLab and Bitbucket reviews without changing the app's current screens or actions.


Summary

ReviewIdentity and ReviewOverview define a shared review contract for GitHub, GitLab, and Bitbucket while leaving existing GitHub callers unchanged.
ReviewPositionSchema, ReviewCapabilitiesSchema, ProviderReviewStateSchema, and ReviewMutationResultSchema preserve provider-specific positions, capability checks, approval states, and write outcomes.
serializeReviewWriteRequest enforces REVIEW_WRITE_REQUEST_MAX_BYTES at 256,000 serialized bytes; parseReviewCursor rejects mismatched scopes, while legacy endpoint limits remain unchanged.

Files
  • packages/app-shared/src/provider-review/contracts.ts — source, added (+384 lines); defines opaque references, actor and credential identity, revisions, file/context records, threads, reactions, inbox scopes, checks, and merge options. Strict schemas reject unknown fields and require https addresses where URLs are present. Versioned resource keys require GitHub user authorization and reject personal Bitbucket integrations; canonical URLs do not authorize access. Positions require paired start fields, a selected-side path, and GitLab base/start revisions while retaining native coordinates. Capabilities evaluate support, version, license, permission, then restrictions; unsupported actions require an explanation and evidence. Recovery choices and expected-head protection remain explicit. GitLab approvals and merge blocking stay distinct; Bitbucket preserves participant states and asynchronous merge tasks. Cursors bind the resource, surface, query, and revision, and limit tokens to 4,096 characters. File content distinguishes available, binary, truncated, and unavailable data; checks distinguish absent, unavailable, and reported results. Results prescribe no retries for confirmed effects, reconciliation for accepted or unresolved effects, declared retry eligibility for rejection, and unfinished-only retries for partial effects. The write limit counts the serialized request, not the unescaped body or character count.
  • packages/app-shared/src/provider-review/index.ts — source, added (+2 lines); exports the review contracts and intent fingerprints without exporting the fixtures.
  • packages/app-shared/src/provider-review/contracts.test.ts — test, added (+341 lines); adds the shared review contract test suite.

providerReviewFixtures, availableCapabilityFixture, and reviewCapabilityFixtures supply test-only ownership and capability cases for GitHub, GitLab, and Bitbucket.
The matrix preserves unknown GitHub withdrawal actions, unsupported Bitbucket actions, and different revision protections instead of assuming identical support.
Adapters must obtain actual capabilities from the authorized provider; fixture availability does not grant runtime permissions.

Files
  • packages/app-shared/src/provider-review/fixtures.ts — test fixture, added (+140 lines); defines personal and organization references, with no personal Bitbucket reference. GitHub leaves unapproval and change-request removal unknown; GitLab supplies supported cases for all actions. Bitbucket excludes reactions, branch updates, and automatic merging, with provider evidence for each exclusion. In the fixtures, GitHub and GitLab merge atomically and attach inline comments to revisions; GitLab also protects approvals, while Bitbucket supplies neither.

ReviewIntentInputSchema and providerReviewIntentFingerprint bind each write intent to the account, review, actor, revision, and complete payload under provider-review-intent:v1.
Strict parsing fixes field order and rejects unknown fields, so edits to comments, review choices, reactions, merge options, or deletion targets change the fingerprint.
legacyGitHubIntentFingerprint preserves old bytes and field order; removal requires retiring old clients and records plus waiting through the 30-day ledger window.

Files
  • packages/app-shared/src/provider-review/intent-fingerprint.ts — source, added (+60 lines); validates write actions, comment queues, draft references, reactions, review choices, merge settings, and branch-deletion preconditions. The input excludes read actions and rejects unknown fields recursively. Fingerprints preserve array order and optional-field distinctions; the legacy alias leaves existing GitHub fingerprints unchanged.
  • packages/app-shared/src/provider-review/intent-fingerprint.test.ts — test, added (+299 lines); adds the intent fingerprint test suite.

PROVIDER_REVIEW_REQUEST_KEYS selects merge-request wording for GitLab and pull-request wording for GitHub and Bitbucket; PROVIDER_REVIEW_STATE_KEYS keeps shared states separate.
Both expose catalog-checked translation keys, not cached text, so callers use the active translator without coupling wording to capability decisions.
The providerReview catalog adds 97 English keys for identity, access, review states, recovery, and merge outcomes; existing GitHub strings remain unchanged.

Files
  • apps/mobile/src/i18n/provider-review-vocabulary.ts — source, added (+153 lines); maps each provider to typed request keys and exports separate state keys. The types check request vocabulary completeness and catalog key validity.
  • apps/mobile/src/i18n/locales/en.json — source, modified (+127 lines); adds wording for review links, ownership, authorization, inbox paging, file context, checks, saved drafts, uncertain outcomes, merge confirmation, and provider limits. The wording distinguishes acceptance from completion and describes unfinished work, GitLab merge blocking, and Bitbucket limitations.
  • apps/mobile/src/i18n/provider-review-vocabulary.test.ts — test, added (+183 lines); adds the mobile vocabulary test suite.

Tests: 4 files added (+963 lines): contracts.test.ts (+341), intent-fingerprint.test.ts (+299), provider-review-vocabulary.test.ts (+183), and fixtures.ts (+140).
Generated: 0 files changed.


Verification

  • No manual tests are reported. Runtime verification remains pending for the cumulative stack; this level adds no visual surface.

Visual Changes

Visual Changes: N/A

Reviewer Notes

  • Scope: level 2 only, mobile-provider-parity-1d93-s1...mobile-provider-parity-1d93-s2.
  • Repository: Kilo-Org/cloud.
  • Worktree: /Users/igor/Projects/.worktrees/mobile-provider-parity-1d93.
  • The handoff reports 78 passing shared tests, followed by nine passing mobile vocabulary tests and a clean bounded recheck.

Human steps

No human steps are required before merge or after merge for this level.

Notes

Runtime verification is pending for the cumulative stack; this level adds no visual surface.

Translation is pending for the 97 new English keys; locale parity remains a completion requirement.

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 ← this PR
  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 (tip)

@kilo-code-bot

kilo-code-bot Bot commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Executive Summary

Incremental review of nullable file/overview line counts found no correctness, security, or contract issues.

Files Reviewed (3 files)
  • packages/app-shared/src/provider-review/contracts.ts
  • packages/app-shared/src/provider-review/contracts.test.ts
  • apps/web/package.json
Previous Review Summary (commit 1428586)

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

Previous review (commit 1428586)

Status: No Issues Found | Recommendation: Merge

Files Reviewed (9 files)
  • apps/mobile/src/i18n/locales/en.json
  • apps/mobile/src/i18n/provider-review-vocabulary.test.ts
  • apps/mobile/src/i18n/provider-review-vocabulary.ts
  • packages/app-shared/src/provider-review/contracts.test.ts
  • packages/app-shared/src/provider-review/contracts.ts
  • packages/app-shared/src/provider-review/fixtures.ts
  • packages/app-shared/src/provider-review/index.ts
  • packages/app-shared/src/provider-review/intent-fingerprint.test.ts
  • packages/app-shared/src/provider-review/intent-fingerprint.ts

Reviewed by grok-4.6 · Input: 163.5K · Output: 10.3K · Cached: 162.7K

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

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