feat(provider-review): add Bitbucket review actions and recovery - #5756
feat(provider-review): add Bitbucket review actions and recovery#5756iscekic wants to merge 9 commits into
Conversation
| const expected = evidence[side]; | ||
| if ( | ||
| endpoint.repository?.uuid !== expected.repositoryId || | ||
| endpoint.repository.workspace?.uuid !== expected.workspaceUuid || |
There was a problem hiding this comment.
CRITICAL: Async merge confirmation treats omitted Bitbucket workspace as identity drift
snapshot() is the only call that requests +source.repository.workspace,+destination.repository.workspace. Bitbucket condensed PR identities omit nested workspace, and merge / mergeTask cannot pass a fields query.
merged() still requires workspace?.uuid === expected.workspaceUuid on both the merge/task body and the snapshot. A 200 merge can recover on a later snapshot-only reconcile, but a 202 SUCCESS path always passes task.merge_result into merged(), so a successful async merge hits conflict() forever and never confirms.
Skip the workspace comparison when the provider body omits workspace, or confirm identity from the snapshot (which requested the field) instead of treating condensed merge/task payloads as drift.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
Code Review SummaryStatus: 1 Issue Found | Recommendation: Address before merge Fix these issues in Kilo Cloud Overview
Issue Details (click to expand)CRITICAL
Files Reviewed (1 file)
Previous Review Summary (commit 99d738d)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 99d738d)Status: 1 Issue Found | Recommendation: Address before merge Fix these issues in Kilo Cloud Overview
Issue Details (click to expand)CRITICAL
Files Reviewed (28 files)
Reviewed by grok-4.6 · Input: 113.6K · Output: 10K · Cached: 208.6K Review guidance: REVIEW.md from base branch |
|
Closing: the owner stopped this workflow section. The branch is retained. |
Summary
BitbucketMergeEvidenceSchema,BitbucketMergeEvidence, andPersistBitbucketMergeEvidencebind recovery to durable, server-observed source/destination identities, excluding client claims.Evidence counts toward storage limits; retries clear stale evidence.
Accepted or confirmed merges without valid evidence return
merge_identity_unavailable; unknown effects block blind retries.Files
apps/web/src/lib/provider-review/operation.ts— M, +52/−6; persists evidence before dispatch and reconciles uncertain merges.packages/app-shared/src/provider-review/contracts.ts— M, +15/−0; validates complete repository, workspace, and branch identities.runBitbucketReviewOperationandBitbucketReviewOperationRequestsupport comments/replies, thread resolution/reopening, approval/change-request toggles, batches, and merges internally; no public facade or mobile controls expose them.Capability, identity, revision, position, target, and payload checks detect drift, not atomic expected-head protection; non-merge receipts remain unresolved.
Merge recovery reads provider/task outcomes without resubmission; batches retain child results and stop at unknown effects, while deletion uses observation without another delete.
Files
apps/web/src/lib/provider-review/bitbucket-write.ts— A, +831/−0; handles fork revisions, merge strategies, and status-only legacy children.BitbucketOAuthRecoverySchemaandBitbucketOAuthRecoveryadd optional signedbitbucketRecoverytocreateOAuthState,VerifiedOAuthState, andStoreBitbucketIntegrationInput; legacy callers remain valid.Bitbucket connect/callback routes validate
reconnectIntegrationIdthroughgetBitbucketOAuthRecovery; atomic credential replacement preserves the integration, workspace, and cache.BitbucketIntegrationRecoveryErrorrejects stale targets, unauthorized recovery, unavailable workspaces, and missing write grants; ordinary connections still reject replacement.Files
apps/web/src/lib/integrations/oauth-state.ts— M, +44/−5; signs strict, expiring recovery context.apps/web/src/lib/integrations/oauth/platforms/bitbucket-connect.ts— M, +24/−6; authenticates owner-scoped reconnect targets.apps/web/src/lib/integrations/oauth/platforms/bitbucket-callback.ts— M, +37/−4; rejects unsigned selectors; skips recovery cache priming.apps/web/src/lib/integrations/platforms/bitbucket/credentials.ts— M, +173/−30; locks encrypted replacements, versions credentials, rejects blocked personal authorizers.getBitbucketReviewGrantStatusreportsreadReady,writeReady, andrecoveryAction; optionalreviewPermissionscannot establish write access when absent.OAuth requests
pullrequest:write, normalizes legacy/implied grants, preserves old read grants, and retains write grants during refresh.BitbucketOAuthScopeErroridentifies missing read scopes; incomplete write grants requirereconnectorreplace_token.Files
packages/worker-utils/src/bitbucket-workspace-access-token.ts— M, +20/−0; computes grant readiness and recovery actions.apps/web/src/lib/integrations/platforms/bitbucket/adapter.ts— M, +17/−4; requests write grants with typed scope failures.services/git-token-service/src/bitbucket-authorization-service.ts— M, +6/−0; preserves normalized grants through credential refresh.apps/web/src/lib/integrations/platforms/bitbucket/workspace-access-token-repository-cache.ts— M, +15/−0; exposes optional permissions, null for unusable connections.BitbucketReviewPermissionsandBitbucketTokenPermissionsshare accessible guidance; connected managers gain OAuth reconnect controls without exposing review actions.Recovery messages distinguish
connection_changed,workspace_unavailable, andmissing_scopes; non-managers retain read-only controls.Files
apps/web/src/components/integrations/BitbucketConnectSetup.tsx— M, +34/−10; requires Pull request Write and links token guidance.apps/web/src/components/integrations/BitbucketConnectedManagement.tsx— M, +2/−0; shows guidance throughout connected recovery states.apps/web/src/components/integrations/BitbucketIntegrationControls.tsx— M, +21/−2; adds scoped reconnect links and scrollable replacement guidance.apps/web/src/components/integrations/BitbucketIntegrationDetails.tsx— M, +6/−0; explains recovery errors and retained connections.Tests: 13 files, +3,558/−41, cover recovery, grants, evidence, and uncertain writes:
apps/web/src/app/api/integrations/bitbucket/callback/route.test.ts(M,+212/−1);apps/web/src/app/api/integrations/bitbucket/connect/route.test.ts(M,+123/−17);apps/web/src/components/integrations/BitbucketConnectSetup.test.ts(A,+98/−0);apps/web/src/components/integrations/BitbucketIntegrationDetails.test.ts(M,+256/−1);apps/web/src/lib/integrations/oauth-state.test.ts(M,+94/−1);apps/web/src/lib/integrations/platforms/bitbucket/adapter.test.ts(M,+39/−5);apps/web/src/lib/integrations/platforms/bitbucket/credentials-reconnect.test.ts(A,+470/−0);apps/web/src/lib/integrations/platforms/bitbucket/credentials.test.ts(M,+230/−11);apps/web/src/lib/provider-review/bitbucket-write.test.ts(A,+1,316/−0);apps/web/src/lib/provider-review/operation.test.ts(M,+461/−2);packages/app-shared/src/provider-review/contracts.test.ts(M,+56/−0);packages/worker-utils/src/bitbucket-workspace-access-token.test.ts(M,+35/−0);services/git-token-service/src/bitbucket-authorization-service.test.ts(M,+168/−3).Generated: 0 files changed.
Verification
Manual runtime checks have not run because verification remains pending in the bot-e2e phase.
Visual Changes
Visual Changes: N/A
Reviewer Notes
Human steps
Automated evidence
Inventory
Kilo-Org/cloudworktree:/Users/igor/Projects/.worktrees/mobile-provider-parity-1d93.mobile-provider-parity-1d93-s11...mobile-provider-parity-1d93-s12.Notes
Runtime verification is pending in the bot-e2e phase; the dispatcher retains that obligation.
Standard CI must execute the database-backed Bitbucket credential and repository-cache suites before completion.
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 #5756 ← this PRmobile-provider-parity-1d93-s13— feat(provider-review): expose the neutral review facade #5763 (tip)